[CacheStorage] Check doomed caches first when dropping cache handles

content/browser/cache_storage/cache_storage.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/cache_storage/cache_storage.h"
 
 #include <stddef.h>
 
 #include <set>
 #include <string>
@@ skipping 95 matching lines @@
   // load the backend, that happens lazily when the cache is used.
   virtual std::unique_ptr<CacheStorageCache> CreateCache(
       const std::string& cache_name) = 0;
 
   // Deletes any pre-existing cache of the same name and then loads it.
   virtual void PrepareNewCacheDestination(const std::string& cache_name,
                                           const CacheCallback& callback) = 0;
 
   // After the backend has been deleted, do any extra house keeping such as
   // removing the cache's directory.
-  virtual void CleanUpDeletedCache(const std::string& key,
-                                   const BoolCallback& callback) = 0;
+  virtual void CleanUpDeletedCache(const std::string& key) = 0;
 
   // Writes the cache names (and sizes) to disk if applicable.
   virtual void WriteIndex(const StringVector& cache_names,
                           const BoolCallback& callback) = 0;
 
   // Loads the cache names from disk if applicable.
   virtual void LoadIndex(std::unique_ptr<std::vector<std::string>> cache_names,
                          const StringVectorCallback& callback) = 0;
 
+  // Called when CacheStorage has created a cache. Used to hold onto a handle to
+  // the cache if necessary.
+  virtual void NotifyCacheCreated(
+      const std::string& cache_name,
+      std::unique_ptr<CacheStorageCacheHandle> cache_handle){};
+
+  // Notification that a cache has been doomed and will be deleted once the last
+  // cache handle has been dropped. If the loader is holding a handle to the
+  // cache, it should drop it now.
+  virtual void NotifyCacheDoomed(const std::string& cache_name){};
+
  protected:
   scoped_refptr<base::SequencedTaskRunner> cache_task_runner_;
   scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
 
   // Owned by CacheStorage which owns this.
   storage::QuotaManagerProxy* quota_manager_proxy_;
 
   base::WeakPtr<storage::BlobStorageContext> blob_context_;
 
   // Raw pointer is safe because this object is owned by cache_storage_.
   CacheStorage* cache_storage_;
 
   GURL origin_;
 };
 
 // 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.
+// cache is doomed.
 class CacheStorage::MemoryLoader : public CacheStorage::CacheLoader {
  public:
   MemoryLoader(base::SequencedTaskRunner* cache_task_runner,
                scoped_refptr<net::URLRequestContextGetter> request_context,
                storage::QuotaManagerProxy* quota_manager_proxy,
                base::WeakPtr<storage::BlobStorageContext> blob_context,
                CacheStorage* cache_storage,
                const GURL& origin)
       : CacheLoader(cache_task_runner,
                     request_context,
                     quota_manager_proxy,
                     blob_context,
                     cache_storage,
                     origin) {}
 
   std::unique_ptr<CacheStorageCache> CreateCache(
       const std::string& cache_name) override {
     return CacheStorageCache::CreateMemoryCache(
         origin_, cache_name, cache_storage_, request_context_getter_,
         quota_manager_proxy_, blob_context_);
   }
 
   void PrepareNewCacheDestination(const std::string& cache_name,
                                   const CacheCallback& callback) override {
     std::unique_ptr<CacheStorageCache> cache = CreateCache(cache_name);
     callback.Run(std::move(cache));
   }
 
-  void CleanUpDeletedCache(const std::string& cache_name,
-                           const BoolCallback& callback) override {
-    CacheHandles::iterator it = cache_handles_.find(cache_name);
-    DCHECK(it != cache_handles_.end());
-    cache_handles_.erase(it);
+  void CleanUpDeletedCache(const std::string& cache_name) override {}
+
+  void WriteIndex(const StringVector& cache_names,
+                  const BoolCallback& callback) override {
     callback.Run(true);
   }
 
-  void WriteIndex(const StringVector& cache_names,
-                  const BoolCallback& callback) override {
-    callback.Run(false);
-  }
-
   void LoadIndex(std::unique_ptr<std::vector<std::string>> cache_names,
                  const StringVectorCallback& callback) override {
     callback.Run(std::move(cache_names));
   }
 
-  void StoreCacheHandle(const std::string& cache_name,
-                        std::unique_ptr<CacheStorageCacheHandle> cache_handle) {
+  void NotifyCacheCreated(
+      const std::string& cache_name,
+      std::unique_ptr<CacheStorageCacheHandle> cache_handle) override {
     DCHECK(!ContainsKey(cache_handles_, cache_name));
     cache_handles_.insert(std::make_pair(cache_name, std::move(cache_handle)));
-  }
+  };
+
+  void NotifyCacheDoomed(const std::string& cache_name) override {
+    DCHECK(ContainsKey(cache_handles_, cache_name));
+    cache_handles_.erase(cache_name);
+  };
 
  private:
   typedef std::map<std::string, std::unique_ptr<CacheStorageCacheHandle>>
       CacheHandles;
   ~MemoryLoader() override {}
 
   // Keep a reference to each cache to ensure that it's not freed before the
   // client calls CacheStorage::Delete or the CacheStorage is
   // freed.
   CacheHandles cache_handles_;
@@ skipping 59 matching lines @@
                                   const std::string& cache_dir) {
     if (cache_dir.empty()) {
       callback.Run(std::unique_ptr<CacheStorageCache>());
       return;
     }
 
     cache_name_to_cache_dir_[cache_name] = cache_dir;
     callback.Run(CreateCache(cache_name));
   }
 
-  void CleanUpDeletedCache(const std::string& cache_name,
-                           const BoolCallback& callback) override {
+  void CleanUpDeletedCache(const std::string& cache_name) override {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
     DCHECK(ContainsKey(cache_name_to_cache_dir_, cache_name));
 
     base::FilePath cache_path =
         origin_path_.AppendASCII(cache_name_to_cache_dir_[cache_name]);
     cache_name_to_cache_dir_.erase(cache_name);
 
     cache_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&SimpleCacheLoader::CleanUpDeleteCacheDirInPool, cache_path,
-                   callback, base::ThreadTaskRunnerHandle::Get()));
+        FROM_HERE, base::Bind(&SimpleCacheLoader::CleanUpDeleteCacheDirInPool,
+                              cache_path));
   }
 
-  static void CleanUpDeleteCacheDirInPool(
-      const base::FilePath& cache_path,
-      const BoolCallback& callback,
-      scoped_refptr<base::SingleThreadTaskRunner> original_task_runner) {
-    bool rv = base::DeleteFile(cache_path, true);
-    original_task_runner->PostTask(FROM_HERE, base::Bind(callback, rv));
+  static void CleanUpDeleteCacheDirInPool(const base::FilePath& cache_path) {
+    base::DeleteFile(cache_path, true /* recursive */);
   }
 
   void WriteIndex(const StringVector& cache_names,
                   const BoolCallback& callback) override {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
     // 1. Create the index file as a string. (WriteIndex)
     // 2. Write the file to disk. (WriteIndexWriteToFileInPool)
 
     CacheStorageIndex index;
@@ skipping 424 matching lines @@
     callback.Run(std::unique_ptr<CacheStorageCacheHandle>(),
                  CACHE_STORAGE_ERROR_STORAGE);
     return;
   }
 
   CacheStorageCache* cache_ptr = cache.get();
 
   cache_map_.insert(std::make_pair(cache_name, std::move(cache)));
   ordered_cache_names_.push_back(cache_name);
 
-  if (memory_only_) {
-    static_cast<MemoryLoader*>(cache_loader_.get())
-        ->StoreCacheHandle(cache_name, CreateCacheHandle(cache_ptr));
-  }
-
   cache_loader_->WriteIndex(
       ordered_cache_names_,
       base::Bind(&CacheStorage::CreateCacheDidWriteIndex,
                  weak_factory_.GetWeakPtr(), callback,
                  base::Passed(CreateCacheHandle(cache_ptr))));
+
+  cache_loader_->NotifyCacheCreated(cache_name, CreateCacheHandle(cache_ptr));
 }
 
 void CacheStorage::CreateCacheDidWriteIndex(
     const CacheAndErrorCallback& callback,
     std::unique_ptr<CacheStorageCacheHandle> cache_handle,
     bool success) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(cache_handle);
 
   // TODO(jkarlin): Handle !success.
 
   callback.Run(std::move(cache_handle), CACHE_STORAGE_OK);
 }
 
 void CacheStorage::HasCacheImpl(const std::string& cache_name,
                                 const BoolAndErrorCallback& callback) {
   bool has_cache = ContainsKey(cache_map_, cache_name);
   callback.Run(has_cache, CACHE_STORAGE_OK);
 }
 
 void CacheStorage::DeleteCacheImpl(const std::string& cache_name,
                                    const BoolAndErrorCallback& callback) {
-  std::unique_ptr<CacheStorageCacheHandle> cache_handle =
-      GetLoadedCache(cache_name);
-  if (!cache_handle) {
+  if (!GetLoadedCache(cache_name)) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(callback, false, CACHE_STORAGE_ERROR_NOT_FOUND));
     return;
   }
 
-  CacheMap::iterator map_iter = cache_map_.find(cache_name);
-  deleted_caches_.insert(
-      std::make_pair(cache_handle->value(), std::move(map_iter->second)));
-  cache_map_.erase(map_iter);
-
   // Delete the name from ordered_cache_names_.
+  StringVector original_ordered_cache_names = ordered_cache_names_;
   StringVector::iterator iter = std::find(
       ordered_cache_names_.begin(), ordered_cache_names_.end(), cache_name);
   DCHECK(iter != ordered_cache_names_.end());
   ordered_cache_names_.erase(iter);
 
-  CacheStorageCache* cache_ptr = cache_handle->value();
-  cache_ptr->GetSizeThenClose(
-      base::Bind(&CacheStorage::DeleteCacheDidClose, weak_factory_.GetWeakPtr(),
-                 cache_name, callback, ordered_cache_names_,
-                 base::Passed(std::move(cache_handle))));
-}
-
-void CacheStorage::DeleteCacheDidClose(
-    const std::string& cache_name,
-    const BoolAndErrorCallback& callback,
-    const StringVector& ordered_cache_names,
-    std::unique_ptr<CacheStorageCacheHandle> cache_handle,
-    int64_t cache_size) {
-  cache_loader_->WriteIndex(
-      ordered_cache_names,
-      base::Bind(&CacheStorage::DeleteCacheDidWriteIndex,
-                 weak_factory_.GetWeakPtr(), cache_name, callback, cache_size));
+  cache_loader_->WriteIndex(ordered_cache_names_,
+                            base::Bind(&CacheStorage::DeleteCacheDidWriteIndex,
+                                       weak_factory_.GetWeakPtr(), cache_name,
+                                       original_ordered_cache_names, callback));
 }
 
 void CacheStorage::DeleteCacheDidWriteIndex(
     const std::string& cache_name,
+    const StringVector& original_ordered_cache_names,
     const BoolAndErrorCallback& callback,
-    int cache_size,
     bool success) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  if (!success) {
+    // Undo any changes if the change couldn't be written to disk.
+    ordered_cache_names_ = original_ordered_cache_names;
+    callback.Run(false, CACHE_STORAGE_ERROR_STORAGE);
+    return;
+  }
+
+  CacheMap::iterator map_iter = cache_map_.find(cache_name);
+  doomed_caches_.insert(
+      std::make_pair(map_iter->second.get(), std::move(map_iter->second)));
+  cache_map_.erase(map_iter);
+
+  cache_loader_->NotifyCacheDoomed(cache_name);
+
+  callback.Run(true, CACHE_STORAGE_OK);
+}
+
+// Call this once the last handle to a doomed cache is gone. It's okay if this
+// doesn't get to complete before shutdown, the cache will be removed from disk
+// on next startup in that case.
+void CacheStorage::DeleteCacheFinalize(
+    std::unique_ptr<CacheStorageCache> doomed_cache) {
+  CacheStorageCache* cache = doomed_cache.get();
+  cache->Size(base::Bind(&CacheStorage::DeleteCacheDidGetSize,
+                         weak_factory_.GetWeakPtr(),
+                         base::Passed(std::move(doomed_cache))));
+}
+
+void CacheStorage::DeleteCacheDidGetSize(
+    std::unique_ptr<CacheStorageCache> cache,
+    int64_t cache_size) {
   quota_manager_proxy_->NotifyStorageModified(
       storage::QuotaClient::kServiceWorkerCache, origin_,
       storage::kStorageTypeTemporary, -1 * cache_size);
 
-  cache_loader_->CleanUpDeletedCache(
-      cache_name, base::Bind(&CacheStorage::DeleteCacheDidCleanUp,
-                             weak_factory_.GetWeakPtr(), callback));
-}
-
-void CacheStorage::DeleteCacheDidCleanUp(const BoolAndErrorCallback& callback,
-                                         bool success) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  callback.Run(true, CACHE_STORAGE_OK);
+  cache_loader_->CleanUpDeletedCache(cache->cache_name());
 }
 
 void CacheStorage::EnumerateCachesImpl(
     const StringsAndErrorCallback& callback) {
   callback.Run(ordered_cache_names_, CACHE_STORAGE_OK);
 }
 
 void CacheStorage::MatchCacheImpl(
     const std::string& cache_name,
     std::unique_ptr<ServiceWorkerFetchRequest> request,
@@ skipping 94 matching lines @@
 }
 
 void CacheStorage::DropCacheHandleRef(CacheStorageCache* cache) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   auto iter = cache_handle_counts_.find(cache);
   DCHECK(iter != cache_handle_counts_.end());
   DCHECK(iter->second >= 1);
 
   iter->second -= 1;
   if (iter->second == 0) {
-    // Delete the CacheStorageCache object. It's either in the main cache map or
-    // the CacheStorage::Delete operation has run on the cache, in which case
-    // it's in the deleted caches map.
-    auto cache_map_iter = cache_map_.find(cache->cache_name());
-
-    if (cache_map_iter == cache_map_.end()) {
-      auto deleted_caches_iter = deleted_caches_.find(cache);
-      DCHECK(deleted_caches_iter != deleted_caches_.end());
-      deleted_caches_.erase(deleted_caches_iter);
+    auto doomed_caches_iter = doomed_caches_.find(cache);
+    if (doomed_caches_iter != doomed_caches_.end()) {
+      // The last reference to a doomed cache is gone, perform clean up.
+      DeleteCacheFinalize(std::move(doomed_caches_iter->second));
+      doomed_caches_.erase(doomed_caches_iter);
       return;
     }
 
+    auto cache_map_iter = cache_map_.find(cache->cache_name());
+    DCHECK(cache_map_iter != cache_map_.end());
+
     cache_map_iter->second.reset();
     cache_handle_counts_.erase(iter);
   }
 }
 
 std::unique_ptr<CacheStorageCacheHandle> CacheStorage::CreateCacheHandle(
     CacheStorageCache* cache) {
   DCHECK(cache);
   return std::unique_ptr<CacheStorageCacheHandle>(new CacheStorageCacheHandle(
       cache->AsWeakPtr(), weak_factory_.GetWeakPtr()));
@@ skipping 122 matching lines @@
 void CacheStorage::PendingSizeCallback(const SizeCallback& callback,
                                        int64_t size) {
   base::WeakPtr<CacheStorage> cache_storage = weak_factory_.GetWeakPtr();
 
   callback.Run(size);
   if (cache_storage)
     scheduler_->CompleteOperationAndRunNext();
 }
 
 }  // namespace content