Change threading in ServiceWorkerCacheStorage

ServiceWorkerCacheStorage currently expects to have its public
functions run by a SequencedTaskRunner.

This CL changes the functions to run on the calling thread and only
uses the TaskRunner for blocking operations.  This way, when calling
the cache and blob storage, we're on the calling thread (IO).

Also added some comments along the way.

BUG=392621

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

Cr-Commit-Position: refs/heads/master@{#289186}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289186 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/service_worker/service_worker_cache.cc

@@ -11,16 +11,16 @@
 namespace content {
 
 // static
-ServiceWorkerCache* ServiceWorkerCache::CreateMemoryCache(
+scoped_ptr<ServiceWorkerCache> ServiceWorkerCache::CreateMemoryCache(
     const std::string& name) {
-  return new ServiceWorkerCache(base::FilePath(), name);
+  return make_scoped_ptr(new ServiceWorkerCache(base::FilePath(), name));
 }
 
 // static
-ServiceWorkerCache* ServiceWorkerCache::CreatePersistentCache(
+scoped_ptr<ServiceWorkerCache> ServiceWorkerCache::CreatePersistentCache(
     const base::FilePath& path,
     const std::string& name) {
-  return new ServiceWorkerCache(path, name);
+  return make_scoped_ptr(new ServiceWorkerCache(path, name));
 }
 
 void ServiceWorkerCache::CreateBackend(
@@ -28,9 +28,13 @@ void ServiceWorkerCache::CreateBackend(
   callback.Run(true);
 }
 
+base::WeakPtr<ServiceWorkerCache> ServiceWorkerCache::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 ServiceWorkerCache::ServiceWorkerCache(const base::FilePath& path,
                                        const std::string& name)
-    : path_(path), name_(name), id_(0) {
+    : path_(path), name_(name), id_(0), weak_ptr_factory_(this) {
 }
 
 ServiceWorkerCache::~ServiceWorkerCache() {

content/browser/service_worker/service_worker_cache.h

@@ -7,6 +7,7 @@
 
 #include "base/callback.h"
 #include "base/files/file_path.h"
+#include "base/memory/weak_ptr.h"
 
 namespace content {
 
@@ -21,9 +22,11 @@ namespace content {
 // InitializeIfNeeded must be called before calling the other public members.
 class ServiceWorkerCache {
  public:
-  static ServiceWorkerCache* CreateMemoryCache(const std::string& name);
-  static ServiceWorkerCache* CreatePersistentCache(const base::FilePath& path,
-                                                   const std::string& name);
+  static scoped_ptr<ServiceWorkerCache> CreateMemoryCache(
+      const std::string& name);
+  static scoped_ptr<ServiceWorkerCache> CreatePersistentCache(
+      const base::FilePath& path,
+      const std::string& name);
   virtual ~ServiceWorkerCache();
 
   // Loads the backend and calls the callback with the result (true for
@@ -36,6 +39,8 @@ class ServiceWorkerCache {
   int32 id() const { return id_; }
   void set_id(int32 id) { id_ = id; }
 
+  base::WeakPtr<ServiceWorkerCache> AsWeakPtr();
+
  private:
   ServiceWorkerCache(const base::FilePath& path, const std::string& name);
 
@@ -43,6 +48,8 @@ class ServiceWorkerCache {
   std::string name_;
   int32 id_;
 
+  base::WeakPtrFactory<ServiceWorkerCache> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerCache);
 };
 

content/browser/service_worker/service_worker_cache_storage.h

@@ -11,20 +11,20 @@
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/id_map.h"
-#include "base/threading/thread_checker.h"
+#include "base/memory/weak_ptr.h"
 
 namespace base {
-class MessageLoopProxy;
+class SequencedTaskRunner;
 }
 
 namespace content {
-
 class ServiceWorkerCache;
 
 // TODO(jkarlin): Constrain the total bytes used per origin.
-// The set of caches for a given origin. It is owned by the
-// ServiceWorkerCacheStorageManager. Provided callbacks are called on the
-// |callback_loop| provided in the constructor.
+
+// ServiceWorkerCacheStorage holds the set of caches for a given origin. It is
+// owned by the ServiceWorkerCacheStorageManager. This class expects to be run
+// on the IO thread.
 class ServiceWorkerCacheStorage {
  public:
   enum CacheStorageError {
@@ -41,10 +41,9 @@ class ServiceWorkerCacheStorage {
   typedef base::Callback<void(const std::vector<std::string>&,
                               CacheStorageError)> StringsAndErrorCallback;
 
-  ServiceWorkerCacheStorage(
-      const base::FilePath& origin_path,
-      bool memory_only,
-      const scoped_refptr<base::MessageLoopProxy>& callback_loop);
+  ServiceWorkerCacheStorage(const base::FilePath& origin_path,
+                            bool memory_only,
+                            base::SequencedTaskRunner* cache_task_runner);
 
   virtual ~ServiceWorkerCacheStorage();
 
@@ -73,10 +72,6 @@ class ServiceWorkerCacheStorage {
 
   // TODO(jkarlin): Add match() function.
 
-  void InitializeCacheCallback(const ServiceWorkerCache* cache,
-                               const CacheAndErrorCallback& callback,
-                               bool success);
-
  private:
   class MemoryLoader;
   class SimpleCacheLoader;
@@ -88,15 +83,62 @@ class ServiceWorkerCacheStorage {
 
   ServiceWorkerCache* GetLoadedCache(const std::string& cache_name) const;
 
-  void LazyInit();
-  void InitCache(ServiceWorkerCache* cache);
-
+  // Initializer and its callback are below.
+  void LazyInit(const base::Closure& closure);
+  void LazyInitDidLoadIndex(
+      const base::Closure& callback,
+      scoped_ptr<std::vector<std::string> > indexed_cache_names);
+  void LazyInitIterateAndLoadCacheName(
+      const base::Closure& callback,
+      scoped_ptr<std::vector<std::string> > indexed_cache_names,
+      const std::vector<std::string>::const_iterator& iter,
+      scoped_ptr<ServiceWorkerCache> cache);
+  void LazyInitDone();
+
+  void DidCreateBackend(base::WeakPtr<ServiceWorkerCache> cache,
+                        const CacheAndErrorCallback& callback,
+                        bool success);
+
+  void AddCacheToMaps(scoped_ptr<ServiceWorkerCache> cache);
+
+  // The CreateCache callbacks are below.
+  void CreateCacheDidCreateCache(const std::string& cache_name,
+                                 const CacheAndErrorCallback& callback,
+                                 scoped_ptr<ServiceWorkerCache> cache);
+  void CreateCacheDidWriteIndex(const CacheAndErrorCallback& callback,
+                                base::WeakPtr<ServiceWorkerCache> cache,
+                                bool success);
+
+  // The DeleteCache callbacks are below.
+  void DeleteCacheDidWriteIndex(const std::string& cache_name,
+                                const BoolAndErrorCallback& callback,
+                                bool success);
+  void DeleteCacheDidCleanUp(const BoolAndErrorCallback& callback,
+                             bool success);
+
+  // Whether or not we've loaded the list of cache names into memory.
   bool initialized_;
+
+  // The list of operations waiting on initialization.
+  std::vector<base::Closure> init_callbacks_;
+
+  // The map of ServiceWorkerCache objects to their integer ids that
+  // ServiceWorkers reference.  Owns the cache objects.
   CacheMap cache_map_;
+
+  // The map of cache names to their integer ids.
   NameMap name_map_;
+
+  // The file path for this CacheStorage.
   base::FilePath origin_path_;
-  scoped_refptr<base::MessageLoopProxy> callback_loop_;
-  scoped_ptr<CacheLoader> cache_loader_;
+
+  // The TaskRunner to run file IO on.
+  scoped_refptr<base::SequencedTaskRunner> cache_task_runner_;
+
+  // Performs backend specific operations (memory vs disk).
+  scoped_refptr<CacheLoader> cache_loader_;
+
+  base::WeakPtrFactory<ServiceWorkerCacheStorage> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerCacheStorage);
 };

content/browser/service_worker/service_worker_cache_storage_manager.cc

@@ -58,7 +58,7 @@ ServiceWorkerCacheStorageManager::~ServiceWorkerCacheStorageManager() {
   for (ServiceWorkerCacheStorageMap::iterator it = cache_storage_map_.begin();
        it != cache_storage_map_.end();
        ++it) {
-    cache_task_runner_->DeleteSoon(FROM_HERE, it->second);
+    delete it->second;
   }
 }
 
@@ -71,12 +71,7 @@ void ServiceWorkerCacheStorageManager::CreateCache(
   ServiceWorkerCacheStorage* cache_storage =
       FindOrCreateServiceWorkerCacheManager(origin);
 
-  cache_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&ServiceWorkerCacheStorage::CreateCache,
-                 base::Unretained(cache_storage),
-                 cache_name,
-                 callback));
+  cache_storage->CreateCache(cache_name, callback);
 }
 
 void ServiceWorkerCacheStorageManager::GetCache(
@@ -87,11 +82,8 @@ void ServiceWorkerCacheStorageManager::GetCache(
 
   ServiceWorkerCacheStorage* cache_storage =
       FindOrCreateServiceWorkerCacheManager(origin);
-  cache_task_runner_->PostTask(FROM_HERE,
-                               base::Bind(&ServiceWorkerCacheStorage::GetCache,
-                                          base::Unretained(cache_storage),
-                                          cache_name,
-                                          callback));
+
+  cache_storage->GetCache(cache_name, callback);
 }
 
 void ServiceWorkerCacheStorageManager::HasCache(
@@ -102,11 +94,7 @@ void ServiceWorkerCacheStorageManager::HasCache(
 
   ServiceWorkerCacheStorage* cache_storage =
       FindOrCreateServiceWorkerCacheManager(origin);
-  cache_task_runner_->PostTask(FROM_HERE,
-                               base::Bind(&ServiceWorkerCacheStorage::HasCache,
-                                          base::Unretained(cache_storage),
-                                          cache_name,
-                                          callback));
+  cache_storage->HasCache(cache_name, callback);
 }
 
 void ServiceWorkerCacheStorageManager::DeleteCache(
@@ -117,12 +105,7 @@ void ServiceWorkerCacheStorageManager::DeleteCache(
 
   ServiceWorkerCacheStorage* cache_storage =
       FindOrCreateServiceWorkerCacheManager(origin);
-  cache_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&ServiceWorkerCacheStorage::DeleteCache,
-                 base::Unretained(cache_storage),
-                 cache_name,
-                 callback));
+  cache_storage->DeleteCache(cache_name, callback);
 }
 
 void ServiceWorkerCacheStorageManager::EnumerateCaches(
@@ -133,11 +116,7 @@ void ServiceWorkerCacheStorageManager::EnumerateCaches(
   ServiceWorkerCacheStorage* cache_storage =
       FindOrCreateServiceWorkerCacheManager(origin);
 
-  cache_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&ServiceWorkerCacheStorage::EnumerateCaches,
-                 base::Unretained(cache_storage),
-                 callback));
+  cache_storage->EnumerateCaches(callback);
 }
 
 ServiceWorkerCacheStorageManager::ServiceWorkerCacheStorageManager(
@@ -158,7 +137,7 @@ ServiceWorkerCacheStorageManager::FindOrCreateServiceWorkerCacheManager(
     ServiceWorkerCacheStorage* cache_storage =
         new ServiceWorkerCacheStorage(ConstructOriginPath(root_path_, origin),
                                       memory_only,
-                                      base::MessageLoopProxy::current());
+                                      cache_task_runner_);
     // The map owns fetch_stores.
     cache_storage_map_.insert(std::make_pair(origin, cache_storage));
     return cache_storage;