Service Worker: Delay stale resource cleanup

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 b25f26b2692ab5602fc477d09e0082217132ce97..67c4e4e32ce56bb336ca2485e95cbf1d168f8874 100644
--- a/content/browser/service_worker/service_worker_storage.h
+++ b/content/browser/service_worker/service_worker_storage.h
@@ -118,7 +118,7 @@ class CONTENT_EXPORT ServiceWorkerStorage
 
   // Adds |id| to the set of resources ids that are in the disk
   // cache but not yet stored with a registration.
-  void StoreUncommittedReponseId(int64 id);
+  void StoreUncommittedResponseId(int64 id);
 
   // Removes |id| from uncommitted list, adds it to the
   // purgeable list and purges it.
@@ -159,7 +159,6 @@ class CONTENT_EXPORT ServiceWorkerStorage
     int64 next_version_id;
     int64 next_resource_id;
     std::set<GURL> origins;
-    std::set<int64> purgeable_resource_ids;
 
     InitialData();
     ~InitialData();
@@ -185,6 +184,9 @@ class CONTENT_EXPORT ServiceWorkerStorage
       const ServiceWorkerDatabase::RegistrationData& data,
       const ResourceList& resources,
       ServiceWorkerDatabase::Status status)> FindInDBCallback;
+  typedef base::Callback<void(const std::vector<int64>& resource_ids,
+                              ServiceWorkerDatabase::Status status)>
+      GetResourcesCallback;
 
   ServiceWorkerStorage(const base::FilePath& path,
                        base::WeakPtr<ServiceWorkerContextCore> context,
@@ -195,6 +197,8 @@ class CONTENT_EXPORT ServiceWorkerStorage
   base::FilePath GetDatabasePath();
   base::FilePath GetDiskCachePath();
 
+  bool IsReady();
+
   bool LazyInitialize(
       const base::Closure& callback);
   void DidReadInitialData(
@@ -262,7 +266,18 @@ class CONTENT_EXPORT ServiceWorkerStorage
   void PurgeResource(int64 id);
   void OnResourcePurged(int64 id, int rv);
 
+  // Deletes purgeable and uncommitted resources left over from the previous
+  // browser session. This must be called once per session before any database
+  // operation that may mutate the purgeable or uncommitted resource lists.
+  void DeleteStaleResources();
+  void DidCollectStaleResources(const std::vector<int64>& stale_resource_ids,
+                                ServiceWorkerDatabase::Status status);
+
   // Static cross-thread helpers.
+  static void CollectStaleResourcesFromDB(
+      ServiceWorkerDatabase* database,
+      scoped_refptr<base::SequencedTaskRunner> original_task_runner,
+      const GetResourcesCallback& callback);
   static void ReadInitialDataFromDB(
       ServiceWorkerDatabase* database,
       scoped_refptr<base::SequencedTaskRunner> original_task_runner,
@@ -320,6 +335,7 @@ class CONTENT_EXPORT ServiceWorkerStorage
   enum State {
     UNINITIALIZED,
     INITIALIZING,
+    HAS_STALE_RESOURCES,

[michaeln, 2014/07/09 02:36:11]

bikesheddy...

This one is a little different then the other states. Would this be better as a
flag, has_checked_for_stale_resources_? If we keep it a state, having
INITIALIZED in both of the names might make sense

INITIALIZED_FOR_READING
INITIALIZED_FOR_WRITING

[falken, 2014/07/09 04:31:14]

I was a bit torn when writing this. I liked State because it constrains the
possible states instead of a combinatoric explosion of flags and states. But it
does seem to make the code more convoluted with the introduction of IsReady. I
switched back to flag, it feels nicer.

     INITIALIZED,
     DISABLED,
   };