Bug 1545345 - Improve worker shutdown. r=baku,smaug

Differential Revision: https://phabricator.services.mozilla.com/D65132

--HG--
extra : moz-landing-system : lando

dom/base/nsIGlobalObject.cpp

@@ -30,6 +30,24 @@ using mozilla::dom::ServiceWorkerRegistration;
 using mozilla::dom::ServiceWorkerRegistrationDescriptor;
 using mozilla::dom::VoidFunction;
 
+bool nsIGlobalObject::IsScriptForbidden(JSObject* aCallback,
+                                        bool aIsJSImplementedWebIDL) const {
+  if (mIsScriptForbidden) {
+    return true;
+  }
+
+  if (NS_IsMainThread()) {
+    if (aIsJSImplementedWebIDL) {
+      return false;
+    }
+    if (!xpc::Scriptability::Get(aCallback).Allowed()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 nsIGlobalObject::~nsIGlobalObject() {
   UnlinkHostObjectURIs();
   DisconnectEventTargetObjects();

dom/base/nsIGlobalObject.h

@@ -51,11 +51,13 @@ class nsIGlobalObject : public nsISupports,
   mozilla::LinkedList<mozilla::DOMEventTargetHelper> mEventTargetObjects;
 
   bool mIsDying;
+  bool mIsScriptForbidden;
 
  protected:
   bool mIsInnerWindow;
 
-  nsIGlobalObject() : mIsDying(false), mIsInnerWindow(false) {}
+  nsIGlobalObject()
+      : mIsDying(false), mIsScriptForbidden(false), mIsInnerWindow(false) {}
 
  public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_IGLOBALOBJECT_IID)
@@ -76,6 +78,14 @@ class nsIGlobalObject : public nsISupports,
    */
   bool IsDying() const { return mIsDying; }
 
+  /**
+   * Is it currently forbidden to call into script?  JS-implemented WebIDL is
+   * a special case that's always allowed because it has the system principal,
+   * and callers should indicate this.
+   */
+  bool IsScriptForbidden(JSObject* aCallback,
+                         bool aIsJSImplementedWebIDL = false) const;
+
   /**
    * Return the JSObject for this global, if it still has one.  Otherwise return
    * null.
@@ -180,6 +190,9 @@ class nsIGlobalObject : public nsISupports,
 
   void StartDying() { mIsDying = true; }
 
+  void StartForbiddingScript() { mIsScriptForbidden = true; }
+  void StopForbiddingScript() { mIsScriptForbidden = false; }
+
   void DisconnectEventTargetObjects();
 
   size_t ShallowSizeOfExcludingThis(mozilla::MallocSizeOf aSizeOf) const;

dom/bindings/CallbackObject.cpp

@@ -223,22 +223,10 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback,
 
   {
     // First, find the real underlying callback.
-    JSObject* realCallback = js::UncheckedUnwrap(wrappedCallback);
+    JS::Rooted<JSObject*> realCallback(ccjs->RootingCx(),
+                                       js::UncheckedUnwrap(wrappedCallback));
 
-    // Check that it's ok to run this callback. JS-implemented WebIDL is always
-    // OK to run, since it runs with Chrome privileges anyway.
-    if (mIsMainThread && !aIsJSImplementedWebIDL) {
-      // Make sure to use realCallback to get the global of the callback
-      // object, not the wrapper.
-      if (!xpc::Scriptability::Get(realCallback).Allowed()) {
-        aRv.ThrowNotSupportedError(
-            "Refusing to execute function from global in which script is "
-            "disabled.");
-        return;
-      }
-    }
-
-    // Now get the global for this callback. Note that for the case of
+    // Get the global for this callback. Note that for the case of
     // JS-implemented WebIDL we never have a window here.
     nsGlobalWindowInner* win = mIsMainThread && !aIsJSImplementedWebIDL
                                    ? xpc::WindowGlobalOrNull(realCallback)
@@ -258,6 +246,15 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback,
       globalObject = xpc::NativeGlobal(realCallback);
       MOZ_ASSERT(globalObject);
     }
+
+    // Make sure to use realCallback to get the global of the callback
+    // object, not the wrapper.
+    if (globalObject->IsScriptForbidden(realCallback, aIsJSImplementedWebIDL)) {
+      aRv.ThrowNotSupportedError(
+          "Refusing to execute function from global in which script is "
+          "disabled.");
+      return;
+    }
   }
 
   // Bail out if there's no useful global.

dom/events/DOMEventTargetHelper.cpp

@@ -333,6 +333,10 @@ nsresult DOMEventTargetHelper::CheckCurrentGlobalCorrectness() const {
     return NS_ERROR_FAILURE;
   }
 
+  if (mParentObject->IsDying()) {
+    return NS_ERROR_FAILURE;
+  }
+
   return NS_OK;
 }
 

dom/events/DOMEventTargetHelper.h

@@ -131,6 +131,16 @@ class DOMEventTargetHelper : public dom::EventTarget,
     return nsPIDOMWindowOuter::GetFromCurrentInner(GetOwner());
   }
 
+  // A global permanently becomes invalid when DisconnectEventTargetObjects() is
+  // called.  Normally this means:
+  // - For the main thread, when nsGlobalWindowInner::FreeInnerObjects is
+  //   called.
+  // - For a worker thread, when clearing the main event queue.  (Which we do
+  //   slightly later than when the spec notionally calls for it to be done.)
+  //
+  // A global may also become temporarily invalid when:
+  // - For the main thread, if the window is no longer the WindowProxy's current
+  //   inner window due to being placed in the bfcache.
   nsresult CheckCurrentGlobalCorrectness() const;
 
   nsPIDOMWindowInner* GetOwner() const { return mOwnerWindow; }

dom/messagechannel/MessagePort.cpp

@@ -99,6 +99,10 @@ class PostMessageRunnable final : public CancelableRunnable {
   void DispatchMessage() const {
     NS_ASSERT_OWNINGTHREAD(Runnable);
 
+    if (NS_FAILED(mPort->CheckCurrentGlobalCorrectness())) {
+      return;
+    }
+
     nsCOMPtr<nsIGlobalObject> globalObject = mPort->GetParentObject();
 
     AutoJSAPI jsapi;

dom/workers/WorkerPrivate.cpp

@@ -3509,9 +3509,21 @@ WorkerPrivate::ProcessAllControlRunnablesLocked() {
 void WorkerPrivate::ClearMainEventQueue(WorkerRanOrNot aRanOrNot) {
   AssertIsOnWorkerThread();
 
-  MOZ_ASSERT(mSyncLoopStack.IsEmpty());
+  MOZ_ASSERT((mPostSyncLoopOperations & ePendingEventQueueClearing)
+                 ? (mSyncLoopStack.Length() == 1)
+                 : mSyncLoopStack.IsEmpty());
   MOZ_ASSERT(!mCancelAllPendingRunnables);
+
   mCancelAllPendingRunnables = true;
+  WorkerGlobalScope* globalScope = GlobalScope();
+  if (globalScope) {
+    // It's appropriate to disconnect event targets at the point that it's no
+    // longer possible for new tasks to be dispatched at the global, and this is
+    // that point.
+    globalScope->DisconnectEventTargetObjects();
+
+    globalScope->WorkerPrivateSaysForbidScript();
+  }
 
   if (WorkerNeverRan == aRanOrNot) {
     for (uint32_t count = mPreStartRunnables.Length(), index = 0; index < count;
@@ -3529,6 +3541,9 @@ void WorkerPrivate::ClearMainEventQueue(WorkerRanOrNot aRanOrNot) {
     ReportUseCounters();
   }
 
+  if (globalScope) {
+    globalScope->WorkerPrivateSaysAllowScript();
+  }
   MOZ_ASSERT(mCancelAllPendingRunnables);
   mCancelAllPendingRunnables = false;
 }
@@ -3905,22 +3920,12 @@ bool WorkerPrivate::DestroySyncLoop(uint32_t aLoopIndex) {
 
   bool result = loopInfo->mResult;
 
-  {
-    // Modifications must be protected by mMutex in DEBUG builds, see comment
-    // about mSyncLoopStack in WorkerPrivate.h.
-#ifdef DEBUG
-    MutexAutoLock lock(mMutex);
-#endif
-
-    // This will delete |loopInfo|!
-    mSyncLoopStack.RemoveElementAt(aLoopIndex);
-  }
-
   auto queue =
       static_cast<ThreadEventQueue<EventQueue>*>(mThread->EventQueue());
   queue->PopEventQueue(nestedEventTarget);
 
-  if (mSyncLoopStack.IsEmpty()) {
+  // Are we making a 1 -> 0 transition here?
+  if (mSyncLoopStack.Length() == 1) {
     if ((mPostSyncLoopOperations & ePendingEventQueueClearing)) {
       ClearMainEventQueue(WorkerRan);
     }
@@ -3932,6 +3937,17 @@ bool WorkerPrivate::DestroySyncLoop(uint32_t aLoopIndex) {
     mPostSyncLoopOperations = 0;
   }
 
+  {
+    // Modifications must be protected by mMutex in DEBUG builds, see comment
+    // about mSyncLoopStack in WorkerPrivate.h.
+#ifdef DEBUG
+    MutexAutoLock lock(mMutex);
+#endif
+
+    // This will delete |loopInfo|!
+    mSyncLoopStack.RemoveElementAt(aLoopIndex);
+  }
+
   return result;
 }
 

dom/workers/WorkerScope.cpp

@@ -182,10 +182,7 @@ JSObject* WorkerGlobalScope::WrapObject(JSContext* aCx,
   MOZ_CRASH("We should never get here!");
 }
 
-void WorkerGlobalScope::NoteTerminating() {
-  DisconnectEventTargetObjects();
-  StartDying();
-}
+void WorkerGlobalScope::NoteTerminating() { StartDying(); }
 
 already_AddRefed<Console> WorkerGlobalScope::GetConsole(ErrorResult& aRv) {
   mWorkerPrivate->AssertIsOnWorkerThread();

dom/workers/WorkerScope.h

@@ -234,6 +234,12 @@ class WorkerGlobalScope : public DOMEventTargetHelper,
   uint64_t WindowID() const;
 
   void FirstPartyStorageAccessGranted();
+
+  // WorkerPrivate wants to be able to forbid script when its state machine
+  // demands it.
+  friend WorkerPrivate;
+  void WorkerPrivateSaysForbidScript() { StartForbiddingScript(); }
+  void WorkerPrivateSaysAllowScript() { StopForbiddingScript(); }
 };
 
 class DedicatedWorkerGlobalScope final : public WorkerGlobalScope {