Service Worker: set active worker to REDUNDANT when unregistered

As per spec update:
https://github.com/slightlyoff/ServiceWorker/issues/353

Now the active worker is set to REDUNDANT when unregistered. If it has a
controllee, this happens when it no longer has a controllee.

This patch adds a Doom function to ServiceWorkerVersion which takes
care of setting REDUNDANT and purging resources once there's
no controllee. This obviates the need for ServiceWorkerStorage to
listen for NoControllees and purge resources then.

BUG=388095

Review URL: https://codereview.chromium.org/377153003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282636 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/service_worker/service_worker_job_unittest.cc

@@ -702,7 +702,7 @@ TEST_F(ServiceWorkerJobTest, AbortAll_RegUnreg) {
 
 // Tests that the waiting worker enters the 'redundant' state upon
 // unregistration.
-TEST_F(ServiceWorkerJobTest, UnregisterSetsRedundant) {
+TEST_F(ServiceWorkerJobTest, UnregisterWaitingSetsRedundant) {
   scoped_refptr<ServiceWorkerRegistration> registration;
   bool called = false;
   job_coordinator()->Register(
@@ -740,4 +740,73 @@ TEST_F(ServiceWorkerJobTest, UnregisterSetsRedundant) {
   EXPECT_EQ(ServiceWorkerVersion::REDUNDANT, version->status());
 }
 
+// Tests that the active worker enters the 'redundant' state upon
+// unregistration.
+TEST_F(ServiceWorkerJobTest, UnregisterActiveSetsRedundant) {
+  scoped_refptr<ServiceWorkerRegistration> registration;
+  bool called = false;
+  job_coordinator()->Register(
+      GURL("http://www.example.com/*"),
+      GURL("http://www.example.com/service_worker.js"),
+      render_process_id_,
+      SaveRegistration(SERVICE_WORKER_OK, &called, &registration));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(called);
+  ASSERT_TRUE(registration);
+
+  scoped_refptr<ServiceWorkerVersion> version = registration->active_version();
+  EXPECT_EQ(ServiceWorkerVersion::RUNNING, version->running_status());
+  EXPECT_EQ(ServiceWorkerVersion::ACTIVATED, version->status());
+
+  called = false;
+  job_coordinator()->Unregister(GURL("http://www.example.com/*"),
+                                SaveUnregistration(SERVICE_WORKER_OK, &called));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  EXPECT_EQ(ServiceWorkerVersion::RUNNING, version->running_status());
+  EXPECT_EQ(ServiceWorkerVersion::REDUNDANT, version->status());
+}
+
+// Tests that the active worker enters the 'redundant' state upon
+// unregistration.
+TEST_F(ServiceWorkerJobTest,
+       UnregisterActiveSetsRedundant_WaitForNoControllee) {
+  scoped_refptr<ServiceWorkerRegistration> registration;
+  bool called = false;
+  job_coordinator()->Register(
+      GURL("http://www.example.com/*"),
+      GURL("http://www.example.com/service_worker.js"),
+      render_process_id_,
+      SaveRegistration(SERVICE_WORKER_OK, &called, &registration));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(called);
+  ASSERT_TRUE(registration);
+
+  scoped_ptr<ServiceWorkerProviderHost> host(
+      new ServiceWorkerProviderHost(33 /* dummy render process id */,
+                                    1 /* dummy provider_id */,
+                                    context()->AsWeakPtr(),
+                                    NULL));
+  registration->active_version()->AddControllee(host.get());
+
+  scoped_refptr<ServiceWorkerVersion> version = registration->active_version();
+  EXPECT_EQ(ServiceWorkerVersion::RUNNING, version->running_status());
+  EXPECT_EQ(ServiceWorkerVersion::ACTIVATED, version->status());
+
+  called = false;
+  job_coordinator()->Unregister(GURL("http://www.example.com/*"),
+                                SaveUnregistration(SERVICE_WORKER_OK, &called));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  EXPECT_EQ(ServiceWorkerVersion::RUNNING, version->running_status());
+  EXPECT_EQ(ServiceWorkerVersion::ACTIVATED, version->status());
+
+  registration->active_version()->RemoveControllee(host.get());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(ServiceWorkerVersion::RUNNING, version->running_status());
+  EXPECT_EQ(ServiceWorkerVersion::REDUNDANT, version->status());
+}
+
 }  // namespace content

content/browser/service_worker/service_worker_register_job.cc

@@ -390,9 +390,9 @@ void ServiceWorkerRegisterJob::CompleteInternal(
   if (status != SERVICE_WORKER_OK) {
     if (registration()) {
       if (new_version()) {
-        new_version()->SetStatus(ServiceWorkerVersion::REDUNDANT);
         DisassociateVersionFromDocuments(context_, new_version());
         registration()->UnsetVersion(new_version());
+        new_version()->Doom();
       }
       if (!registration()->active_version()) {
         context_->storage()->DeleteRegistration(

content/browser/service_worker/service_worker_register_job.h

@@ -85,7 +85,8 @@ class ServiceWorkerRegisterJob
     ~Internal();
     scoped_refptr<ServiceWorkerRegistration> registration;
 
-    // Holds 'installing' or 'waiting' version depending on the phase.
+    // Holds the version created by this job. It can be the 'installing',
+    // 'waiting', or 'active' version depending on the phase.
     scoped_refptr<ServiceWorkerVersion> new_version;
   };
 

content/browser/service_worker/service_worker_registration.h

@@ -83,7 +83,6 @@ class CONTENT_EXPORT ServiceWorkerRegistration
   // listeners via OnVersionAttributesChanged.
   void UnsetVersion(ServiceWorkerVersion* version);
 
-
  private:
   ~ServiceWorkerRegistration();
   friend class base::RefCounted<ServiceWorkerRegistration>;

content/browser/service_worker/service_worker_script_cache_map.h

@@ -20,7 +20,7 @@ class ServiceWorkerContextCore;
 class ServiceWorkerVersion;
 
 // Class that maintains the mapping between urls and a resource id
-// for a particular versions implicit script resources.
+// for a particular version's implicit script resources.
 class ServiceWorkerScriptCacheMap {
  public:
   int64 Lookup(const GURL& url);

content/browser/service_worker/service_worker_storage.cc

@@ -450,8 +450,6 @@ void ServiceWorkerStorage::NotifyDoneInstallingRegistration(
             &ServiceWorkerDatabase::PurgeUncommittedResourceIds),
             base::Unretained(database_.get()),
             ids));
-
-    StartPurgingResources(resources);
   }
 }
 
@@ -465,6 +463,12 @@ bool ServiceWorkerStorage::IsDisabled() const {
   return state_ == DISABLED;
 }
 
+void ServiceWorkerStorage::PurgeResources(const ResourceList& resources) {
+  if (!has_checked_for_stale_resources_)
+    DeleteStaleResources();
+  StartPurgingResources(resources);
+}
+
 ServiceWorkerStorage::ServiceWorkerStorage(
     const base::FilePath& path,
     base::WeakPtr<ServiceWorkerContextCore> context,
@@ -705,7 +709,8 @@ void ServiceWorkerStorage::DidStoreRegistration(
   registered_origins_.insert(origin);
   callback.Run(SERVICE_WORKER_OK);
 
-  SchedulePurgeResources(deleted_version_id, newly_purgeable_resources);
+  if (!context_ || !context_->GetLiveVersion(deleted_version_id))
+    StartPurgingResources(newly_purgeable_resources);
 }
 
 void ServiceWorkerStorage::DidUpdateToActiveState(
@@ -734,7 +739,8 @@ void ServiceWorkerStorage::DidDeleteRegistration(
     registered_origins_.erase(origin);
   callback.Run(SERVICE_WORKER_OK);
 
-  SchedulePurgeResources(version_id, newly_purgeable_resources);
+  if (!context_ || !context_->GetLiveVersion(version_id))
+    StartPurgingResources(newly_purgeable_resources);
 }
 
 scoped_refptr<ServiceWorkerRegistration>
@@ -844,32 +850,6 @@ void ServiceWorkerStorage::OnDiskCacheInitialized(int rv) {
   ServiceWorkerMetrics::CountInitDiskCacheResult(rv == net::OK);
 }
 
-void ServiceWorkerStorage::OnNoControllees(ServiceWorkerVersion* version) {
-  std::map<int64, std::vector<int64> >::iterator it =
-      deleted_version_resource_ids_.find(version->version_id());
-  DCHECK(it != deleted_version_resource_ids_.end());
-  StartPurgingResources(it->second);
-  deleted_version_resource_ids_.erase(it);
-  version->RemoveListener(this);
-}
-
-void ServiceWorkerStorage::SchedulePurgeResources(
-    int64 version_id,
-    const std::vector<int64>& resources) {
-  DCHECK(deleted_version_resource_ids_.find(version_id) ==
-         deleted_version_resource_ids_.end());
-  if (resources.empty())
-    return;
-  scoped_refptr<ServiceWorkerVersion> version =
-      context_ ? context_->GetLiveVersion(version_id) : NULL;
-  if (version && version->HasControllee()) {
-    deleted_version_resource_ids_[version_id] = resources;
-    version->AddListener(this);
-  } else {
-    StartPurgingResources(resources);
-  }
-}
-
 void ServiceWorkerStorage::StartPurgingResources(
     const std::vector<int64>& ids) {
   DCHECK(has_checked_for_stale_resources_);

content/browser/service_worker/service_worker_storage.h

@@ -92,8 +92,8 @@ class CONTENT_EXPORT ServiceWorkerStorage
 
   // Commits |registration| with the installed but not activated |version|
   // to storage, overwritting any pre-existing registration data for the scope.
-  // A pre-existing version's script resources will remain available until
-  // it no longer controls a page, or a browser restart occurs.
+  // A pre-existing version's script resources remain available if that version
+  // is live. PurgeResources should be called when it's OK to delete them.
   void StoreRegistration(
       ServiceWorkerRegistration* registration,
       ServiceWorkerVersion* version,
@@ -104,9 +104,9 @@ class CONTENT_EXPORT ServiceWorkerStorage
       ServiceWorkerRegistration* registration,
       const StatusCallback& callback);
 
-  // Deletes the registration data for |registration_id|. The script resources
-  // for the registration's stored version will remain available until that
-  // version no longer controls a page, or a browser restart occurs.
+  // Deletes the registration data for |registration_id|. If the registration's
+  // version is live, its script resources will remain available.
+  // PurgeResources should be called when it's OK to delete them.
   void DeleteRegistration(int64 registration_id,
                           const GURL& origin,
                           const StatusCallback& callback);
@@ -143,8 +143,13 @@ class CONTENT_EXPORT ServiceWorkerStorage
   void Disable();
   bool IsDisabled() const;
 
+  // |resources| must already be on the purgeable list.
+  void PurgeResources(const ResourceList& resources);
+
  private:
   friend class ServiceWorkerResourceStorageTest;
+  FRIEND_TEST_ALL_PREFIXES(ServiceWorkerResourceStorageTest,
+                           DeleteRegistration_NoLiveVersion);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerResourceStorageTest,
                            DeleteRegistration_WaitingVersion);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerResourceStorageTest,
@@ -253,11 +258,6 @@ class CONTENT_EXPORT ServiceWorkerStorage
   ServiceWorkerDiskCache* disk_cache();
   void OnDiskCacheInitialized(int rv);
 
-  // ServiceWorkerVersion::Listener override
-  virtual void OnNoControllees(ServiceWorkerVersion* version) OVERRIDE;
-
-  void SchedulePurgeResources(int64 version_id,
-                              const std::vector<int64>& resources);
   void StartPurgingResources(const std::vector<int64>& ids);
   void StartPurgingResources(const ResourceList& resources);
   void ContinuePurgingResources();
@@ -351,7 +351,6 @@ class CONTENT_EXPORT ServiceWorkerStorage
   std::deque<int64> purgeable_resource_ids_;
   bool is_purge_pending_;
   bool has_checked_for_stale_resources_;
-  std::map<int64, std::vector<int64> > deleted_version_resource_ids_;
 
   base::WeakPtrFactory<ServiceWorkerStorage> weak_factory_;
 

content/browser/service_worker/service_worker_storage_unittest.cc

@@ -577,6 +577,38 @@ class ServiceWorkerResourceStorageDiskTest
   base::ScopedTempDir user_data_directory_;
 };
 
+TEST_F(ServiceWorkerResourceStorageTest, DeleteRegistration_NoLiveVersion) {
+  bool was_called = false;
+  ServiceWorkerStatusCode result = SERVICE_WORKER_ERROR_FAILED;
+  std::set<int64> verify_ids;
+
+  registration_->SetWaitingVersion(NULL);
+  registration_ = NULL;
+
+  // 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.
+  storage()->DeleteRegistration(
+      registration_id_,
+      scope_.GetOrigin(),
+      base::Bind(&VerifyPurgeableListStatusCallback,
+                 base::Unretained(storage()->database_.get()),
+                 &verify_ids,
+                 &was_called,
+                 &result));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(was_called);
+  EXPECT_EQ(SERVICE_WORKER_OK, result);
+  EXPECT_EQ(2u, verify_ids.size());
+  verify_ids.clear();
+  EXPECT_EQ(ServiceWorkerDatabase::STATUS_OK,
+            storage()->database_->GetPurgeableResourceIds(&verify_ids));
+  EXPECT_TRUE(verify_ids.empty());
+
+  EXPECT_FALSE(VerifyBasicResponse(storage(), resource_id1_, false));
+  EXPECT_FALSE(VerifyBasicResponse(storage(), resource_id2_, false));
+}
+
 TEST_F(ServiceWorkerResourceStorageTest, DeleteRegistration_WaitingVersion) {
   bool was_called = false;
   ServiceWorkerStatusCode result = SERVICE_WORKER_ERROR_FAILED;
@@ -598,6 +630,19 @@ TEST_F(ServiceWorkerResourceStorageTest, DeleteRegistration_WaitingVersion) {
   EXPECT_EQ(SERVICE_WORKER_OK, result);
   EXPECT_EQ(2u, verify_ids.size());
   verify_ids.clear();
+  EXPECT_EQ(ServiceWorkerDatabase::STATUS_OK,
+            storage()->database_->GetPurgeableResourceIds(&verify_ids));
+  EXPECT_EQ(2u, verify_ids.size());
+
+  EXPECT_TRUE(VerifyBasicResponse(storage(), resource_id1_, false));
+  EXPECT_TRUE(VerifyBasicResponse(storage(), resource_id2_, false));
+
+  // Doom the version, now it happens.
+  registration_->waiting_version()->Doom();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(SERVICE_WORKER_OK, result);
+  EXPECT_EQ(2u, verify_ids.size());
+  verify_ids.clear();
   EXPECT_EQ(ServiceWorkerDatabase::STATUS_OK,
             storage()->database_->GetPurgeableResourceIds(&verify_ids));
   EXPECT_TRUE(verify_ids.empty());
@@ -632,6 +677,7 @@ TEST_F(ServiceWorkerResourceStorageTest, DeleteRegistration_ActiveVersion) {
                  &verify_ids,
                  &was_called,
                  &result));
+  registration_->active_version()->Doom();
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(was_called);
   EXPECT_EQ(SERVICE_WORKER_OK, result);
@@ -781,6 +827,7 @@ TEST_F(ServiceWorkerResourceStorageTest, UpdateRegistration) {
                  &verify_ids,
                  &was_called,
                  &result));
+  registration_->active_version()->Doom();
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(was_called);
   EXPECT_EQ(SERVICE_WORKER_OK, result);

content/browser/service_worker/service_worker_unregister_job.cc

@@ -68,13 +68,11 @@ void ServiceWorkerUnregisterJob::OnRegistrationFound(
 
 void ServiceWorkerUnregisterJob::DeleteRegistration(
     const scoped_refptr<ServiceWorkerRegistration>& registration) {
-  // TODO(nhiroki): When we've implemented the installing version, terminate and
-  // set it to redundant here as per spec.
-
-  if (registration->waiting_version()) {
-    registration->waiting_version()->SetStatus(
-        ServiceWorkerVersion::REDUNDANT);
-  }
+  // TODO: Also doom installing version.
+  if (registration->waiting_version())
+    registration->waiting_version()->Doom();
+  if (registration->active_version())
+    registration->active_version()->Doom();
 
   context_->storage()->DeleteRegistration(
       registration->id(),

content/browser/service_worker/service_worker_version.cc

@@ -99,6 +99,7 @@ ServiceWorkerVersion::ServiceWorkerVersion(
       status_(NEW),
       context_(context),
       script_cache_map_(this, context),
+      is_doomed_(false),
       weak_factory_(this) {
   DCHECK(context_);
   DCHECK(registration);
@@ -386,10 +387,11 @@ void ServiceWorkerVersion::RemoveControllee(
   controllee_by_id_.Remove(found->second);
   controllee_map_.erase(found);
   RemoveProcessFromWorker(provider_host->process_id());
-  if (!HasControllee()) {
-    ScheduleStopWorker();
-    FOR_EACH_OBSERVER(Listener, listeners_, OnNoControllees(this));
-  }
+  if (HasControllee())
+    return;
+  ScheduleStopWorker();
+  if (is_doomed_)
+    DoomInternal();
 }
 
 void ServiceWorkerVersion::AddPotentialControllee(
@@ -410,6 +412,14 @@ void ServiceWorkerVersion::RemoveListener(Listener* listener) {
   listeners_.RemoveObserver(listener);
 }
 
+void ServiceWorkerVersion::Doom() {
+  if (is_doomed_)
+    return;
+  is_doomed_ = true;
+  if (!HasControllee())
+    DoomInternal();
+}
+
 void ServiceWorkerVersion::OnStarted() {
   DCHECK_EQ(RUNNING, running_status());
   // Fire all start callbacks.
@@ -656,4 +666,13 @@ void ServiceWorkerVersion::ScheduleStopWorker() {
                  base::Bind(&ServiceWorkerUtils::NoOpStatusCallback)));
 }
 
+void ServiceWorkerVersion::DoomInternal() {
+  SetStatus(REDUNDANT);
+  if (!context_)
+    return;
+  std::vector<ServiceWorkerDatabase::ResourceRecord> resources;
+  script_cache_map_.GetResources(&resources);
+  context_->storage()->PurgeResources(resources);
+}
+
 }  // namespace content

content/browser/service_worker/service_worker_version.h

@@ -84,7 +84,6 @@ class CONTENT_EXPORT ServiceWorkerVersion
                                         const base::string16& message,
                                         int line_number,
                                         const GURL& source_url) {};
-    virtual void OnNoControllees(ServiceWorkerVersion* version) {};
   };
 
   ServiceWorkerVersion(
@@ -218,6 +217,11 @@ class CONTENT_EXPORT ServiceWorkerVersion
   ServiceWorkerScriptCacheMap* script_cache_map() { return &script_cache_map_; }
   EmbeddedWorkerInstance* embedded_worker() { return embedded_worker_.get(); }
 
+  // Dooms this version to have REDUNDANT status and its resources deleted.  If
+  // the version is controlling a page, these changes will happen when the
+  // version no longer controls any pages.
+  void Doom();
+
  private:
   typedef ServiceWorkerVersion self;
   typedef std::map<ServiceWorkerProviderHost*, int> ControlleeMap;
@@ -262,6 +266,7 @@ class CONTENT_EXPORT ServiceWorkerVersion
                                const std::vector<int>& sent_message_port_ids);
 
   void ScheduleStopWorker();
+  void DoomInternal();
 
   const int64 version_id_;
   int64 registration_id_;
@@ -287,6 +292,7 @@ class CONTENT_EXPORT ServiceWorkerVersion
   ServiceWorkerScriptCacheMap script_cache_map_;
   base::OneShotTimer<ServiceWorkerVersion> stop_worker_timer_;
   base::OneShotTimer<ServiceWorkerVersion> update_timer_;
+  bool is_doomed_;
 
   base::WeakPtrFactory<ServiceWorkerVersion> weak_factory_;