Bug 1231213 - Update the update algorithm to better match the spec r=asuth
☠☠ backed out by 3cf55b7f12f2 ☠ ☠
authorPerry Jiang <perry@mozilla.com>
Wed, 14 Aug 2019 16:20:54 +0000
changeset 487985 7e09ad9ceea6cc1dafe35ceed18af6a8b3bc007b
parent 487984 a275eb0b1a191cce3d7b6461ee6b816b3d87ddac
child 487986 0fe31de4a2712c918b59fba1c9de7e20db92d3bd
push id36434
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:44:30 +0000
treeherdermozilla-central@144fbfb409b7 [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;
     }
   };
 });