Bug 1231213 - Update the update algorithm to better match the spec r=asuth
authorPerry Jiang <perry@mozilla.com>
Thu, 15 Aug 2019 17:27:51 +0000
changeset 488314 8f031439c3bcc4c1f9a0859500c1bd22eaeb622c
parent 488313 7b8515d888a9910d3b1d87f8b8d9f8b46003ce87
child 488315 140cb3a190e91200a67ac8fcfeebc5194d5cfa81
push id113906
push userncsoregi@mozilla.com
push dateFri, 16 Aug 2019 04:07:24 +0000
treeherdermozilla-inbound@d887276421d3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1231213
milestone70.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 1231213 - Update the update algorithm to better match the spec r=asuth - Throw a TypeError when a registration isn't found in the "scope to registration map" - Synchronously (before enqueuing a job) check for an existing newest worker - Synchronously check if an installing worker is attempting to update itself Differential Revision: https://phabricator.services.mozilla.com/D41618
dom/serviceworkers/ServiceWorkerManager.cpp
dom/serviceworkers/ServiceWorkerRegistration.cpp
testing/web-platform/meta/service-workers/service-worker/update-not-allowed.https.html.ini
testing/web-platform/tests/service-workers/service-worker/resources/update-during-installation-worker.js
--- a/dom/serviceworkers/ServiceWorkerManager.cpp
+++ b/dom/serviceworkers/ServiceWorkerManager.cpp
@@ -2331,60 +2331,45 @@ void ServiceWorkerManager::Update(
 
   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();
-}
-
-}  // namespace
-
 void ServiceWorkerManager::UpdateInternal(
     nsIPrincipal* aPrincipal, const nsACString& aScope,
     ServiceWorkerUpdateFinishCallback* aCallback) {
   MOZ_ASSERT(aPrincipal);
   MOZ_ASSERT(aCallback);
 
   nsAutoCString scopeKey;
   nsresult rv = PrincipalToScopeKey(aPrincipal, scopeKey);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
 
   RefPtr<ServiceWorkerRegistrationInfo> registration =
       GetRegistration(scopeKey, aScope);
   if (NS_WARN_IF(!registration)) {
+    ErrorResult error;
+    error.ThrowTypeError<MSG_SW_UPDATE_BAD_REGISTRATION>(
+        NS_ConvertUTF8toUTF16(aScope), NS_LITERAL_STRING("uninstalled"));
+    aCallback->UpdateFailed(error);
+
+    // In case the callback does not consume the exception
+    error.SuppressException();
     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) {
-    RejectUpdateWithInvalidStateError(*aCallback);
-    return;
-  }
-
-  if (newest->State() == ServiceWorkerState::Installing) {
-    RejectUpdateWithInvalidStateError(*aCallback);
-    return;
-  }
+  MOZ_DIAGNOSTIC_ASSERT(newest,
+                        "The Update algorithm should have been aborted already "
+                        "if there wasn't a newest worker");
 
   RefPtr<ServiceWorkerJobQueue> queue = GetOrCreateJobQueue(scopeKey, aScope);
 
   // "Invoke Update algorithm, or its equivalent, with client, registration as
   // its argument."
   RefPtr<ServiceWorkerUpdateJob> job = new ServiceWorkerUpdateJob(
       aPrincipal, registration->Scope(), newest->ScriptSpec(),
       registration->GetUpdateViaCache());
--- a/dom/serviceworkers/ServiceWorkerRegistration.cpp
+++ b/dom/serviceworkers/ServiceWorkerRegistration.cpp
@@ -7,16 +7,17 @@
 #include "ServiceWorkerRegistration.h"
 
 #include "mozilla/dom/DOMMozPromiseRequestHolder.h"
 #include "mozilla/dom/Notification.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/PushManager.h"
 #include "mozilla/dom/ServiceWorker.h"
 #include "mozilla/dom/ServiceWorkerRegistrationBinding.h"
+#include "mozilla/dom/ServiceWorkerUtils.h"
 #include "mozilla/dom/WorkerPrivate.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsISupportsPrimitives.h"
 #include "nsPIDOMWindow.h"
 #include "RemoteServiceWorkerRegistrationImpl.h"
 #include "ServiceWorkerRegistrationImpl.h"
 
 namespace mozilla {
@@ -194,16 +195,51 @@ already_AddRefed<Promise> ServiceWorkerR
     return nullptr;
   }
 
   RefPtr<Promise> outer = Promise::Create(global, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
+  /**
+   * `ServiceWorker` objects are not exposed on worker threads yet, so calling
+   * `ServiceWorkerRegistration::Get{Installing,Waiting,Active}` won't work.
+   */
+  const bool hasNewestWorker = mDescriptor.GetInstalling() ||
+                               mDescriptor.GetWaiting() ||
+                               mDescriptor.GetActive();
+
+  /**
+   * If newestWorker is null, return a promise rejected with an
+   * "InvalidStateError" DOMException and abort these steps.
+   */
+  if (!hasNewestWorker) {
+    outer->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return outer.forget();
+  }
+
+  /**
+   * If the context object’s relevant settings object’s global object
+   * globalObject is a ServiceWorkerGlobalScope object, and globalObject’s
+   * associated service worker's state is "installing", return a promise
+   * rejected with an "InvalidStateError" DOMException and abort these steps.
+   */
+  if (!NS_IsMainThread()) {
+    WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate();
+    MOZ_ASSERT(workerPrivate);
+
+    if (workerPrivate->IsServiceWorker() &&
+        (workerPrivate->GetServiceWorkerDescriptor().State() ==
+         ServiceWorkerState::Installing)) {
+      outer->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
+      return outer.forget();
+    }
+  }
+
   RefPtr<ServiceWorkerRegistration> self = this;
 
   mInner->Update(
       [outer, self](const ServiceWorkerRegistrationDescriptor& aDesc) {
         nsIGlobalObject* global = self->GetParentObject();
         MOZ_DIAGNOSTIC_ASSERT(global);
         RefPtr<ServiceWorkerRegistration> ref =
             global->GetOrCreateServiceWorkerRegistration(aDesc);
--- a/testing/web-platform/meta/service-workers/service-worker/update-not-allowed.https.html.ini
+++ b/testing/web-platform/meta/service-workers/service-worker/update-not-allowed.https.html.ini
@@ -1,13 +1,5 @@
 [update-not-allowed.https.html]
   disabled:
     if (os == "android") and not e10s: https://bugzilla.mozilla.org/show_bug.cgi?id=1499972
     if (os == "android") and e10s: bug 1550895 (frequently fails on geckoview)
   expected: ERROR
-  [ServiceWorkerRegistration.update() from active service worker succeeds while installing service worker.]
-    expected:
-      if sw-e10s: TIMEOUT
-      FAIL
-
-  [ServiceWorkerRegistration.update() from client succeeds while installing service worker.]
-    expected: FAIL
-
--- a/testing/web-platform/tests/service-workers/service-worker/resources/update-during-installation-worker.js
+++ b/testing/web-platform/tests/service-workers/service-worker/resources/update-during-installation-worker.js
@@ -9,49 +9,53 @@ const installFinished = new Promise(reso
 });
 
 addEventListener('install', event => {
   fireInstallEvent();
   event.waitUntil(installFinished);
 });
 
 addEventListener('message', event => {
+  let resolveWaitUntil;
+  event.waitUntil(new Promise(resolve => { resolveWaitUntil = resolve; }));
+
   // Use a dedicated MessageChannel for every request so senders can wait for
   // individual requests to finish, and concurrent requests (to different
   // workers) don't cause race conditions.
   const port = event.data;
   port.onmessage = (event) => {
     switch (event.data) {
       case 'awaitInstallEvent':
         installEventFired.then(() => {
             port.postMessage('installEventFired');
-        });
+        }).finally(resolveWaitUntil);
         break;
 
       case 'finishInstall':
         installFinished.then(() => {
             port.postMessage('installFinished');
-        });
+        }).finally(resolveWaitUntil);
         finishInstall();
         break;
 
       case 'callUpdate': {
         const channel = new MessageChannel();
         registration.update().then(() => {
             channel.port2.postMessage({
                 success: true,
             });
         }).catch((exception) => {
             channel.port2.postMessage({
                 success: false,
                 exception: exception.name,
             });
-        });
+        }).finally(resolveWaitUntil);
         port.postMessage(channel.port1, [channel.port1]);
         break;
       }
 
       default:
         port.postMessage('Unexpected command ' + event.data);
+        resolveWaitUntil();
         break;
     }
   };
 });