service worker: Move resource purge scheduling to storage control impl

This CL moves service worker script resource purging logic from
ServiceWorkerRegistry to ServiceWorkerStorageControlImpl. See [1] for
the motivation and how the new purging logic would work.

[1] https://docs.google.com/document/d/1VnZN2k9YbpX1Xy-Yvo93DKDnW4-M2W-rwjkGF1swqLo/edit?usp=sharing

Bug: 1055677
Change-Id: I4f8e9a54b47a9ec39538b8126ef8fb3e05f04c3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2348734
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797531}

content/browser/service_worker/service_worker_registry.cc

@@ -1164,22 +1164,6 @@ void ServiceWorkerRegistry::DidStoreRegistration(
     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(stored_registration_id);
   if (registration) {
@@ -1215,9 +1199,6 @@ void ServiceWorkerRegistry::DidDeleteRegistration(
     return;
   }
 
-  if (!context_->GetLiveVersion(deleted_version_id))
-    storage()->PurgeResources(newly_purgeable_resources);
-
   scoped_refptr<ServiceWorkerRegistration> registration =
       context_->GetLiveRegistration(registration_id);
   if (registration)

content/browser/service_worker/service_worker_storage_control_impl.cc

@@ -49,9 +49,18 @@ class ServiceWorkerLiveVersionRefImpl
 
   void set_purgeable_resources(
       const std::vector<int64_t>& purgeable_resources) {
-    DCHECK(purgeable_resources_.empty());
+    if (!purgeable_resources_.empty()) {
+      // This setter method can be called multiple times but the resource ids
+      // should be the same.
+      DCHECK(std::set<int64_t>(purgeable_resources_.begin(),
+                               purgeable_resources_.end()) ==
+             std::set<int64_t>(purgeable_resources.begin(),
+                               purgeable_resources.end()));
+      return;
+    }
     purgeable_resources_ = purgeable_resources;
   }
+
   const std::vector<int64_t>& purgeable_resources() const {
     return purgeable_resources_;
   }
@@ -90,8 +99,7 @@ void ServiceWorkerStorageControlImpl::Bind(
 void ServiceWorkerStorageControlImpl::OnNoLiveVersion(int64_t version_id) {
   auto it = live_versions_.find(version_id);
   DCHECK(it != live_versions_.end());
-  if (purge_resources_when_no_live_versions_ &&
-      it->second->purgeable_resources().size() > 0) {
+  if (it->second->purgeable_resources().size() > 0) {
     storage_->PurgeResources(it->second->purgeable_resources());
   }
   live_versions_.erase(it);
@@ -101,11 +109,6 @@ void ServiceWorkerStorageControlImpl::LazyInitializeForTest() {
   storage_->LazyInitializeForTest();
 }
 
-void ServiceWorkerStorageControlImpl::
-    EnableResourcePurgingOnNoLiveVersionForTest() {
-  purge_resources_when_no_live_versions_ = true;
-}
-
 void ServiceWorkerStorageControlImpl::GetRegisteredOrigins(
     GetRegisteredOriginsCallback callback) {
   storage_->GetRegisteredOrigins(std::move(callback));
@@ -469,8 +472,7 @@ ServiceWorkerStorageControlImpl::CreateLiveVersionReferenceRemote(
 void ServiceWorkerStorageControlImpl::MaybePurgeResources(
     int64_t version_id,
     const std::vector<int64_t>& purgeable_resources) {
-  if (!purge_resources_when_no_live_versions_ ||
-      version_id == blink::mojom::kInvalidServiceWorkerVersionId ||
+  if (version_id == blink::mojom::kInvalidServiceWorkerVersionId ||
       purgeable_resources.size() == 0) {
     return;
   }

content/browser/service_worker/service_worker_storage_control_impl.h

@@ -47,12 +47,6 @@ class CONTENT_EXPORT ServiceWorkerStorageControlImpl
 
   void LazyInitializeForTest();
 
-  // When enabled, the instance of this class manages when to purge resources
-  // (service worker scripts) which are no longer used.
-  // TODO(crbug.com/1055677): Remove this once resource purging logic is
-  // completely moved from ServiceWorkerRegistry to this class.
-  void EnableResourcePurgingOnNoLiveVersionForTest();
-
  private:
   // storage::mojom::ServiceWorkerStorageControl implementations:
   void GetRegisteredOrigins(GetRegisteredOriginsCallback callback) override;
@@ -199,8 +193,6 @@ class CONTENT_EXPORT ServiceWorkerStorageControlImpl
                  std::unique_ptr<ServiceWorkerLiveVersionRefImpl>>
       live_versions_;
 
-  bool purge_resources_when_no_live_versions_ = false;
-
   base::WeakPtrFactory<ServiceWorkerStorageControlImpl> weak_ptr_factory_{this};
 };
 

content/browser/service_worker/service_worker_storage_control_impl_unittest.cc

@@ -185,7 +185,6 @@ class ServiceWorkerStorageControlImplTest : public testing::Test {
         /*quota_manager_proxy=*/nullptr);
     storage_impl_ =
         std::make_unique<ServiceWorkerStorageControlImpl>(std::move(storage));
-    storage_impl_->EnableResourcePurgingOnNoLiveVersionForTest();
   }
 
   void DestroyStorage() {

content/browser/service_worker/service_worker_storage_unittest.cc

@@ -1481,8 +1481,6 @@ TEST_F(ServiceWorkerResourceStorageTest,
 }
 
 TEST_F(ServiceWorkerResourceStorageTest, DeleteRegistration_NoLiveVersion) {
-  registration_->SetWaitingVersion(nullptr);
-
   // Deleting the registration should result in the resources being added to the
   // purgeable list and then doomed in the disk cache and removed from that
   // list.
@@ -1490,9 +1488,15 @@ TEST_F(ServiceWorkerResourceStorageTest, DeleteRegistration_NoLiveVersion) {
   storage()->SetPurgingCompleteCallbackForTest(loop.QuitClosure());
   EXPECT_EQ(blink::ServiceWorkerStatusCode::kOk,
             DeleteRegistration(registration_, scope_.GetOrigin()));
+  // At this point registration_->waiting_version() has a remote reference, so
+  // the resources should be in the purgeable list.
   EXPECT_EQ(2u, GetPurgeableResourceIdsFromDB().size());
+
+  registration_->SetWaitingVersion(nullptr);
   loop.Run();
 
+  // registration_->waiting_version() is cleared. The resources should be
+  // purged at this point.
   EXPECT_TRUE(GetPurgeableResourceIdsFromDB().empty());
   EXPECT_FALSE(VerifyBasicResponse(storage_control(), resource_id1_, false));
   EXPECT_FALSE(VerifyBasicResponse(storage_control(), resource_id2_, false));
@@ -1770,9 +1774,6 @@ TEST_F(ServiceWorkerResourceStorageTest, UpdateRegistration_NoLiveVersion) {
   live_version->set_fetch_handler_existence(
       ServiceWorkerVersion::FetchHandlerExistence::EXISTS);
 
-  // Destroy the active version.
-  registration_->UnsetVersion(registration_->active_version());
-
   // Writing the registration should purge the old version's resources,
   // since it's not live.
   base::RunLoop loop;
@@ -1782,6 +1783,9 @@ TEST_F(ServiceWorkerResourceStorageTest, UpdateRegistration_NoLiveVersion) {
       StoreRegistration(registration_.get(), registration_->waiting_version()));
   EXPECT_EQ(2u, GetPurgeableResourceIdsFromDB().size());
 
+  // Destroy the active version.
+  registration_->UnsetVersion(registration_->active_version());
+
   // The resources should be purged.
   loop.Run();
   EXPECT_TRUE(GetPurgeableResourceIdsFromDB().empty());

content/browser/service_worker/service_worker_version.cc

@@ -374,11 +374,9 @@ void ServiceWorkerVersion::SetStatus(Status status) {
   } else if (status == REDUNDANT) {
     embedded_worker_->OnWorkerVersionDoomed();
 
-    // Tell the storage system that this worker's script resources can now be
-    // deleted.
-    std::vector<storage::mojom::ServiceWorkerResourceRecordPtr> resources;
-    script_cache_map_.GetResources(&resources);
-    context_->storage()->PurgeResources(resources);
+    // Drop the remote reference to tell the storage system that the worker
+    // script resources can now be deleted.
+    remote_reference_.reset();
   }
 }