Remove ServiceWorkerVersion dependencies from ServiceWorkerStorage::Did{Store,Delete}Registration

When ServiceWorkerStorage is moved to the Storage Service it
wouldn't be able to access live ServiceWorkerVersions directly.
This CL moves live version lookup to ServiceWorkerRegistry as a
preparation for moving ServiceWorkerStorage to the Storage Service.

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

content/browser/service_worker/service_worker_registry.cc

@@ -551,12 +551,30 @@ void ServiceWorkerRegistry::DidGetAllRegistrations(
 void ServiceWorkerRegistry::DidStoreRegistration(
     const ServiceWorkerDatabase::RegistrationData& data,
     StatusCallback callback,
-    blink::ServiceWorkerStatusCode status) {
+    blink::ServiceWorkerStatusCode status,
+    int64_t deleted_version_id,
+    const std::vector<int64_t>& newly_purgeable_resources) {
   if (status != blink::ServiceWorkerStatusCode::kOk) {
     std::move(callback).Run(status);
     return;
   }
 
+  // Purge the deleted version's resources now if needed. This is subtle. The
+  // version might still be used for a long time even after it's deleted. We can
+  // only purge safely once the version is REDUNDANT, since it will never be
+  // used again.
+  //
+  // If the deleted version's ServiceWorkerVersion doesn't exist, we can assume
+  // it's effectively REDUNDANT so it's safe to purge now. This is because the
+  // caller is assumed to promote the new version to active unless the deleted
+  // version is doing work, and it can't be doing work if it's not live.
+  //
+  // If the ServiceWorkerVersion does exist, it triggers purging once it reaches
+  // REDUNDANT. Otherwise, purging happens on the next browser session (via
+  // DeleteStaleResources).
+  if (!context_->GetLiveVersion(deleted_version_id))
+    storage()->PurgeResources(newly_purgeable_resources);
+
   scoped_refptr<ServiceWorkerRegistration> registration =
       context_->GetLiveRegistration(data.registration_id);
   if (registration) {
@@ -570,10 +588,12 @@ void ServiceWorkerRegistry::DidStoreRegistration(
 
 void ServiceWorkerRegistry::DidDeleteRegistration(
     StatusCallback callback,
-    blink::ServiceWorkerStatusCode status) {
-  // TODO(crbug.com/1039200): Move code from
-  // ServiceWorkerStorage::DidDeleteRegistration() which depends on
-  // ServiceWorkerContextCore.
+    blink::ServiceWorkerStatusCode status,
+    int64_t deleted_version_id,
+    const std::vector<int64_t>& newly_purgeable_resources) {
+  if (!context_->GetLiveVersion(deleted_version_id))
+    storage()->PurgeResources(newly_purgeable_resources);
+
   std::move(callback).Run(status);
 }
 

content/browser/service_worker/service_worker_registry.h

@@ -170,11 +170,17 @@ class CONTENT_EXPORT ServiceWorkerRegistry {
       blink::ServiceWorkerStatusCode status,
       std::unique_ptr<RegistrationList> registration_data_list);
 
-  void DidStoreRegistration(const ServiceWorkerDatabase::RegistrationData& data,
-                            StatusCallback callback,
-                            blink::ServiceWorkerStatusCode status);
-  void DidDeleteRegistration(StatusCallback callback,
-                             blink::ServiceWorkerStatusCode status);
+  void DidStoreRegistration(
+      const ServiceWorkerDatabase::RegistrationData& data,
+      StatusCallback callback,
+      blink::ServiceWorkerStatusCode status,
+      int64_t deleted_version_id,
+      const std::vector<int64_t>& newly_purgeable_resources);
+  void DidDeleteRegistration(
+      StatusCallback callback,
+      blink::ServiceWorkerStatusCode status,
+      int64_t deleted_version_id,
+      const std::vector<int64_t>& newly_purgeable_resources);
 
   // The ServiceWorkerContextCore object must outlive this.
   ServiceWorkerContextCore* const context_;

content/browser/service_worker/service_worker_storage.cc

@@ -96,7 +96,7 @@ ServiceWorkerStorage::InitialData::~InitialData() {
 ServiceWorkerStorage::DidDeleteRegistrationParams::DidDeleteRegistrationParams(
     int64_t registration_id,
     GURL origin,
-    StatusCallback callback)
+    DeleteRegistrationCallback callback)
     : registration_id(registration_id),
       origin(origin),
       callback(std::move(callback)) {}
@@ -365,7 +365,7 @@ void ServiceWorkerStorage::GetAllRegistrations(
 void ServiceWorkerStorage::StoreRegistrationData(
     const ServiceWorkerDatabase::RegistrationData& registration_data,
     const ResourceList& resources,
-    StatusCallback callback) {
+    StoreRegistrationDataCallback callback) {
   DCHECK_EQ(state_, STORAGE_STATE_INITIALIZED);
 
   if (!has_checked_for_stale_resources_)
@@ -472,9 +472,10 @@ void ServiceWorkerStorage::UpdateNavigationPreloadHeader(
       base::BindOnce(&DidUpdateNavigationPreloadState, std::move(callback)));
 }
 
-void ServiceWorkerStorage::DeleteRegistration(int64_t registration_id,
-                                              const GURL& origin,
-                                              StatusCallback callback) {
+void ServiceWorkerStorage::DeleteRegistration(
+    int64_t registration_id,
+    const GURL& origin,
+    DeleteRegistrationCallback callback) {
   DCHECK_EQ(state_, STORAGE_STATE_INITIALIZED);
 
   if (!has_checked_for_stale_resources_)
@@ -1009,6 +1010,13 @@ void ServiceWorkerStorage::PurgeResources(const ResourceList& resources) {
   StartPurgingResources(resources);
 }
 
+void ServiceWorkerStorage::PurgeResources(
+    const std::vector<int64_t>& resource_ids) {
+  if (!has_checked_for_stale_resources_)
+    DeleteStaleResources();
+  StartPurgingResources(resource_ids);
+}
+
 ServiceWorkerStorage::ServiceWorkerStorage(
     const base::FilePath& user_data_directory,
     ServiceWorkerContextCore* context,
@@ -1163,7 +1171,7 @@ void ServiceWorkerStorage::DidGetAllRegistrations(
 }
 
 void ServiceWorkerStorage::DidStoreRegistrationData(
-    StatusCallback callback,
+    StoreRegistrationDataCallback callback,
     const ServiceWorkerDatabase::RegistrationData& new_version,
     const GURL& origin,
     const ServiceWorkerDatabase::RegistrationData& deleted_version,
@@ -1171,7 +1179,9 @@ void ServiceWorkerStorage::DidStoreRegistrationData(
     ServiceWorkerDatabase::Status status) {
   if (status != ServiceWorkerDatabase::STATUS_OK) {
     ScheduleDeleteAndStartOver();
-    std::move(callback).Run(DatabaseStatusToStatusCode(status));
+    std::move(callback).Run(DatabaseStatusToStatusCode(status),
+                            deleted_version.version_id,
+                            newly_purgeable_resources);
     return;
   }
   registered_origins_.insert(origin);
@@ -1185,23 +1195,9 @@ void ServiceWorkerStorage::DidStoreRegistrationData(
             deleted_version.resources_total_size_bytes);
   }
 
-  // Purge the deleted version's resources now if needed. This is subtle. The
-  // version might still be used for a long time even after it's deleted. We can
-  // only purge safely once the version is REDUNDANT, since it will never be
-  // used again.
-  //
-  // If the deleted version's ServiceWorkerVersion doesn't exist, we can assume
-  // it's effectively REDUNDANT so it's safe to purge now. This is because the
-  // caller is assumed to promote the new version to active unless the deleted
-  // version is doing work, and it can't be doing work if it's not live.
-  //
-  // If the ServiceWorkerVersion does exist, it triggers purging once it reaches
-  // REDUNDANT. Otherwise, purging happens on the next browser session (via
-  // DeleteStaleResources).
-  if (!context_->GetLiveVersion(deleted_version.version_id))
-    StartPurgingResources(newly_purgeable_resources);
-
-  std::move(callback).Run(blink::ServiceWorkerStatusCode::kOk);
+  std::move(callback).Run(blink::ServiceWorkerStatusCode::kOk,
+                          deleted_version.version_id,
+                          newly_purgeable_resources);
 }
 
 void ServiceWorkerStorage::DidUpdateToActiveState(
@@ -1222,9 +1218,12 @@ void ServiceWorkerStorage::DidDeleteRegistration(
     ServiceWorkerDatabase::Status status) {
   if (status != ServiceWorkerDatabase::STATUS_OK) {
     ScheduleDeleteAndStartOver();
-    std::move(params->callback).Run(DatabaseStatusToStatusCode(status));
+    std::move(params->callback)
+        .Run(DatabaseStatusToStatusCode(status), deleted_version.version_id,
+             newly_purgeable_resources);
     return;
   }
+
   if (quota_manager_proxy_) {
     // Can be nullptr in tests.
     quota_manager_proxy_->NotifyStorageModified(
@@ -1233,12 +1232,13 @@ void ServiceWorkerStorage::DidDeleteRegistration(
         blink::mojom::StorageType::kTemporary,
         -deleted_version.resources_total_size_bytes);
   }
+
   if (origin_state == OriginState::kDelete)
     registered_origins_.erase(params->origin);
-  std::move(params->callback).Run(blink::ServiceWorkerStatusCode::kOk);
 
-  if (!context_->GetLiveVersion(deleted_version.version_id))
-    StartPurgingResources(newly_purgeable_resources);
+  std::move(params->callback)
+      .Run(blink::ServiceWorkerStatusCode::kOk, deleted_version.version_id,
+           newly_purgeable_resources);
 }
 
 void ServiceWorkerStorage::DidWriteUncommittedResourceIds(
@@ -1541,7 +1541,7 @@ void ServiceWorkerStorage::DeleteRegistrationFromDB(
     scoped_refptr<base::SequencedTaskRunner> original_task_runner,
     int64_t registration_id,
     const GURL& origin,
-    DeleteRegistrationCallback callback) {
+    DeleteRegistrationInDBCallback callback) {
   DCHECK(database);
 
   ServiceWorkerDatabase::RegistrationData deleted_version;

content/browser/service_worker/service_worker_storage.h

@@ -81,6 +81,14 @@ class CONTENT_EXPORT ServiceWorkerStorage {
   using GetAllRegistrationsCallback =
       base::OnceCallback<void(blink::ServiceWorkerStatusCode status,
                               std::unique_ptr<RegistrationList> registrations)>;
+  using StoreRegistrationDataCallback = base::OnceCallback<void(
+      blink::ServiceWorkerStatusCode status,
+      int64_t deleted_version_id,
+      const std::vector<int64_t>& newly_purgeable_resources)>;
+  using DeleteRegistrationCallback = base::OnceCallback<void(
+      blink::ServiceWorkerStatusCode status,
+      int64_t deleted_version_id,
+      const std::vector<int64_t>& newly_purgeable_resources)>;
   using GetUserDataCallback =
       base::OnceCallback<void(const std::vector<std::string>& data,
                               blink::ServiceWorkerStatusCode status)>;
@@ -152,7 +160,7 @@ class CONTENT_EXPORT ServiceWorkerStorage {
   void StoreRegistrationData(
       const ServiceWorkerDatabase::RegistrationData& registration_data,
       const ResourceList& resources,
-      StatusCallback callback);
+      StoreRegistrationDataCallback callback);
 
   // Updates the state of the registration's stored version to active.
   void UpdateToActiveState(int64_t registration_id,
@@ -181,7 +189,7 @@ class CONTENT_EXPORT ServiceWorkerStorage {
   // called only from ServiceWorkerRegistry.
   void DeleteRegistration(int64_t registration_id,
                           const GURL& origin,
-                          StatusCallback callback);
+                          DeleteRegistrationCallback callback);
 
   // Removes traces of deleted data on disk.
   void PerformStorageCleanup(base::OnceClosure callback);
@@ -280,6 +288,7 @@ class CONTENT_EXPORT ServiceWorkerStorage {
   // uncommitted resources, as long as the caller does its own cleanup to remove
   // the uncommitted resource keys.
   void PurgeResources(const ResourceList& resources);
+  void PurgeResources(const std::vector<int64_t>& resource_ids);
 
   void LazyInitializeForTest();
 
@@ -316,11 +325,11 @@ class CONTENT_EXPORT ServiceWorkerStorage {
   struct DidDeleteRegistrationParams {
     int64_t registration_id;
     GURL origin;
-    StatusCallback callback;
+    DeleteRegistrationCallback callback;
 
     DidDeleteRegistrationParams(int64_t registration_id,
                                 GURL origin,
-                                StatusCallback callback);
+                                DeleteRegistrationCallback callback);
     ~DidDeleteRegistrationParams();
   };
 
@@ -339,7 +348,7 @@ class CONTENT_EXPORT ServiceWorkerStorage {
       const ServiceWorkerDatabase::RegistrationData& deleted_version_data,
       const std::vector<int64_t>& newly_purgeable_resources,
       ServiceWorkerDatabase::Status status)>;
-  using DeleteRegistrationCallback = base::OnceCallback<void(
+  using DeleteRegistrationInDBCallback = base::OnceCallback<void(
       OriginState origin_state,
       const ServiceWorkerDatabase::RegistrationData& deleted_version_data,
       const std::vector<int64_t>& newly_purgeable_resources,
@@ -389,7 +398,7 @@ class CONTENT_EXPORT ServiceWorkerStorage {
       std::unique_ptr<RegistrationList> registration_data_list,
       ServiceWorkerDatabase::Status status);
   void DidStoreRegistrationData(
-      StatusCallback callback,
+      StoreRegistrationDataCallback callback,
       const ServiceWorkerDatabase::RegistrationData& new_version,
       const GURL& origin,
       const ServiceWorkerDatabase::RegistrationData& deleted_version,
@@ -461,7 +470,7 @@ class CONTENT_EXPORT ServiceWorkerStorage {
       scoped_refptr<base::SequencedTaskRunner> original_task_runner,
       int64_t registration_id,
       const GURL& origin,
-      DeleteRegistrationCallback callback);
+      DeleteRegistrationInDBCallback callback);
   static void WriteRegistrationInDB(
       ServiceWorkerDatabase* database,
       scoped_refptr<base::SequencedTaskRunner> original_task_runner,