Move ServiceWorkerCache backend creation to a lazy init function.

Previously the ServiceWorkerCacheStorage called ServiceWorkerCache::CreateBackend on Get() and Create().  Now the ServiceWorkerCache does it itself lazily.  This is simpler for the ServiceWorkerCache's caller and should improve performance as backends are created only when needed.  This makes the downstream refcounting CL simpler.

Related CLs:
1. https://crrev.com/542703002: Change ownership of the parameters to ServiceWorkerCache:: Put and Match.
* 2. https://crrev.com/545533002: Move ServiceWorkerCache backend creation to a lazy init function.
3. https://crrev.com/548533002: Make ServiceWorkerCacheStorage::CacheLoader::LoadCache synchronous
4. https://crrev.com/549493002: Expose ServiceWorkerCache objects to ServiceWorkerCacheStorageManager clients.

BUG=392621

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

Cr-Commit-Position: refs/heads/master@{#294372}

content/browser/service_worker/service_worker_cache.cc

@@ -650,51 +650,9 @@ base::WeakPtr<ServiceWorkerCache> ServiceWorkerCache::AsWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
-void ServiceWorkerCache::CreateBackend(const ErrorCallback& callback) {
-  DCHECK(!backend_);
-
-  // Use APP_CACHE as opposed to DISK_CACHE to prevent cache eviction.
-  net::CacheType cache_type =
-      path_.empty() ? net::MEMORY_CACHE : net::APP_CACHE;
-
-  scoped_ptr<ScopedBackendPtr> backend_ptr(new ScopedBackendPtr());
-
-  // Temporary pointer so that backend_ptr can be Pass()'d in Bind below.
-  ScopedBackendPtr* backend = backend_ptr.get();
-
-  net::CompletionCallback create_cache_callback =
-      base::Bind(CreateBackendDidCreate,
-                 callback,
-                 base::Passed(backend_ptr.Pass()),
-                 weak_ptr_factory_.GetWeakPtr());
-
-  // TODO(jkarlin): Use the cache MessageLoopProxy that ServiceWorkerCacheCore
-  // has for disk caches.
-  // TODO(jkarlin): Switch to SimpleCache after it supports APP_CACHE and after
-  // debugging why the QuickStressBody unittest fails with it.
-  int rv = disk_cache::CreateCacheBackend(
-      cache_type,
-      net::CACHE_BACKEND_SIMPLE,
-      path_,
-      kMaxCacheBytes,
-      false, /* force */
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE).get(),
-      NULL,
-      backend,
-      create_cache_callback);
-  if (rv != net::ERR_IO_PENDING)
-    create_cache_callback.Run(rv);
-}
-
 void ServiceWorkerCache::Put(scoped_ptr<ServiceWorkerFetchRequest> request,
                              scoped_ptr<ServiceWorkerResponse> response,
                              const ErrorCallback& callback) {
-  DCHECK(backend_);
-
-  scoped_ptr<disk_cache::Entry*> entry(new disk_cache::Entry*);
-
-  disk_cache::Entry** entry_ptr = entry.get();
-
   scoped_ptr<storage::BlobDataHandle> blob_data_handle;
 
   if (!response->blob_uuid.empty()) {
@@ -710,27 +668,36 @@ void ServiceWorkerCache::Put(scoped_ptr<ServiceWorkerFetchRequest> request,
     }
   }
 
-  ServiceWorkerFetchRequest* request_ptr = request.get();
-
-  net::CompletionCallback create_entry_callback =
-      base::Bind(PutDidCreateEntry,
+  base::Closure continuation = base::Bind(&ServiceWorkerCache::PutImpl,
+                                          weak_ptr_factory_.GetWeakPtr(),
                                           base::Passed(request.Pass()),
                                           base::Passed(response.Pass()),
-                 callback,
-                 base::Passed(entry.Pass()),
                                           base::Passed(blob_data_handle.Pass()),
-                 request_context_);
+                                          callback);
 
-  int rv = backend_->CreateEntry(
-      request_ptr->url.spec(), entry_ptr, create_entry_callback);
+  if (!initialized_) {
+    Init(continuation);
+    return;
+  }
 
-  if (rv != net::ERR_IO_PENDING)
-    create_entry_callback.Run(rv);
+  continuation.Run();
 }
 
 void ServiceWorkerCache::Match(scoped_ptr<ServiceWorkerFetchRequest> request,
                                const ResponseCallback& callback) {
-  DCHECK(backend_);
+  if (!initialized_) {
+    Init(base::Bind(&ServiceWorkerCache::Match,
+                    weak_ptr_factory_.GetWeakPtr(),
+                    base::Passed(request.Pass()),
+                    callback));
+    return;
+  }
+  if (!backend_) {
+    callback.Run(ErrorTypeStorage,
+                 scoped_ptr<ServiceWorkerResponse>(),
+                 scoped_ptr<storage::BlobDataHandle>());
+    return;
+  }
 
   scoped_ptr<disk_cache::Entry*> entry(new disk_cache::Entry*);
 
@@ -753,7 +720,17 @@ void ServiceWorkerCache::Match(scoped_ptr<ServiceWorkerFetchRequest> request,
 
 void ServiceWorkerCache::Delete(scoped_ptr<ServiceWorkerFetchRequest> request,
                                 const ErrorCallback& callback) {
-  DCHECK(backend_);
+  if (!initialized_) {
+    Init(base::Bind(&ServiceWorkerCache::Delete,
+                    weak_ptr_factory_.GetWeakPtr(),
+                    base::Passed(request.Pass()),
+                    callback));
+    return;
+  }
+  if (!backend_) {
+    callback.Run(ErrorTypeStorage);
+    return;
+  }
 
   scoped_ptr<disk_cache::Entry*> entry(new disk_cache::Entry*);
 
@@ -774,7 +751,15 @@ void ServiceWorkerCache::Delete(scoped_ptr<ServiceWorkerFetchRequest> request,
 }
 
 void ServiceWorkerCache::Keys(const RequestsCallback& callback) {
-  DCHECK(backend_);
+  if (!initialized_) {
+    Init(base::Bind(
+        &ServiceWorkerCache::Keys, weak_ptr_factory_.GetWeakPtr(), callback));
+    return;
+  }
+  if (!backend_) {
+    callback.Run(ErrorTypeStorage, scoped_ptr<Requests>());
+    return;
+  }
 
   // 1. Iterate through all of the entries, open them, and add them to a vector.
   // 2. For each open entry:
@@ -802,10 +787,6 @@ void ServiceWorkerCache::Keys(const RequestsCallback& callback) {
     open_entry_callback.Run(rv);
 }
 
-bool ServiceWorkerCache::HasCreatedBackend() const {
-  return backend_;
-}
-
 ServiceWorkerCache::ServiceWorkerCache(
     const base::FilePath& path,
     net::URLRequestContext* request_context,
@@ -813,9 +794,42 @@ ServiceWorkerCache::ServiceWorkerCache(
     : path_(path),
       request_context_(request_context),
       blob_storage_context_(blob_context),
+      initialized_(false),
       weak_ptr_factory_(this) {
 }
 
+void ServiceWorkerCache::PutImpl(
+    scoped_ptr<ServiceWorkerFetchRequest> request,
+    scoped_ptr<ServiceWorkerResponse> response,
+    scoped_ptr<storage::BlobDataHandle> blob_data_handle,
+    const ErrorCallback& callback) {
+  if (!backend_) {
+    callback.Run(ErrorTypeStorage);
+    return;
+  }
+
+  scoped_ptr<disk_cache::Entry*> entry(new disk_cache::Entry*);
+
+  disk_cache::Entry** entry_ptr = entry.get();
+
+  ServiceWorkerFetchRequest* request_ptr = request.get();
+
+  net::CompletionCallback create_entry_callback =
+      base::Bind(PutDidCreateEntry,
+                 base::Passed(request.Pass()),
+                 base::Passed(response.Pass()),
+                 callback,
+                 base::Passed(entry.Pass()),
+                 base::Passed(blob_data_handle.Pass()),
+                 request_context_);
+
+  int rv = backend_->CreateEntry(
+      request_ptr->url.spec(), entry_ptr, create_entry_callback);
+
+  if (rv != net::ERR_IO_PENDING)
+    create_entry_callback.Run(rv);
+}
+
 // static
 void ServiceWorkerCache::KeysDidOpenNextEntry(
     scoped_ptr<KeysContext> keys_context,
@@ -899,4 +913,60 @@ void ServiceWorkerCache::KeysDidReadHeaders(
   KeysProcessNextEntry(keys_context.Pass(), iter + 1);
 }
 
+void ServiceWorkerCache::CreateBackend(const ErrorCallback& callback) {
+  DCHECK(!backend_);
+
+  // Use APP_CACHE as opposed to DISK_CACHE to prevent cache eviction.
+  net::CacheType cache_type =
+      path_.empty() ? net::MEMORY_CACHE : net::APP_CACHE;
+
+  scoped_ptr<ScopedBackendPtr> backend_ptr(new ScopedBackendPtr());
+
+  // Temporary pointer so that backend_ptr can be Pass()'d in Bind below.
+  ScopedBackendPtr* backend = backend_ptr.get();
+
+  net::CompletionCallback create_cache_callback =
+      base::Bind(CreateBackendDidCreate,
+                 callback,
+                 base::Passed(backend_ptr.Pass()),
+                 weak_ptr_factory_.GetWeakPtr());
+
+  // TODO(jkarlin): Use the cache MessageLoopProxy that ServiceWorkerCacheCore
+  // has for disk caches.
+  int rv = disk_cache::CreateCacheBackend(
+      cache_type,
+      net::CACHE_BACKEND_SIMPLE,
+      path_,
+      kMaxCacheBytes,
+      false, /* force */
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE).get(),
+      NULL,
+      backend,
+      create_cache_callback);
+  if (rv != net::ERR_IO_PENDING)
+    create_cache_callback.Run(rv);
+}
+
+void ServiceWorkerCache::Init(const base::Closure& callback) {
+  init_callbacks_.push_back(callback);
+
+  // If this isn't the first call to Init then return as the initialization
+  // has already started.
+  if (init_callbacks_.size() > 1u)
+    return;
+
+  CreateBackend(base::Bind(&ServiceWorkerCache::InitDone,
+                           weak_ptr_factory_.GetWeakPtr()));
+}
+
+void ServiceWorkerCache::InitDone(ErrorType error) {
+  initialized_ = true;
+  for (std::vector<base::Closure>::iterator it = init_callbacks_.begin();
+       it != init_callbacks_.end();
+       ++it) {
+    it->Run();
+  }
+  init_callbacks_.clear();
+}
+
 }  // namespace content

content/browser/service_worker/service_worker_cache.h

@@ -59,13 +59,10 @@ class CONTENT_EXPORT ServiceWorkerCache {
       net::URLRequestContext* request_context,
       base::WeakPtr<storage::BlobStorageContext> blob_context);
 
+  // Operations in progress will complete after the cache is deleted but pending
+  // operations (those operations waiting for init to finish) won't.
   virtual ~ServiceWorkerCache();
 
-  // Loads the backend and calls the callback with the result (true for
-  // success). This must be called before member functions that require a
-  // backend are called. The callback will always be called.
-  void CreateBackend(const ErrorCallback& callback);
-
   // Returns ErrorTypeNotFound if not found. The callback will always be called.
   // |request| must remain valid until the callback is called.
   void Match(scoped_ptr<ServiceWorkerFetchRequest> request,
@@ -90,9 +87,6 @@ class CONTENT_EXPORT ServiceWorkerCache {
   // callback will always be called.
   void Keys(const RequestsCallback& callback);
 
-  // Call to determine if CreateBackend must be called.
-  bool HasCreatedBackend() const;
-
   void set_backend(scoped_ptr<disk_cache::Backend> backend) {
     backend_ = backend.Pass();
   }
@@ -107,6 +101,11 @@ class CONTENT_EXPORT ServiceWorkerCache {
                      net::URLRequestContext* request_context,
                      base::WeakPtr<storage::BlobStorageContext> blob_context);
 
+  void PutImpl(scoped_ptr<ServiceWorkerFetchRequest> request,
+               scoped_ptr<ServiceWorkerResponse> response,
+               scoped_ptr<storage::BlobDataHandle> blob_data_handle,
+               const ErrorCallback& callback);
+
   // Static callbacks for the Keys function.
   static void KeysDidOpenNextEntry(scoped_ptr<KeysContext> keys_context,
                                    int rv);
@@ -117,10 +116,19 @@ class CONTENT_EXPORT ServiceWorkerCache {
       const Entries::iterator& iter,
       scoped_ptr<ServiceWorkerRequestResponseHeaders> headers);
 
+  // Loads the backend and calls the callback with the result (true for
+  // success). The callback will always be called.
+  void CreateBackend(const ErrorCallback& callback);
+
+  void Init(const base::Closure& callback);
+  void InitDone(ErrorType error);
+
   scoped_ptr<disk_cache::Backend> backend_;
   base::FilePath path_;
   net::URLRequestContext* request_context_;
   base::WeakPtr<storage::BlobStorageContext> blob_storage_context_;
+  bool initialized_;
+  std::vector<base::Closure> init_callbacks_;
 
   base::WeakPtrFactory<ServiceWorkerCache> weak_ptr_factory_;
 

content/browser/service_worker/service_worker_cache_storage.cc

@@ -437,16 +437,7 @@ void ServiceWorkerCacheStorage::GetCache(
     return;
   }
 
-  ServiceWorkerCache* cache = cache_context->cache.get();
-
-  if (cache->HasCreatedBackend())
-    return callback.Run(cache_context->id, CACHE_STORAGE_ERROR_NO_ERROR);
-
-  cache->CreateBackend(base::Bind(&ServiceWorkerCacheStorage::DidCreateBackend,
-                                  weak_factory_.GetWeakPtr(),
-                                  cache->AsWeakPtr(),
-                                  cache_context->id,
-                                  callback));
+  callback.Run(cache_context->id, CACHE_STORAGE_ERROR_NO_ERROR);
 }
 
 void ServiceWorkerCacheStorage::HasCache(const std::string& cache_name,
@@ -518,23 +509,6 @@ void ServiceWorkerCacheStorage::EnumerateCaches(
   callback.Run(names, CACHE_STORAGE_ERROR_NO_ERROR);
 }
 
-void ServiceWorkerCacheStorage::DidCreateBackend(
-    base::WeakPtr<ServiceWorkerCache> cache,
-    CacheID cache_id,
-    const CacheAndErrorCallback& callback,
-    ServiceWorkerCache::ErrorType error) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  if (error != ServiceWorkerCache::ErrorTypeOK || !cache) {
-    // TODO(jkarlin): This should delete the directory and try again in case
-    // the cache is simply corrupt.
-    callback.Run(kInvalidCacheID, CACHE_STORAGE_ERROR_STORAGE);
-    return;
-  }
-
-  callback.Run(cache_id, CACHE_STORAGE_ERROR_NO_ERROR);
-}
-
 // Init is run lazily so that it is called on the proper MessageLoop.
 void ServiceWorkerCacheStorage::LazyInit(const base::Closure& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -664,15 +638,11 @@ void ServiceWorkerCacheStorage::CreateCacheDidWriteIndex(
     bool success) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!cache) {
-    callback.Run(false, CACHE_STORAGE_ERROR_CLOSING);
+    callback.Run(kInvalidCacheID, CACHE_STORAGE_ERROR_CLOSING);
     return;
   }
 
-  cache->CreateBackend(base::Bind(&ServiceWorkerCacheStorage::DidCreateBackend,
-                                  weak_factory_.GetWeakPtr(),
-                                  cache,
-                                  id,
-                                  callback));
+  callback.Run(id, CACHE_STORAGE_ERROR_NO_ERROR);
 }
 
 void ServiceWorkerCacheStorage::DeleteCacheDidWriteIndex(

content/browser/service_worker/service_worker_cache_storage.h

@@ -113,11 +113,6 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
       scoped_ptr<ServiceWorkerCache> cache);
   void LazyInitDone();
 
-  void DidCreateBackend(base::WeakPtr<ServiceWorkerCache> cache,
-                        CacheID cache_id,
-                        const CacheAndErrorCallback& callback,
-                        ServiceWorkerCache::ErrorType error);
-
   CacheContext* AddCacheToMaps(const std::string& cache_name,
                                scoped_ptr<ServiceWorkerCache> cache);
 

content/browser/service_worker/service_worker_cache_unittest.cc

@@ -76,7 +76,6 @@ class ServiceWorkerCacheTest : public testing::Test {
           url_request_context,
           blob_storage_context->context()->AsWeakPtr());
     }
-    CreateBackend();
   }
 
   virtual void TearDown() OVERRIDE {
@@ -123,15 +122,6 @@ class ServiceWorkerCacheTest : public testing::Test {
         GURL("http://example.com/no_body.html"), 200, "OK", headers, "");
   }
 
-  void CreateBackend() {
-    scoped_ptr<base::RunLoop> loop(new base::RunLoop());
-    cache_->CreateBackend(base::Bind(&ServiceWorkerCacheTest::ErrorTypeCallback,
-                                     base::Unretained(this),
-                                     base::Unretained(loop.get())));
-    loop->Run();
-    EXPECT_EQ(ServiceWorkerCache::ErrorTypeOK, callback_error_);
-  }
-
   scoped_ptr<ServiceWorkerFetchRequest> CopyFetchRequest(
       const ServiceWorkerFetchRequest& request) {
     return make_scoped_ptr(new ServiceWorkerFetchRequest(request.url,
@@ -293,7 +283,7 @@ TEST_P(ServiceWorkerCacheTestP, PutBody) {
   EXPECT_TRUE(Put(body_request_, body_response_));
 }
 
-TEST_P(ServiceWorkerCacheTestP, PutBodyDropBlobRef) {
+TEST_F(ServiceWorkerCacheTest, PutBodyDropBlobRef) {
   scoped_ptr<base::RunLoop> loop(new base::RunLoop());
   cache_->Put(CopyFetchRequest(body_request_),
               CopyFetchResponse(body_response_),