Bug 1174486 part 4. Stop reporting exceptions in nsJSUtils::EvaluateString and have its consumers use AutoJSAPIs that take ownership of error reporting instead. r=bholley

dom/base/nsGlobalWindow.cpp

@@ -12463,6 +12463,7 @@ nsGlobalWindow::RunTimeoutHandler(nsTimeout* aTimeout,
     // New script entry point required, due to the "Create a script" sub-step of
     // http://www.whatwg.org/specs/web-apps/current-work/#timer-initialisation-steps
     AutoEntryScript entryScript(this, reason, true, aScx->GetNativeContext());
+    entryScript.TakeOwnershipOfErrorReporting();
     JS::CompileOptions options(entryScript.cx());
     options.setFileAndLine(filename, lineNo)
            .setVersion(JSVERSION_DEFAULT);

dom/base/nsJSUtils.cpp

@@ -98,41 +98,6 @@ nsJSUtils::GetCurrentlyRunningCodeInnerWindowID(JSContext *aContext)
   return innerWindowID;
 }
 
-void
-nsJSUtils::ReportPendingException(JSContext *aContext)
-{
-  if (JS_IsExceptionPending(aContext)) {
-    bool saved = JS_SaveFrameChain(aContext);
-    {
-      // JS_SaveFrameChain set the compartment of aContext to null, so we need
-      // to enter a compartment.  The question is, which one? We don't want to
-      // enter the original compartment of aContext (or the compartment of the
-      // current exception on aContext, for that matter) because when we
-      // JS_ReportPendingException the JS engine can try to duck-type the
-      // exception and produce a JSErrorReport.  It will then pass that
-      // JSErrorReport to the error reporter on aContext, which might expose
-      // information from it to script via onerror handlers.  So it's very
-      // important that the duck typing happen in the same compartment as the
-      // onerror handler.  In practice, that's the compartment of the window (or
-      // otherwise default global) of aContext, so use that here.
-      nsIScriptContext* scx = GetScriptContextFromJSContext(aContext);
-      JS::Rooted<JSObject*> scope(aContext);
-      scope = scx ? scx->GetWindowProxy() : nullptr;
-      if (!scope) {
-        // The SafeJSContext has no default object associated with it.
-        MOZ_ASSERT(NS_IsMainThread());
-        MOZ_ASSERT(aContext == nsContentUtils::GetSafeJSContext());
-        scope = xpc::UnprivilegedJunkScope(); // Usage approved by bholley
-      }
-      JSAutoCompartment ac(aContext, scope);
-      JS_ReportPendingException(aContext);
-    }
-    if (saved) {
-      JS_RestoreFrameChain(aContext);
-    }
-  }
-}
-
 nsresult
 nsJSUtils::CompileFunction(AutoJSAPI& jsapi,
                            JS::AutoObjectVector& aScopeChain,
@@ -199,6 +164,8 @@ nsJSUtils::EvaluateString(JSContext* aCx,
   PROFILER_LABEL("nsJSUtils", "EvaluateString",
     js::ProfileEntry::Category::JS);
 
+  MOZ_ASSERT(JS::ContextOptionsRef(aCx).autoJSAPIOwnsErrorReporting(),
+             "Caller must own error reporting");
   MOZ_ASSERT_IF(aCompileOptions.versionSet,
                 aCompileOptions.version != JSVERSION_UNKNOWN);
   MOZ_ASSERT_IF(aEvaluateOptions.coerceToString, !aCompileOptions.noScriptRval);
@@ -222,10 +189,6 @@ nsJSUtils::EvaluateString(JSContext* aCx,
   nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager();
   NS_ENSURE_TRUE(ssm->ScriptAllowed(aEvaluationGlobal), NS_OK);
 
-  // We need to prevent AutoLastFrameCheck from reporting and clearing
-  // any pending exceptions.
-  AutoDontReportUncaught dontReport(aCx);
-
   bool ok = true;
   // Scope the JSAutoCompartment so that we can later wrap the return value
   // into the caller's cx.
@@ -269,7 +232,7 @@ nsJSUtils::EvaluateString(JSContext* aCx,
   }
 
   if (!ok) {
-    ReportPendingException(aCx);
+    rv = NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW;
     if (!aCompileOptions.noScriptRval) {
       aRetValue.setUndefined();
     }

dom/base/nsJSUtils.h

@@ -53,13 +53,6 @@ public:
    */
   static uint64_t GetCurrentlyRunningCodeInnerWindowID(JSContext *aContext);
 
-  /**
-   * Report a pending exception on aContext, if any.  Note that this
-   * can be called when the context has a JS stack.  If that's the
-   * case, the stack will be set aside before reporting the exception.
-   */
-  static void ReportPendingException(JSContext *aContext);
-
   static nsresult CompileFunction(mozilla::dom::AutoJSAPI& jsapi,
                                   JS::AutoObjectVector& aScopeChain,
                                   JS::CompileOptions& aOptions,
@@ -86,7 +79,9 @@ public:
 
   // aEvaluationGlobal is the global to evaluate in.  The return value
   // will then be wrapped back into the compartment aCx is in when
-  // this function is called.
+  // this function is called.  For all the EvaluateString overloads,
+  // the JSContext must come from an AutoJSAPI that has had
+  // TakeOwnershipOfErrorReporting() called on it.
   static nsresult EvaluateString(JSContext* aCx,
                                  const nsAString& aScript,
                                  JS::Handle<JSObject*> aEvaluationGlobal,

dom/base/nsScriptLoader.cpp

@@ -1120,6 +1120,7 @@ nsScriptLoader::EvaluateScript(nsScriptLoadRequest* aRequest,
   // http://www.whatwg.org/specs/web-apps/current-work/#execute-the-script-block
   AutoEntryScript entryScript(globalObject, "<script> element", true,
                               context->GetNativeContext());
+  entryScript.TakeOwnershipOfErrorReporting();
   JS::Rooted<JSObject*> global(entryScript.cx(),
                                globalObject->GetGlobalJSObject());
 

dom/jsurl/nsJSProtocolHandler.cpp

@@ -251,6 +251,10 @@ nsresult nsJSThunk::EvaluateScript(nsIChannel *aChannel,
     // http://www.whatwg.org/specs/web-apps/current-work/#javascript-protocol
     AutoEntryScript entryScript(innerGlobal, "javascript: URI", true,
                                 scriptContext->GetNativeContext());
+    // We want to make sure we report any exceptions that happen before we
+    // return, since whatever happens inside our execution shouldn't affect any
+    // other scripts that might happen to be running.
+    entryScript.TakeOwnershipOfErrorReporting();
     JSContext* cx = entryScript.cx();
     JS::Rooted<JSObject*> globalJSObject(cx, innerGlobal->GetGlobalJSObject());
     NS_ENSURE_TRUE(globalJSObject, NS_ERROR_UNEXPECTED);
@@ -278,20 +282,13 @@ nsresult nsJSThunk::EvaluateScript(nsIChannel *aChannel,
     rv = nsJSUtils::EvaluateString(cx, NS_ConvertUTF8toUTF16(script),
                                    globalJSObject, options, evalOptions, &v);
 
-    // If there's an error on cx as a result of that call, report
-    // it now -- either we're just running under the event loop,
-    // so we shouldn't propagate JS exceptions out of here, or we
-    // can't be sure that our caller is JS (and if it's not we'll
-    // lose the error), or it might be JS that then proceeds to
-    // cause an error of its own (which will also make us lose
-    // this error).
-    ::JS_ReportPendingException(cx);
-
     if (NS_FAILED(rv) || !(v.isString() || v.isUndefined())) {
         return NS_ERROR_MALFORMED_URI;
     } else if (v.isUndefined()) {
         return NS_ERROR_DOM_RETVAL_UNDEFINED;
     } else {
+        MOZ_ASSERT(rv != NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW,
+                   "How did we get a non-undefined return value?");
         nsAutoJSString result;
         if (!result.init(cx, v)) {
             return NS_ERROR_OUT_OF_MEMORY;

dom/plugins/base/nsNPAPIPlugin.cpp

@@ -1499,6 +1499,7 @@ _evaluate(NPP npp, NPObject* npobj, NPString *script, NPVariant *result)
   }
 
   dom::AutoEntryScript aes(win, "NPAPI NPN_evaluate");
+  aes.TakeOwnershipOfErrorReporting();
   JSContext* cx = aes.cx();
 
   JS::Rooted<JSObject*> obj(cx, nsNPObjWrapper::GetNewOrUsed(npp, cx, npobj));

dom/xbl/nsXBLProtoImplField.cpp

@@ -406,6 +406,7 @@ nsXBLProtoImplField::InstallField(JS::Handle<JSObject*> aBoundNode,
   // We are going to run script via EvaluateString, so we need a script entry
   // point, but as this is XBL related it does not appear in the HTML spec.
   AutoEntryScript entryScript(globalObject, "XBL <field> initialization", true);
+  entryScript.TakeOwnershipOfErrorReporting();
   JSContext* cx = entryScript.cx();
 
   NS_ASSERTION(!::JS_IsExceptionPending(cx),
@@ -441,6 +442,12 @@ nsXBLProtoImplField::InstallField(JS::Handle<JSObject*> aBoundNode,
     return rv;
   }
 
+  if (rv == NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW) {
+    // Report the exception now, before we try using the JSContext for
+    // the JS_DefineUCProperty call.
+    entryScript.ReportException();
+  }
+
   // Now, enter the node's compartment, wrap the eval result, and define it on
   // the bound node.
   JSAutoCompartment ac2(cx, aBoundNode);

xpcom/base/ErrorList.h

@@ -527,6 +527,12 @@
    * the second assignment throws NS_SUCCESS_DOM_NO_OPERATION.
    */
   ERROR(NS_SUCCESS_DOM_NO_OPERATION,               SUCCESS(1)),
+
+  /*
+   * A success code that indicates that evaluating a string of JS went
+   * just fine except it threw an exception.
+   */
+  ERROR(NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW,    SUCCESS(2)),
 #undef MODULE