Bug 1144750: Don't attempt to report errors that will cause the warnings only reporter to assert. r=bholley
authorDave Townsend <dtownsend@oxymoronical.com>
Wed, 18 Mar 2015 14:51:18 -0700
changeset 265001 29d74068461390bb18611efd4385e73db5907125
parent 265000 d8a86f570ca973f53495042a46ae988e186d8b1c
child 265002 0ac8484970b4303c26d1e60503e121a70045a518
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [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:
+    explicit AutoClearPendingException(JSContext *cxArg)
+      : cx(cxArg)
+    { }
+
+    ~AutoClearPendingException() {
+        cx->clearPendingException();
+    }
+};
+
 } // namespace js
 
 #endif /* jsexn_h */