Don't prematurely delete script resources when registration is deleted

content/browser/service_worker/service_worker_storage.h

Index: content/browser/service_worker/service_worker_storage.h
diff --git a/content/browser/service_worker/service_worker_storage.h b/content/browser/service_worker/service_worker_storage.h
index 55238a45f76b9f4555a49a35ace39a1dea82e663..5218f8039679eaa9b92575065a0a0b6b40b385f8 100644
--- a/content/browser/service_worker/service_worker_storage.h
+++ b/content/browser/service_worker/service_worker_storage.h
@@ -16,6 +16,7 @@
 #include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "content/browser/service_worker/service_worker_database.h"
+#include "content/browser/service_worker/service_worker_version.h"
 #include "content/common/content_export.h"
 #include "content/common/service_worker/service_worker_status_code.h"
 #include "url/gurl.h"
@@ -37,11 +38,11 @@ class ServiceWorkerRegistration;
 class ServiceWorkerRegistrationInfo;
 class ServiceWorkerResponseReader;
 class ServiceWorkerResponseWriter;
-class ServiceWorkerVersion;
 
 // This class provides an interface to store and retrieve ServiceWorker
 // registration data.
-class CONTENT_EXPORT ServiceWorkerStorage {
+class CONTENT_EXPORT ServiceWorkerStorage
+    : public ServiceWorkerVersion::Listener {
  public:
   typedef std::vector<ServiceWorkerDatabase::ResourceRecord> ResourceList;
   typedef base::Callback<void(ServiceWorkerStatusCode status)> StatusCallback;
@@ -55,7 +56,7 @@ class CONTENT_EXPORT ServiceWorkerStorage {
       void(ServiceWorkerStatusCode status, int result)>
           CompareCallback;
 
-  ~ServiceWorkerStorage();
+  virtual ~ServiceWorkerStorage();
 
   static scoped_ptr<ServiceWorkerStorage> Create(
       const base::FilePath& path,
@@ -69,6 +70,9 @@ class CONTENT_EXPORT ServiceWorkerStorage {
       base::WeakPtr<ServiceWorkerContextCore> context,
       ServiceWorkerStorage* old_storage);
 
+  // ServiceWorkerVersion::Listener override
+  virtual void OnNoControllees(ServiceWorkerVersion* version) OVERRIDE;

[michaeln, 2014/06/30 22:31:54]

please put this method in the private section, its an impl detail that this
class implements this method and not part of the api consumers of this class
should be calling directly

[falken, 2014/07/01 06:17:39]

Done.

+
   // Finds registration for |document_url| or |pattern| or |registration_id|.
   // The Find methods will find stored and initially installing registrations.
   // Returns SERVICE_WORKER_OK with non-null registration if registration
@@ -103,10 +107,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 either a browser restart or
-  // DeleteVersionResources is called.
+  // 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.
   void DeleteRegistration(int64 registration_id,
                           const GURL& origin,
                           const StatusCallback& callback);
@@ -144,9 +147,11 @@ class CONTENT_EXPORT ServiceWorkerStorage {
   bool IsDisabled() const;
 
  private:
-  friend class ServiceWorkerStorageTest;
-  FRIEND_TEST_ALL_PREFIXES(ServiceWorkerStorageTest,
+  friend class ServiceWorkerResourceStorageTest;
+  FRIEND_TEST_ALL_PREFIXES(ServiceWorkerResourceStorageTest,
                            ResourceIdsAreStoredAndPurged);
+  FRIEND_TEST_ALL_PREFIXES(ServiceWorkerResourceStorageTest,
+                           ResourceIdsAreStoredAndPurged2);
 
   struct InitialData {
     int64 next_registration_id;
@@ -168,10 +173,11 @@ class CONTENT_EXPORT ServiceWorkerStorage {
       const GURL& origin,
       const std::vector<int64>& newly_purgeable_resources,
       ServiceWorkerDatabase::Status status)> WriteRegistrationCallback;
-  typedef base::Callback<void(
-      bool origin_is_deletable,
-      const std::vector<int64>& newly_purgeable_resources,
-      ServiceWorkerDatabase::Status status)> DeleteRegistrationCallback;
+  typedef base::Callback<
+      void(bool origin_is_deletable,
+           int64 version_id,
+           const std::vector<int64>& newly_purgeable_resources,
+           ServiceWorkerDatabase::Status status)> DeleteRegistrationCallback;
   typedef base::Callback<void(
       const ServiceWorkerDatabase::RegistrationData& data,
       const ResourceList& resources,
@@ -224,6 +230,7 @@ class CONTENT_EXPORT ServiceWorkerStorage {
       const GURL& origin,
       const StatusCallback& callback,
       bool origin_is_deletable,
+      int64 version_id,
       const std::vector<int64>& newly_purgeable_resources,
       ServiceWorkerDatabase::Status status);
 
@@ -320,8 +327,9 @@ class CONTENT_EXPORT ServiceWorkerStorage {
   scoped_refptr<base::MessageLoopProxy> disk_cache_thread_;
   scoped_refptr<quota::QuotaManagerProxy> quota_manager_proxy_;
   scoped_ptr<ServiceWorkerDiskCache> disk_cache_;
-  std::deque<int64> purgeable_reource_ids_;
+  std::deque<int64> purgeable_resource_ids_;
   bool is_purge_pending_;
+  std::map<int64, std::vector<int64> > deleted_version_resource_ids_;
 
   base::WeakPtrFactory<ServiceWorkerStorage> weak_factory_;