Bug 1144750: Don't attempt to report errors that will cause the warnings only reporter to assert. r=bholley
☠☠ backed out by 08cef2ba28a0 ☠ ☠
authorDave Townsend <dtownsend@oxymoronical.com>
Wed, 18 Mar 2015 14:51:18 -0700
changeset 266240 7588e6588dd381d20d81ef8f0b4be1bfc0d6eefc
parent 266239 8f15eadc9430720531072045f63107bcd0514353
child 266241 a2a653839bca6b09210df6f3377468e161b89e94
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1144750
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1144750: Don't attempt to report errors that will cause the warnings only reporter to assert. r=bholley
dom/base/ScriptSettings.cpp
js/src/jscntxt.cpp
js/src/jsexn.cpp
js/src/jsexn.h
--- a/dom/base/ScriptSettings.cpp
+++ b/dom/base/ScriptSettings.cpp
@@ -307,17 +307,16 @@ AutoJSAPI::AutoJSAPI()
   , mOldAutoJSAPIOwnsErrorReporting(false)
 {
 }
 
 AutoJSAPI::~AutoJSAPI()
 {
   if (mOwnErrorReporting) {
     MOZ_ASSERT(NS_IsMainThread(), "See corresponding assertion in TakeOwnershipOfErrorReporting()");
-    JS::ContextOptionsRef(cx()).setAutoJSAPIOwnsErrorReporting(mOldAutoJSAPIOwnsErrorReporting);
 
     if (HasException()) {
 
       // AutoJSAPI uses a JSAutoNullableCompartment, and may be in a null
       // compartment when the destructor is called. However, the JS engine
       // requires us to be in a compartment when we fetch the pending exception.
       // In this case, we enter the privileged junk scope and don't dispatch any
       // error events.
@@ -337,16 +336,23 @@ AutoJSAPI::~AutoJSAPI()
           DispatchScriptErrorEvent(win, JS_GetRuntime(cx()), xpcReport, exn);
         } else {
           xpcReport->LogToConsole();
         }
       } else {
         NS_WARNING("OOMed while acquiring uncaught exception from JSAPI");
       }
     }
+
+    // We need to do this _after_ processing the existing exception, because the
+    // JS engine can throw while doing that, and uses this bit to determine what
+    // to do in that case: squelch the exception if the bit is set, otherwise
+    // call the error reporter. Calling WarningOnlyErrorReporter with a
+    // non-warning will assert, so we need to make sure we do the former.
+    JS::ContextOptionsRef(cx()).setAutoJSAPIOwnsErrorReporting(mOldAutoJSAPIOwnsErrorReporting);
   }
 
   if (mOldErrorReporter.isSome()) {
     JS_SetErrorReporter(JS_GetRuntime(cx()), mOldErrorReporter.value());
   }
 }
 
 void
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -224,16 +224,23 @@ ReportError(JSContext *cx, const char *m
         reportp->errorNumber == JSMSG_UNCAUGHT_EXCEPTION)
     {
         reportp->flags |= JSREPORT_EXCEPTION;
     }
 
     if (cx->options().autoJSAPIOwnsErrorReporting() || JS_IsRunning(cx)) {
         if (ErrorToException(cx, message, reportp, callback, userRef))
             return;
+
+        /*
+         * The AutoJSAPI error reporter only allows warnings to be reported so
+         * just ignore this error rather than try to report it.
+         */
+        if (cx->options().autoJSAPIOwnsErrorReporting())
+            return;
     }
 
     /*
      * Call the error reporter only if an exception wasn't raised.
      */
     if (message)
         CallErrorReporter(cx, message, reportp);
 }
--- a/js/src/jsexn.cpp
+++ b/js/src/jsexn.cpp
@@ -603,16 +603,22 @@ js::ErrorToException(JSContext *cx, cons
     // Flag the error report passed in to indicate an exception was raised.
     reportp->flags |= JSREPORT_EXCEPTION;
     return true;
 }
 
 static bool
 IsDuckTypedErrorObject(JSContext *cx, HandleObject exnObject, const char **filename_strp)
 {
+    /*
+     * This function is called from ErrorReport::init and so should not generate
+     * any new exceptions.
+     */
+    AutoClearPendingException acpe(cx);
+
     bool found;
     if (!JS_HasProperty(cx, exnObject, js_message_str, &found) || !found)
         return false;
 
     const char *filename_str = *filename_strp;
     if (!JS_HasProperty(cx, exnObject, filename_str, &found) || !found) {
         /* Now try "fileName", in case this quacks like an Error */
         filename_str = js_fileName_str;
--- a/js/src/jsexn.h
+++ b/js/src/jsexn.h
@@ -110,11 +110,25 @@ static inline JSExnType
 ExnTypeFromProtoKey(JSProtoKey key)
 {
     JSExnType type = static_cast<JSExnType>(key - JSProto_Error);
     MOZ_ASSERT(type >= JSEXN_ERR);
     MOZ_ASSERT(type < JSEXN_LIMIT);
     return type;
 }
 
+class AutoClearPendingException
+{
+    JSContext *cx;
+
+  public:
+    AutoClearPendingException(JSContext *cxArg)
+      : cx(cxArg)
+    { }
+
+    ~AutoClearPendingException() {
+        cx->clearPendingException();
+    }
+};
+
 } // namespace js
 
 #endif /* jsexn_h */