[ServiceWorker] Stop resurrecting uninstalled workers

Previously, service workers were resurrected when they were updated
or when the same script was registered while the service worker was
still alive. Now, registering a service worker will always create a
new instance, regardless of whether there already is a running
instance or not.

Chrome status:
https://chromestatus.com/feature/5646687634718720

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/r176Lvgxfys/4W4IGM4uAgAJ

Bug: 971571,988582
Change-Id: Id7748377af107099d7c10fb2e455f30f7423e5f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1768759
Commit-Queue: Yannic Bonenberger <yannic.bonenberger@gmail.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705159}

content/browser/service_worker/service_worker_provider_host.cc

@@ -597,8 +597,15 @@ void ServiceWorkerProviderHost::AddMatchingRegistration(
   if (!IsContextSecureForServiceWorker())
     return;
   size_t key = registration->scope().spec().size();
-  if (base::Contains(matching_registrations_, key))
-    return;
+  if (base::Contains(matching_registrations_, key)) {
+    if (registration == matching_registrations_[key]) {
+      // TODO(https://crbug.com/971571): Verify this can never happen and
+      // replace with a DCHECK.
+      return;
+    }
+    uninstalled_matching_registrations_[key].emplace(
+        std::move(matching_registrations_[key]));
+  }
   registration->AddListener(this);
   matching_registrations_[key] = registration;
   ReturnRegistrationForReadyIfNeeded();
@@ -608,12 +615,18 @@ void ServiceWorkerProviderHost::RemoveMatchingRegistration(
     ServiceWorkerRegistration* registration) {
   DCHECK_NE(controller_registration_, registration);
 #if DCHECK_IS_ON()
-  DCHECK(IsMatchingRegistration(registration));
+  DCHECK(IsMatchingRegistration(registration) ||
+         IsMatchingUninstalledRegistration(registration));
 #endif  // DCHECK_IS_ON()
 
   registration->RemoveListener(this);
   size_t key = registration->scope().spec().size();
-  matching_registrations_.erase(key);
+  if (registration == matching_registrations_[key]) {
+    matching_registrations_.erase(key);
+    return;
+  }
+  DCHECK(base::Contains(uninstalled_matching_registrations_, key));
+  uninstalled_matching_registrations_.erase(key);
 }
 
 ServiceWorkerRegistration* ServiceWorkerProviderHost::MatchRegistration()
@@ -803,6 +816,15 @@ bool ServiceWorkerProviderHost::IsMatchingRegistration(
     return false;
   return true;
 }
+
+bool ServiceWorkerProviderHost::IsMatchingUninstalledRegistration(
+    ServiceWorkerRegistration* registration) {
+  size_t key = registration->scope().spec().size();
+  if (!base::Contains(uninstalled_matching_registrations_, key)) {
+    return false;
+  }
+  return base::Contains(uninstalled_matching_registrations_[key], registration);
+}
 #endif  // DCHECK_IS_ON()
 
 void ServiceWorkerProviderHost::RemoveAllMatchingRegistrations() {
@@ -812,6 +834,13 @@ void ServiceWorkerProviderHost::RemoveAllMatchingRegistrations() {
     registration->RemoveListener(this);
   }
   matching_registrations_.clear();
+
+  for (const auto& it : uninstalled_matching_registrations_) {
+    for (const auto& registration : it.second) {
+      registration->RemoveListener(this);
+    }
+  }
+  uninstalled_matching_registrations_.clear();
 }
 
 void ServiceWorkerProviderHost::ReturnRegistrationForReadyIfNeeded() {

content/browser/service_worker/service_worker_provider_host.h

@@ -373,9 +373,17 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   // This is subtle: it doesn't keep all registrations (e.g., from storage) in
   // memory, but just the ones that are possibly the longest-matching one. The
   // best match from storage is added at load time. That match can't uninstall
-  // while this host is a controllee, so all the other stored registrations can
-  // be ignored. Only a newly installed registration can claim it, and new
-  // installing registrations are added as matches.
+  // while this host is a controllee, but it can be unregistered and newly
+  // installed registations can override the entry if they have the same scope
+  // as an existing match. Since it is possible that multiple service workers
+  // for the same scope exist at any given time, e.g. when a worker is
+  // registered while an unregistered worker for the same scope is controlling
+  // clients, it's the callers responsibility to remove the matching
+  // registration when they no longer need it.
+  //
+  // Note: Chrome's implementation of .ready differs from the spec s.t. the
+  // ready promise is only resolved once it has been accessed by the user.
+  // https://github.com/w3c/ServiceWorker/issues/1477
   void AddMatchingRegistration(ServiceWorkerRegistration* registration);
   void RemoveMatchingRegistration(ServiceWorkerRegistration* registration);
 
@@ -515,6 +523,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
 
 #if DCHECK_IS_ON()
   bool IsMatchingRegistration(ServiceWorkerRegistration* registration) const;
+  bool IsMatchingUninstalledRegistration(
+      ServiceWorkerRegistration* registration);
 #endif  // DCHECK_IS_ON()
 
   // Discards all references to matching registrations.
@@ -674,6 +684,14 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   // AddMatchingRegistration().
   ServiceWorkerRegistrationMap matching_registrations_;
 
+  // Similar to ServiceWorkerRegistrationMap, but with multiple registrations.
+  using ServiceWorkerRegistrationMultiSet =
+      std::map<size_t, std::set<scoped_refptr<ServiceWorkerRegistration>>>;
+  // Contains registrations that have been uninstalled, e.g., by
+  // |registration.unregister()|, but are still alive, e.g., because they are
+  // controlling clients.
+  ServiceWorkerRegistrationMultiSet uninstalled_matching_registrations_;
+
   // Contains all ServiceWorkerRegistrationObjectHost instances corresponding to
   // the service worker registration JavaScript objects for the hosted execution
   // context (service worker global scope or service worker client) in the

content/browser/service_worker/service_worker_register_job.cc

@@ -124,15 +124,7 @@ void ServiceWorkerRegisterJob::StartImpl() {
                                weak_factory_.GetWeakPtr());
   }
 
-  scoped_refptr<ServiceWorkerRegistration> registration =
-      context_->storage()->GetUninstallingRegistration(scope_);
-  if (registration.get())
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::BindOnce(std::move(next_step),
-                       blink::ServiceWorkerStatusCode::kOk, registration));
-  else
-    context_->storage()->FindRegistrationForScope(scope_, std::move(next_step));
+  context_->storage()->FindRegistrationForScope(scope_, std::move(next_step));
 }
 
 void ServiceWorkerRegisterJob::Abort() {

third_party/blink/web_tests/TestExpectations

@@ -2878,10 +2878,6 @@ crbug.com/626703 external/wpt/mediacapture-streams/MediaStream-MediaElement-srcO
 crbug.com/626703 virtual/audio-service/external/wpt/mediacapture-streams/MediaStream-MediaElement-srcObject.https.html [ Timeout ]
 crbug.com/626703 virtual/feature-policy-permissions/external/wpt/mediacapture-streams/MediaStream-MediaElement-srcObject.https.html [ Timeout ]
 crbug.com/699040 external/wpt/svg/text/reftests/text-xml-space-001.svg [ Failure ]
-crbug.com/626703 external/wpt/service-workers/service-worker/ready.https.html [ Timeout ]
-crbug.com/626703 virtual/omt-service-worker-startup/external/wpt/service-workers/service-worker/ready.https.html [ Timeout ]
-crbug.com/626703 virtual/omt-worker-fetch/external/wpt/service-workers/service-worker/ready.https.html [ Timeout ]
-crbug.com/626703 virtual/cache-storage-sequence/external/wpt/service-workers/service-worker/ready.https.html [ Timeout ]
 crbug.com/626703 [ Linux ] external/wpt/css/css-fonts/math-script-level-and-math-style/math-script-level-auto-and-math-style-002.tentative.html [ Failure ]
 crbug.com/626703 [ Mac ] external/wpt/css/css-fonts/math-script-level-and-math-style/math-script-level-auto-and-math-style-002.tentative.html [ Failure ]
 crbug.com/626703 [ Win ] external/wpt/css/css-fonts/math-script-level-and-math-style/math-script-level-auto-and-math-style-002.tentative.html [ Failure ]

third_party/blink/web_tests/external/wpt/service-workers/service-worker/clients-matchall-client-types.https-expected.txt

 This is a testharness.js-based test.
 FAIL Verify matchAll() with window client type assert_array_equals: property 2, expected "https://web-platform.test:8444/service-workers/service-worker/resources/clients-matchall-client-types-iframe.html" but got "https://web-platform.test:8444/service-workers/service-worker/resources/url-modified-via-pushstate.html"
-FAIL Verify matchAll() with {window, sharedworker, worker} client types promise_test: Unhandled rejection with value: object "Error: wait_for_state must be passed a ServiceWorker"
+FAIL Verify matchAll() with {window, sharedworker, worker} client types assert_array_equals: property 2, expected "https://web-platform.test:8444/service-workers/service-worker/resources/clients-matchall-client-types-iframe.html" but got "https://web-platform.test:8444/service-workers/service-worker/resources/url-modified-via-pushstate.html"
 Harness: the test ran to completion.
 

third_party/blink/web_tests/external/wpt/service-workers/service-worker/ready.https-expected.txt

@@ -7,6 +7,7 @@ PASS ready on an iframe whose parent registers a new service worker
 PASS ready on an iframe that installs a new service worker
 PASS ready after a longer matched registration registered
 PASS access ready after it has been resolved
-FAIL access ready on uninstalling registration that is resurrected assert_not_equals: ready promise should resolve before timeout got disallowed value null
+PASS resolve ready after unregistering and reregistering
+FAIL resolve ready before unregistering and reregistering assert_equals: Resolves with the first registration expected "https://web-platform.test:8444/service-workers/service-worker/resources/empty-worker.js" but got "https://web-platform.test:8444/service-workers/service-worker/resources/empty-worker.js?2"
 Harness: the test ran to completion.
 

third_party/blink/web_tests/external/wpt/service-workers/service-worker/unregister-then-register-new-script.https-expected.txt

-This is a testharness.js-based test.
-FAIL Registering a new script URL while an unregistered registration is in use assert_equals: before activated registration.installing expected null but got object "[object ServiceWorker]"
-PASS Registering a new script URL that 404s does not resurrect unregistered registration
-FAIL Registering a new script URL that fails to install does not resurrect unregistered registration assert_not_equals: New registration is different got disallowed value object "[object ServiceWorkerRegistration]"
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/service-workers/service-worker/unregister-then-register.https-expected.txt

-This is a testharness.js-based test.
-PASS Unregister then register resolves to a new value
-FAIL Unregister then register does not resolve to the original value even if the registration is in use. assert_not_equals: Unregister and register should always create a new registration got disallowed value object "[object ServiceWorkerRegistration]"
-PASS Unregister then register does not affect existing controllee
-FAIL Unregister then register does not resurrect the registration assert_equals: Registration is new expected null but got object "[object ServiceWorker]"
-Harness: the test ran to completion.
-