Bug 1458836 - No needs to inform WorkerHolders when in Closing state, r=bkelly
authorAndrea Marchesini <amarchesini@mozilla.com>
Thu, 03 May 2018 17:03:13 +0200
changeset 472948 e73e0231a9e3e5abf3da65461cffad8f3cc2d6e9
parent 472947 3bcc98e23806594f2f9bccdd2596c37a5db7187d
child 472949 c07fc14c5821f15874e781fe57b4908b770a478c
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1458836
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1458836 - No needs to inform WorkerHolders when in Closing state, r=bkelly
dom/workers/WorkerHolder.cpp
dom/workers/WorkerHolder.h
dom/workers/WorkerPrivate.cpp
--- a/dom/workers/WorkerHolder.cpp
+++ b/dom/workers/WorkerHolder.cpp
@@ -38,16 +38,18 @@ WorkerHolder::~WorkerHolder()
 }
 
 bool
 WorkerHolder::HoldWorker(WorkerPrivate* aWorkerPrivate,
                          WorkerStatus aFailStatus)
 {
   AssertOnOwningThread(mThread);
   MOZ_ASSERT(aWorkerPrivate);
+  MOZ_ASSERT(aFailStatus >= Terminating);
+
   aWorkerPrivate->AssertIsOnWorkerThread();
 
   if (!aWorkerPrivate->AddHolder(this, aFailStatus)) {
     return false;
   }
 
   mWorkerPrivate = aWorkerPrivate;
   return true;
--- a/dom/workers/WorkerHolder.h
+++ b/dom/workers/WorkerHolder.h
@@ -13,42 +13,44 @@ namespace mozilla {
 namespace dom {
 
 class WorkerPrivate;
 
 /**
  * Use this chart to help figure out behavior during each of the closing
  * statuses. Details below.
  *
- * +=============================================+
- * |             Closing Statuses                |
- * +=============+=============+=================+
- * |    status   | clear queue | abort execution |
- * +=============+=============+=================+
- * |   Closing   |     yes     |       no        |
- * +-------------+-------------+-----------------+
- * | Terminating |     yes     |       yes       |
- * +-------------+-------------+-----------------+
- * |  Canceling  |     yes     |       yes       |
- * +-------------+-------------+-----------------+
- * |   Killing   |     yes     |       yes       |
- * +-------------+-------------+-----------------+
+ * +========================================================+
+ * |                     Closing Statuses                   |
+ * +=============+=============+=================+==========+
+ * |    status   | clear queue | abort execution | notified |
+ * +=============+=============+=================+==========+
+ * |   Closing   |     yes     |       no        |    no    |
+ * +-------------+-------------+-----------------+----------+
+ * | Terminating |     yes     |       yes       |   yes    |
+ * +-------------+-------------+-----------------+----------+
+ * |  Canceling  |     yes     |       yes       |   yes    |
+ * +-------------+-------------+-----------------+----------+
+ * |   Killing   |     yes     |       yes       |   yes    |
+ * +-------------+-------------+-----------------+----------+
  */
 
 enum WorkerStatus
 {
   // Not yet scheduled.
   Pending = 0,
 
   // This status means that the worker is active.
   Running,
 
   // Inner script called close() on the worker global scope. Setting this
   // status causes the worker to clear its queue of events but does not abort
-  // the currently running script.
+  // the currently running script. WorkerHolder/WorkerRef objects are not going
+  // to be notified because the behavior of APIs/Components should not change
+  // during this status yet.
   Closing,
 
   // Outer script called terminate() on the worker or the worker object was
   // garbage collected in its outer script. Setting this status causes the
   // worker to abort immediately and clear its queue of events.
   Terminating,
 
   // Either the user navigated away from the owning page or the owning page fell
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -4053,21 +4053,17 @@ WorkerPrivate::RemoveHolder(WorkerHolder
   }
 }
 
 void
 WorkerPrivate::NotifyHolders(WorkerStatus aStatus)
 {
   AssertIsOnWorkerThread();
 
-  NS_ASSERTION(aStatus > Running, "Bad status!");
-
-  if (aStatus >= Closing) {
-    CancelAllTimeouts();
-  }
+  NS_ASSERTION(aStatus > Closing, "Bad status!");
 
   nsTObserverArray<WorkerHolder*>::ForwardIterator iter(mHolders);
   while (iter.HasMore()) {
     WorkerHolder* holder = iter.GetNext();
     if (!holder->Notify(aStatus)) {
       NS_WARNING("Failed to notify holder!");
     }
   }
@@ -4561,18 +4557,24 @@ WorkerPrivate::NotifyInternal(WorkerStat
     // dispatched after we clear the queue below.
     if (aStatus == Closing) {
       Close();
     }
   }
 
   MOZ_ASSERT(previousStatus != Pending);
 
+  if (aStatus >= Closing) {
+    CancelAllTimeouts();
+  }
+
   // Let all our holders know the new status.
-  NotifyHolders(aStatus);
+  if (aStatus > Closing) {
+    NotifyHolders(aStatus);
+  }
 
   // If this is the first time our status has changed then we need to clear the
   // main event queue.
   if (previousStatus == Running) {
     // NB: If we're in a sync loop, we can't clear the queue immediately,
     // because this is the wrong queue. So we have to defer it until later.
     if (!mSyncLoopStack.IsEmpty()) {
       mPendingEventQueueClearing = true;