Bug 1438945 - Fix the suspend and freezing logic. r=asuth
authorBlake Kaplan <mrbkap@gmail.com>
Mon, 19 Nov 2018 15:18:33 -0800
changeset 447140 c52099a44a4e780047f71f4a3a5eec59cf2e7b1c
parent 447139 93c96481ec70d75496045840d5c42c90ee8ec761
child 447141 c9e6293a64a6e3d120ae2b7c56965e9249d5877b
push id35067
push userrmaries@mozilla.com
push dateTue, 20 Nov 2018 05:04:04 +0000
treeherdermozilla-central@74c27e915067 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1438945
milestone65.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 1438945 - Fix the suspend and freezing logic. r=asuth This implements the behavior that as long as there's one non-frozen or non-suspended actor, we resume or thaw the manager. Differential Revision: https://phabricator.services.mozilla.com/D11822
dom/workers/sharedworkers/SharedWorkerManager.cpp
dom/workers/sharedworkers/SharedWorkerManager.h
dom/workers/sharedworkers/SharedWorkerParent.cpp
dom/workers/sharedworkers/SharedWorkerParent.h
--- a/dom/workers/sharedworkers/SharedWorkerManager.cpp
+++ b/dom/workers/sharedworkers/SharedWorkerManager.cpp
@@ -22,16 +22,18 @@ SharedWorkerManager::SharedWorkerManager
                                          const RemoteWorkerData& aData,
                                          nsIPrincipal* aLoadingPrincipal)
   : mPBackgroundEventTarget(aPBackgroundEventTarget)
   , mLoadingPrincipal(aLoadingPrincipal)
   , mDomain(aData.domain())
   , mResolvedScriptURL(aData.resolvedScriptURL())
   , mName(aData.name())
   , mIsSecureContext(aData.isSecureContext())
+  , mSuspended(false)
+  , mFrozen(false)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aLoadingPrincipal);
 }
 
 SharedWorkerManager::~SharedWorkerManager()
 {
   nsCOMPtr<nsIEventTarget> target =
@@ -85,31 +87,22 @@ SharedWorkerManager::MatchOnMainThread(c
 
 void
 SharedWorkerManager::AddActor(SharedWorkerParent* aParent)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aParent);
   MOZ_ASSERT(!mActors.Contains(aParent));
 
-  uint32_t frozen = 0;
-
-  for (SharedWorkerParent* actor : mActors) {
-    if (actor->IsFrozen()) {
-      ++frozen;
-    }
-  }
-
-  bool hadActors = !mActors.IsEmpty();
-
   mActors.AppendElement(aParent);
 
-  if (hadActors && frozen == mActors.Length() - 1) {
-    mRemoteWorkerController->Thaw();
-  }
+  // NB: We don't update our Suspended/Frozen state here, yet. The aParent is
+  // responsible for doing so from SharedWorkerParent::ManagerCreated.
+  // XXX But we could avoid iterating all of our actors because if aParent is
+  // not frozen and we are, we would just need to thaw ourselves.
 }
 
 void
 SharedWorkerManager::RemoveActor(SharedWorkerParent* aParent)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aParent);
   MOZ_ASSERT(mActors.Contains(aParent));
@@ -117,16 +110,19 @@ SharedWorkerManager::RemoveActor(SharedW
   uint64_t windowID = aParent->WindowID();
   if (windowID) {
     mRemoteWorkerController->RemoveWindowID(windowID);
   }
 
   mActors.RemoveElement(aParent);
 
   if (!mActors.IsEmpty()) {
+    // Our remaining actors could be all suspended or frozen.
+    UpdateSuspend();
+    UpdateFrozen();
     return;
   }
 
   // Time to go.
 
   mRemoteWorkerController->Terminate();
   mRemoteWorkerController = nullptr;
 
@@ -143,23 +139,28 @@ SharedWorkerManager::UpdateSuspend()
   uint32_t suspended = 0;
 
   for (SharedWorkerParent* actor : mActors) {
     if (actor->IsSuspended()) {
       ++suspended;
     }
   }
 
-  if (suspended != 0 && suspended != mActors.Length()) {
+  // Call Suspend only when all of our actors' windows are suspended and call
+  // Resume only when one of them resumes.
+  if ((mSuspended && suspended == mActors.Length()) ||
+      (!mSuspended && suspended != mActors.Length())) {
     return;
   }
 
-  if (suspended) {
+  if (!mSuspended) {
+    mSuspended = true;
     mRemoteWorkerController->Suspend();
   } else {
+    mSuspended = false;
     mRemoteWorkerController->Resume();
   }
 }
 
 void
 SharedWorkerManager::UpdateFrozen()
 {
   AssertIsOnBackgroundThread();
@@ -168,23 +169,28 @@ SharedWorkerManager::UpdateFrozen()
   uint32_t frozen = 0;
 
   for (SharedWorkerParent* actor : mActors) {
     if (actor->IsFrozen()) {
       ++frozen;
     }
   }
 
-  if (frozen != 0 && frozen != mActors.Length()) {
+  // Similar to UpdateSuspend, above, we only want to be frozen when all of our
+  // actors are frozen.
+  if ((mFrozen && frozen == mActors.Length()) ||
+      (!mFrozen && frozen != mActors.Length())) {
     return;
   }
 
-  if (frozen) {
+  if (!mFrozen) {
+    mFrozen = true;
     mRemoteWorkerController->Freeze();
   } else {
+    mFrozen = false;
     mRemoteWorkerController->Thaw();
   }
 }
 
 bool
 SharedWorkerManager::IsSecureContext() const
 {
   return mIsSecureContext;
--- a/dom/workers/sharedworkers/SharedWorkerManager.h
+++ b/dom/workers/sharedworkers/SharedWorkerManager.h
@@ -79,16 +79,18 @@ private:
 
   nsCOMPtr<nsIEventTarget> mPBackgroundEventTarget;
 
   nsCOMPtr<nsIPrincipal> mLoadingPrincipal;
   nsCString mDomain;
   nsCString mResolvedScriptURL;
   nsString mName;
   bool mIsSecureContext;
+  bool mSuspended;
+  bool mFrozen;
 
   // Raw pointers because SharedWorkerParent unregisters itself in ActorDestroy().
   nsTArray<SharedWorkerParent*> mActors;
 
   RefPtr<RemoteWorkerController> mRemoteWorkerController;
 };
 
 } // dom namespace
--- a/dom/workers/sharedworkers/SharedWorkerParent.cpp
+++ b/dom/workers/sharedworkers/SharedWorkerParent.cpp
@@ -155,23 +155,18 @@ SharedWorkerParent::ManagerCreated(Share
   if (mStatus == eClosed) {
     aWorkerManager->RemoveActor(this);
     return;
   }
 
   mStatus = eActive;
   mWorkerManager = aWorkerManager;
 
-  if (mFrozen) {
-    mWorkerManager->UpdateFrozen();
-  }
-
-  if (mSuspended) {
-    mWorkerManager->UpdateSuspend();
-  }
+  mWorkerManager->UpdateFrozen();
+  mWorkerManager->UpdateSuspend();
 }
 
 void
 SharedWorkerParent::ErrorPropagation(nsresult aError)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(NS_FAILED(aError));
   MOZ_ASSERT(mStatus == ePending || mStatus == eClosed);
--- a/dom/workers/sharedworkers/SharedWorkerParent.h
+++ b/dom/workers/sharedworkers/SharedWorkerParent.h
@@ -56,17 +56,17 @@ public:
   IsSuspended() const
   {
     return mSuspended;
   }
 
   bool
   IsFrozen() const
   {
-    return mSuspended;
+    return mFrozen;
   }
 
   uint64_t
   WindowID() const
   {
     return mWindowID;
   }