Backed out changeset 4d8423d5a83b (bug 1130065)

dom/workers/ServiceWorkerManager.cpp

@@ -130,6 +130,7 @@ ServiceWorkerRegistrationInfo::Clear()
     mWaitingWorker->UpdateState(ServiceWorkerState::Redundant);
     // Fire statechange.
     mWaitingWorker = nullptr;
+    mWaitingToActivate = false;
   }
 
   if (mActiveWorker) {
@@ -198,6 +199,8 @@ ServiceWorkerManager::~ServiceWorkerManager()
   mServiceWorkerRegistrationInfos.Clear();
 }
 
+class ServiceWorkerRegisterJob;
+
 class ContinueLifecycleTask : public nsISupports
 {
   NS_DECL_ISUPPORTS
@@ -213,8 +216,6 @@ public:
 
 NS_IMPL_ISUPPORTS0(ContinueLifecycleTask);
 
-class ServiceWorkerRegisterJob;
-
 class ContinueInstallTask MOZ_FINAL : public ContinueLifecycleTask
 {
   nsRefPtr<ServiceWorkerRegisterJob> mJob;
@@ -237,7 +238,10 @@ public:
   { }
 
   void
-  ContinueAfterWorkerEvent(bool aSuccess, bool aActivateImmediately /* unused */) MOZ_OVERRIDE;
+  ContinueAfterWorkerEvent(bool aSuccess, bool aActivateImmediately /* unused */) MOZ_OVERRIDE
+  {
+    mRegistration->FinishActivate(aSuccess);
+  }
 };
 
 class ContinueLifecycleRunnable MOZ_FINAL : public nsRunnable
@@ -603,10 +607,6 @@ public:
     nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
     MOZ_ASSERT(swm->mSetOfScopesBeingUpdated.Contains(mRegistration->mScope));
     swm->mSetOfScopesBeingUpdated.Remove(mRegistration->mScope);
-    // This is effectively the end of Step 4.3 of the [[Update]] algorithm.
-    // The invocation of [[Install]] is not part of the atomic block.
-
-    // Begin [[Install]] atomic step 4.
     if (mRegistration->mInstallingWorker) {
       // FIXME(nsm): Terminate and stuff
       mRegistration->mInstallingWorker->UpdateState(ServiceWorkerState::Redundant);
@@ -619,13 +619,15 @@ public:
 
     Succeed();
 
-    // Step 4.6 "Queue a task..." for updatefound.
     nsCOMPtr<nsIRunnable> upr =
       NS_NewRunnableMethodWithArg<ServiceWorkerRegistrationInfo*>(swm,
                                                                   &ServiceWorkerManager::FireUpdateFound,
                                                                   mRegistration);
     NS_DispatchToMainThread(upr);
 
+    nsMainThreadPtrHandle<ContinueLifecycleTask> handle(
+        new nsMainThreadPtrHolder<ContinueLifecycleTask>(new ContinueInstallTask(this)));
+
     nsRefPtr<ServiceWorker> serviceWorker;
     nsresult rv =
       swm->CreateServiceWorker(mRegistration->mInstallingWorker->ScriptSpec(),
@@ -633,21 +635,15 @@ public:
                                getter_AddRefs(serviceWorker));
 
     if (NS_WARN_IF(NS_FAILED(rv))) {
-      ContinueAfterInstallEvent(false /* aSuccess */, false /* aActivateImmediately */);
+      ContinueAfterInstallEvent(false /* success */, false /* activate immediately */);
       return;
     }
 
-    nsMainThreadPtrHandle<ContinueLifecycleTask> handle(
-        new nsMainThreadPtrHolder<ContinueLifecycleTask>(new ContinueInstallTask(this)));
-
     nsRefPtr<LifecycleEventWorkerRunnable> r =
       new LifecycleEventWorkerRunnable(serviceWorker->GetWorkerPrivate(), NS_LITERAL_STRING("install"), handle);
 
     AutoJSAPI jsapi;
     jsapi.Init();
-
-    // This triggers Step 4.7 "Queue a task to run the following substeps..."
-    // which sends the install event to the worker.
     r->Dispatch(jsapi.cx());
   }
 
@@ -729,8 +725,7 @@ private:
   FailCommon(nsresult aRv)
   {
     mCallback = nullptr;
-    nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-    swm->MaybeRemoveRegistration(mRegistration);
+    MaybeRemoveRegistration();
     // Ensures that the job can't do anything useful from this point on.
     mRegistration = nullptr;
     Done(aRv);
@@ -748,8 +743,23 @@ private:
   }
 
   void
-  ContinueAfterInstallEvent(bool aInstallEventSuccess, bool aActivateImmediately)
+  MaybeRemoveRegistration()
   {
+    MOZ_ASSERT(mRegistration);
+    nsRefPtr<ServiceWorkerInfo> newest = mRegistration->Newest();
+    if (!newest) {
+      nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+      swm->RemoveRegistration(mRegistration);
+    }
+  }
+
+  void
+  ContinueAfterInstallEvent(bool aSuccess, bool aActivateImmediately)
+  {
+    // By this point the callback should've been notified about success or fail
+    // and nulled.
+    MOZ_ASSERT(!mCallback);
+
     if (!mRegistration->mInstallingWorker) {
       NS_WARNING("mInstallingWorker was null.");
       return Done(NS_ERROR_DOM_ABORT_ERR);
@@ -758,12 +768,12 @@ private:
     nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
 
     // "If installFailed is true"
-    if (!aInstallEventSuccess) {
+    if (!aSuccess) {
       mRegistration->mInstallingWorker->UpdateState(ServiceWorkerState::Redundant);
       mRegistration->mInstallingWorker = nullptr;
       swm->InvalidateServiceWorkerRegistrationWorker(mRegistration,
                                                      WhichServiceWorker::INSTALLING_WORKER);
-      swm->MaybeRemoveRegistration(mRegistration);
+      MaybeRemoveRegistration();
       return Done(NS_ERROR_DOM_ABORT_ERR);
     }
 
@@ -780,14 +790,14 @@ private:
     // swapping it with waiting worker.
     mRegistration->mInstallingWorker->UpdateState(ServiceWorkerState::Installed);
     mRegistration->mWaitingWorker = mRegistration->mInstallingWorker.forget();
+    mRegistration->mWaitingToActivate = false;
     swm->InvalidateServiceWorkerRegistrationWorker(mRegistration,
                                                    WhichServiceWorker::INSTALLING_WORKER | WhichServiceWorker::WAITING_WORKER);
 
     // FIXME(nsm): Bug 982711 Deal with activateImmediately.
     NS_WARN_IF_FALSE(!aActivateImmediately, "Immediate activation using replace() is not supported yet");
-    Done(NS_OK);
-    // Activate() is invoked out of band of atomic.
     mRegistration->TryToActivate();
+    Done(NS_OK);
   }
 };
 
@@ -806,8 +816,6 @@ ContinueUpdateRunnable::Run()
 void
 ContinueInstallTask::ContinueAfterWorkerEvent(bool aSuccess, bool aActivateImmediately)
 {
-  // This does not start the job immediately if there are other jobs in the
-  // queue, which captures the "atomic" behaviour we want.
   mJob->ContinueAfterInstallEvent(aSuccess, aActivateImmediately);
 }
 
@@ -1090,20 +1098,18 @@ LifecycleEventWorkerRunnable::DispatchLifecycleEvent(JSContext* aCx, WorkerPriva
 void
 ServiceWorkerRegistrationInfo::TryToActivate()
 {
+  mWaitingToActivate = true;
   if (!IsControllingDocuments()) {
     Activate();
   }
 }
 
-void
-ContinueActivateTask::ContinueAfterWorkerEvent(bool aSuccess, bool aActivateImmediately /* unused */)
-{
-  mRegistration->FinishActivate(aSuccess);
-}
-
 void
 ServiceWorkerRegistrationInfo::Activate()
 {
+  MOZ_ASSERT(mWaitingToActivate);
+  mWaitingToActivate = false;
+
   nsRefPtr<ServiceWorkerInfo> activatingWorker = mWaitingWorker;
   nsRefPtr<ServiceWorkerInfo> exitingWorker = mActiveWorker;
 
@@ -1124,18 +1130,18 @@ ServiceWorkerRegistrationInfo::Activate()
   mWaitingWorker = nullptr;
   mActiveWorker->UpdateState(ServiceWorkerState::Activating);
 
-  // FIXME(nsm): Unlink appcache if there is one.
-
   swm->CheckPendingReadyPromises();
   swm->StoreRegistration(mPrincipal, this);
 
   // "Queue a task to fire a simple event named controllerchange..."
   nsCOMPtr<nsIRunnable> controllerChangeRunnable =
-    NS_NewRunnableMethodWithArg<ServiceWorkerRegistrationInfo*>(swm,
-                                                                &ServiceWorkerManager::FireControllerChange,
-                                                                this);
+    NS_NewRunnableMethodWithArg<ServiceWorkerRegistrationInfo*>(swm, &ServiceWorkerManager::FireControllerChange, this);
   NS_DispatchToMainThread(controllerChangeRunnable);
 
+  // XXXnsm I have my doubts about this. Leaving the main thread means that
+  // subsequent calls to Activate() not from a Register() call, i.e. due to all
+  // controlled documents going away, may lead to two or more calls being
+  // interleaved.
   MOZ_ASSERT(mActiveWorker);
   nsRefPtr<ServiceWorker> serviceWorker;
   nsresult rv =
@@ -1143,11 +1149,7 @@ ServiceWorkerRegistrationInfo::Activate()
                              mScope,
                              getter_AddRefs(serviceWorker));
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    nsCOMPtr<nsIRunnable> r =
-      NS_NewRunnableMethodWithArg<bool>(this,
-                                        &ServiceWorkerRegistrationInfo::FinishActivate,
-                                        false /* success */);
-    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(r)));
+    FinishActivate(false /* success */);
     return;
   }
 
@@ -1683,10 +1685,7 @@ ServiceWorkerManager::HandleError(JSContext* aCx,
 void
 ServiceWorkerRegistrationInfo::FinishActivate(bool aSuccess)
 {
-  if (mPendingUninstall || !mActiveWorker) {
-    return;
-  }
-
+  MOZ_ASSERT(mActiveWorker);
   if (aSuccess) {
     mActiveWorker->UpdateState(ServiceWorkerState::Activated);
   } else {
@@ -1942,6 +1941,31 @@ ServiceWorkerManager::MaybeStartControlling(nsIDocument* aDoc)
   }
 }
 
+class ServiceWorkerActivateAfterUnloadingJob MOZ_FINAL : public ServiceWorkerJob
+{
+  nsRefPtr<ServiceWorkerRegistrationInfo> mRegistration;
+public:
+  ServiceWorkerActivateAfterUnloadingJob(ServiceWorkerJobQueue* aQueue,
+                                         ServiceWorkerRegistrationInfo* aReg)
+    : ServiceWorkerJob(aQueue)
+    , mRegistration(aReg)
+  { }
+
+  void
+  Start()
+  {
+    if (mRegistration->mPendingUninstall) {
+      mRegistration->Clear();
+      nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+      swm->RemoveRegistration(mRegistration);
+    } else {
+      mRegistration->TryToActivate();
+    }
+
+    Done(NS_OK);
+  }
+};
+
 void
 ServiceWorkerManager::MaybeStopControlling(nsIDocument* aDoc)
 {
@@ -1954,12 +1978,12 @@ ServiceWorkerManager::MaybeStopControlling(nsIDocument* aDoc)
   if (registration) {
     registration->StopControllingADocument();
     if (!registration->IsControllingDocuments()) {
-      if (registration->mPendingUninstall) {
-        registration->Clear();
-        RemoveRegistration(registration);
-      } else {
-        registration->TryToActivate();
-      }
+      ServiceWorkerJobQueue* queue = GetOrCreateJobQueue(registration->mScope);
+      // The remaining tasks touch registration->mPendingUninstall, so queue
+      // them up in a job.
+      nsRefPtr<ServiceWorkerActivateAfterUnloadingJob> job =
+        new ServiceWorkerActivateAfterUnloadingJob(queue, registration);
+      queue->Append(job);
     }
   }
 }
@@ -2413,14 +2437,4 @@ ServiceWorkerManager::CreateNewRegistration(const nsCString& aScope,
   return registration;
 }
 
-void
-ServiceWorkerManager::MaybeRemoveRegistration(ServiceWorkerRegistrationInfo* aRegistration)
-{
-  MOZ_ASSERT(aRegistration);
-  nsRefPtr<ServiceWorkerInfo> newest = aRegistration->Newest();
-  if (!newest) {
-    RemoveRegistration(aRegistration);
-  }
-}
-
 END_WORKERS_NAMESPACE

dom/workers/ServiceWorkerManager.h

@@ -49,7 +49,6 @@ class ServiceWorkerJobQueue;
 
 class ServiceWorkerJob : public nsISupports
 {
-protected:
   // The queue keeps the jobs alive, so they can hold a rawptr back to the
   // queue.
   ServiceWorkerJobQueue* mQueue;
@@ -152,6 +151,7 @@ public:
   // removed since documents may be controlled. It is marked as
   // pendingUninstall and when all controlling documents go away, removed.
   bool mPendingUninstall;
+  bool mWaitingToActivate;
 
   explicit ServiceWorkerRegistrationInfo(const nsACString& aScope,
                                          nsIPrincipal* aPrincipal);
@@ -289,9 +289,12 @@ class ServiceWorkerManager MOZ_FINAL
   : public nsIServiceWorkerManager
   , public nsIIPCBackgroundChildCreateCallback
 {
+  friend class ActivationRunnable;
   friend class GetReadyPromiseRunnable;
   friend class GetRegistrationsRunnable;
   friend class GetRegistrationRunnable;
+  friend class QueueFireUpdateFoundRunnable;
+  friend class ServiceWorkerActivateAfterUnloadingJob;
   friend class ServiceWorkerRegisterJob;
   friend class ServiceWorkerRegistrationInfo;
   friend class ServiceWorkerUnregisterJob;
@@ -490,9 +493,6 @@ private:
                                       void* aUnused);
 
   nsClassHashtable<nsISupportsHashKey, PendingReadyPromise> mPendingReadyPromises;
- 
-  void
-  MaybeRemoveRegistration(ServiceWorkerRegistrationInfo* aRegistration);
 
   mozilla::ipc::PBackgroundChild* mActor;
 

dom/workers/test/serviceworkers/install_event_error_worker.js

@@ -1,4 +1,4 @@
 // Worker that errors on receiving an install event.
 oninstall = function(e) {
   undefined.doSomething;
-};
+}

dom/workers/test/serviceworkers/mochitest.ini

@@ -1,5 +1,5 @@
 [DEFAULT]
-skip-if = buildapp == 'b2g'
+skip-if = buildapp == 'b2g' || android_version == "10" # bug 1056702
 support-files =
   worker.js
   worker2.js
@@ -22,13 +22,18 @@ support-files =
   message_posting_worker.js
 
 [test_unregister.html]
+skip-if = true # bug 1094375
 [test_installation_simple.html]
+skip-if = true # bug 1094375
 [test_get_serviced.html]
 [test_install_event.html]
 [test_navigator.html]
 [test_scopes.html]
+skip-if = true # bug 1126470 and many others
 [test_controller.html]
 [test_workerUpdate.html]
+skip-if = true # Enable after Bug 982726 postMessage is landed.
 [test_workerUnregister.html]
+skip-if = true # Enable after Bug 982726 postMessage is landed.
 [test_post_message.html]
 [test_post_message_advanced.html]

dom/workers/test/serviceworkers/simpleregister/ready.html

@@ -3,11 +3,13 @@
   <body>
     <script type="text/javascript">
 
-      window.addEventListener('message', function(evt) {
-        navigator.serviceWorker.ready.then(function() {
-          evt.ports[0].postMessage("WOW!");
-        });
-      }, false);
+       window.addEventListener('message', function(evt) {
+         navigator.serviceWorker.ready.then(function() {
+           navigator.serviceWorker.oncontrollerchange = function(e) {
+             evt.ports[0].postMessage("WOW!");
+           }
+         });
+       }, false);
 
     </script>
   </body>

dom/workers/test/serviceworkers/test_install_event.html

@@ -38,10 +38,9 @@
 
   function installError() {
     // Silence worker errors so they don't cause the test to fail.
-    window.onerror = function(e) {}
+    window.onerror = function() { }
     return navigator.serviceWorker.register("install_event_error_worker.js", { scope: "./install_event" })
       .then(function(swr) {
-        ok(swr.installing instanceof ServiceWorker, "There should be an installing worker if promise resolves.");
         ok(swr.installing.state == "installing", "Installing worker's state should be 'installing'");
         return new Promise(function(resolve, reject) {
           swr.installing.onstatechange = function(e) {
@@ -86,8 +85,7 @@
   }
 
   function runTest() {
-    Promise.resolve()
-      .then(simpleRegister)
+    simpleRegister()
       .then(nextRegister)
       .then(installError)
       .then(activateError)

dom/workers/test/serviceworkers/test_installation_simple.html

@@ -75,6 +75,25 @@
     });
   }
 
+  function abortPrevious() {
+    var p = navigator.serviceWorker.register("worker2.js", { scope: "foo/" });
+    var q = navigator.serviceWorker.register("worker3.js", { scope: "foo/" });
+
+    return Promise.all([
+      p.then(function(wr) {
+        ok(false, "First registration should fail with AbortError");
+      }, function(e) {
+        ok(e.name === "AbortError", "First registration should fail with AbortError");
+      }),
+
+      q.then(function(wr) {
+        ok(wr instanceof ServiceWorkerRegistration, "Second registration should succeed");
+      }, function(e) {
+        ok(false, "Second registration should succeed");
+      })
+    ]);
+  }
+
   function networkError404() {
     return navigator.serviceWorker.register("404.js", { scope: "network_error/"}).then(function(w) {
         ok(false, "Should fail with NetworkError");
@@ -92,6 +111,7 @@
         is(swr, undefined, "A failed registration for a scope with no prior controllers should clear itself");
       });
     }, function(e) {
+    info("NSM " + e.name);
       ok(e instanceof Error, "Registration should fail with parse error");
     });
   }
@@ -162,6 +182,7 @@
       .then(sameOriginScope)
       .then(httpsOnly)
       .then(realWorker)
+      .then(abortPrevious)
       .then(networkError404)
       .then(parseError)
       .then(updatefound)

dom/workers/test/serviceworkers/test_scopes.html

@@ -62,7 +62,7 @@
       ok(getScope(p("sub/dir/afoo")) === p("sub/dir/a"), "Scope should match");
       ok(getScope(p("star*wars")) === p("star*"), "Scope should match");
       ok(getScope(p("star/a.html")) === p(""), "Scope should match");
-      resolve();
+      resolve(true);
     });
   }
 

dom/workers/test/serviceworkers/unregister/index.html

@@ -19,19 +19,22 @@
     info("unregister/index.html should not to be launched directly!");
   }
 
-  SimpleTest.requestFlakyTimeout("Unfortunately we have no way to test for a page being uncontrolled except waiting for ready to not resolve");
   var tId = setTimeout(function() {
+    info("tId timeout!");
     parent.postMessage({ controlled: false }, "*");
     tId = null;
   }, 2000);
 
   navigator.serviceWorker.ready.then(function() {
+  info("Got ready");
     if (tId == null) {
+    info("tId was null");
       parent.postMessage("FAIL!!!", "*");
       return;
     }
 
     clearTimeout(tId);
+    info("tId was non-null");
     parent.postMessage({ controlled: true }, "*");
   });