Bug 1250975. Stop passing a JSContext argument to WorkerRunnable::PreDispatch and its overrides. r=khuey
☠☠ backed out by e048438a36a1 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 25 Feb 2016 16:05:39 -0500
changeset 334732 bb404647f14c6905477ab1e9cd412896b9c9e8cc
parent 334731 257324c2ae1725b5221082c5c22f986408290fd6
child 334733 5f8c7166c9262e1a5a38f06ee31793c5ee8856db
push id11625
push usercykesiopka.bmo@gmail.com
push dateThu, 25 Feb 2016 23:32:13 +0000
reviewerskhuey
bugs1250975
milestone47.0a1
Bug 1250975. Stop passing a JSContext argument to WorkerRunnable::PreDispatch and its overrides. r=khuey
dom/base/WebSocket.cpp
dom/notification/Notification.cpp
dom/workers/RuntimeService.cpp
dom/workers/ServiceWorkerPrivate.cpp
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerRunnable.cpp
dom/workers/WorkerRunnable.h
dom/workers/XMLHttpRequest.cpp
--- a/dom/base/WebSocket.cpp
+++ b/dom/base/WebSocket.cpp
@@ -2495,40 +2495,41 @@ class CancelRunnable final : public Work
 {
 public:
   CancelRunnable(WorkerPrivate* aWorkerPrivate, WebSocketImpl* aImpl)
     : WorkerRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount)
     , mImpl(aImpl)
   {
   }
 
-  bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
+  bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
     aWorkerPrivate->AssertIsOnWorkerThread();
     aWorkerPrivate->ModifyBusyCountFromWorker(aCx, true);
     return !NS_FAILED(mImpl->CancelInternal());
   }
 
-  void PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate, bool aRunResult)
+  void PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
+               bool aRunResult) override
   {
     aWorkerPrivate->ModifyBusyCountFromWorker(aCx, false);
   }
 
   bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     // We don't call WorkerRunnable::PreDispatch because it would assert the
     // wrong thing about which thread we're on.
     AssertIsOnMainThread();
     return true;
   }
 
   void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult)
+               bool aDispatchResult) override
   {
     // We don't call WorkerRunnable::PostDispatch because it would assert the
     // wrong thing about which thread we're on.
     AssertIsOnMainThread();
   }
 
 private:
   RefPtr<WebSocketImpl> mImpl;
@@ -2667,48 +2668,49 @@ public:
   WorkerRunnableDispatcher(WebSocketImpl* aImpl, WorkerPrivate* aWorkerPrivate,
                            already_AddRefed<nsIRunnable>&& aEvent)
     : WorkerRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount)
     , mWebSocketImpl(aImpl)
     , mEvent(aEvent)
   {
   }
 
-  bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
+  bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
     aWorkerPrivate->AssertIsOnWorkerThread();
     aWorkerPrivate->ModifyBusyCountFromWorker(aCx, true);
 
     // No messages when disconnected.
     if (mWebSocketImpl->mDisconnectingOrDisconnected) {
       NS_WARNING("Dispatching a WebSocket event after the disconnection!");
       return true;
     }
 
     return !NS_FAILED(mEvent->Run());
   }
 
-  void PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate, bool aRunResult)
+  void PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
+               bool aRunResult) override
   {
     aWorkerPrivate->ModifyBusyCountFromWorker(aCx, false);
   }
 
   bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
+  PreDispatch(WorkerPrivate* aWorkerPrivate) 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
     // channel implementation is running on (probably the main thread or socket
     // transport thread).
     return true;
   }
 
   void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
-               bool aDispatchResult)
+               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
     // channel implementation is running on (probably the main thread or socket
     // transport thread).
   }
 
 private:
--- a/dom/notification/Notification.cpp
+++ b/dom/notification/Notification.cpp
@@ -368,17 +368,17 @@ class NotificationWorkerRunnable : publi
 {
 protected:
   explicit NotificationWorkerRunnable(WorkerPrivate* aWorkerPrivate)
     : WorkerRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount)
   {
   }
 
   bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     // We don't call WorkerRunnable::PreDispatch because it would assert the
     // wrong thing about which thread we're on.
     AssertIsOnMainThread();
     return true;
   }
 
   void
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -1064,17 +1064,17 @@ public:
   WorkerTaskRunnable(WorkerPrivate* aWorkerPrivate, WorkerTask* aTask)
   : WorkerRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount), mTask(aTask)
   {
     MOZ_ASSERT(aTask);
   }
 
 private:
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     // May be called on any thread!
     return true;
   }
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                bool aDispatchResult) override
--- a/dom/workers/ServiceWorkerPrivate.cpp
+++ b/dom/workers/ServiceWorkerPrivate.cpp
@@ -728,17 +728,17 @@ public:
   ClearWindowAllowedRunnable(WorkerPrivate* aWorkerPrivate,
                              AllowWindowInteractionHandler* aHandler)
   : WorkerRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount)
   , mHandler(aHandler)
   { }
 
 private:
   bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     // WorkerRunnable asserts that the dispatch is from parent thread if
     // the busy count modification is WorkerThreadUnchangedBusyCount.
     // Since this runnable will be dispatched from the timer thread, we override
     // PreDispatch and PostDispatch to skip the check.
     return true;
   }
 
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -364,17 +364,17 @@ public:
   WorkerFinishedRunnable(WorkerPrivate* aWorkerPrivate,
                          WorkerPrivate* aFinishedWorker)
   : WorkerControlRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount),
     mFinishedWorker(aFinishedWorker)
   { }
 
 private:
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     // Silence bad assertions.
     return true;
   }
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                bool aDispatchResult) override
@@ -589,17 +589,17 @@ class CloseEventRunnable final : public 
 {
 public:
   explicit CloseEventRunnable(WorkerPrivate* aWorkerPrivate)
   : WorkerRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount)
   { }
 
 private:
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     MOZ_CRASH("Don't call Dispatch() on CloseEventRunnable!");
   }
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                bool aDispatchResult) override
   {
@@ -852,17 +852,17 @@ public:
   // ModifyBusyCount... but that doesn't actually use its JSContext argument.
   bool Dispatch()
   {
     return WorkerControlRunnable::Dispatch(nullptr);
   }
 
 private:
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     aWorkerPrivate->AssertIsOnParentThread();
     // Modify here, but not in PostRun! This busy count addition will be matched
     // by the CloseEventRunnable.
     return aWorkerPrivate->ModifyBusyCount(true);
   }
 
   virtual void
@@ -1171,17 +1171,17 @@ public:
   explicit TimerRunnable(WorkerPrivate* aWorkerPrivate)
   : WorkerRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount)
   { }
 
 private:
   ~TimerRunnable() {}
 
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     // Silence bad assertions.
     return true;
   }
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                bool aDispatchResult) override
@@ -1218,17 +1218,17 @@ public:
 private:
   virtual bool
   IsDebuggerRunnable() const override
   {
     return true;
   }
 
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     // Silence bad assertions.
     return true;
   }
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                bool aDispatchResult) override
@@ -1266,17 +1266,17 @@ class KillCloseEventRunnable final : pub
   {
   public:
     explicit KillScriptRunnable(WorkerPrivate* aWorkerPrivate)
     : WorkerControlRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount)
     { }
 
   private:
     virtual bool
-    PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+    PreDispatch(WorkerPrivate* aWorkerPrivate) override
     {
       // Silence bad assertions, this is dispatched from the timer thread.
       return true;
     }
 
     virtual void
     PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                  bool aDispatchResult) override
@@ -1332,17 +1332,17 @@ private:
   ~KillCloseEventRunnable()
   {
     if (mTimer) {
       mTimer->Cancel();
     }
   }
 
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     MOZ_CRASH("Don't call Dispatch() on KillCloseEventRunnable!");
   }
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                bool aDispatchResult) override
   {
@@ -1486,17 +1486,17 @@ public:
   GarbageCollectRunnable(WorkerPrivate* aWorkerPrivate, bool aShrinking,
                          bool aCollectChildren)
   : WorkerControlRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount),
     mShrinking(aShrinking), mCollectChildren(aCollectChildren)
   { }
 
 private:
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     // Silence bad assertions, this can be dispatched from either the main
     // thread or the timer thread..
     return true;
   }
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
@@ -1677,17 +1677,17 @@ public:
 
 private:
   ~DummyRunnable()
   {
     mWorkerPrivate->AssertIsOnWorkerThread();
   }
 
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     MOZ_ASSERT_UNREACHABLE("Should never call Dispatch on this!");
     return true;
   }
 
   virtual void
   PostDispatch(JSContext* aCx,
                WorkerPrivate* aWorkerPrivate,
--- a/dom/workers/WorkerRunnable.cpp
+++ b/dom/workers/WorkerRunnable.cpp
@@ -54,29 +54,28 @@ WorkerRunnable::DefaultGlobalObject() co
   if (IsDebuggerRunnable()) {
     return mWorkerPrivate->DebuggerGlobalScope();
   } else {
     return mWorkerPrivate->GlobalScope();
   }
 }
 
 bool
-WorkerRunnable::PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
+WorkerRunnable::PreDispatch(WorkerPrivate* aWorkerPrivate)
 {
 #ifdef DEBUG
   MOZ_ASSERT(aWorkerPrivate);
 
   switch (mBehavior) {
     case ParentThreadUnchangedBusyCount:
       aWorkerPrivate->AssertIsOnWorkerThread();
       break;
 
     case WorkerThreadModifyBusyCount:
       aWorkerPrivate->AssertIsOnParentThread();
-      MOZ_ASSERT(aCx);
       break;
 
     case WorkerThreadUnchangedBusyCount:
       aWorkerPrivate->AssertIsOnParentThread();
       break;
 
     default:
       MOZ_ASSERT_UNREACHABLE("Unknown behavior!");
@@ -91,34 +90,35 @@ WorkerRunnable::PreDispatch(JSContext* a
 }
 
 bool
 WorkerRunnable::Dispatch(JSContext* aCx)
 {
   bool ok;
 
   if (!aCx) {
-    ok = PreDispatch(nullptr, mWorkerPrivate);
+    ok = PreDispatch(mWorkerPrivate);
     if (ok) {
       ok = DispatchInternal();
     }
     PostDispatch(nullptr, mWorkerPrivate, ok);
     return ok;
   }
 
   JSAutoRequest ar(aCx);
 
   JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
 
   Maybe<JSAutoCompartment> ac;
   if (global) {
     ac.emplace(aCx, global);
   }
 
-  ok = PreDispatch(aCx, mWorkerPrivate);
+  ok = PreDispatch(mWorkerPrivate);
+  MOZ_ASSERT(!JS_IsExceptionPending(aCx));
 
   if (ok && !DispatchInternal()) {
     ok = false;
   }
 
   PostDispatch(aCx, mWorkerPrivate, ok);
 
   return ok;
@@ -409,21 +409,21 @@ WorkerRunnable::Cancel()
 void
 WorkerDebuggerRunnable::PostDispatch(JSContext* aCx,
                                      WorkerPrivate* aWorkerPrivate,
                                      bool aDispatchResult)
 {
   // The only way aDispatchResult can be false here is if either PreDispatch or
   // DispatchInternal returned false.
   //
-  // Our PreDispatch always returns true and is final.  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.
+  // 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,
                                        nsIEventTarget* aSyncLoopTarget)
 : WorkerRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount),
   mSyncLoopTarget(aSyncLoopTarget)
 {
@@ -465,20 +465,20 @@ WorkerSyncRunnable::DispatchInternal()
 void
 MainThreadWorkerSyncRunnable::PostDispatch(JSContext* aCx,
                                            WorkerPrivate* aWorkerPrivate,
                                            bool aDispatchResult)
 {
   // The only way aDispatchResult can be false here is if either PreDispatch or
   // DispatchInternal returned false.
   //
-  // Our PreDispatch always returns true and is final.  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
+  // 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(
@@ -534,20 +534,20 @@ StopSyncLoopRunnable::DispatchInternal()
 void
 MainThreadStopSyncLoopRunnable::PostDispatch(JSContext* aCx,
                                              WorkerPrivate* aWorkerPrivate,
                                              bool aDispatchResult)
 {
   // The only way aDispatchResult can be false here is if either PreDispatch or
   // DispatchInternal returned false.
   //
-  // Our PreDispatch always returns true and is final.  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.
+  // 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
 WorkerControlRunnable::WorkerControlRunnable(WorkerPrivate* aWorkerPrivate,
                                              TargetAndBusyBehavior aBehavior)
 : WorkerRunnable(aWorkerPrivate, aBehavior)
 {
@@ -650,18 +650,17 @@ WorkerCheckAPIExposureOnMainThreadRunnab
   ErrorResult rv;
   WorkerMainThreadRunnable::Dispatch(rv);
   bool ok = !rv.Failed();
   rv.SuppressException();
   return ok;
 }
 
 bool
-WorkerSameThreadRunnable::PreDispatch(JSContext* aCx,
-                                      WorkerPrivate* aWorkerPrivate)
+WorkerSameThreadRunnable::PreDispatch(WorkerPrivate* aWorkerPrivate)
 {
   // We don't call WorkerRunnable::PreDispatch, because we're using
   // WorkerThreadModifyBusyCount for mBehavior, and WorkerRunnable will assert
   // that PreDispatch is on the parent thread in that case.
   aWorkerPrivate->AssertIsOnWorkerThread();
   return true;
 }
 
--- a/dom/workers/WorkerRunnable.h
+++ b/dom/workers/WorkerRunnable.h
@@ -110,20 +110,19 @@ protected:
   IsDebuggerRunnable() const;
 
   nsIGlobalObject*
   DefaultGlobalObject() const;
 
   // By default asserts that Dispatch() is being called on the right thread
   // (ParentThread if |mTarget| is WorkerThread, or WorkerThread otherwise).
   // Also increments the busy count of |mWorkerPrivate| if targeting the
-  // WorkerThread.  The JSContext passed in here is the one that was passed to
-  // Dispatch().
+  // WorkerThread.
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate);
+  PreDispatch(WorkerPrivate* aWorkerPrivate);
 
   // 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,
@@ -180,17 +179,17 @@ protected:
 private:
   virtual bool
   IsDebuggerRunnable() const override
   {
     return true;
   }
 
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override final
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override final
   {
     AssertIsOnMainThread();
 
     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
@@ -246,32 +245,21 @@ protected:
   : WorkerSyncRunnable(aWorkerPrivate, Move(aSyncLoopTarget))
   {
     AssertIsOnMainThread();
   }
 
   virtual ~MainThreadWorkerSyncRunnable()
   { }
 
-  // Hook for subclasses that want to override our PreDispatch.  This override
-  // must be infallible and must not leave an exception hanging out on the
-  // JSContext.  We pass the JSContext through so callees that expect to be able
-  // to do something with script have a way to do it.  We'd pass an AutoJSAPI,
-  // but some of our subclasses are dispatched with a null JSContext*, so we
-  // can't do that sort of thing ourselves.
-  virtual void InfalliblePreDispatch(JSContext* aCx)
-  {}
-
 private:
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override final
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     AssertIsOnMainThread();
-    InfalliblePreDispatch(aCx);
-    MOZ_ASSERT_IF(aCx, !JS_IsExceptionPending(aCx));
     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
@@ -336,20 +324,18 @@ public:
     AssertIsOnMainThread();
   }
 
 protected:
   virtual ~MainThreadStopSyncLoopRunnable()
   { }
 
 private:
-  // If this function stops being final, reevaluate the assumptions PostDispatch
-  // makes.
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override final
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override final
   {
     AssertIsOnMainThread();
     return true;
   }
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                bool aDispatchResult) override;
@@ -399,17 +385,17 @@ protected:
   explicit MainThreadWorkerControlRunnable(WorkerPrivate* aWorkerPrivate)
   : WorkerControlRunnable(aWorkerPrivate, WorkerThreadUnchangedBusyCount)
   { }
 
   virtual ~MainThreadWorkerControlRunnable()
   { }
 
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override
   {
     AssertIsOnMainThread();
     return true;
   }
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                bool aDispatchResult) override;
@@ -427,17 +413,17 @@ protected:
   explicit WorkerSameThreadRunnable(WorkerPrivate* aWorkerPrivate)
   : WorkerRunnable(aWorkerPrivate, WorkerThreadModifyBusyCount)
   { }
 
   virtual ~WorkerSameThreadRunnable()
   { }
 
   virtual bool
-  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override;
+  PreDispatch(WorkerPrivate* aWorkerPrivate) override;
 
   virtual void
   PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
                bool aDispatchResult) override;
 
   // We just delegate PostRun to WorkerRunnable, since it does exactly
   // what we want.
 };
--- a/dom/workers/XMLHttpRequest.cpp
+++ b/dom/workers/XMLHttpRequest.cpp
@@ -536,16 +536,20 @@ class EventRunnable final : public MainT
   uint16_t mReadyState;
   bool mUploadEvent;
   bool mProgressEvent;
   bool mLengthComputable;
   bool mUseCachedArrayBufferResponse;
   nsresult mResponseTextResult;
   nsresult mStatusResult;
   nsresult mResponseResult;
+  // mScopeObj is used in PreDispatch.  It's passed to our constructor as a
+  // handle, and we only use it while under Dispatch.  Also, the caller ensures
+  // that this scope outlives us.
+  JS::Handle<JSObject*> mScopeObj;
 
 public:
   class MOZ_RAII StateDataAutoRooter : private JS::CustomAutoRooter
   {
     XMLHttpRequest::StateData* mStateData;
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 
   public:
@@ -560,44 +564,51 @@ public:
     virtual void trace(JSTracer* aTrc)
     {
       JS::TraceEdge(aTrc, &mStateData->mResponse,
                     "XMLHttpRequest::StateData::mResponse");
     }
   };
 
   EventRunnable(Proxy* aProxy, bool aUploadEvent, const nsString& aType,
-                bool aLengthComputable, uint64_t aLoaded, uint64_t aTotal)
+                bool aLengthComputable, uint64_t aLoaded, uint64_t aTotal,
+                JS::Handle<JSObject*> aScopeObj)
   : MainThreadProxyRunnable(aProxy->mWorkerPrivate, aProxy),
     StructuredCloneHolder(CloningSupported, TransferringNotSupported,
                           SameProcessDifferentThread),
     mType(aType), mResponse(JS::UndefinedValue()), mLoaded(aLoaded),
     mTotal(aTotal), mEventStreamId(aProxy->mInnerEventStreamId), mStatus(0),
     mReadyState(0), mUploadEvent(aUploadEvent), mProgressEvent(true),
     mLengthComputable(aLengthComputable), mUseCachedArrayBufferResponse(false),
-    mResponseTextResult(NS_OK), mStatusResult(NS_OK), mResponseResult(NS_OK)
+    mResponseTextResult(NS_OK), mStatusResult(NS_OK), mResponseResult(NS_OK),
+    mScopeObj(aScopeObj)
   { }
 
-  EventRunnable(Proxy* aProxy, bool aUploadEvent, const nsString& aType)
+  EventRunnable(Proxy* aProxy, bool aUploadEvent, const nsString& aType,
+                JS::Handle<JSObject*> aScopeObj)
   : MainThreadProxyRunnable(aProxy->mWorkerPrivate, aProxy),
     StructuredCloneHolder(CloningSupported, TransferringNotSupported,
                           SameProcessDifferentThread),
     mType(aType), mResponse(JS::UndefinedValue()), mLoaded(0), mTotal(0),
     mEventStreamId(aProxy->mInnerEventStreamId), mStatus(0), mReadyState(0),
     mUploadEvent(aUploadEvent), mProgressEvent(false), mLengthComputable(0),
     mUseCachedArrayBufferResponse(false), mResponseTextResult(NS_OK),
-    mStatusResult(NS_OK), mResponseResult(NS_OK)
+    mStatusResult(NS_OK), mResponseResult(NS_OK), mScopeObj(aScopeObj)
   { }
 
+  void Dispatch()
+  {
+    MainThreadProxyRunnable::Dispatch(nullptr);
+  }
 private:
   ~EventRunnable()
   { }
 
-  virtual void
-  InfalliblePreDispatch(JSContext* aCx) override final;
+  virtual bool
+  PreDispatch(WorkerPrivate* /* unused */) override final;
 
   virtual bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override;
 };
 
 class SyncTeardownRunnable final : public WorkerThreadProxySyncRunnable
 {
 public:
@@ -1050,49 +1061,48 @@ Proxy::HandleEvent(nsIDOMEvent* aEvent)
   if (NS_FAILED(aEvent->GetTarget(getter_AddRefs(target)))) {
     NS_WARNING("Failed to get target!");
     return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsIXMLHttpRequestUpload> uploadTarget = do_QueryInterface(target);
   ProgressEvent* progressEvent = aEvent->InternalDOMEvent()->AsProgressEvent();
 
-  RefPtr<EventRunnable> runnable;
-
   if (mInOpen && type.EqualsASCII(sEventStrings[STRING_readystatechange])) {
     uint16_t readyState = 0;
     if (NS_SUCCEEDED(mXHR->GetReadyState(&readyState)) &&
         readyState == nsIXMLHttpRequest::OPENED) {
       mInnerEventStreamId++;
     }
   }
 
-  if (progressEvent) {
-    runnable = new EventRunnable(this, !!uploadTarget, type,
-                                 progressEvent->LengthComputable(),
-                                 progressEvent->Loaded(),
-                                 progressEvent->Total());
-  }
-  else {
-    runnable = new EventRunnable(this, !!uploadTarget, type);
-  }
-
   {
     AutoSafeJSContext cx;
     JSAutoRequest ar(cx);
 
     JS::Rooted<JS::Value> value(cx);
     if (!GetOrCreateDOMReflectorNoWrap(cx, mXHR, &value)) {
       return NS_ERROR_FAILURE;
     }
 
     JS::Rooted<JSObject*> scope(cx, &value.toObject());
-    JSAutoCompartment ac(cx, scope);
-
-    runnable->Dispatch(cx);
+
+    RefPtr<EventRunnable> runnable;
+    if (progressEvent) {
+      runnable = new EventRunnable(this, !!uploadTarget, type,
+                                   progressEvent->LengthComputable(),
+                                   progressEvent->Loaded(),
+                                   progressEvent->Total(),
+                                   scope);
+    }
+    else {
+      runnable = new EventRunnable(this, !!uploadTarget, type, scope);
+    }
+
+    runnable->Dispatch();
   }
 
   if (!uploadTarget) {
     if (type.EqualsASCII(sEventStrings[STRING_loadstart])) {
       NS_ASSERTION(!mMainThreadSeenLoadStart, "Huh?!");
       mMainThreadSeenLoadStart = true;
     }
     else if (mMainThreadSeenLoadStart &&
@@ -1165,82 +1175,79 @@ LoadStartDetectionRunnable::HandleEvent(
     }
   }
 #endif
 
   mReceivedLoadStart = true;
   return NS_OK;
 }
 
-void
-EventRunnable::InfalliblePreDispatch(JSContext* aCx)
+bool
+EventRunnable::PreDispatch(WorkerPrivate* /* unused */)
 {
   AssertIsOnMainThread();
-  MOZ_ASSERT(aCx);
-  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
 
   AutoJSAPI jsapi;
-  DebugOnly<bool> ok =
-    jsapi.Init(xpc::NativeGlobal(JS::CurrentGlobalOrNull(aCx)), aCx);
+  DebugOnly<bool> ok = jsapi.Init(xpc::NativeGlobal(mScopeObj));
   MOZ_ASSERT(ok);
-  MOZ_ASSERT(jsapi.cx() == aCx);
   jsapi.TakeOwnershipOfErrorReporting();
+  JSContext* cx = jsapi.cx();
 
   RefPtr<nsXMLHttpRequest>& xhr = mProxy->mXHR;
   MOZ_ASSERT(xhr);
 
   if (NS_FAILED(xhr->GetResponseType(mResponseType))) {
     MOZ_ASSERT(false, "This should never fail!");
   }
 
   mResponseTextResult = xhr->GetResponseText(mResponseText);
   if (NS_SUCCEEDED(mResponseTextResult)) {
     mResponseResult = mResponseTextResult;
     if (mResponseText.IsVoid()) {
       mResponse.setNull();
     }
   }
   else {
-    JS::Rooted<JS::Value> response(aCx);
-    mResponseResult = xhr->GetResponse(aCx, &response);
+    JS::Rooted<JS::Value> response(cx);
+    mResponseResult = xhr->GetResponse(cx, &response);
     if (NS_SUCCEEDED(mResponseResult)) {
       if (!response.isGCThing()) {
         mResponse = response;
       } else {
         bool doClone = true;
-        JS::Rooted<JS::Value> transferable(aCx);
-        JS::Rooted<JSObject*> obj(aCx, response.isObjectOrNull() ?
+        JS::Rooted<JS::Value> transferable(cx);
+        JS::Rooted<JSObject*> obj(cx, response.isObjectOrNull() ?
                                   response.toObjectOrNull() : nullptr);
         if (obj && JS_IsArrayBufferObject(obj)) {
           // Use cached response if the arraybuffer has been transfered.
           if (mProxy->mArrayBufferResponseWasTransferred) {
             MOZ_ASSERT(JS_IsDetachedArrayBufferObject(obj));
             mUseCachedArrayBufferResponse = true;
             doClone = false;
           } else {
             MOZ_ASSERT(!JS_IsDetachedArrayBufferObject(obj));
-            JS::AutoValueArray<1> argv(aCx);
+            JS::AutoValueArray<1> argv(cx);
             argv[0].set(response);
-            obj = JS_NewArrayObject(aCx, argv);
+            obj = JS_NewArrayObject(cx, argv);
             if (obj) {
               transferable.setObject(*obj);
               // Only cache the response when the readyState is DONE.
               if (xhr->ReadyState() == nsIXMLHttpRequest::DONE) {
                 mProxy->mArrayBufferResponseWasTransferred = true;
               }
             } else {
               mResponseResult = NS_ERROR_OUT_OF_MEMORY;
               doClone = false;
             }
           }
         }
 
         if (doClone) {
           ErrorResult rv;
-          Write(aCx, response, transferable, rv);
+          Write(cx, response, transferable, rv);
           if (NS_WARN_IF(rv.Failed())) {
             NS_WARNING("Failed to clone response!");
             mResponseResult = rv.StealNSResult();
             mProxy->mArrayBufferResponseWasTransferred = false;
           }
         }
       }
     }
@@ -1248,16 +1255,18 @@ EventRunnable::InfalliblePreDispatch(JSC
 
   mStatusResult = xhr->GetStatus(&mStatus);
 
   xhr->GetStatusText(mStatusText);
 
   mReadyState = xhr->ReadyState();
 
   xhr->GetResponseURL(mResponseURL);
+
+  return true;
 }
 
 bool
 EventRunnable::WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
 {
   if (mEventStreamId != mProxy->mOuterEventStreamId) {
     // Threads raced, this event is now obsolete.
     return true;