service worker: Add ServiceWorkerRegistry::CreateNewRegistration()

The purpose of this change is to have a single point of creating
ServiceWorkerRegistration. This way,
* We can hide NewRegistrationId() inside ServiceWorkerRegistry.
  This is useful because we plan to move id management into the
  Storage Service and we don't want to expose it other than
  ServiceWorkerRegistry.
* ServiceWorkerRegistry can track all live registrations. In
  subsequent CLs we plan to hook ServiceWorkerRegistration creation
  so that ServiceWorkerRegistry can detect if the registration is
  stored or not.

Bug: 1039200
Change-Id: Ieeeb4c3b156488de8fe6a527515ec23998c5ea3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2010599
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733542}

content/browser/service_worker/embedded_worker_instance_unittest.cc

@@ -118,9 +118,7 @@ class EmbeddedWorkerInstanceTest : public testing::Test,
     RegistrationAndVersionPair pair;
     blink::mojom::ServiceWorkerRegistrationOptions options;
     options.scope = scope;
-    pair.first = base::MakeRefCounted<ServiceWorkerRegistration>(
-        options, context()->storage()->NewRegistrationId(),
-        context()->AsWeakPtr());
+    pair.first = context()->registry()->CreateNewRegistration(options);
     pair.second = base::MakeRefCounted<ServiceWorkerVersion>(
         pair.first.get(), script_url, blink::mojom::ScriptType::kClassic,
         context()->storage()->NewVersionId(), context()->AsWeakPtr());

content/browser/service_worker/service_worker_navigation_loader_unittest.cc

@@ -342,8 +342,7 @@ class ServiceWorkerNavigationLoaderTest : public testing::Test {
     blink::mojom::ServiceWorkerRegistrationOptions options;
     options.scope = GURL("https://example.com/");
     registration_ =
-        new ServiceWorkerRegistration(options, storage()->NewRegistrationId(),
-                                      helper_->context()->AsWeakPtr());
+        helper_->context()->registry()->CreateNewRegistration(options);
     version_ = new ServiceWorkerVersion(
         registration_.get(), GURL("https://example.com/service_worker.js"),
         blink::mojom::ScriptType::kClassic, storage()->NewVersionId(),

content/browser/service_worker/service_worker_new_script_loader_unittest.cc

@@ -180,9 +180,7 @@ class ServiceWorkerNewScriptLoaderTest : public testing::Test {
   void SetUpRegistrationWithOptions(
       const GURL& script_url,
       blink::mojom::ServiceWorkerRegistrationOptions options) {
-    registration_ = base::MakeRefCounted<ServiceWorkerRegistration>(
-        options, context()->storage()->NewRegistrationId(),
-        context()->AsWeakPtr());
+    registration_ = context()->registry()->CreateNewRegistration(options);
     SetUpVersion(script_url);
   }
 

content/browser/service_worker/service_worker_object_host_unittest.cc

@@ -116,9 +116,8 @@ class ServiceWorkerObjectHostTest : public testing::Test {
 
     blink::mojom::ServiceWorkerRegistrationOptions options;
     options.scope = scope;
-    registration_ = new ServiceWorkerRegistration(
-        options, helper_->context()->storage()->NewRegistrationId(),
-        helper_->context()->AsWeakPtr());
+    registration_ =
+        helper_->context()->registry()->CreateNewRegistration(options);
     version_ = new ServiceWorkerVersion(
         registration_.get(), script_url, blink::mojom::ScriptType::kClassic,
         helper_->context()->storage()->NewVersionId(),

content/browser/service_worker/service_worker_register_job.cc

@@ -422,16 +422,16 @@ void ServiceWorkerRegisterJob::OnUpdateCheckFinished(
 void ServiceWorkerRegisterJob::RegisterAndContinue() {
   SetPhase(REGISTER);
 
-  int64_t registration_id = context_->storage()->NewRegistrationId();
-  if (registration_id == blink::mojom::kInvalidServiceWorkerRegistrationId) {
+  blink::mojom::ServiceWorkerRegistrationOptions options(
+      scope_, worker_script_type_, update_via_cache_);
+  scoped_refptr<ServiceWorkerRegistration> new_registration =
+      context_->registry()->CreateNewRegistration(options);
+  if (!new_registration) {
     Complete(blink::ServiceWorkerStatusCode::kErrorAbort);
     return;
   }
 
-  blink::mojom::ServiceWorkerRegistrationOptions options(
-      scope_, worker_script_type_, update_via_cache_);
-  set_registration(new ServiceWorkerRegistration(options, registration_id,
-                                                 context_->AsWeakPtr()));
+  set_registration(std::move(new_registration));
   AddRegistrationToMatchingProviderHosts(registration());
   UpdateAndContinue();
 }

content/browser/service_worker/service_worker_registration.h

@@ -71,6 +71,8 @@ class CONTENT_EXPORT ServiceWorkerRegistration
     kUninstalled,
   };
 
+  // The constructor should be called only from ServiceWorkerRegistry other than
+  // tests.
   ServiceWorkerRegistration(
       const blink::mojom::ServiceWorkerRegistrationOptions& options,
       int64_t registration_id,

content/browser/service_worker/service_worker_registration_unittest.cc

@@ -345,8 +345,7 @@ TEST_F(ServiceWorkerRegistrationTest, NavigationPreload) {
   blink::mojom::ServiceWorkerRegistrationOptions options;
   options.scope = kScope;
   scoped_refptr<ServiceWorkerRegistration> registration =
-      base::MakeRefCounted<ServiceWorkerRegistration>(
-          options, storage()->NewRegistrationId(), context()->AsWeakPtr());
+      context()->registry()->CreateNewRegistration(options);
   scoped_refptr<ServiceWorkerVersion> version_1 =
       base::MakeRefCounted<ServiceWorkerVersion>(
           registration.get(), kScript, blink::mojom::ScriptType::kClassic,
@@ -394,8 +393,7 @@ class ServiceWorkerActivationTest : public ServiceWorkerRegistrationTest,
 
     blink::mojom::ServiceWorkerRegistrationOptions options;
     options.scope = kScope;
-    registration_ = base::MakeRefCounted<ServiceWorkerRegistration>(
-        options, storage()->NewRegistrationId(), context()->AsWeakPtr());
+    registration_ = context()->registry()->CreateNewRegistration(options);
 
     // Create an active version.
     scoped_refptr<ServiceWorkerVersion> version_1 =
@@ -881,12 +879,11 @@ class ServiceWorkerRegistrationObjectHostTest
     return status.value();
   }
 
-  scoped_refptr<ServiceWorkerRegistration> CreateRegistration(
+  scoped_refptr<ServiceWorkerRegistration> CreateNewRegistration(
       const GURL& scope) {
     blink::mojom::ServiceWorkerRegistrationOptions options;
     options.scope = scope;
-    return base::MakeRefCounted<ServiceWorkerRegistration>(
-        options, storage()->NewRegistrationId(), context()->AsWeakPtr());
+    return context()->registry()->CreateNewRegistration(options);
   }
 
   scoped_refptr<ServiceWorkerVersion> CreateVersion(
@@ -914,7 +911,7 @@ class ServiceWorkerRegistrationObjectHostTest
 
     // Prepare ServiceWorkerRegistration and ServiceWorkerVersion.
     scoped_refptr<ServiceWorkerRegistration> registration =
-        CreateRegistration(scope);
+        CreateNewRegistration(scope);
     scoped_refptr<ServiceWorkerVersion> version =
         CreateVersion(registration.get(), script_url);
 
@@ -1113,7 +1110,7 @@ TEST_P(ServiceWorkerRegistrationObjectHostUpdateTest,
   const GURL kScope("https://www.example.com/");
   const GURL kScriptUrl("https://www.example.com/sw.js");
   scoped_refptr<ServiceWorkerRegistration> registration =
-      CreateRegistration(kScope);
+      CreateNewRegistration(kScope);
   scoped_refptr<ServiceWorkerVersion> version =
       CreateVersion(registration.get(), kScriptUrl);
 
@@ -1144,7 +1141,7 @@ TEST_P(ServiceWorkerRegistrationObjectHostUpdateTest,
   const GURL kScope("https://www.example.com/");
   const GURL kScriptUrl("https://www.example.com/sw.js");
   scoped_refptr<ServiceWorkerRegistration> registration =
-      CreateRegistration(kScope);
+      CreateNewRegistration(kScope);
 
   scoped_refptr<ServiceWorkerVersion> version =
       CreateVersion(registration.get(), kScriptUrl);

content/browser/service_worker/service_worker_registry.cc

@@ -64,6 +64,16 @@ ServiceWorkerRegistry::ServiceWorkerRegistry(
 
 ServiceWorkerRegistry::~ServiceWorkerRegistry() = default;
 
+scoped_refptr<ServiceWorkerRegistration>
+ServiceWorkerRegistry::CreateNewRegistration(
+    blink::mojom::ServiceWorkerRegistrationOptions options) {
+  int64_t registration_id = storage()->NewRegistrationId();
+  if (registration_id == blink::mojom::kInvalidServiceWorkerRegistrationId)
+    return nullptr;
+  return base::MakeRefCounted<ServiceWorkerRegistration>(
+      std::move(options), registration_id, context_->AsWeakPtr());
+}
+
 void ServiceWorkerRegistry::FindRegistrationForClientUrl(
     const GURL& client_url,
     FindRegistrationCallback callback) {

content/browser/service_worker/service_worker_registry.h

@@ -68,6 +68,12 @@ class CONTENT_EXPORT ServiceWorkerRegistry {
 
   ServiceWorkerStorage* storage() const { return storage_.get(); }
 
+  // Creates a new in-memory representation of registration. Can be null when
+  // storage is disabled. This method must be called after storage is
+  // initialized.
+  scoped_refptr<ServiceWorkerRegistration> CreateNewRegistration(
+      blink::mojom::ServiceWorkerRegistrationOptions options);
+
   // TODO(crbug.com/1039200): Move corresponding comments from
   // ServiceWorkerStorage.
   void FindRegistrationForClientUrl(const GURL& client_url,

content/browser/service_worker/service_worker_test_utils.cc

@@ -355,8 +355,7 @@ CreateServiceWorkerRegistrationAndVersion(ServiceWorkerContextCore* context,
 
   blink::mojom::ServiceWorkerRegistrationOptions options;
   options.scope = scope;
-  auto registration = base::MakeRefCounted<ServiceWorkerRegistration>(
-      options, storage->NewRegistrationId(), context->AsWeakPtr());
+  auto registration = context->registry()->CreateNewRegistration(options);
   auto version = base::MakeRefCounted<ServiceWorkerVersion>(
       registration.get(), script, blink::mojom::ScriptType::kClassic,
       storage->NewVersionId(), context->AsWeakPtr());

content/browser/service_worker/service_worker_updated_script_loader_unittest.cc

@@ -98,9 +98,7 @@ class ServiceWorkerUpdatedScriptLoaderTest : public testing::Test {
   void SetUpRegistrationWithOptions(
       const GURL& script_url,
       blink::mojom::ServiceWorkerRegistrationOptions options) {
-    registration_ = base::MakeRefCounted<ServiceWorkerRegistration>(
-        options, context()->storage()->NewRegistrationId(),
-        context()->AsWeakPtr());
+    registration_ = context()->registry()->CreateNewRegistration(options);
     SetUpVersion(script_url);
   }
 

content/browser/service_worker/service_worker_version_browsertest.cc

@@ -433,9 +433,8 @@ class ServiceWorkerVersionBrowserTest : public ContentBrowserTest {
     blink::mojom::ServiceWorkerRegistrationOptions options;
     options.scope = scope;
     options.type = script_type;
-    registration_ = new ServiceWorkerRegistration(
-        options, wrapper()->context()->storage()->NewRegistrationId(),
-        wrapper()->context()->AsWeakPtr());
+    registration_ =
+        wrapper()->context()->registry()->CreateNewRegistration(options);
     // Set the update check time to avoid triggering updates in the middle of
     // tests.
     registration_->set_last_update_check(base::Time::Now());

content/browser/service_worker/service_worker_version_unittest.cc

@@ -90,9 +90,8 @@ class ServiceWorkerVersionTest : public testing::Test {
     scope_ = GURL("https://www.example.com/test/");
     blink::mojom::ServiceWorkerRegistrationOptions options;
     options.scope = scope_;
-    registration_ = new ServiceWorkerRegistration(
-        options, helper_->context()->storage()->NewRegistrationId(),
-        helper_->context()->AsWeakPtr());
+    registration_ =
+        helper_->context()->registry()->CreateNewRegistration(options);
     version_ = new ServiceWorkerVersion(
         registration_.get(),
         GURL("https://www.example.com/test/service_worker.js"),
@@ -1115,9 +1114,8 @@ TEST_F(ServiceWorkerVersionTest, BadOrigin) {
   const GURL scope("bad-origin://www.example.com/test/");
   blink::mojom::ServiceWorkerRegistrationOptions options;
   options.scope = scope;
-  auto registration = base::MakeRefCounted<ServiceWorkerRegistration>(
-      options, helper_->context()->storage()->NewRegistrationId(),
-      helper_->context()->AsWeakPtr());
+  auto registration =
+      helper_->context()->registry()->CreateNewRegistration(options);
   auto version = base::MakeRefCounted<ServiceWorkerVersion>(
       registration_.get(),
       GURL("bad-origin://www.example.com/test/service_worker.js"),