Bug 1072144 part 3. Hoist the exception reporting out of WorkerRunnable::PostRun into WorkerRunnable::Run and make it unconditional. r=khuey

dom/workers/WorkerPrivate.cpp

@@ -514,9 +514,9 @@ private:
       rv.SuppressException();
       return false;
     }
-    // Make sure to propagate exceptions from rv onto aCx, so that our PostRun
+    // Make sure to propagate exceptions from rv onto aCx, so that they will get
-    // can report it.  We do this for all failures on rv, because now we're
+    // reported after we return.  We do this for all failures on rv, because now
-    // using rv to track all the state we care about.
+    // we're using rv to track all the state we care about.
     //
     // This is a little dumb, but aCx is in the null compartment here because we
     // set it up that way in our Run(), since we had not created the global at
@@ -574,9 +574,9 @@ private:
       rv.SuppressException();
       return false;
     }
-    // Make sure to propagate exceptions from rv onto aCx, so that our PostRun
+    // Make sure to propagate exceptions from rv onto aCx, so that they will get
-    // can report it.  We do this for all failures on rv, because now we're
+    // reported after we return.  We do this for all failures on rv, because now
-    // using rv to track all the state we care about.
+    // we're using rv to track all the state we care about.
     if (rv.MaybeSetPendingException(aCx)) {
       return false;
     }

dom/workers/WorkerRunnable.cpp

@@ -184,10 +184,6 @@ WorkerRunnable::PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
   if (mBehavior == WorkerThreadModifyBusyCount) {
     aWorkerPrivate->ModifyBusyCountFromWorker(false);
   }
-
-  if (!aRunResult) {
-    JS_ReportPendingException(aCx);
-  }
 }
 
 // static
@@ -349,6 +345,8 @@ WorkerRunnable::Run()
   }
 
   bool result = WorkerRun(cx, mWorkerPrivate);
+  MOZ_ASSERT_IF(result, !JS_IsExceptionPending(cx));
+  JS_ReportPendingException(cx);
 
   // In the case of CompileScriptRunnnable, WorkerRun above can cause us to
   // lazily create a global, so we construct aes here before calling PostRun.

dom/workers/WorkerRunnable.h

@@ -136,6 +136,10 @@ protected:
   // multiple globals per worker???).  If it wasn't in a compartment, aCx will
   // be in either the debugger global's compartment or the worker's global's
   // compartment depending on whether IsDebuggerRunnable() is true.
+  //
+  // Immediately after WorkerRun returns, the caller will assert that either it
+  // returns false or there is no exception pending on aCx.  Then it will report
+  // any pending exceptions on aCx.
   virtual bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) = 0;
 
@@ -144,9 +148,7 @@ protected:
   // busy count was previously modified in PreDispatch().
   //
   // The aCx passed here is the same one as was passed to WorkerRun and is
-  // still in the same compartment.  If aRunResult is false, any failures on
+  // still in the same compartment.
-  // aCx are reported.  Otherwise failures are left to linger on the JSContext
-  // and maim later code (XXXbz: Aiming to fix that in bug 1072144).
   virtual void
   PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate, bool aRunResult);