Bug 1251045 part 3. Remove the JSContext argument of WorkerRunnable::PostDispatch and its overrides. r=khuey

dom/base/WebSocket.cpp

@@ -2523,8 +2523,7 @@ public:
   }
 
   void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     // We don't call WorkerRunnable::PostDispatch because it would assert the
     // wrong thing about which thread we're on.
@@ -2704,8 +2703,7 @@ public:
   }
 
   void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     // We don't call WorkerRunnable::PreDispatch because it would assert the
     // wrong thing about which thread we're on.  We're on whichever thread the

dom/notification/Notification.cpp

@@ -382,8 +382,7 @@ protected:
   }
 
   void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     // We don't call WorkerRunnable::PostDispatch because it would assert the
     // wrong thing about which thread we're on.

dom/workers/RuntimeService.cpp

@@ -1076,8 +1076,7 @@ private:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     // May be called on any thread!
   }

dom/workers/ServiceWorkerPrivate.cpp

@@ -743,8 +743,7 @@ private:
   }
 
   void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     // Silence bad assertions.
   }

dom/workers/ServiceWorkerRegistration.cpp

@@ -338,8 +338,7 @@ public:
   }
 
   void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aSuccess) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aSuccess) override
   {
     if (!aSuccess) {
       mStatus.SuppressException();

dom/workers/WorkerPrivate.cpp

@@ -376,8 +376,7 @@ private:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     // Silence bad assertions.
   }
@@ -608,8 +607,7 @@ private:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     MOZ_CRASH("Don't call Dispatch() on CloseEventRunnable!");
   }
@@ -874,8 +872,7 @@ private:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     aWorkerPrivate->AssertIsOnParentThread();
     if (!aDispatchResult) {
@@ -1095,8 +1092,7 @@ private:
   { }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     aWorkerPrivate->AssertIsOnWorkerThread();
 
@@ -1191,8 +1187,7 @@ private:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     // Silence bad assertions.
   }
@@ -1238,8 +1233,7 @@ private:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     // Silence bad assertions.
   }
@@ -1286,8 +1280,7 @@ class KillCloseEventRunnable final : public WorkerRunnable
     }
 
     virtual void
-    PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-                 bool aDispatchResult) override
+    PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
     {
       // Silence bad assertions, this is dispatched from the timer thread.
     }
@@ -1351,8 +1344,7 @@ private:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     MOZ_CRASH("Don't call Dispatch() on KillCloseEventRunnable!");
   }
@@ -1507,8 +1499,7 @@ private:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-                bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     // Silence bad assertions, this can be dispatched from either the main
     // thread or the timer thread..
@@ -1697,9 +1688,7 @@ private:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx,
-               WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
   {
     MOZ_ASSERT_UNREACHABLE("Should never call Dispatch on this!");
   }

dom/workers/WorkerRunnable.cpp

@@ -70,9 +70,6 @@ WorkerRunnable::PreDispatch(WorkerPrivate* aWorkerPrivate)
       break;
 
     case WorkerThreadModifyBusyCount:
-      aWorkerPrivate->AssertIsOnParentThread();
-      break;
-
     case WorkerThreadUnchangedBusyCount:
       aWorkerPrivate->AssertIsOnParentThread();
       break;
@@ -99,7 +96,7 @@ WorkerRunnable::Dispatch(JSContext* aCx)
     if (ok) {
       ok = DispatchInternal();
     }
-    PostDispatch(nullptr, mWorkerPrivate, ok);
+    PostDispatch(mWorkerPrivate, ok);
     return ok;
   }
 
@@ -112,12 +109,18 @@ WorkerRunnable::Dispatch(JSContext* aCx)
     ac.emplace(aCx, global);
   }
 
+  MOZ_ASSERT(!JS_IsExceptionPending(aCx));
+
   ok = PreDispatch(mWorkerPrivate);
+  MOZ_ASSERT(!JS_IsExceptionPending(aCx));
+
   if (ok && !DispatchInternal()) {
     ok = false;
   }
+  MOZ_ASSERT(!JS_IsExceptionPending(aCx));
 
-  PostDispatch(aCx, mWorkerPrivate, ok);
+  PostDispatch(mWorkerPrivate, ok);
+  MOZ_ASSERT(!JS_IsExceptionPending(aCx));
 
   return ok;
 }
@@ -149,7 +152,7 @@ WorkerRunnable::DispatchInternal()
 }
 
 void
-WorkerRunnable::PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
+WorkerRunnable::PostDispatch(WorkerPrivate* aWorkerPrivate,
                              bool aDispatchResult)
 {
   MOZ_ASSERT(aWorkerPrivate);
@@ -162,7 +165,6 @@ WorkerRunnable::PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
 
     case WorkerThreadModifyBusyCount:
       aWorkerPrivate->AssertIsOnParentThread();
-      MOZ_ASSERT(aCx);
       break;
 
     case WorkerThreadUnchangedBusyCount:
@@ -178,9 +180,6 @@ WorkerRunnable::PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
     if (mBehavior == WorkerThreadModifyBusyCount) {
       aWorkerPrivate->ModifyBusyCount(false);
     }
-    if (aCx) {
-      JS_ReportPendingException(aCx);
-    }
   }
 }
 
@@ -405,19 +404,9 @@ WorkerRunnable::Cancel()
 }
 
 void
-WorkerDebuggerRunnable::PostDispatch(JSContext* aCx,
-                                     WorkerPrivate* aWorkerPrivate,
+WorkerDebuggerRunnable::PostDispatch(WorkerPrivate* aWorkerPrivate,
                                      bool aDispatchResult)
 {
-  // The only way aDispatchResult can be false here is if either PreDispatch or
-  // DispatchInternal returned false.
-  //
-  // PreDispatch can never throw on a JSContext.  We inherit DispatchInternal
-  // from WorkerRunnable and don't allow overriding it in our subclasses.
-  // WorkerRunnable::DispatchInternal only fails if one of its runnable
-  // dispatching functions fails, and none of those cases can throw a JS
-  // exception.  So we can never have a JS exception here.
-  MOZ_ASSERT_IF(aCx, !JS_IsExceptionPending(aCx));
 }
 
 WorkerSyncRunnable::WorkerSyncRunnable(WorkerPrivate* aWorkerPrivate,
@@ -461,22 +450,9 @@ WorkerSyncRunnable::DispatchInternal()
 }
 
 void
-MainThreadWorkerSyncRunnable::PostDispatch(JSContext* aCx,
-                                           WorkerPrivate* aWorkerPrivate,
+MainThreadWorkerSyncRunnable::PostDispatch(WorkerPrivate* aWorkerPrivate,
                                            bool aDispatchResult)
 {
-  // The only way aDispatchResult can be false here is if either PreDispatch or
-  // DispatchInternal returned false.
-  //
-  // PreDispatch can never throw on a JSContext.  We inherit DispatchInternal
-  // from WorkerSyncRunnable and don't allow overriding it in our subclasses.
-  // WorkerSyncRunnable::DispatchInternal only returns false if if dispatch to
-  // the syncloop target fails or if calling up to
-  // WorkerRunnable::DispatchInternal fails.  WorkerRunnable::DispatchInternal
-  // only fails if one of its runnable dispatching functions fails, and none of
-  // those cases can throw a JS exception.  So we can never have a JS exception
-  // here.
-  MOZ_ASSERT_IF(aCx, !JS_IsExceptionPending(aCx));
 }
 
 StopSyncLoopRunnable::StopSyncLoopRunnable(
@@ -530,18 +506,9 @@ StopSyncLoopRunnable::DispatchInternal()
 }
 
 void
-MainThreadStopSyncLoopRunnable::PostDispatch(JSContext* aCx,
-                                             WorkerPrivate* aWorkerPrivate,
+MainThreadStopSyncLoopRunnable::PostDispatch(WorkerPrivate* aWorkerPrivate,
                                              bool aDispatchResult)
 {
-  // The only way aDispatchResult can be false here is if either PreDispatch or
-  // DispatchInternal returned false.
-  //
-  // PreDispatch can never throw on a JSContext.  We inherit DispatchInternal
-  // from StopSyncLoopRunnable, and that itself is final and only returns false
-  // if dispatch to the syncloop target fails.  So we can never have a JS
-  // exception here.
-  MOZ_ASSERT_IF(aCx, !JS_IsExceptionPending(aCx));
 }
 
 #ifdef DEBUG
@@ -586,15 +553,10 @@ WorkerControlRunnable::DispatchInternal()
 }
 
 void
-MainThreadWorkerControlRunnable::PostDispatch(JSContext* aCx,
-                                              WorkerPrivate* aWorkerPrivate,
+MainThreadWorkerControlRunnable::PostDispatch(WorkerPrivate* aWorkerPrivate,
                                               bool aDispatchResult)
 {
   AssertIsOnMainThread();
-
-  if (aCx && !aDispatchResult) {
-    JS_ReportPendingException(aCx);
-  }
 }
 
 NS_IMPL_ISUPPORTS_INHERITED0(WorkerControlRunnable, WorkerRunnable)
@@ -663,8 +625,7 @@ WorkerSameThreadRunnable::PreDispatch(WorkerPrivate* aWorkerPrivate)
 }
 
 void
-WorkerSameThreadRunnable::PostDispatch(JSContext* aCx,
-                                       WorkerPrivate* aWorkerPrivate,
+WorkerSameThreadRunnable::PostDispatch(WorkerPrivate* aWorkerPrivate,
                                        bool aDispatchResult)
 {
   // We don't call WorkerRunnable::PostDispatch, because we're using

dom/workers/WorkerRunnable.h

@@ -121,12 +121,8 @@ protected:
 
   // By default asserts that Dispatch() is being called on the right thread
   // (ParentThread if |mTarget| is WorkerThread, or WorkerThread otherwise).
-  // Also reports any Dispatch() failures as an exception on |aCx|, and busy
-  // count if targeting the WorkerThread and Dispatch() failed.  The JSContext
-  // passed in here is the one that was passed to Dispatch().
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult);
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult);
 
   // Must be implemented by subclasses. Called on the target thread.  The return
   // value will be passed to PostRun().  The JSContext passed in here comes from
@@ -191,18 +187,8 @@ private:
     return true;
   }
 
-  // We want to be able to assert in PostDispatch that no exceptions were thrown
-  // on aCx.  We can do that if we know no one is subclassing our
-  // DispatchInternal to do weird things.
-  virtual bool
-  DispatchInternal() override final
-  {
-    return WorkerRunnable::DispatchInternal();
-  }
-
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override;
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override;
 };
 
 // This runnable is used to send a message directly to a worker's sync loop.
@@ -258,18 +244,8 @@ private:
     return true;
   }
 
-  // We want to be able to assert in PostDispatch that no exceptions were thrown
-  // on aCx.  We can do that if we know no one is subclassing our
-  // DispatchInternal to do weird things.
-  virtual bool
-  DispatchInternal() override final
-  {
-    return WorkerSyncRunnable::DispatchInternal();
-  }
-
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override;
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override;
 };
 
 // This runnable is used to stop a sync loop . As sync loops keep the busy count
@@ -303,8 +279,6 @@ private:
   virtual bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override;
 
-  // If this stops being final, reevaluate the assumptions
-  // MainThreadWorkerSyncRunnable::PostDispatch makes.
   virtual bool
   DispatchInternal() override final;
 };
@@ -337,8 +311,7 @@ private:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override;
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override;
 };
 
 // This runnable is processed as soon as it is received by the worker,
@@ -397,8 +370,7 @@ protected:
   }
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override;
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override;
 };
 
 // A WorkerRunnable that should be dispatched from the worker to itself for
@@ -421,8 +393,7 @@ protected:
   PreDispatch(WorkerPrivate* aWorkerPrivate) override;
 
   virtual void
-  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult) override;
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override;
 
   // We just delegate PostRun to WorkerRunnable, since it does exactly
   // what we want.