Bug 1872913 - Ensure mMainThreadDebuggeeEventTarget is not paused during canceling of a worker. r=asuth

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

dom/workers/EventWithOptionsRunnable.cpp

@@ -138,18 +138,6 @@ bool EventWithOptionsRunnable::WorkerRun(JSContext* aCx,
       return true;
     }
 
-    // Once a window has frozen its workers, their
-    // mMainThreadDebuggeeEventTargets should be paused, and their
-    // WorkerDebuggeeRunnables should not be being executed. The same goes for
-    // WorkerDebuggeeRunnables sent from child to parent workers, but since a
-    // frozen parent worker runs only control runnables anyway, that is taken
-    // care of naturally.
-    MOZ_ASSERT(!aWorkerPrivate->IsFrozen());
-
-    // Similarly for paused windows; all its workers should have been informed.
-    // (Subworkers are unaffected by paused windows.)
-    MOZ_ASSERT(!aWorkerPrivate->IsParentWindowPaused());
-
     aWorkerPrivate->AssertInnerWindowIsCorrect();
 
     return BuildAndFireEvent(aCx, aWorkerPrivate,

dom/workers/MessageEventRunnable.cpp

@@ -91,18 +91,6 @@ bool MessageEventRunnable::WorkerRun(JSContext* aCx,
       return true;
     }
 
-    // Once a window has frozen its workers, their
-    // mMainThreadDebuggeeEventTargets should be paused, and their
-    // WorkerDebuggeeRunnables should not be being executed. The same goes for
-    // WorkerDebuggeeRunnables sent from child to parent workers, but since a
-    // frozen parent worker runs only control runnables anyway, that is taken
-    // care of naturally.
-    MOZ_ASSERT(!aWorkerPrivate->IsFrozen());
-
-    // Similarly for paused windows; all its workers should have been informed.
-    // (Subworkers are unaffected by paused windows.)
-    MOZ_ASSERT(!aWorkerPrivate->IsParentWindowPaused());
-
     aWorkerPrivate->AssertInnerWindowIsCorrect();
 
     return DispatchDOMEvent(aCx, aWorkerPrivate,

dom/workers/WorkerError.cpp

@@ -92,18 +92,6 @@ class ReportErrorRunnable final : public WorkerDebuggeeRunnable {
     } else {
       AssertIsOnMainThread();
 
-      // Once a window has frozen its workers, their
-      // mMainThreadDebuggeeEventTargets should be paused, and their
-      // WorkerDebuggeeRunnables should not be being executed. The same goes for
-      // WorkerDebuggeeRunnables sent from child to parent workers, but since a
-      // frozen parent worker runs only control runnables anyway, that is taken
-      // care of naturally.
-      MOZ_ASSERT(!aWorkerPrivate->IsFrozen());
-
-      // Similarly for paused windows; all its workers should have been
-      // informed. (Subworkers are unaffected by paused windows.)
-      MOZ_ASSERT(!aWorkerPrivate->IsParentWindowPaused());
-
       if (aWorkerPrivate->IsSharedWorker()) {
         aWorkerPrivate->GetRemoteWorkerController()
             ->ErrorPropagationOnMainThread(mReport.get(),
@@ -175,17 +163,9 @@ class ReportGenericErrorRunnable final : public WorkerDebuggeeRunnable {
   }
 
   bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override {
-    // Once a window has frozen its workers, their
+    if (!aWorkerPrivate->IsAcceptingEvents()) {
-    // mMainThreadDebuggeeEventTargets should be paused, and their
+      return true;
-    // WorkerDebuggeeRunnables should not be being executed. The same goes for
+    }
-    // WorkerDebuggeeRunnables sent from child to parent workers, but since a
-    // frozen parent worker runs only control runnables anyway, that is taken
-    // care of naturally.
-    MOZ_ASSERT(!aWorkerPrivate->IsFrozen());
-
-    // Similarly for paused windows; all its workers should have been informed.
-    // (Subworkers are unaffected by paused windows.)
-    MOZ_ASSERT(!aWorkerPrivate->IsParentWindowPaused());
 
     if (aWorkerPrivate->IsSharedWorker()) {
       aWorkerPrivate->GetRemoteWorkerController()->ErrorPropagationOnMainThread(
@@ -206,10 +186,6 @@ class ReportGenericErrorRunnable final : public WorkerDebuggeeRunnable {
       return true;
     }
 
-    if (!aWorkerPrivate->IsAcceptingEvents()) {
-      return true;
-    }
-
     RefPtr<mozilla::dom::EventTarget> parentEventTarget =
         aWorkerPrivate->ParentEventTargetRef();
     RefPtr<Event> event =

dom/workers/WorkerPrivate.cpp

@@ -1779,6 +1779,14 @@ bool WorkerPrivate::Notify(WorkerStatus aStatus) {
     mCancelingTimer = nullptr;
   }
 
+  // The NotifyRunnable kicks off a series of events that need the
+  // CancelingOnParentRunnable to be executed always.
+  // Note that we already advanced mParentStatus above and we check that
+  // status in all other (asynchronous) call sites of SetIsPaused.
+  if (!mParent) {
+    MOZ_ALWAYS_SUCCEEDS(mMainThreadDebuggeeEventTarget->SetIsPaused(false));
+  }
+
   RefPtr<NotifyRunnable> runnable = new NotifyRunnable(this, aStatus);
   return runnable->Dispatch();
 }
@@ -1788,8 +1796,15 @@ bool WorkerPrivate::Freeze(const nsPIDOMWindowInner* aWindow) {
 
   mParentFrozen = true;
 
-  // WorkerDebuggeeRunnables sent from a worker to content must not be delivered
+  bool isCanceling = false;
-  // while the worker is frozen.
+  {
+    MutexAutoLock lock(mMutex);
+
+    isCanceling = mParentStatus >= Canceling;
+  }
+
+  // WorkerDebuggeeRunnables sent from a worker to content must not be
+  // delivered while the worker is frozen.
   //
   // Since a top-level worker and all its children share the same
   // mMainThreadDebuggeeEventTarget, it's sufficient to do this only in the
@@ -1799,16 +1814,13 @@ bool WorkerPrivate::Freeze(const nsPIDOMWindowInner* aWindow) {
     // allocated mMainThreadDebuggeeEventTarget yet.
     if (mMainThreadDebuggeeEventTarget) {
       // Pausing a ThrottledEventQueue is infallible.
-      MOZ_ALWAYS_SUCCEEDS(mMainThreadDebuggeeEventTarget->SetIsPaused(true));
+      MOZ_ALWAYS_SUCCEEDS(
+          mMainThreadDebuggeeEventTarget->SetIsPaused(!isCanceling));
     }
   }
 
-  {
+  if (isCanceling) {
-    MutexAutoLock lock(mMutex);
+    return true;
-
-    if (mParentStatus >= Canceling) {
-      return true;
-    }
   }
 
   DisableDebugger();
@@ -1823,26 +1835,32 @@ bool WorkerPrivate::Thaw(const nsPIDOMWindowInner* aWindow) {
 
   mParentFrozen = false;
 
-  // Delivery of WorkerDebuggeeRunnables to the window may resume.
-  //
-  // Since a top-level worker and all its children share the same
-  // mMainThreadDebuggeeEventTarget, it's sufficient to do this only in the
-  // top-level worker.
-  if (aWindow) {
-    // Since the worker is no longer frozen, only a paused parent window should
-    // require the queue to remain paused.
-    //
-    // This can only fail if the ThrottledEventQueue cannot dispatch its
-    // executor to the main thread, in which case the main thread was never
-    // going to draw runnables from it anyway, so the failure doesn't matter.
-    Unused << mMainThreadDebuggeeEventTarget->SetIsPaused(
-        IsParentWindowPaused());
-  }
-
   {
-    MutexAutoLock lock(mMutex);
+    bool isCanceling = false;
 
-    if (mParentStatus >= Canceling) {
+    {
+      MutexAutoLock lock(mMutex);
+
+      isCanceling = mParentStatus >= Canceling;
+    }
+
+    // Delivery of WorkerDebuggeeRunnables to the window may resume.
+    //
+    // Since a top-level worker and all its children share the same
+    // mMainThreadDebuggeeEventTarget, it's sufficient to do this only in the
+    // top-level worker.
+    if (aWindow) {
+      // Since the worker is no longer frozen, only a paused parent window
+      // should require the queue to remain paused.
+      //
+      // This can only fail if the ThrottledEventQueue cannot dispatch its
+      // executor to the main thread, in which case the main thread was never
+      // going to draw runnables from it anyway, so the failure doesn't matter.
+      Unused << mMainThreadDebuggeeEventTarget->SetIsPaused(
+          IsParentWindowPaused() && !isCanceling);
+    }
+
+    if (isCanceling) {
       return true;
     }
   }
@@ -1861,8 +1879,18 @@ void WorkerPrivate::ParentWindowPaused() {
   // This is called from WorkerPrivate construction, and we may not have
   // allocated mMainThreadDebuggeeEventTarget yet.
   if (mMainThreadDebuggeeEventTarget) {
-    // Pausing a ThrottledEventQueue is infallible.
+    bool isCanceling = false;
-    MOZ_ALWAYS_SUCCEEDS(mMainThreadDebuggeeEventTarget->SetIsPaused(true));
+
+    {
+      MutexAutoLock lock(mMutex);
+
+      isCanceling = mParentStatus >= Canceling;
+    }
+
+    // If we are already canceling we might wait for CancelingOnParentRunnable
+    // to be executed, so do not pause.
+    MOZ_ALWAYS_SUCCEEDS(
+        mMainThreadDebuggeeEventTarget->SetIsPaused(!isCanceling));
   }
 }
 
@@ -1872,12 +1900,11 @@ void WorkerPrivate::ParentWindowResumed() {
   MOZ_ASSERT(mParentWindowPaused);
   mParentWindowPaused = false;
 
+  bool isCanceling = false;
   {
     MutexAutoLock lock(mMutex);
 
-    if (mParentStatus >= Canceling) {
+    isCanceling = mParentStatus >= Canceling;
-      return;
-    }
   }
 
   // Since the window is no longer paused, the queue should only remain paused
@@ -1886,7 +1913,8 @@ void WorkerPrivate::ParentWindowResumed() {
   // This can only fail if the ThrottledEventQueue cannot dispatch its executor
   // to the main thread, in which case the main thread was never going to draw
   // runnables from it anyway, so the failure doesn't matter.
-  Unused << mMainThreadDebuggeeEventTarget->SetIsPaused(IsFrozen());
+  Unused << mMainThreadDebuggeeEventTarget->SetIsPaused(IsFrozen() &&
+                                                        !isCanceling);
 }
 
 void WorkerPrivate::PropagateStorageAccessPermissionGranted() {

dom/workers/WorkerPrivate.h

@@ -623,7 +623,7 @@ class WorkerPrivate final
 
   PerformanceStorage* GetPerformanceStorage();
 
-  bool IsAcceptingEvents() {
+  bool IsAcceptingEvents() MOZ_EXCLUDES(mMutex) {
     AssertIsOnParentThread();
 
     MutexAutoLock lock(mMutex);