Bug 1254393. Take ownership of error reporting on the AutoEntryScript in nsXPCWrappedJSClass::CallMethod. r=bholley

There are several cases to consider in terms of where exceptions might come from
here:

1)  We could have an exception on the JSContext already when we set up the
AutoEntryScript.  In practice this does not happen, and once this change is made
we will add an assert to that effect in AutoJSAPI::InitInternal.  See bug 1112920.

2)  We could have an exception thrown by the XPCCallContext constructor, but it
never does that when initialized, as here, without a JSObject*.

3)  We can have an exception thrown by CallMethod itself, if the callee method
wants a JSContext or optional argc.  This patch switches that to using
CheckForException, which means it will behave exactly like exceptions thrown
from the actual implementation of the method we're calling in terms of how it's
reported and whether it's rethrown to callers, if any.

4)  We can have exceptions thrown various places (e.g. during argument and
retval/outparam conversions) after this point, but those are all under the scope
of the AutoScriptEvaluate, whose destructor will restore the JSContext state to whatever it
was when the AutoScriptEvaluate was created.  Since the AutoScriptEvaluate goes
out of scope before the AutoEntryScript does, there will be no dangling
exception on the JSContext at that point.

5)  We can have exceptions thrown by the actual method call.  Those are
reported, as needed, via CheckForException, and will continue to be so reported.

So in general there are two behavior changes here:

* We now treat the "callee wants JSContext or argc" case as the same as an
  exception from the callee, which is not what we used to do.

* If the object we're calling comes from an inner window whose outer window has
  been torn down, we will now correctly report the exception against that inner
  window.  Before this patch, what happend is that we would init the
  AutoEntryScript with the inner window, but FindJSContext would not find an
  nsIScriptContext on it (since its outer has been torn down), so we would use
  the safe JS context.  Then in xpc::SystemErrorReporter we would check for a
  window associated with the JSContext, not find one, check for an addon sandbox
  current compartment, see we're not in one, and then use the privileged junk
  scope for its error reporting.  The new setup in AutoJSAPI::ReportError checks
  for the current compartent being a window, which of course it is.
This commit is contained in:
Boris Zbarsky 2016-03-08 17:21:40 -05:00
Родитель 3e5ab54341
Коммит 8db444c416
2 изменённых файлов: 12 добавлений и 6 удалений

Просмотреть файл

@ -775,12 +775,14 @@ nsXPCWrappedJSClass::CleanupOutparams(JSContext* cx, uint16_t methodIndex,
nsresult
nsXPCWrappedJSClass::CheckForException(XPCCallContext & ccx,
AutoEntryScript& aes,
const char * aPropertyName,
const char * anInterfaceName,
bool aForceReport)
{
XPCContext * xpcc = ccx.GetXPCContext();
JSContext * cx = ccx.GetJSContext();
MOZ_ASSERT(cx == aes.cx());
nsCOMPtr<nsIException> xpc_exception;
/* this one would be set by our error reporter */
@ -811,7 +813,7 @@ nsXPCWrappedJSClass::CheckForException(XPCCallContext & ccx,
// Clear the pending exception now, because xpc_exception might be JS-
// implemented, so invoking methods on it might re-enter JS, which we can't
// do with an exception on the stack.
JS_ClearPendingException(cx);
aes.ClearException();
if (xpc_exception) {
nsresult e_result;
@ -860,7 +862,8 @@ nsXPCWrappedJSClass::CheckForException(XPCCallContext & ccx,
// just so that we can tell the JS engine to pass it back to us via the
// error reporting callback. This is all very dumb.
JS_SetPendingException(cx, js_exception);
reportable = !JS_ReportPendingException(cx);
aes.ReportException();
reportable = false;
}
if (reportable) {
@ -979,6 +982,7 @@ nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS* wrapper, uint16_t methodIndex,
NativeGlobal(js::GetGlobalForObjectCrossCompartment(wrapper->GetJSObject()));
AutoEntryScript aes(nativeGlobal, "XPCWrappedJS method call",
/* aIsMainThread = */ true);
aes.TakeOwnershipOfErrorReporting();
XPCCallContext ccx(NATIVE_CALLER, aes.cx());
if (!ccx.IsValid())
return retval;
@ -997,7 +1001,7 @@ nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS* wrapper, uint16_t methodIndex,
// Throw and warn for good measure.
JS_ReportError(cx, str);
NS_WARNING(str);
return NS_ERROR_FAILURE;
return CheckForException(ccx, aes, name, GetInterfaceName(), false);
}
RootedValue fval(cx);
@ -1221,7 +1225,7 @@ pre_call_clean_up:
// do the deed - note exceptions
JS_ClearPendingException(cx);
MOZ_ASSERT(!aes.HasException());
RootedValue rval(cx);
if (XPT_MD_IS_GETTER(info->flags)) {
@ -1267,7 +1271,8 @@ pre_call_clean_up:
// May also want to check if we're moving from content->chrome and force
// a report in that case.
return CheckForException(ccx, name, GetInterfaceName(), forceReport);
return CheckForException(ccx, aes, name, GetInterfaceName(),
forceReport);
}
XPCJSRuntime::Get()->SetPendingException(nullptr); // XXX necessary?

Просмотреть файл

@ -2335,11 +2335,12 @@ public:
const nsAString& aName,
nsIVariant** aResult);
private:
static nsresult CheckForException(XPCCallContext & ccx,
mozilla::dom::AutoEntryScript& aes,
const char * aPropertyName,
const char * anInterfaceName,
bool aForceReport);
private:
virtual ~nsXPCWrappedJSClass();
nsXPCWrappedJSClass(); // not implemented