Bug 1189685 - Part 1: Ensure that the state of all ServiceWorker instances is up to date when dispatching statechange events; r=bkelly
authorEhsan Akhgari <ehsan@mozilla.com>
Sun, 25 Oct 2015 19:30:16 -0400
changeset 269509 b0e600058af69fa8548d33b619845e7340ae319d
parent 269508 b1095ad5a4b84117cfbde34da2e9e77351b566db
child 269510 fb0887080d19cce421e023b958c832d992b3f466
push id67112
push usereakhgari@mozilla.com
push dateMon, 26 Oct 2015 16:38:02 +0000
treeherdermozilla-inbound@fb0887080d19 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1189685
milestone44.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 1189685 - Part 1: Ensure that the state of all ServiceWorker instances is up to date when dispatching statechange events; r=bkelly
dom/workers/ServiceWorker.cpp
dom/workers/ServiceWorker.h
dom/workers/ServiceWorkerManager.cpp
--- a/dom/workers/ServiceWorker.cpp
+++ b/dom/workers/ServiceWorker.cpp
@@ -96,21 +96,11 @@ ServiceWorker::PostMessage(JSContext* aC
     return;
   }
 
   UniquePtr<ServiceWorkerClientInfo> clientInfo(new ServiceWorkerClientInfo(window->GetExtantDoc()));
   ServiceWorkerPrivate* workerPrivate = mInfo->WorkerPrivate();
   aRv = workerPrivate->SendMessageEvent(aCx, aMessage, aTransferable, Move(clientInfo));
 }
 
-void
-ServiceWorker::QueueStateChangeEvent(ServiceWorkerState aState)
-{
-  nsCOMPtr<nsIRunnable> r =
-    NS_NewRunnableMethodWithArg<ServiceWorkerState>(this,
-                                                    &ServiceWorker::DispatchStateChange,
-                                                    aState);
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(r)));
-}
-
 } // namespace workers
 } // namespace dom
 } // namespace mozilla
--- a/dom/workers/ServiceWorker.h
+++ b/dom/workers/ServiceWorker.h
@@ -50,23 +50,19 @@ public:
   }
 
   void
   GetScriptURL(nsString& aURL) const;
 
   void
   DispatchStateChange(ServiceWorkerState aState)
   {
-    SetState(aState);
     DOMEventTargetHelper::DispatchTrustedEvent(NS_LITERAL_STRING("statechange"));
   }
 
-  void
-  QueueStateChangeEvent(ServiceWorkerState aState);
-
 #ifdef XP_WIN
 #undef PostMessage
 #endif
 
   void
   PostMessage(JSContext* aCx, JS::Handle<JS::Value> aMessage,
               const Optional<Sequence<JS::Value>>& aTransferable,
               ErrorResult& aRv);
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -4207,35 +4207,70 @@ ServiceWorkerInfo::RemoveWorker(ServiceW
   aWorker->GetScriptURL(workerURL);
   MOZ_ASSERT(workerURL.Equals(NS_ConvertUTF8toUTF16(mScriptSpec)));
 #endif
   MOZ_ASSERT(mInstances.Contains(aWorker));
 
   mInstances.RemoveElement(aWorker);
 }
 
+namespace {
+
+class ChangeStateUpdater final : public nsRunnable
+{
+public:
+  ChangeStateUpdater(const nsTArray<ServiceWorker*>& aInstances,
+                     ServiceWorkerState aState)
+    : mState(aState)
+  {
+    for (size_t i = 0; i < aInstances.Length(); ++i) {
+      mInstances.AppendElement(aInstances[i]);
+    }
+  }
+
+  NS_IMETHODIMP Run()
+  {
+    // We need to update the state of all instances atomically before notifying
+    // them to make sure that the observed state for all instances inside
+    // statechange event handlers is correct.
+    for (size_t i = 0; i < mInstances.Length(); ++i) {
+      mInstances[i]->SetState(mState);
+    }
+    for (size_t i = 0; i < mInstances.Length(); ++i) {
+      mInstances[i]->DispatchStateChange(mState);
+    }
+
+    return NS_OK;
+  }
+
+private:
+  nsAutoTArray<RefPtr<ServiceWorker>, 1> mInstances;
+  ServiceWorkerState mState;
+};
+
+}
+
 void
 ServiceWorkerInfo::UpdateState(ServiceWorkerState aState)
 {
 #ifdef DEBUG
   // Any state can directly transition to redundant, but everything else is
   // ordered.
   if (aState != ServiceWorkerState::Redundant) {
     MOZ_ASSERT_IF(mState == ServiceWorkerState::EndGuard_, aState == ServiceWorkerState::Installing);
     MOZ_ASSERT_IF(mState == ServiceWorkerState::Installing, aState == ServiceWorkerState::Installed);
     MOZ_ASSERT_IF(mState == ServiceWorkerState::Installed, aState == ServiceWorkerState::Activating);
     MOZ_ASSERT_IF(mState == ServiceWorkerState::Activating, aState == ServiceWorkerState::Activated);
   }
   // Activated can only go to redundant.
   MOZ_ASSERT_IF(mState == ServiceWorkerState::Activated, aState == ServiceWorkerState::Redundant);
 #endif
   mState = aState;
-  for (uint32_t i = 0; i < mInstances.Length(); ++i) {
-    mInstances[i]->QueueStateChangeEvent(mState);
-  }
+  nsCOMPtr<nsIRunnable> r = new ChangeStateUpdater(mInstances, mState);
+  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(r.forget())));
 }
 
 ServiceWorkerInfo::ServiceWorkerInfo(ServiceWorkerRegistrationInfo* aReg,
                                      const nsACString& aScriptSpec,
                                      const nsAString& aCacheName)
   : mRegistration(aReg)
   , mScriptSpec(aScriptSpec)
   , mCacheName(aCacheName)