[CacheStorage] Garbage collect unreferenced caches

content/browser/cache_storage/cache_storage.cc

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 aa0ffb33f4dc6169a5e6a19bc49968d1504f9ecd..99393b2cc1d258edd47227e38142d50a139327c6 100644
--- a/content/browser/cache_storage/cache_storage.cc
+++ b/content/browser/cache_storage/cache_storage.cc
@@ -4,6 +4,7 @@
 
 #include "content/browser/cache_storage/cache_storage.h"
 
+#include <set>
 #include <string>
 
 #include "base/barrier_closure.h"
@@ -327,6 +328,8 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
                             const std::string& serialized) {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+    scoped_ptr<std::set<std::string>> cache_dirs(new std::set<std::string>);
+
     CacheStorageIndex index;
     if (index.ParseFromString(serialized)) {
       for (int i = 0, max = index.cache_size(); i < max; ++i) {
@@ -334,9 +337,14 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
         DCHECK(cache.has_cache_dir());
         names->push_back(cache.name());
         cache_name_to_cache_dir_[cache.name()] = cache.cache_dir();
+        cache_dirs->insert(cache.cache_dir());
       }
     }
 
+    cache_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&DeleteUnreferencedCachesInPool, origin_path_,
+                              base::Passed(&cache_dirs)));
+
     // TODO(jkarlin): Delete caches that are in the directory and not returned

[jsbell, 2015/10/23 21:20:22]

Remove this TODO?

[jkarlin, 2015/10/23 23:08:45]

Thanks! The whole purpose of the CL was to remove that TODO. Done.

     // in LoadIndex.
     callback.Run(names.Pass());
@@ -346,6 +354,20 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
   friend class MigratedLegacyCacheDirectoryNameTest;
   ~SimpleCacheLoader() override {}
 
+  // Iterates over the caches and deletes any directory not found in
+  // |cache_dirs|. Runs on cache_task_runner_
+  static void DeleteUnreferencedCachesInPool(
+      const base::FilePath& cache_base_dir,
+      scoped_ptr<std::set<std::string>> cache_dirs) {
+    base::FileEnumerator file_enum(cache_base_dir, false /* recursive */,

[jsbell, 2015/10/23 21:20:22]

I'm suddenly paranoid that the docs for FileEnumerator don't actually state that
it's not invalidated by changes to the directory, but we use this pattern
(FileEnumerator + DeleteFile) all over the code base, so ... I guess it must be
okay.

[jkarlin, 2015/10/23 23:08:45]

No longer deletes while iterating, just to be safe.

+                                   base::FileEnumerator::DIRECTORIES);
+    base::FilePath cache_path;
+    while (!(cache_path = file_enum.Next()).empty()) {
+      if (!ContainsKey(*cache_dirs, cache_path.BaseName().AsUTF8Unsafe()))
+        base::DeleteFile(cache_path, true /* recursive */);
+    }
+  }
+
   // Runs on cache_task_runner_
   static std::string MigrateCachesIfNecessaryInPool(
       const std::string& body,