Bug 1107777 - Add autoJSAPIOwnsErrorReporting flag to JSContext options. AutoJSAPI sets it. r=bholley

dom/base/ScriptSettings.cpp

@@ -304,7 +304,7 @@ FindJSContext(nsIGlobalObject* aGlobalObject)
 AutoJSAPI::AutoJSAPI()
   : mCx(nullptr)
   , mOwnErrorReporting(false)
-  , mOldDontReportUncaught(false)
+  , mOldAutoJSAPIOwnsErrorReporting(false)
 {
 }
 
@@ -312,7 +312,7 @@ AutoJSAPI::~AutoJSAPI()
 {
   if (mOwnErrorReporting) {
     MOZ_ASSERT(NS_IsMainThread(), "See corresponding assertion in TakeOwnershipOfErrorReporting()");
-    JS::ContextOptionsRef(cx()).setDontReportUncaught(mOldDontReportUncaught);
+    JS::ContextOptionsRef(cx()).setAutoJSAPIOwnsErrorReporting(mOldAutoJSAPIOwnsErrorReporting);
 
     if (HasException()) {
 
@@ -377,7 +377,7 @@ AutoJSAPI::AutoJSAPI(nsIGlobalObject* aGlobalObject,
                      bool aIsMainThread,
                      JSContext* aCx)
   : mOwnErrorReporting(false)
-  , mOldDontReportUncaught(false)
+  , mOldAutoJSAPIOwnsErrorReporting(false)
 {
   MOZ_ASSERT(aGlobalObject);
   MOZ_ASSERT(aGlobalObject->GetGlobalJSObject(), "Must have a JS global");
@@ -472,11 +472,12 @@ AutoJSAPI::InitWithLegacyErrorReporting(nsGlobalWindow* aWindow)
   return InitWithLegacyErrorReporting(static_cast<nsIGlobalObject*>(aWindow));
 }
 
-// Even with dontReportUncaught, the JS engine still sends warning reports
-// to the JSErrorReporter as soon as they are generated. These go directly to
-// the console, so we can handle them easily here.
+// Even with autoJSAPIOwnsErrorReporting, the JS engine still sends warning
+// reports to the JSErrorReporter as soon as they are generated. These go
+// directly to the console, so we can handle them easily here.
 //
-// Eventually, SpiderMonkey will have a special-purpose callback for warnings only.
+// Eventually, SpiderMonkey will have a special-purpose callback for warnings
+// only.
 void
 WarningOnlyErrorReporter(JSContext* aCx, const char* aMessage, JSErrorReport* aRep)
 {
@@ -496,8 +497,8 @@ AutoJSAPI::TakeOwnershipOfErrorReporting()
   mOwnErrorReporting = true;
 
   JSRuntime *rt = JS_GetRuntime(cx());
-  mOldDontReportUncaught = JS::ContextOptionsRef(cx()).dontReportUncaught();
-  JS::ContextOptionsRef(cx()).setDontReportUncaught(true);
+  mOldAutoJSAPIOwnsErrorReporting = JS::ContextOptionsRef(cx()).autoJSAPIOwnsErrorReporting();
+  JS::ContextOptionsRef(cx()).setAutoJSAPIOwnsErrorReporting(true);
   JS_SetErrorReporter(rt, WarningOnlyErrorReporter);
 }
 

dom/base/ScriptSettings.h

@@ -307,7 +307,7 @@ private:
 
   // Track state between the old and new error reporting modes.
   bool mOwnErrorReporting;
-  bool mOldDontReportUncaught;
+  bool mOldAutoJSAPIOwnsErrorReporting;
   Maybe<JSErrorReporter> mOldErrorReporter;
 
   void InitInternal(JSObject* aGlobal, JSContext* aCx, bool aIsMainThread);

js/src/jsapi.cpp

@@ -3741,7 +3741,7 @@ struct AutoLastFrameCheck
     ~AutoLastFrameCheck() {
         if (cx->isExceptionPending() &&
             !JS_IsRunning(cx) &&
-            !cx->options().dontReportUncaught()) {
+            (!cx->options().dontReportUncaught() && !cx->options().autoJSAPIOwnsErrorReporting())) {
             js_ReportUncaughtException(cx);
         }
     }

js/src/jsapi.h

@@ -1601,7 +1601,8 @@ class JS_PUBLIC_API(ContextOptions) {
   public:
     ContextOptions()
       : privateIsNSISupports_(false),
-        dontReportUncaught_(false)
+        dontReportUncaught_(false),
+        autoJSAPIOwnsErrorReporting_(false)
     {
     }
 
@@ -1625,9 +1626,28 @@ class JS_PUBLIC_API(ContextOptions) {
         return *this;
     }
 
+    bool autoJSAPIOwnsErrorReporting() const { return autoJSAPIOwnsErrorReporting_; }
+    ContextOptions &setAutoJSAPIOwnsErrorReporting(bool flag) {
+        autoJSAPIOwnsErrorReporting_ = flag;
+        return *this;
+    }
+    ContextOptions &toggleAutoJSAPIOwnsErrorReporting() {
+        autoJSAPIOwnsErrorReporting_ = !autoJSAPIOwnsErrorReporting_;
+        return *this;
+    }
+
+
   private:
     bool privateIsNSISupports_ : 1;
     bool dontReportUncaught_ : 1;
+    // dontReportUncaught isn't respected by all JSAPI codepaths, particularly the
+    // JS_ReportError* functions that eventually report the error even when dontReportUncaught is
+    // set, if script is not running. We want a way to indicate that the embedder will always
+    // handle any exceptions, and that SpiderMonkey should just leave them on the context. This is
+    // the way we want to do all future error handling in Gecko - stealing the exception explicitly
+    // from the context and handling it as per the situation. This will eventually become the
+    // default and these 2 flags should go away.
+    bool autoJSAPIOwnsErrorReporting_ : 1;
 };
 
 JS_PUBLIC_API(ContextOptions &)

js/src/jscntxt.cpp

@@ -301,12 +301,17 @@ ReportError(JSContext *cx, const char *message, JSErrorReport *reportp,
         reportp->flags |= JSREPORT_EXCEPTION;
     }
 
+    if (cx->options().autoJSAPIOwnsErrorReporting() || JS_IsRunning(cx)) {
+        if (js_ErrorToException(cx, message, reportp, callback, userRef)) {
+            return;
+        }
+    }
+
     /*
      * Call the error reporter only if an exception wasn't raised.
      */
-    if (!JS_IsRunning(cx) || !js_ErrorToException(cx, message, reportp, callback, userRef)) {
-        if (message)
-            CallErrorReporter(cx, message, reportp);
+    if (message) {
+        CallErrorReporter(cx, message, reportp);
     }
 }
 

js/xpconnect/src/XPCWrappedJSClass.cpp

@@ -277,7 +277,8 @@ nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject(JSContext* cx,
             }
 
             // Don't report if reporting was disabled by someone else.
-            if (!ContextOptionsRef(cx).dontReportUncaught())
+            if (!ContextOptionsRef(cx).dontReportUncaught() &&
+                !ContextOptionsRef(cx).autoJSAPIOwnsErrorReporting())
                 JS_ReportPendingException(cx);
         } else if (!success) {
             NS_WARNING("QI hook ran OOMed - this is probably a bug!");