Bug 1488792: Reject calls to update() while service worker in 'installing' state r=asuth
authorYaron Tausky <ytausky@mozilla.com>
Wed, 10 Oct 2018 08:20:01 +0000
changeset 496160 9a70e53741767b0c2e149e16f5a33594bebc89ba
parent 496157 c9d9dd203994cd2251869d7b470564281d5d995f
child 496161 1fead446e3b9c18764a813099b7e6d932ac7d918
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1488792
milestone64.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 1488792: Reject calls to update() while service worker in 'installing' state r=asuth The service workers spec mandates that calling ServiceWorkerRegistration.update() on a registration whose newest worker is in the 'installing' state fail immediately. This commit implements this requirement and tests it. Differential Revision: https://phabricator.services.mozilla.com/D5241
dom/serviceworkers/ServiceWorkerManager.cpp
testing/web-platform/tests/service-workers/service-worker/resources/update-during-installation-worker.js
testing/web-platform/tests/service-workers/service-worker/update-not-allowed.https.html
--- a/dom/serviceworkers/ServiceWorkerManager.cpp
+++ b/dom/serviceworkers/ServiceWorkerManager.cpp
@@ -2429,16 +2429,30 @@ ServiceWorkerManager::Update(nsIPrincipa
   ServiceWorkerUpdaterChild* actor =
     new ServiceWorkerUpdaterChild(promise, successRunnable, failureRunnable);
 
   mActor->SendPServiceWorkerUpdaterConstructor(actor,
                                                aPrincipal->OriginAttributesRef(),
                                                nsCString(aScope));
 }
 
+namespace {
+
+void
+RejectUpdateWithInvalidStateError(ServiceWorkerUpdateFinishCallback& aCallback)
+{
+  ErrorResult error(NS_ERROR_DOM_INVALID_STATE_ERR);
+  aCallback.UpdateFailed(error);
+
+  // In case the callback does not consume the exception
+  error.SuppressException();
+}
+
+}
+
 void
 ServiceWorkerManager::UpdateInternal(nsIPrincipal* aPrincipal,
                                      const nsACString& aScope,
                                      ServiceWorkerUpdateFinishCallback* aCallback)
 {
   MOZ_ASSERT(aPrincipal);
   MOZ_ASSERT(aCallback);
 
@@ -2454,22 +2468,22 @@ ServiceWorkerManager::UpdateInternal(nsI
     return;
   }
 
   // "Let newestWorker be the result of running Get Newest Worker algorithm
   // passing registration as its argument.
   // If newestWorker is null, return a promise rejected with "InvalidStateError"
   RefPtr<ServiceWorkerInfo> newest = registration->Newest();
   if (!newest) {
-    ErrorResult error(NS_ERROR_DOM_INVALID_STATE_ERR);
-    aCallback->UpdateFailed(error);
-
-    // In case the callback does not consume the exception
-    error.SuppressException();
-
+    RejectUpdateWithInvalidStateError(*aCallback);
+    return;
+  }
+
+  if (newest->State() == ServiceWorkerState::Installing) {
+    RejectUpdateWithInvalidStateError(*aCallback);
     return;
   }
 
   RefPtr<ServiceWorkerJobQueue> queue = GetOrCreateJobQueue(scopeKey, aScope);
 
   // "Invoke Update algorithm, or its equivalent, with client, registration as
   // its argument."
   RefPtr<ServiceWorkerUpdateJob> job =
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/service-workers/service-worker/resources/update-during-installation-worker.js
@@ -0,0 +1,23 @@
+'use strict';
+
+const installMayFinish = new Promise(resolve => {
+    self.finishInstall = resolve;
+});
+
+let report = { installEventFired: false };
+
+addEventListener('install', event => {
+    report.installEventFired = true;
+    let attemptUpdate = registration.update().catch(exception => {
+        report.exception = exception.name;
+    });
+    event.waitUntil(Promise.all([installMayFinish, attemptUpdate]));
+});
+
+addEventListener('message', event => {
+    if (event.data === 'finishInstall') {
+        finishInstall();
+    } else {
+        event.source.postMessage(report);
+    }
+});
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/service-workers/service-worker/update-not-allowed.https.html
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="resources/test-helpers.sub.js"></script>
+<script>
+'use strict';
+
+async function spin_up_service_worker(test) {
+    const script = 'resources/update-during-installation-worker.js';
+    const scope = 'resources/blank.html';
+
+    let registration = await service_worker_unregister_and_register(test, script, scope);
+    test.add_cleanup(() => {
+        if (registration.installing) {
+            registration.installing.postMessage('finishInstall');
+        }
+        registration.unregister();
+    });
+
+    return registration;
+}
+
+promise_test(async t => {
+    const registration = await spin_up_service_worker(t);
+    const worker = registration.installing;
+
+    // spin_up_service_worker installs a cleanup hook that ensures the
+    // worker finished its installation by sending it a
+    // 'finishInstall' message, thus making sure that the registration
+    // will be cleanly removed at the end of the test.
+    assert_equals(worker.state, 'installing');
+    promise_rejects(t, 'InvalidStateError', registration.update());
+}, 'ServiceWorkerRegistration.update() from client throws while installing service worker.')
+
+promise_test(async t => {
+    const registration = await spin_up_service_worker(t);
+    const worker = registration.installing;
+    worker.postMessage('finishInstall');
+
+    // By waiting for both states at the same time, the test fails
+    // quickly if the installation fails, avoiding a timeout.
+    await Promise.race([wait_for_state(t, worker, 'activated'),
+                        wait_for_state(t, worker, 'redundant')]);
+    assert_equals(worker.state, 'activated', 'Service worker should be activated.');
+
+    const response = await new Promise(resolve => {
+        navigator.serviceWorker.onmessage = event => { resolve(event.data); };
+        worker.postMessage('PING');
+    });
+
+    // We check that the service worker instance that replied to the
+    // message is the same one that received the 'install' event since
+    // it's possible for them to be two distinct execution
+    // environments.
+    assert_true(response.installEventFired, 'Service worker should have been installed.');
+    assert_equals(response.exception, 'InvalidStateError', 'update() should have thrown.');
+}, 'ServiceWorkerRegistration.update() from installing service worker throws.');
+</script>