Bug 1237992 - service worker activate should be executed after install onstatechange events are fired. r=bkelly
authordimi <dlee@mozilla.com>
Thu, 25 Feb 2016 09:03:28 +0800
changeset 285476 87a62c4c347446c5bf6bcdd1fa4808df3175463b
parent 285475 63fdedf32876a6c2776e72d8d882c8afd7475c5f
child 285477 4b043b29bb042320fa0be83d30aff95ac5903f22
push id72393
push usercbook@mozilla.com
push dateThu, 25 Feb 2016 07:45:28 +0000
treeherdermozilla-inbound@87a62c4c3474 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1237992
milestone47.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 1237992 - service worker activate should be executed after install onstatechange events are fired. r=bkelly
dom/workers/ServiceWorkerManager.cpp
dom/workers/ServiceWorkerManager.h
dom/workers/test/serviceworkers/test_claim_oninstall.html
testing/web-platform/mozilla/meta/MANIFEST.json
testing/web-platform/mozilla/tests/service-workers/service-worker/activate-event-after-install-state-change.https.html
testing/web-platform/mozilla/tests/service-workers/service-worker/resources/skip-waiting-installed-worker.js
testing/web-platform/mozilla/tests/service-workers/service-worker/skip-waiting-installed.https.html
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -1237,17 +1237,17 @@ public:
     mRegistration->mWaitingWorker->UpdateState(ServiceWorkerState::Installed);
     mRegistration->NotifyListenersOnChange();
     swm->StoreRegistration(mPrincipal, mRegistration);
     swm->InvalidateServiceWorkerRegistrationWorker(mRegistration,
                                                    WhichServiceWorker::INSTALLING_WORKER | WhichServiceWorker::WAITING_WORKER);
 
     Done(NS_OK);
     // Activate() is invoked out of band of atomic.
-    mRegistration->TryToActivate();
+    mRegistration->TryToActivateAsync();
   }
 
 private:
   const InstallType mType;
 };
 
 class ServiceWorkerRegisterJob final : public ServiceWorkerJobBase,
                                        public serviceWorkerScriptCache::CompareCallback
@@ -1849,19 +1849,33 @@ ServiceWorkerManager::AppendPendingOpera
 
   if (!mShuttingDown) {
     PendingOperation* opt = mPendingOperations.AppendElement();
     opt->mRunnable = aRunnable;
   }
 }
 
 void
+ServiceWorkerRegistrationInfo::TryToActivateAsync()
+{
+  nsCOMPtr<nsIRunnable> r =
+  NS_NewRunnableMethod(this,
+                       &ServiceWorkerRegistrationInfo::TryToActivate);
+  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(r)));
+}
+
+/*
+ * TryToActivate should not be called directly, use TryToACtivateAsync instead.
+ */
+void
 ServiceWorkerRegistrationInfo::TryToActivate()
 {
-  if (!IsControllingDocuments() || mWaitingWorker->SkipWaitingFlag()) {
+  if (!IsControllingDocuments() ||
+      // Waiting worker will be removed if the registration is removed
+      (mWaitingWorker && mWaitingWorker->SkipWaitingFlag())) {
     Activate();
   }
 }
 
 void
 ContinueActivateTask::ContinueAfterWorkerEvent(bool aSuccess)
 {
   mRegistration->FinishActivate(aSuccess);
@@ -3404,17 +3418,17 @@ ServiceWorkerManager::StopControllingADo
     } else {
       // If the registration has an active worker that is running
       // this might be a good time to stop it.
       if (aRegistration->mActiveWorker) {
         ServiceWorkerPrivate* serviceWorkerPrivate =
           aRegistration->mActiveWorker->WorkerPrivate();
         serviceWorkerPrivate->NoteStoppedControllingDocuments();
       }
-      aRegistration->TryToActivate();
+      aRegistration->TryToActivateAsync();
     }
   }
 }
 
 NS_IMETHODIMP
 ServiceWorkerManager::GetScopeForUrl(nsIPrincipal* aPrincipal,
                                      const nsAString& aUrl, nsAString& aScope)
 {
@@ -4185,17 +4199,17 @@ ServiceWorkerManager::SetSkipWaitingFlag
 
   if (registration->mInstallingWorker &&
       (registration->mInstallingWorker->ID() == aServiceWorkerID)) {
     registration->mInstallingWorker->SetSkipWaitingFlag();
   } else if (registration->mWaitingWorker &&
              (registration->mWaitingWorker->ID() == aServiceWorkerID)) {
     registration->mWaitingWorker->SetSkipWaitingFlag();
     if (registration->mWaitingWorker->State() == ServiceWorkerState::Installed) {
-      registration->TryToActivate();
+      registration->TryToActivateAsync();
     }
   } else {
     NS_WARNING("Failed to set skipWaiting flag, no matching worker.");
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
--- a/dom/workers/ServiceWorkerManager.h
+++ b/dom/workers/ServiceWorkerManager.h
@@ -135,16 +135,19 @@ public:
 
   void
   Clear();
 
   void
   PurgeActiveWorker();
 
   void
+  TryToActivateAsync();
+
+  void
   TryToActivate();
 
   void
   Activate();
 
   void
   FinishActivate(bool aSuccess);
 
--- a/dom/workers/test/serviceworkers/test_claim_oninstall.html
+++ b/dom/workers/test/serviceworkers/test_claim_oninstall.html
@@ -33,23 +33,28 @@
     ok(registration.installing, "Worker should be in installing state");
 
     navigator.serviceWorker.oncontrollerchange = function() {
       ok(false, "Claim should not succeed when the worker is not active.");
     }
 
     var p = new Promise(function(res, rej) {
       registration.installing.onstatechange = function(e) {
+        ok(registration.waiting, "Worker should be in waitinging state");
+
         // The worker will become active only if claim will reject inside the
         // install handler.
+        registration.waiting.onstatechange = function(e) {
+          ok(registration.active, "Claim should reject if the worker is not active");
+          ok(navigator.serviceWorker.controller === null, "Client is not controlled.");
+          e.target.onstatechange = null;
+          res();
+        }
 
-        ok(registration.active, "Claim should reject if the worker is not active");
-        ok(navigator.serviceWorker.controller === null, "Client is not controlled.");
         e.target.onstatechange = null;
-        res();
       }
     });
 
     return p;
   }
 
   function runTest() {
     register()
--- a/testing/web-platform/mozilla/meta/MANIFEST.json
+++ b/testing/web-platform/mozilla/meta/MANIFEST.json
@@ -29,16 +29,22 @@
           }
         ],
         "service-workers/service-worker/ServiceWorkerGlobalScope/update.https.html": [
           {
             "path": "service-workers/service-worker/ServiceWorkerGlobalScope/update.https.html",
             "url": "/_mozilla/service-workers/service-worker/ServiceWorkerGlobalScope/update.https.html"
           }
         ],
+        "service-workers/service-worker/activate-event-after-install-state-change.https.html": [
+          {
+            "path": "service-workers/service-worker/activate-event-after-install-state-change.https.html",
+            "url": "/_mozilla/service-workers/service-worker/activate-event-after-install-state-change.https.html"
+          }
+        ],
         "service-workers/service-worker/activation-after-registration.https.html": [
           {
             "path": "service-workers/service-worker/activation-after-registration.https.html",
             "url": "/_mozilla/service-workers/service-worker/activation-after-registration.https.html"
           }
         ],
         "service-workers/service-worker/active.https.html": [
           {
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/mozilla/tests/service-workers/service-worker/activate-event-after-install-state-change.https.html
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<title>Service Worker: registration events</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="resources/test-helpers.sub.js"></script>
+<script>
+promise_test(function(t) {
+  var script = 'resources/empty-worker.js';
+  var scope = 'resources/blank.html';
+  var registration;
+
+  return service_worker_unregister_and_register(t, script, scope)
+    .then(function(registration) {
+        var sw = registration.installing;
+
+        return new Promise(t.step_func(function(resolve) {
+          sw.onstatechange = t.step_func(function() {
+            if (sw.state === 'installed') {
+              assert_equals(registration.active, null,
+                            'installed event should be fired before activating service worker');
+              resolve();
+            }
+          });
+        }));
+      })
+    .then(function() {
+        return service_worker_unregister_and_done(t, scope);
+      })
+    .catch(unreached_rejection(t));
+  }, 'installed event should be fired before activating service worker');
+
+</script>
--- a/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/skip-waiting-installed-worker.js
+++ b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/skip-waiting-installed-worker.js
@@ -1,33 +1,24 @@
 self.state = 'starting';
 
 self.addEventListener('install', function() {
     self.state = 'installing';
   });
 
-self.addEventListener('activate', function() {
-    self.state = 'activating';
-  });
-
 self.addEventListener('message', function(event) {
     var port = event.data.port;
     if (self.state !== 'installing') {
       port.postMessage('FAIL: Worker should be waiting in installed state');
       return;
     }
     self.skipWaiting()
       .then(function(result) {
           if (result !== undefined) {
             port.postMessage('FAIL: Promise should be resolved with undefined');
             return;
           }
-          if (self.state !== 'activating') {
-            port.postMessage(
-                'FAIL: Promise should be resolved after worker activated');
-            return;
-          }
           port.postMessage('PASS');
         })
       .catch(function(e) {
           port.postMessage('FAIL: unexpected exception: ' + e);
         });
   });
--- a/testing/web-platform/mozilla/tests/service-workers/service-worker/skip-waiting-installed.https.html
+++ b/testing/web-platform/mozilla/tests/service-workers/service-worker/skip-waiting-installed.https.html
@@ -5,52 +5,56 @@
 <script src="/resources/testharnessreport.js"></script>
 <script src="resources/test-helpers.sub.js"></script>
 <script>
 
 promise_test(function(t) {
     var scope = 'resources/blank.html';
     var url1 = 'resources/empty.js';
     var url2 = 'resources/skip-waiting-installed-worker.js';
-    var frame, frame_sw, service_worker, onmessage, oncontrollerchanged;
+    var frame, frame_sw, service_worker, registration, onmessage, oncontrollerchanged;
     var saw_message = new Promise(function(resolve) {
         onmessage = function(e) {
             var message = e.data;
             assert_equals(
                 message, 'PASS',
-                'skipWaiting promise should be resolved after activated');
+                'skipWaiting promise should be resolved with undefined');
+
+            assert_equals(registration.active.scriptURL, normalizeURL(url2),
+                          "skipWaiting should make worker become active");
             resolve();
         };
       });
     var saw_controllerchanged = new Promise(function(resolve) {
         oncontrollerchanged = function() {
             assert_equals(
                 frame_sw.controller.scriptURL, normalizeURL(url2),
                 'Controller scriptURL should change to the second one');
             resolve();
         };
       });
     return service_worker_unregister_and_register(t, url1, scope)
-      .then(function(registration) {
-          return wait_for_state(t, registration.installing, 'activated');
+      .then(function(r) {
+          return wait_for_state(t, r.installing, 'activated');
         })
       .then(function() {
           return with_iframe(scope);
         })
       .then(function(f) {
           frame = f;
           frame_sw = f.contentWindow.navigator.serviceWorker;
           assert_equals(
               frame_sw.controller.scriptURL, normalizeURL(url1),
               'Document controller scriptURL should equal to the first one');
           frame_sw.oncontrollerchange = t.step_func(oncontrollerchanged);
           return navigator.serviceWorker.register(url2, {scope: scope});
         })
-      .then(function(registration) {
-          service_worker = registration.installing;
+      .then(function(r) {
+          registration = r;
+          service_worker = r.installing;
           return wait_for_state(t, service_worker, 'installed');
         })
       .then(function() {
           var channel = new MessageChannel();
           channel.port1.onmessage = t.step_func(onmessage);
           service_worker.postMessage({port: channel.port2}, [channel.port2]);
           return Promise.all([saw_message, saw_controllerchanged]);
         })