service worker: Return live version reference from GetRegistrationsForOrigin()

The method is used by ServiceWorkerRegistry and it creates a
ServiceWorkerVersion in DidGetRegistrationsForOrigins(). The method
should pass a ServiceWorkerLiveVersionRef so that a ServiceWorkerVersion
can keep it during the lifetime of the ServiceWorkerVersion.

This CL:
* Merges SerializedServiceWorkerRegistration into
  ServiceWorkerFindRegistrationResult as these are almost the same.
* Moves |status| out of ServiceWorkerFindRegistrationResult because
  it doesn't much make sense for GetRegistrationsForOrigin()
* Tweaks ServiceWorkerStorageControlImpl to create
  ServiceWorkerLiveVersionRef for GetRegistrationsForOrigin()

This CL doesn't pass a live version reference to ServiceWorkerVersion
yet. A follow-up CL will pass the reference to it.

Bug: 1055677
Change-Id: I408f741e7f6a199fdc162f4e68c98233b2b6a50a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2331961
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794335}

components/services/storage/public/mojom/service_worker_storage_control.mojom

@@ -12,12 +12,6 @@ import "services/network/public/mojom/url_response_head.mojom";
 import "url/mojom/origin.mojom";
 import "url/mojom/url.mojom";
 
-// This struct represents a serialized service worker registration.
-struct SerializedServiceWorkerRegistration {
-  ServiceWorkerRegistrationData registration_data;
-  array<ServiceWorkerResourceRecord> resources;
-};
-
 // An interface that is used to keep track of which service worker versions are
 // being used by clients of the storage service. This is an empty interface that
 // is mapped internally by the storage service to a single version.
@@ -30,20 +24,15 @@ struct SerializedServiceWorkerRegistration {
 // owned by the C++ ServiceWorkerVersion instance.
 interface ServiceWorkerLiveVersionRef {};
 
-// Conveys a result of finding a registration. If a registration is found,
-// |status| will be kOk. |version_reference|, |registration| and |resources| are
-// null or empty if there is no matching registration.
-//
+// Conveys a result of finding a registration.
 // The Storage Service (components/services/storage) supplies this
 // information and the //content consumes the information.
 struct ServiceWorkerFindRegistrationResult {
-  // The result of a find operation.
-  ServiceWorkerDatabaseStatus status;
   // A reference to a service worker version associated with
   // |registration->version_id|.
-  pending_remote<ServiceWorkerLiveVersionRef>? version_reference;
+  pending_remote<ServiceWorkerLiveVersionRef> version_reference;
   // Stored registration.
-  ServiceWorkerRegistrationData? registration;
+  ServiceWorkerRegistrationData registration;
   // Resources associated with |registration|.
   array<ServiceWorkerResourceRecord> resources;
 };
@@ -122,19 +111,22 @@ interface ServiceWorkerStorageControl {
 
   // Reads a stored registration for |client_id|.
   FindRegistrationForClientUrl(url.mojom.Url client_url) =>
-      (ServiceWorkerFindRegistrationResult result);
+      (ServiceWorkerDatabaseStatus status,
+       ServiceWorkerFindRegistrationResult? result);
   // Reads a stored registration for |scope|.
   FindRegistrationForScope(url.mojom.Url scope) =>
-      (ServiceWorkerFindRegistrationResult result);
+      (ServiceWorkerDatabaseStatus status,
+       ServiceWorkerFindRegistrationResult? result);
   // Reads a stored registration for |registration_id|. |origin| is to
   // be used as a hint to look up the registration faster.
   FindRegistrationForId(int64 registration_id, url.mojom.Url? origin) =>
-      (ServiceWorkerFindRegistrationResult result);
+      (ServiceWorkerDatabaseStatus status,
+       ServiceWorkerFindRegistrationResult? result);
 
   // Returns all stored registrations for a given origin.
   GetRegistrationsForOrigin(url.mojom.Origin origin) =>
       (ServiceWorkerDatabaseStatus status,
-       array<SerializedServiceWorkerRegistration> registrations);
+       array<ServiceWorkerFindRegistrationResult> registrations);
 
   // Returns the total resource size for a given origin.
   GetUsageForOrigin(url.mojom.Origin origin) =>

content/browser/service_worker/service_worker_registry.cc

@@ -1006,8 +1006,8 @@ void ServiceWorkerRegistry::DidGetRegistrationsForOrigin(
     GetRegistrationsCallback callback,
     const url::Origin& origin_filter,
     storage::mojom::ServiceWorkerDatabaseStatus database_status,
-    std::vector<storage::mojom::SerializedServiceWorkerRegistrationPtr>
-        serialized_registrations) {
+    std::vector<storage::mojom::ServiceWorkerFindRegistrationResultPtr>
+        entries) {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
 
   blink::ServiceWorkerStatusCode status =
@@ -1024,10 +1024,10 @@ void ServiceWorkerRegistry::DidGetRegistrationsForOrigin(
   // Add all stored registrations.
   std::set<int64_t> registration_ids;
   std::vector<scoped_refptr<ServiceWorkerRegistration>> registrations;
-  for (const auto& entry : serialized_registrations) {
-    registration_ids.insert(entry->registration_data->registration_id);
+  for (const auto& entry : entries) {
+    registration_ids.insert(entry->registration->registration_id);
     registrations.push_back(
-        GetOrCreateRegistration(*entry->registration_data, entry->resources));
+        GetOrCreateRegistration(*entry->registration, entry->resources));
   }
 
   // Add unstored registrations that are being installed.

content/browser/service_worker/service_worker_registry.h

@@ -291,8 +291,8 @@ class CONTENT_EXPORT ServiceWorkerRegistry {
       GetRegistrationsCallback callback,
       const url::Origin& origin_filter,
       storage::mojom::ServiceWorkerDatabaseStatus database_status,
-      std::vector<storage::mojom::SerializedServiceWorkerRegistrationPtr>
-          serialized_registrations);
+      std::vector<storage::mojom::ServiceWorkerFindRegistrationResultPtr>
+          entries);
   void DidGetAllRegistrations(
       GetRegistrationsInfosCallback callback,
       storage::mojom::ServiceWorkerDatabaseStatus database_status,

content/browser/service_worker/service_worker_storage_control_impl.cc

@@ -13,34 +13,6 @@ namespace content {
 
 namespace {
 
-void DidGetRegistrationsForOrigin(
-    ServiceWorkerStorageControlImpl::GetRegistrationsForOriginCallback callback,
-    storage::mojom::ServiceWorkerDatabaseStatus status,
-    std::unique_ptr<ServiceWorkerStorage::RegistrationList>
-        registration_data_list,
-    std::unique_ptr<std::vector<ServiceWorkerStorage::ResourceList>>
-        resources_list) {
-  if (status != storage::mojom::ServiceWorkerDatabaseStatus::kOk) {
-    std::move(callback).Run(
-        status,
-        std::vector<storage::mojom::SerializedServiceWorkerRegistrationPtr>());
-    return;
-  }
-
-  DCHECK_EQ(registration_data_list->size(), resources_list->size());
-
-  std::vector<storage::mojom::SerializedServiceWorkerRegistrationPtr>
-      registrations;
-  for (size_t i = 0; i < registration_data_list->size(); ++i) {
-    registrations.push_back(
-        storage::mojom::SerializedServiceWorkerRegistration::New(
-            std::move((*registration_data_list)[i]),
-            std::move((*resources_list)[i])));
-  }
-
-  std::move(callback).Run(status, std::move(registrations));
-}
-
 void DidGetUserDataForAllRegistrations(
     ServiceWorkerStorageControlImpl::GetUserDataForAllRegistrationsCallback
         callback,
@@ -193,7 +165,9 @@ void ServiceWorkerStorageControlImpl::GetRegistrationsForOrigin(
     GetRegistrationsForOriginCallback callback) {
   storage_->GetRegistrationsForOrigin(
       origin,
-      base::BindOnce(&DidGetRegistrationsForOrigin, std::move(callback)));
+      base::BindOnce(
+          &ServiceWorkerStorageControlImpl::DidGetRegistrationsForOrigin,
+          weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
 void ServiceWorkerStorageControlImpl::GetUsageForOrigin(
@@ -406,30 +380,56 @@ void ServiceWorkerStorageControlImpl::ApplyPolicyUpdates(
 
 void ServiceWorkerStorageControlImpl::DidFindRegistration(
     base::OnceCallback<
-        void(storage::mojom::ServiceWorkerFindRegistrationResultPtr)> callback,
+        void(storage::mojom::ServiceWorkerDatabaseStatus status,
+             storage::mojom::ServiceWorkerFindRegistrationResultPtr)> callback,
     storage::mojom::ServiceWorkerRegistrationDataPtr data,
     std::unique_ptr<ResourceList> resources,
     storage::mojom::ServiceWorkerDatabaseStatus status) {
-  ResourceList resource_list =
-      resources ? std::move(*resources) : ResourceList();
+  if (status != storage::mojom::ServiceWorkerDatabaseStatus::kOk) {
+    std::move(callback).Run(status, /*result=*/nullptr);
+    return;
+  }
+
+  DCHECK(resources);
+  DCHECK(data);
 
+  ResourceList resource_list = std::move(*resources);
   mojo::PendingRemote<storage::mojom::ServiceWorkerLiveVersionRef>
-      remote_reference;
-  if (data &&
-      data->version_id != blink::mojom::kInvalidServiceWorkerVersionId) {
-    DCHECK_EQ(status, storage::mojom::ServiceWorkerDatabaseStatus::kOk);
-    auto it = live_versions_.find(data->version_id);
-    if (it == live_versions_.end()) {
-      remote_reference = CreateLiveVersionReference(data->version_id);
-    } else {
-      it->second->Add(remote_reference.InitWithNewPipeAndPassReceiver());
-    }
-  }
+      remote_reference = CreateLiveVersionReferenceRemote(data->version_id);
 
   std::move(callback).Run(
-      storage::mojom::ServiceWorkerFindRegistrationResult::New(
-          status, std::move(remote_reference), std::move(data),
-          std::move(resource_list)));
+      status, storage::mojom::ServiceWorkerFindRegistrationResult::New(
+                  std::move(remote_reference), std::move(data),
+                  std::move(resource_list)));
+}
+
+void ServiceWorkerStorageControlImpl::DidGetRegistrationsForOrigin(
+    GetRegistrationsForOriginCallback callback,
+    storage::mojom::ServiceWorkerDatabaseStatus status,
+    std::unique_ptr<ServiceWorkerStorage::RegistrationList>
+        registration_data_list,
+    std::unique_ptr<std::vector<ResourceList>> resources_list) {
+  if (status != storage::mojom::ServiceWorkerDatabaseStatus::kOk) {
+    std::move(callback).Run(
+        status,
+        std::vector<storage::mojom::ServiceWorkerFindRegistrationResultPtr>());
+    return;
+  }
+
+  DCHECK_EQ(registration_data_list->size(), resources_list->size());
+
+  std::vector<storage::mojom::ServiceWorkerFindRegistrationResultPtr>
+      registrations;
+  for (size_t i = 0; i < registration_data_list->size(); ++i) {
+    int64_t version_id = (*registration_data_list)[i]->version_id;
+    registrations.push_back(
+        storage::mojom::ServiceWorkerFindRegistrationResult::New(
+            CreateLiveVersionReferenceRemote(version_id),
+            std::move((*registration_data_list)[i]),
+            std::move((*resources_list)[i])));
+  }
+
+  std::move(callback).Run(status, std::move(registrations));
 }
 
 void ServiceWorkerStorageControlImpl::DidStoreRegistration(
@@ -457,23 +457,27 @@ void ServiceWorkerStorageControlImpl::DidGetNewVersionId(
   mojo::PendingRemote<storage::mojom::ServiceWorkerLiveVersionRef>
       remote_reference;
   if (version_id != blink::mojom::kInvalidServiceWorkerVersionId) {
-    remote_reference = CreateLiveVersionReference(version_id);
+    remote_reference = CreateLiveVersionReferenceRemote(version_id);
   }
   std::move(callback).Run(version_id, std::move(remote_reference));
 }
 
 mojo::PendingRemote<storage::mojom::ServiceWorkerLiveVersionRef>
-ServiceWorkerStorageControlImpl::CreateLiveVersionReference(
+ServiceWorkerStorageControlImpl::CreateLiveVersionReferenceRemote(
     int64_t version_id) {
   DCHECK_NE(version_id, blink::mojom::kInvalidServiceWorkerVersionId);
-  DCHECK(!base::Contains(live_versions_, version_id));
 
   mojo::PendingRemote<storage::mojom::ServiceWorkerLiveVersionRef>
       remote_reference;
-  auto reference = std::make_unique<ServiceWorkerLiveVersionRefImpl>(
-      weak_ptr_factory_.GetWeakPtr(), version_id);
-  reference->Add(remote_reference.InitWithNewPipeAndPassReceiver());
-  live_versions_[version_id] = std::move(reference);
+  auto it = live_versions_.find(version_id);
+  if (it == live_versions_.end()) {
+    auto reference = std::make_unique<ServiceWorkerLiveVersionRefImpl>(
+        weak_ptr_factory_.GetWeakPtr(), version_id);
+    reference->Add(remote_reference.InitWithNewPipeAndPassReceiver());
+    live_versions_[version_id] = std::move(reference);
+  } else {
+    it->second->Add(remote_reference.InitWithNewPipeAndPassReceiver());
+  }
   return remote_reference;
 }
 

content/browser/service_worker/service_worker_storage_control_impl.h

@@ -161,10 +161,17 @@ class CONTENT_EXPORT ServiceWorkerStorageControlImpl
   // Callbacks for ServiceWorkerStorage methods.
   void DidFindRegistration(
       base::OnceCallback<void(
+          storage::mojom::ServiceWorkerDatabaseStatus status,
           storage::mojom::ServiceWorkerFindRegistrationResultPtr)> callback,
       storage::mojom::ServiceWorkerRegistrationDataPtr data,
       std::unique_ptr<ResourceList> resources,
       storage::mojom::ServiceWorkerDatabaseStatus status);
+  void DidGetRegistrationsForOrigin(
+      GetRegistrationsForOriginCallback callback,
+      storage::mojom::ServiceWorkerDatabaseStatus status,
+      std::unique_ptr<ServiceWorkerStorage::RegistrationList>
+          registration_data_list,
+      std::unique_ptr<std::vector<ResourceList>> resources_list);
   void DidStoreRegistration(
       StoreRegistrationCallback callback,
       storage::mojom::ServiceWorkerDatabaseStatus status,
@@ -179,7 +186,7 @@ class CONTENT_EXPORT ServiceWorkerStorageControlImpl
   void DidGetNewVersionId(GetNewVersionIdCallback callback, int64_t version_id);
 
   mojo::PendingRemote<storage::mojom::ServiceWorkerLiveVersionRef>
-  CreateLiveVersionReference(int64_t version_id);
+  CreateLiveVersionReferenceRemote(int64_t version_id);
 
   void MaybePurgeResources(int64_t version_id,
                            const std::vector<int64_t>& purgeable_resources);