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

--HG--
extra : moz-landing-system : lando

dom/serviceworkers/ServiceWorkerManager.cpp

@@ -2336,19 +2336,6 @@ void ServiceWorkerManager::Update(
       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) {
@@ -2364,22 +2351,20 @@ void ServiceWorkerManager::UpdateInternal(
   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);
 

dom/serviceworkers/ServiceWorkerRegistration.cpp

@@ -12,6 +12,7 @@
 #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"
@@ -199,6 +200,41 @@ already_AddRefed<Promise> ServiceWorkerRegistration::Update(ErrorResult& aRv) {
     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(

testing/web-platform/meta/service-workers/service-worker/update-not-allowed.https.html.ini

@@ -3,11 +3,3 @@
     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
-

testing/web-platform/tests/service-workers/service-worker/resources/update-during-installation-worker.js

@@ -14,6 +14,9 @@ addEventListener('install', event => {
 });
 
 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.
@@ -23,13 +26,13 @@ addEventListener('message', event => {
       case 'awaitInstallEvent':
         installEventFired.then(() => {
             port.postMessage('installEventFired');
-        });
+        }).finally(resolveWaitUntil);
         break;
 
       case 'finishInstall':
         installFinished.then(() => {
             port.postMessage('installFinished');
-        });
+        }).finally(resolveWaitUntil);
         finishInstall();
         break;
 
@@ -44,13 +47,14 @@ addEventListener('message', event => {
                 success: false,
                 exception: exception.name,
             });
-        });
+        }).finally(resolveWaitUntil);
         port.postMessage(channel.port1, [channel.port1]);
         break;
       }
 
       default:
         port.postMessage('Unexpected command ' + event.data);
+        resolveWaitUntil();
         break;
     }
   };