Expose ServiceWorkerCache objects to ServiceWorkerCacheStorageManager clients.

The CL provides reference counted ServiceWorkerCaches on ServiceWorkerCacheStorage::Get and Create.  Previously only CacheIDs were provided from those functions and there was no way for the client to access the ServiceWorkerCache itself.

This CL makes the following modifications:

1. Make ServiceWorkerCache refcounted
2. Return ServiceWorkerCache* instead of a CacheID from Create() and Put()
3. Removed CacheID from ServiceWorkerCache
4. Removed CacheContext from ServiceWorkerCache (since it really only existed to hold a CacheID)
5. Now keeps track of CacheIDs in ServiceWorkerCacheListener
6. Added memory unit tests

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/549493002

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

content/browser/service_worker/service_worker_cache.cc

@@ -605,7 +605,7 @@ struct ServiceWorkerCache::KeysContext {
       entries[i]->Close();
     if (enumerated_entry)
       enumerated_entry->Close();
-    if (cache && backend_iterator)
+    if (cache && backend_iterator && cache->backend_)
       cache->backend_->EndEnumeration(&backend_iterator);
   }
 
@@ -627,19 +627,19 @@ struct ServiceWorkerCache::KeysContext {
 };
 
 // static
-scoped_ptr<ServiceWorkerCache> ServiceWorkerCache::CreateMemoryCache(
+scoped_refptr<ServiceWorkerCache> ServiceWorkerCache::CreateMemoryCache(
     net::URLRequestContext* request_context,
     base::WeakPtr<storage::BlobStorageContext> blob_context) {
-  return make_scoped_ptr(
+  return make_scoped_refptr(
       new ServiceWorkerCache(base::FilePath(), request_context, blob_context));
 }
 
 // static
-scoped_ptr<ServiceWorkerCache> ServiceWorkerCache::CreatePersistentCache(
+scoped_refptr<ServiceWorkerCache> ServiceWorkerCache::CreatePersistentCache(
     const base::FilePath& path,
     net::URLRequestContext* request_context,
     base::WeakPtr<storage::BlobStorageContext> blob_context) {
-  return make_scoped_ptr(
+  return make_scoped_refptr(
       new ServiceWorkerCache(path, request_context, blob_context));
 }
 
@@ -787,6 +787,10 @@ void ServiceWorkerCache::Keys(const RequestsCallback& callback) {
     open_entry_callback.Run(rv);
 }
 
+void ServiceWorkerCache::Close() {
+  backend_.reset();
+}
+
 ServiceWorkerCache::ServiceWorkerCache(
     const base::FilePath& path,
     net::URLRequestContext* request_context,
@@ -849,6 +853,12 @@ void ServiceWorkerCache::KeysDidOpenNextEntry(
     return;
   }
 
+  if (!cache->backend_) {
+    keys_context->original_callback.Run(ErrorTypeNotFound,
+                                        scoped_ptr<Requests>());
+    return;
+  }
+
   // Store the entry.
   keys_context->entries.push_back(keys_context->enumerated_entry);
   keys_context->enumerated_entry = NULL;

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/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "net/base/completion_callback.h"
@@ -33,7 +34,8 @@ class ServiceWorkerRequestResponseHeaders;
 // Represents a ServiceWorker Cache as seen in
 // https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html.
 // InitializeIfNeeded must be called before calling the other public members.
-class CONTENT_EXPORT ServiceWorkerCache {
+class CONTENT_EXPORT ServiceWorkerCache
+    : public base::RefCounted<ServiceWorkerCache> {
  public:
   enum ErrorType {
     ErrorTypeOK = 0,
@@ -51,34 +53,27 @@ class CONTENT_EXPORT ServiceWorkerCache {
   typedef base::Callback<void(ErrorType, scoped_ptr<Requests>)>
       RequestsCallback;
 
-  static scoped_ptr<ServiceWorkerCache> CreateMemoryCache(
+  static scoped_refptr<ServiceWorkerCache> CreateMemoryCache(
       net::URLRequestContext* request_context,
       base::WeakPtr<storage::BlobStorageContext> blob_context);
-  static scoped_ptr<ServiceWorkerCache> CreatePersistentCache(
+  static scoped_refptr<ServiceWorkerCache> CreatePersistentCache(
       const base::FilePath& path,
       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();
-
   // 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,
              const ResponseCallback& callback);
 
   // Puts the request and response object in the cache. The response body (if
   // present) is stored in the cache, but not the request body. Returns
-  // ErrorTypeOK on success. The callback will always be called. |request| and
-  // |response| must remain valid until the callback is called.
+  // ErrorTypeOK on success. The callback will always be called.
   void Put(scoped_ptr<ServiceWorkerFetchRequest> request,
            scoped_ptr<ServiceWorkerResponse> response,
            const ErrorCallback& callback);
 
   // Returns ErrorNotFound if not found. Otherwise deletes and returns
-  // ErrorTypeOK. The callback will always be called. |request| must remain
-  // valid until the callback is called.
+  // ErrorTypeOK. The callback will always be called.
   void Delete(scoped_ptr<ServiceWorkerFetchRequest> request,
               const ErrorCallback& callback);
 
@@ -87,6 +82,9 @@ class CONTENT_EXPORT ServiceWorkerCache {
   // callback will always be called.
   void Keys(const RequestsCallback& callback);
 
+  // Prevent further operations on this object and delete the backend.
+  void Close();
+
   void set_backend(scoped_ptr<disk_cache::Backend> backend) {
     backend_ = backend.Pass();
   }
@@ -94,6 +92,8 @@ class CONTENT_EXPORT ServiceWorkerCache {
   base::WeakPtr<ServiceWorkerCache> AsWeakPtr();
 
  private:
+  friend class base::RefCounted<ServiceWorkerCache>;
+
   struct KeysContext;
   typedef std::vector<disk_cache::Entry*> Entries;
 
@@ -101,6 +101,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();
+
   void PutImpl(scoped_ptr<ServiceWorkerFetchRequest> request,
                scoped_ptr<ServiceWorkerResponse> response,
                scoped_ptr<storage::BlobDataHandle> blob_data_handle,
@@ -123,6 +127,8 @@ class CONTENT_EXPORT ServiceWorkerCache {
   void Init(const base::Closure& callback);
   void InitDone(ErrorType error);
 
+  // The backend can be deleted via the Close function at any time so always
+  // check for its existence before use.
   scoped_ptr<disk_cache::Backend> backend_;
   base::FilePath path_;
   net::URLRequestContext* request_context_;

content/browser/service_worker/service_worker_cache_listener.cc

@@ -49,7 +49,10 @@ WebServiceWorkerCacheError ToWebServiceWorkerCacheError(
 ServiceWorkerCacheListener::ServiceWorkerCacheListener(
     ServiceWorkerVersion* version,
     base::WeakPtr<ServiceWorkerContextCore> context)
-    : version_(version), context_(context), weak_factory_(this) {
+    : version_(version),
+      context_(context),
+      next_cache_id_(0),
+      weak_factory_(this) {
 }
 
 ServiceWorkerCacheListener::~ServiceWorkerCacheListener() {
@@ -143,13 +146,15 @@ void ServiceWorkerCacheListener::Send(const IPC::Message& message) {
 
 void ServiceWorkerCacheListener::OnCacheStorageGetCallback(
     int request_id,
-    int cache_id,
+    const scoped_refptr<ServiceWorkerCache>& cache,
     ServiceWorkerCacheStorage::CacheStorageError error) {
   if (error != ServiceWorkerCacheStorage::CACHE_STORAGE_ERROR_NO_ERROR) {
     Send(ServiceWorkerMsg_CacheStorageGetError(
         request_id, ToWebServiceWorkerCacheError(error)));
     return;
   }
+
+  CacheID cache_id = StoreCacheReference(cache);
   Send(ServiceWorkerMsg_CacheStorageGetSuccess(request_id, cache_id));
 }
 
@@ -173,13 +178,14 @@ void ServiceWorkerCacheListener::OnCacheStorageHasCallback(
 
 void ServiceWorkerCacheListener::OnCacheStorageCreateCallback(
     int request_id,
-    int cache_id,
+    const scoped_refptr<ServiceWorkerCache>& cache,
     ServiceWorkerCacheStorage::CacheStorageError error) {
   if (error != ServiceWorkerCacheStorage::CACHE_STORAGE_ERROR_NO_ERROR) {
     Send(ServiceWorkerMsg_CacheStorageCreateError(
         request_id, ToWebServiceWorkerCacheError(error)));
     return;
   }
+  CacheID cache_id = StoreCacheReference(cache);
   Send(ServiceWorkerMsg_CacheStorageCreateSuccess(request_id, cache_id));
 }
 
@@ -213,4 +219,28 @@ void ServiceWorkerCacheListener::OnCacheStorageKeysCallback(
   Send(ServiceWorkerMsg_CacheStorageKeysSuccess(request_id, string16s));
 }
 
+ServiceWorkerCacheListener::CacheID
+ServiceWorkerCacheListener::StoreCacheReference(
+    const scoped_refptr<ServiceWorkerCache>& cache) {
+  CacheToIDMap::iterator it = cache_to_id_map_.find(cache.get());
+  if (it == cache_to_id_map_.end()) {
+    CacheID cache_id = next_cache_id_++;
+    cache_to_id_map_.insert(std::make_pair(cache.get(), cache_id));
+    id_to_cache_map_.insert(std::make_pair(cache_id, cache));
+    return cache_id;
+  }
+
+  return it->second;
+}
+
+void ServiceWorkerCacheListener::DropCacheReference(CacheID cache_id) {
+  IDToCacheMap::iterator it = id_to_cache_map_.find(cache_id);
+  if (it != id_to_cache_map_.end())
+    return;
+
+  size_t deleted = cache_to_id_map_.erase(it->second.get());
+  DCHECK(deleted == 1u);
+  id_to_cache_map_.erase(it);
+}
+
 }  // namespace content

content/browser/service_worker/service_worker_cache_listener.h

@@ -35,12 +35,19 @@ class ServiceWorkerCacheListener : public EmbeddedWorkerInstance::Listener {
                            const base::string16& cache_name);
   void OnCacheStorageKeys(int request_id);
 
+  // TODO(gavinp,jkarlin): Plumb a message up from the renderer saying that the
+  // renderer is done with a cache id.
+
  private:
+  typedef int32_t CacheID;  // TODO(jkarlin): Bump to 64 bit.
+  typedef std::map<ServiceWorkerCache*, CacheID> CacheToIDMap;
+  typedef std::map<CacheID, scoped_refptr<ServiceWorkerCache> > IDToCacheMap;
+
   void Send(const IPC::Message& message);
 
   void OnCacheStorageGetCallback(
       int request_id,
-      int cache_id,
+      const scoped_refptr<ServiceWorkerCache>& cache,
       ServiceWorkerCacheStorage::CacheStorageError error);
   void OnCacheStorageHasCallback(
       int request_id,
@@ -48,7 +55,7 @@ class ServiceWorkerCacheListener : public EmbeddedWorkerInstance::Listener {
       ServiceWorkerCacheStorage::CacheStorageError error);
   void OnCacheStorageCreateCallback(
       int request_id,
-      int cache_id,
+      const scoped_refptr<ServiceWorkerCache>& cache,
       ServiceWorkerCacheStorage::CacheStorageError error);
   void OnCacheStorageDeleteCallback(
       int request_id,
@@ -59,12 +66,22 @@ class ServiceWorkerCacheListener : public EmbeddedWorkerInstance::Listener {
       const std::vector<std::string>& strings,
       ServiceWorkerCacheStorage::CacheStorageError error);
 
+  // Hangs onto a scoped_refptr for the cache if it isn't already doing so.
+  // Returns a unique cache_id. Call DropCacheReference when the client is done
+  // with this cache.
+  CacheID StoreCacheReference(const scoped_refptr<ServiceWorkerCache>& cache);
+  void DropCacheReference(CacheID cache_id);
+
   // The ServiceWorkerVersion to use for messaging back to the renderer thread.
   ServiceWorkerVersion* version_;
 
   // The ServiceWorkerContextCore should always outlive this.
   base::WeakPtr<ServiceWorkerContextCore> context_;
 
+  IDToCacheMap id_to_cache_map_;
+  CacheToIDMap cache_to_id_map_;
+  CacheID next_cache_id_;
+
   base::WeakPtrFactory<ServiceWorkerCacheListener> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerCacheListener);

content/browser/service_worker/service_worker_cache_storage.cc

@@ -22,28 +22,13 @@
 
 namespace content {
 
-// static
-const int ServiceWorkerCacheStorage::kInvalidCacheID = -1;
-
-// The meta information related to each ServiceWorkerCache that the
-// ServiceWorkerCacheManager needs to keep track of.
-// TODO(jkarlin): Add reference counting so that the deletion of javascript
-// objects can delete the ServiceWorkerCache.
-struct ServiceWorkerCacheStorage::CacheContext {
-  CacheContext(const std::string& name,
-               CacheID id,
-               scoped_ptr<ServiceWorkerCache> cache)
-      : name(name), id(id), cache(cache.Pass()) {}
-  std::string name;
-  CacheID id;
-  scoped_ptr<ServiceWorkerCache> cache;
-};
 
 // Handles the loading and clean up of ServiceWorkerCache objects. The
 // callback of every public method is guaranteed to be called.
 class ServiceWorkerCacheStorage::CacheLoader {
  public:
-  typedef base::Callback<void(scoped_ptr<ServiceWorkerCache>)> CacheCallback;
+  typedef base::Callback<void(const scoped_refptr<ServiceWorkerCache>&)>
+      CacheCallback;
   typedef base::Callback<void(bool)> BoolCallback;
   typedef base::Callback<void(scoped_ptr<std::vector<std::string> >)>
       StringsCallback;
@@ -59,7 +44,7 @@ class ServiceWorkerCacheStorage::CacheLoader {
 
   // Creates a ServiceWorkerCache with the given name. It does not attempt to
   // load the backend, that happens lazily when the cache is used.
-  virtual scoped_ptr<ServiceWorkerCache> CreateServiceWorkerCache(
+  virtual scoped_refptr<ServiceWorkerCache> CreateServiceWorkerCache(
       const std::string& cache_name) = 0;
 
   // Deletes any pre-existing cache of the same name and then loads it.
@@ -85,6 +70,10 @@ class ServiceWorkerCacheStorage::CacheLoader {
   base::WeakPtr<storage::BlobStorageContext> blob_context_;
 };
 
+// Creates memory-only ServiceWorkerCaches. Because these caches have no
+// persistent storage it is not safe to free them from memory if they might be
+// used again. Therefore this class holds a reference to each cache until the
+// cache is deleted.
 class ServiceWorkerCacheStorage::MemoryLoader
     : public ServiceWorkerCacheStorage::CacheLoader {
  public:
@@ -93,7 +82,7 @@ class ServiceWorkerCacheStorage::MemoryLoader
                base::WeakPtr<storage::BlobStorageContext> blob_context)
       : CacheLoader(cache_task_runner, request_context, blob_context) {}
 
-  virtual scoped_ptr<ServiceWorkerCache> CreateServiceWorkerCache(
+  virtual scoped_refptr<ServiceWorkerCache> CreateServiceWorkerCache(
       const std::string& cache_name) OVERRIDE {
     return ServiceWorkerCache::CreateMemoryCache(request_context_,
                                                  blob_context_);
@@ -101,11 +90,17 @@ class ServiceWorkerCacheStorage::MemoryLoader
 
   virtual void CreateCache(const std::string& cache_name,
                            const CacheCallback& callback) OVERRIDE {
-    callback.Run(CreateServiceWorkerCache(cache_name).Pass());
+    scoped_refptr<ServiceWorkerCache> cache =
+        ServiceWorkerCache::CreateMemoryCache(request_context_, blob_context_);
+    cache_refs_.insert(std::make_pair(cache_name, cache));
+    callback.Run(cache);
   }
 
   virtual void CleanUpDeletedCache(const std::string& cache_name,
                                    const BoolCallback& callback) OVERRIDE {
+    CacheRefMap::iterator it = cache_refs_.find(cache_name);
+    DCHECK(it != cache_refs_.end());
+    cache_refs_.erase(it);
     callback.Run(true);
   }
 
@@ -120,7 +115,13 @@ class ServiceWorkerCacheStorage::MemoryLoader
   }
 
  private:
+  typedef std::map<std::string, scoped_refptr<ServiceWorkerCache> > CacheRefMap;
   virtual ~MemoryLoader() {}
+
+  // Keep a reference to each cache to ensure that it's not freed before the
+  // client calls ServiceWorkerCacheStorage::Delete or the CacheStorage is
+  // freed.
+  CacheRefMap cache_refs_;
 };
 
 class ServiceWorkerCacheStorage::SimpleCacheLoader
@@ -134,7 +135,7 @@ class ServiceWorkerCacheStorage::SimpleCacheLoader
         origin_path_(origin_path),
         weak_ptr_factory_(this) {}
 
-  virtual scoped_ptr<ServiceWorkerCache> CreateServiceWorkerCache(
+  virtual scoped_refptr<ServiceWorkerCache> CreateServiceWorkerCache(
       const std::string& cache_name) OVERRIDE {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -176,7 +177,7 @@ class ServiceWorkerCacheStorage::SimpleCacheLoader
                                     base::WeakPtr<SimpleCacheLoader> loader,
                                     bool success) {
     if (!success || !loader) {
-      callback.Run(scoped_ptr<ServiceWorkerCache>());
+      callback.Run(scoped_refptr<ServiceWorkerCache>());
       return;
     }
 
@@ -218,9 +219,8 @@ class ServiceWorkerCacheStorage::SimpleCacheLoader
 
     for (CacheMap::const_iterator it = caches.begin(); it != caches.end();
          ++it) {
-      const CacheContext* cache = it->second;
       ServiceWorkerCacheStorageIndex::Cache* index_cache = index.add_cache();
-      index_cache->set_name(cache->name);
+      index_cache->set_name(it->first);
       index_cache->set_size(0);  // TODO(jkarlin): Make this real.
     }
 
@@ -337,9 +337,9 @@ ServiceWorkerCacheStorage::ServiceWorkerCacheStorage(
     net::URLRequestContext* request_context,
     base::WeakPtr<storage::BlobStorageContext> blob_context)
     : initialized_(false),
-      next_cache_id_(0),
       origin_path_(path),
       cache_task_runner_(cache_task_runner),
+      memory_only_(memory_only),
       weak_factory_(this) {
   if (memory_only)
     cache_loader_.reset(new MemoryLoader(
@@ -350,7 +350,6 @@ ServiceWorkerCacheStorage::ServiceWorkerCacheStorage(
 }
 
 ServiceWorkerCacheStorage::~ServiceWorkerCacheStorage() {
-  STLDeleteContainerPairSecondPointers(cache_map_.begin(), cache_map_.end());
 }
 
 void ServiceWorkerCacheStorage::CreateCache(
@@ -364,8 +363,9 @@ void ServiceWorkerCacheStorage::CreateCache(
     return;
   }
 
-  if (GetLoadedCache(cache_name)) {
-    callback.Run(kInvalidCacheID, CACHE_STORAGE_ERROR_EXISTS);
+  if (cache_map_.find(cache_name) != cache_map_.end()) {
+    callback.Run(scoped_refptr<ServiceWorkerCache>(),
+                 CACHE_STORAGE_ERROR_EXISTS);
     return;
   }
 
@@ -390,13 +390,14 @@ void ServiceWorkerCacheStorage::GetCache(
     return;
   }
 
-  CacheContext* cache_context = GetLoadedCache(cache_name);
-  if (!cache_context) {
-    callback.Run(kInvalidCacheID, CACHE_STORAGE_ERROR_NOT_FOUND);
+  scoped_refptr<ServiceWorkerCache> cache = GetLoadedCache(cache_name);
+  if (!cache.get()) {
+    callback.Run(scoped_refptr<ServiceWorkerCache>(),
+                 CACHE_STORAGE_ERROR_NOT_FOUND);
     return;
   }
 
-  callback.Run(cache_context->id, CACHE_STORAGE_ERROR_NO_ERROR);
+  callback.Run(cache, CACHE_STORAGE_ERROR_NO_ERROR);
 }
 
 void ServiceWorkerCacheStorage::HasCache(const std::string& cache_name,
@@ -411,7 +412,7 @@ void ServiceWorkerCacheStorage::HasCache(const std::string& cache_name,
     return;
   }
 
-  bool has_cache = GetLoadedCache(cache_name) != NULL;
+  bool has_cache = cache_map_.find(cache_name) != cache_map_.end();
 
   callback.Run(has_cache, CACHE_STORAGE_ERROR_NO_ERROR);
 }
@@ -429,15 +430,17 @@ void ServiceWorkerCacheStorage::DeleteCache(
     return;
   }
 
-  scoped_ptr<CacheContext> cache_context(GetLoadedCache(cache_name));
-  if (!cache_context) {
+  CacheMap::iterator it = cache_map_.find(cache_name);
+  if (it == cache_map_.end()) {
     callback.Run(false, CACHE_STORAGE_ERROR_NOT_FOUND);
     return;
   }
 
-  name_map_.erase(cache_name);
-  cache_map_.erase(cache_context->id);
-  cache_context.reset();
+  base::WeakPtr<ServiceWorkerCache> cache = it->second;
+  if (cache)
+    cache->Close();
+
+  cache_map_.erase(it);
 
   // Update the Index
   cache_loader_->WriteIndex(
@@ -460,7 +463,7 @@ void ServiceWorkerCacheStorage::EnumerateCaches(
   }
 
   std::vector<std::string> names;
-  for (NameMap::const_iterator it = name_map_.begin(); it != name_map_.end();
+  for (CacheMap::const_iterator it = cache_map_.begin(); it != cache_map_.end();
        ++it) {
     names.push_back(it->first);
   }
@@ -500,12 +503,9 @@ void ServiceWorkerCacheStorage::LazyInitDidLoadIndex(
     scoped_ptr<std::vector<std::string> > indexed_cache_names) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  for (std::vector<std::string>::iterator it = indexed_cache_names->begin();
-       it != indexed_cache_names->end();
-       ++it) {
-    scoped_ptr<ServiceWorkerCache> cache =
-        cache_loader_->CreateServiceWorkerCache(*it);
-    AddCacheToMaps(*it, cache.Pass());
+  for (size_t i = 0u, max = indexed_cache_names->size(); i < max; ++i) {
+    cache_map_.insert(std::make_pair(indexed_cache_names->at(i),
+                                     base::WeakPtr<ServiceWorkerCache>()));
   }
 
   initialized_ = true;
@@ -517,53 +517,36 @@ void ServiceWorkerCacheStorage::LazyInitDidLoadIndex(
   init_callbacks_.clear();
 }
 
-ServiceWorkerCacheStorage::CacheContext*
-ServiceWorkerCacheStorage::AddCacheToMaps(
-    const std::string& cache_name,
-    scoped_ptr<ServiceWorkerCache> cache) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  CacheID id = next_cache_id_++;
-  CacheContext* cache_context = new CacheContext(cache_name, id, cache.Pass());
-  cache_map_.insert(std::make_pair(id, cache_context));  // Takes ownership
-  name_map_.insert(std::make_pair(cache_name, id));
-  return cache_context;
-}
-
 void ServiceWorkerCacheStorage::CreateCacheDidCreateCache(
     const std::string& cache_name,
     const CacheAndErrorCallback& callback,
-    scoped_ptr<ServiceWorkerCache> cache) {
+    const scoped_refptr<ServiceWorkerCache>& cache) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  if (!cache) {
-    callback.Run(kInvalidCacheID, CACHE_STORAGE_ERROR_CLOSING);
+  if (!cache.get()) {
+    callback.Run(scoped_refptr<ServiceWorkerCache>(),
+                 CACHE_STORAGE_ERROR_CLOSING);
     return;
   }
 
-  CacheContext* cache_context = AddCacheToMaps(cache_name, cache.Pass());
+  cache_map_.insert(std::make_pair(cache_name, cache->AsWeakPtr()));
 
   cache_loader_->WriteIndex(
       cache_map_,
       base::Bind(&ServiceWorkerCacheStorage::CreateCacheDidWriteIndex,
                  weak_factory_.GetWeakPtr(),
                  callback,
-                 cache_context->cache->AsWeakPtr(),
-                 cache_context->id));
+                 cache));
 }
 
 void ServiceWorkerCacheStorage::CreateCacheDidWriteIndex(
     const CacheAndErrorCallback& callback,
-    base::WeakPtr<ServiceWorkerCache> cache,
-    CacheID id,
+    const scoped_refptr<ServiceWorkerCache>& cache,
     bool success) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (!cache) {
-    callback.Run(kInvalidCacheID, CACHE_STORAGE_ERROR_CLOSING);
-    return;
-  }
+  DCHECK(cache.get());
 
-  callback.Run(id, CACHE_STORAGE_ERROR_NO_ERROR);
+  callback.Run(cache, CACHE_STORAGE_ERROR_NO_ERROR);
 }
 
 void ServiceWorkerCacheStorage::DeleteCacheDidWriteIndex(
@@ -587,18 +570,25 @@ void ServiceWorkerCacheStorage::DeleteCacheDidCleanUp(
   callback.Run(true, CACHE_STORAGE_ERROR_NO_ERROR);
 }
 
-ServiceWorkerCacheStorage::CacheContext*
-ServiceWorkerCacheStorage::GetLoadedCache(const std::string& cache_name) const {
+scoped_refptr<ServiceWorkerCache> ServiceWorkerCacheStorage::GetLoadedCache(
+    const std::string& cache_name) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(initialized_);
 
-  NameMap::const_iterator name_iter = name_map_.find(cache_name);
-  if (name_iter == name_map_.end())
-    return NULL;
+  CacheMap::iterator map_iter = cache_map_.find(cache_name);
+  if (map_iter == cache_map_.end())
+    return scoped_refptr<ServiceWorkerCache>();
+
+  base::WeakPtr<ServiceWorkerCache> cache = map_iter->second;
+
+  if (!cache) {
+    scoped_refptr<ServiceWorkerCache> new_cache =
+        cache_loader_->CreateServiceWorkerCache(cache_name);
+    map_iter->second = new_cache->AsWeakPtr();
+    return new_cache;
+  }
 
-  CacheMap::const_iterator map_iter = cache_map_.find(name_iter->second);
-  DCHECK(map_iter != cache_map_.end());
-  return map_iter->second;
+  return make_scoped_refptr(cache.get());
 }
 
 }  // namespace content

content/browser/service_worker/service_worker_cache_storage.h

@@ -34,13 +34,6 @@ namespace content {
 // on the IO thread.
 class CONTENT_EXPORT ServiceWorkerCacheStorage {
  public:
-  // TODO(jkarlin): Convert this (and everything that uses it) to int64_t so
-  // that we don't run out of id space.
-  typedef int32_t CacheID;
-
-  // The CacheID returned on failed cache operations.
-  const static int kInvalidCacheID;
-
   enum CacheStorageError {
     CACHE_STORAGE_ERROR_NO_ERROR,
     CACHE_STORAGE_ERROR_NOT_IMPLEMENTED,
@@ -51,7 +44,8 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
   };
 
   typedef base::Callback<void(bool, CacheStorageError)> BoolAndErrorCallback;
-  typedef base::Callback<void(int, CacheStorageError)> CacheAndErrorCallback;
+  typedef base::Callback<void(const scoped_refptr<ServiceWorkerCache>&,
+                              CacheStorageError)> CacheAndErrorCallback;
   typedef base::Callback<void(const std::vector<std::string>&,
                               CacheStorageError)> StringsAndErrorCallback;
 
@@ -93,12 +87,13 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
   class MemoryLoader;
   class SimpleCacheLoader;
   class CacheLoader;
-  struct CacheContext;
 
-  typedef std::map<CacheID, CacheContext*> CacheMap;
-  typedef std::map<std::string, CacheID> NameMap;
+  typedef std::map<std::string, base::WeakPtr<ServiceWorkerCache> > CacheMap;
 
-  CacheContext* GetLoadedCache(const std::string& cache_name) const;
+  // Return a ServiceWorkerCache for the given name if the name is known. If the
+  // ServiceWorkerCache has been deleted, creates a new one.
+  scoped_refptr<ServiceWorkerCache> GetLoadedCache(
+      const std::string& cache_name);
 
   // Initializer and its callback are below.
   void LazyInit(const base::Closure& closure);
@@ -106,16 +101,16 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
       const base::Closure& callback,
       scoped_ptr<std::vector<std::string> > indexed_cache_names);
 
-  CacheContext* AddCacheToMaps(const std::string& cache_name,
-                               scoped_ptr<ServiceWorkerCache> cache);
+  void AddCacheToMap(const std::string& cache_name,
+                     base::WeakPtr<ServiceWorkerCache> cache);
 
   // The CreateCache callbacks are below.
-  void CreateCacheDidCreateCache(const std::string& cache_name,
+  void CreateCacheDidCreateCache(
+      const std::string& cache_name,
       const CacheAndErrorCallback& callback,
-                                 scoped_ptr<ServiceWorkerCache> cache);
+      const scoped_refptr<ServiceWorkerCache>& cache);
   void CreateCacheDidWriteIndex(const CacheAndErrorCallback& callback,
-                                base::WeakPtr<ServiceWorkerCache> cache,
-                                CacheID id,
+                                const scoped_refptr<ServiceWorkerCache>& cache,
                                 bool success);
 
   // The DeleteCache callbacks are below.
@@ -131,14 +126,8 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
   // The list of operations waiting on initialization.
   std::vector<base::Closure> init_callbacks_;
 
-  // The map of CacheIDs to their CacheContext objects. Owns the CacheContext
-  // object. The CacheIDs are used by JavaScript to reference a
-  // ServiceWorkerCache.
+  // The map of cache names to ServiceWorkerCache objects.
   CacheMap cache_map_;
-  CacheID next_cache_id_;  // The next CacheID to use in cache_map_
-
-  // The map of cache names to their integer ids.
-  NameMap name_map_;
 
   // The file path for this CacheStorage.
   base::FilePath origin_path_;
@@ -146,6 +135,9 @@ class CONTENT_EXPORT ServiceWorkerCacheStorage {
   // The TaskRunner to run file IO on.
   scoped_refptr<base::SequencedTaskRunner> cache_task_runner_;
 
+  // Whether or not to store data in disk or memory.
+  bool memory_only_;
+
   // Performs backend specific operations (memory vs disk).
   scoped_ptr<CacheLoader> cache_loader_;
 

content/browser/service_worker/service_worker_cache_storage_manager_unittest.cc

@@ -24,9 +24,9 @@ class ServiceWorkerCacheStorageManagerTest : public testing::Test {
   ServiceWorkerCacheStorageManagerTest()
       : browser_thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP),
         callback_bool_(false),
-        callback_cache_id_(0),
         callback_error_(
             ServiceWorkerCacheStorage::CACHE_STORAGE_ERROR_NO_ERROR),
+        callback_cache_error_(ServiceWorkerCache::ErrorTypeOK),
         origin1_("http://example1.com"),
         origin2_("http://example2.com") {}
 
@@ -74,9 +74,9 @@ class ServiceWorkerCacheStorageManagerTest : public testing::Test {
 
   void CacheAndErrorCallback(
       base::RunLoop* run_loop,
-      int cache_id,
+      const scoped_refptr<ServiceWorkerCache>& cache,
       ServiceWorkerCacheStorage::CacheStorageError error) {
-    callback_cache_id_ = cache_id;
+    callback_cache_ = cache;
     callback_error_ = error;
     run_loop->Quit();
   }
@@ -90,6 +90,23 @@ class ServiceWorkerCacheStorageManagerTest : public testing::Test {
     run_loop->Quit();
   }
 
+  void CachePutCallback(base::RunLoop* run_loop,
+                        ServiceWorkerCache::ErrorType error) {
+    callback_cache_error_ = error;
+    run_loop->Quit();
+  }
+
+  void CacheMatchCallback(
+      base::RunLoop* run_loop,
+      ServiceWorkerCache::ErrorType error,
+      scoped_ptr<ServiceWorkerResponse> response,
+      scoped_ptr<storage::BlobDataHandle> blob_data_handle) {
+    callback_cache_error_ = error;
+    callback_cache_response_ = response.Pass();
+    // Deliberately drop the data handle as only the url is being tested.
+    run_loop->Quit();
+  }
+
   bool CreateCache(const GURL& origin, const std::string& cache_name) {
     scoped_ptr<base::RunLoop> loop(new base::RunLoop());
     cache_manager_->CreateCache(
@@ -103,9 +120,9 @@ class ServiceWorkerCacheStorageManagerTest : public testing::Test {
     bool error = callback_error_ !=
                  ServiceWorkerCacheStorage::CACHE_STORAGE_ERROR_NO_ERROR;
     if (error)
-      EXPECT_EQ(ServiceWorkerCacheStorage::kInvalidCacheID, callback_cache_id_);
+      EXPECT_TRUE(!callback_cache_.get());
     else
-      EXPECT_LE(0, callback_cache_id_);
+      EXPECT_TRUE(callback_cache_.get());
     return !error;
   }
 
@@ -122,9 +139,9 @@ class ServiceWorkerCacheStorageManagerTest : public testing::Test {
     bool error = callback_error_ !=
                  ServiceWorkerCacheStorage::CACHE_STORAGE_ERROR_NO_ERROR;
     if (error)
-      EXPECT_EQ(ServiceWorkerCacheStorage::kInvalidCacheID, callback_cache_id_);
+      EXPECT_TRUE(!callback_cache_.get());
     else
-      EXPECT_LE(0, callback_cache_id_);
+      EXPECT_TRUE(callback_cache_.get());
     return !error;
   }
 
@@ -169,6 +186,43 @@ class ServiceWorkerCacheStorageManagerTest : public testing::Test {
     return !error;
   }
 
+  bool CachePut(const scoped_refptr<ServiceWorkerCache>& cache,
+                const std::string& url) {
+    scoped_ptr<ServiceWorkerFetchRequest> request(
+        new ServiceWorkerFetchRequest());
+    scoped_ptr<ServiceWorkerResponse> response(new ServiceWorkerResponse());
+    request->url = GURL("http://example.com/foo");
+    response->url = GURL("http://example.com/foo");
+    scoped_ptr<base::RunLoop> loop(new base::RunLoop());
+    cache->Put(
+        request.Pass(),
+        response.Pass(),
+        base::Bind(&ServiceWorkerCacheStorageManagerTest::CachePutCallback,
+                   base::Unretained(this),
+                   base::Unretained(loop.get())));
+    loop->Run();
+
+    bool error = callback_cache_error_ != ServiceWorkerCache::ErrorTypeOK;
+    return !error;
+  }
+
+  bool CacheMatch(const scoped_refptr<ServiceWorkerCache>& cache,
+                  const std::string& url) {
+    scoped_ptr<ServiceWorkerFetchRequest> request(
+        new ServiceWorkerFetchRequest());
+    request->url = GURL("http://example.com/foo");
+    scoped_ptr<base::RunLoop> loop(new base::RunLoop());
+    cache->Match(
+        request.Pass(),
+        base::Bind(&ServiceWorkerCacheStorageManagerTest::CacheMatchCallback,
+                   base::Unretained(this),
+                   base::Unretained(loop.get())));
+    loop->Run();
+
+    bool error = callback_cache_error_ != ServiceWorkerCache::ErrorTypeOK;
+    return !error;
+  }
+
   bool VerifyKeys(const std::vector<std::string>& expected_keys) {
     if (expected_keys.size() != callback_strings_.size())
       return false;
@@ -191,9 +245,11 @@ class ServiceWorkerCacheStorageManagerTest : public testing::Test {
   base::ScopedTempDir temp_dir_;
   scoped_ptr<ServiceWorkerCacheStorageManager> cache_manager_;
 
+  scoped_refptr<ServiceWorkerCache> callback_cache_;
   int callback_bool_;
-  ServiceWorkerCacheStorage::CacheID callback_cache_id_;
   ServiceWorkerCacheStorage::CacheStorageError callback_error_;
+  ServiceWorkerCache::ErrorType callback_cache_error_;
+  scoped_ptr<ServiceWorkerResponse> callback_cache_response_;
   std::vector<std::string> callback_strings_;
 
   const GURL origin1_;
@@ -234,12 +290,11 @@ TEST_P(ServiceWorkerCacheStorageManagerTestP, CreateTwoCaches) {
   EXPECT_TRUE(CreateCache(origin1_, "bar"));
 }
 
-TEST_P(ServiceWorkerCacheStorageManagerTestP, IDsDiffer) {
+TEST_P(ServiceWorkerCacheStorageManagerTestP, CachePointersDiffer) {
   EXPECT_TRUE(CreateCache(origin1_, "foo"));
-  ServiceWorkerCacheStorage::CacheID id1 = callback_cache_id_;
+  scoped_refptr<ServiceWorkerCache> cache = callback_cache_;
   EXPECT_TRUE(CreateCache(origin1_, "bar"));
-  ServiceWorkerCacheStorage::CacheID id2 = callback_cache_id_;
-  EXPECT_NE(id1, id2);
+  EXPECT_TRUE(cache.get() != callback_cache_.get());
 }
 
 TEST_P(ServiceWorkerCacheStorageManagerTestP, Create2CachesSameNameDiffSWs) {
@@ -249,9 +304,9 @@ TEST_P(ServiceWorkerCacheStorageManagerTestP, Create2CachesSameNameDiffSWs) {
 
 TEST_P(ServiceWorkerCacheStorageManagerTestP, GetCache) {
   EXPECT_TRUE(CreateCache(origin1_, "foo"));
-  int cache_id = callback_cache_id_;
+  scoped_refptr<ServiceWorkerCache> cache = callback_cache_;
   EXPECT_TRUE(Get(origin1_, "foo"));
-  EXPECT_EQ(cache_id, callback_cache_id_);
+  EXPECT_TRUE(cache.get() == callback_cache_.get());
 }
 
 TEST_P(ServiceWorkerCacheStorageManagerTestP, GetNonExistent) {
@@ -318,9 +373,9 @@ TEST_P(ServiceWorkerCacheStorageManagerTestP, DeletedKeysGone) {
 
 TEST_P(ServiceWorkerCacheStorageManagerTestP, Chinese) {
   EXPECT_TRUE(CreateCache(origin1_, "你好"));
-  int cache_id = callback_cache_id_;
+  scoped_refptr<ServiceWorkerCache> cache = callback_cache_;
   EXPECT_TRUE(Get(origin1_, "你好"));
-  EXPECT_EQ(cache_id, callback_cache_id_);
+  EXPECT_TRUE(cache.get() == callback_cache_.get());
   EXPECT_TRUE(Keys(origin1_));
   EXPECT_EQ(1u, callback_strings_.size());
   EXPECT_TRUE("你好" == callback_strings_[0]);
@@ -328,9 +383,9 @@ TEST_P(ServiceWorkerCacheStorageManagerTestP, Chinese) {
 
 TEST_F(ServiceWorkerCacheStorageManagerTest, EmptyKey) {
   EXPECT_TRUE(CreateCache(origin1_, ""));
-  int cache_id = callback_cache_id_;
+  scoped_refptr<ServiceWorkerCache> cache = callback_cache_;
   EXPECT_TRUE(Get(origin1_, ""));
-  EXPECT_EQ(cache_id, callback_cache_id_);
+  EXPECT_EQ(cache.get(), callback_cache_.get());
   EXPECT_TRUE(Keys(origin1_));
   EXPECT_EQ(1u, callback_strings_.size());
   EXPECT_TRUE("" == callback_strings_[0]);
@@ -385,6 +440,42 @@ TEST_F(ServiceWorkerCacheStorageManagerTest, BadOriginName) {
   EXPECT_STREQ("foo", callback_strings_[0].c_str());
 }
 
+// With a persistent cache if the client drops its reference to a
+// ServiceWorkerCache
+// it should be deleted.
+TEST_F(ServiceWorkerCacheStorageManagerTest, DropReference) {
+  EXPECT_TRUE(CreateCache(origin1_, "foo"));
+  base::WeakPtr<ServiceWorkerCache> cache = callback_cache_->AsWeakPtr();
+  callback_cache_ = NULL;
+  EXPECT_TRUE(!cache);
+}
+
+// With a memory cache the cache can't be freed from memory until the client
+// calls delete.
+TEST_F(ServiceWorkerCacheStorageManagerMemoryOnlyTest,
+       MemoryLosesReferenceOnlyAfterDelete) {
+  EXPECT_TRUE(CreateCache(origin1_, "foo"));
+  base::WeakPtr<ServiceWorkerCache> cache = callback_cache_->AsWeakPtr();
+  callback_cache_ = NULL;
+  EXPECT_TRUE(cache);
+  EXPECT_TRUE(Delete(origin1_, "foo"));
+  EXPECT_FALSE(cache);
+}
+
+TEST_P(ServiceWorkerCacheStorageManagerTestP, RecreateCacheOnDemand) {
+  EXPECT_TRUE(CreateCache(origin1_, "foo"));
+  EXPECT_TRUE(CachePut(callback_cache_, "bar"));
+  callback_cache_ = NULL;
+  EXPECT_TRUE(Get(origin1_, "foo"));
+  EXPECT_TRUE(CacheMatch(callback_cache_, "bar"));
+}
+
+TEST_P(ServiceWorkerCacheStorageManagerTestP, DeleteBeforeRelease) {
+  EXPECT_TRUE(CreateCache(origin1_, "foo"));
+  EXPECT_TRUE(Delete(origin1_, "foo"));
+  EXPECT_TRUE(callback_cache_->AsWeakPtr());
+}
+
 INSTANTIATE_TEST_CASE_P(ServiceWorkerCacheStorageManagerTests,
                         ServiceWorkerCacheStorageManagerTestP,
                         ::testing::Values(false, true));

content/browser/service_worker/service_worker_cache_unittest.cc

@@ -82,7 +82,7 @@ class ServiceWorkerCacheTest : public testing::Test {
     base::RunLoop().RunUntilIdle();
     disk_cache::SimpleBackendImpl::FlushWorkerPoolForTesting();
     base::RunLoop().RunUntilIdle();
-    cache_.reset();
+    cache_ = NULL;
     base::RunLoop().RunUntilIdle();
     disk_cache::SimpleBackendImpl::FlushWorkerPoolForTesting();
     base::RunLoop().RunUntilIdle();
@@ -255,7 +255,7 @@ class ServiceWorkerCacheTest : public testing::Test {
   storage::BlobStorageContext* blob_storage_context_;
 
   base::ScopedTempDir temp_dir_;
-  scoped_ptr<ServiceWorkerCache> cache_;
+  scoped_refptr<ServiceWorkerCache> cache_;
 
   ServiceWorkerFetchRequest body_request_;
   ServiceWorkerResponse body_response_;