Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(9199)

Unified Diff: content/browser/cache_storage/cache_storage.cc

Issue 2186433004: [CacheStorage] Deleting a cache could delete the wrong directory (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@fix1
Patch Set: Rebase Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | content/browser/cache_storage/cache_storage_manager_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/cache_storage/cache_storage.cc
diff --git a/content/browser/cache_storage/cache_storage.cc b/content/browser/cache_storage/cache_storage.cc
index bb5269bc3e155586ad693ee1b4e4703e522efda7..326feed6f6ff63ad593281efc8a484768a0e523d 100644
--- a/content/browser/cache_storage/cache_storage.cc
+++ b/content/browser/cache_storage/cache_storage.cc
@@ -113,7 +113,7 @@ class CacheStorage::CacheLoader {
// 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) = 0;
+ virtual void CleanUpDeletedCache(CacheStorageCache* cache) = 0;
// Writes the cache names (and sizes) to disk if applicable.
virtual void WriteIndex(const StringVector& cache_names,
@@ -127,12 +127,12 @@ class CacheStorage::CacheLoader {
// the cache if necessary.
virtual void NotifyCacheCreated(
const std::string& cache_name,
- std::unique_ptr<CacheStorageCacheHandle> cache_handle){};
+ 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){};
+ // Notification that the cache for |cache_handle| has been doomed. If the
+ // loader is holding a handle to the cache, it should drop it now.
+ virtual void NotifyCacheDoomed(
+ std::unique_ptr<CacheStorageCacheHandle> cache_handle) {};
protected:
scoped_refptr<base::SequencedTaskRunner> cache_task_runner_;
@@ -181,8 +181,7 @@ class CacheStorage::MemoryLoader : public CacheStorage::CacheLoader {
callback.Run(std::move(cache));
}
- void CleanUpDeletedCache(const std::string& cache_name) override {
- }
+ void CleanUpDeletedCache(CacheStorageCache* cache) override {}
void WriteIndex(const StringVector& cache_names,
const BoolCallback& callback) override {
@@ -201,9 +200,10 @@ class CacheStorage::MemoryLoader : public CacheStorage::CacheLoader {
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);
+ void NotifyCacheDoomed(
+ std::unique_ptr<CacheStorageCacheHandle> cache_handle) override {
+ DCHECK(ContainsKey(cache_handles_, cache_handle->value()->cache_name()));
+ cache_handles_.erase(cache_handle->value()->cache_name());
};
private:
@@ -284,13 +284,13 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
callback.Run(CreateCache(cache_name));
}
- void CleanUpDeletedCache(const std::string& cache_name) override {
+ void CleanUpDeletedCache(CacheStorageCache* cache) override {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
- DCHECK(ContainsKey(cache_name_to_cache_dir_, cache_name));
+ DCHECK(ContainsKey(doomed_cache_to_path_, cache));
base::FilePath cache_path =
- origin_path_.AppendASCII(cache_name_to_cache_dir_[cache_name]);
- cache_name_to_cache_dir_.erase(cache_name);
+ origin_path_.AppendASCII(doomed_cache_to_path_[cache]);
+ doomed_cache_to_path_.erase(cache);
cache_task_runner_->PostTask(
FROM_HERE, base::Bind(&SimpleCacheLoader::CleanUpDeleteCacheDirInPool,
@@ -390,6 +390,16 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
callback.Run(std::move(names));
}
+ void NotifyCacheDoomed(
+ std::unique_ptr<CacheStorageCacheHandle> cache_handle) override {
+ DCHECK(ContainsKey(cache_name_to_cache_dir_,
+ cache_handle->value()->cache_name()));
+ auto iter =
+ cache_name_to_cache_dir_.find(cache_handle->value()->cache_name());
+ doomed_cache_to_path_[cache_handle->value()] = iter->second;
+ cache_name_to_cache_dir_.erase(iter);
+ };
+
private:
friend class MigratedLegacyCacheDirectoryNameTest;
~SimpleCacheLoader() override {}
@@ -475,6 +485,7 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
const base::FilePath origin_path_;
std::map<std::string, std::string> cache_name_to_cache_dir_;
+ std::map<CacheStorageCache*, std::string> doomed_cache_to_path_;
base::WeakPtrFactory<SimpleCacheLoader> weak_ptr_factory_;
};
@@ -784,12 +795,17 @@ void CacheStorage::DeleteCacheDidWriteIndex(
return;
}
+ // Make sure that a cache handle exists for the doomed cache to ensure that
+ // DeleteCacheFinalize is called.
+ std::unique_ptr<CacheStorageCacheHandle> cache_handle =
+ GetLoadedCache(cache_name);
+
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);
+ cache_loader_->NotifyCacheDoomed(std::move(cache_handle));
callback.Run(true, CACHE_STORAGE_OK);
}
@@ -812,7 +828,7 @@ void CacheStorage::DeleteCacheDidGetSize(
storage::QuotaClient::kServiceWorkerCache, origin_,
storage::kStorageTypeTemporary, -1 * cache_size);
- cache_loader_->CleanUpDeletedCache(cache->cache_name());
+ cache_loader_->CleanUpDeletedCache(cache.get());
}
void CacheStorage::EnumerateCachesImpl(
« no previous file with comments | « no previous file | content/browser/cache_storage/cache_storage_manager_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698