[CacheStorage] Garbage collect unreferenced caches

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 <set>
 #include <string>
 
 #include "base/barrier_closure.h"
 #include "base/files/file_util.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/guid.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/numerics/safe_conversions.h"
@@ skipping 303 matching lines @@
         base::Bind(&SimpleCacheLoader::LoadIndexDidReadFile,
                    weak_ptr_factory_.GetWeakPtr(), base::Passed(&names),
                    callback));
   }
 
   void LoadIndexDidReadFile(scoped_ptr<std::vector<std::string>> names,
                             const StringVectorCallback& callback,
                             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) {
         const CacheStorageIndex::Cache& cache = index.cache(i);
         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());
   }
 
  private:
   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,
       const base::FilePath& index_path) {
     CacheStorageIndex index;
     base::FilePath origin_path = index_path.DirName();
 
     if (!index.ParseFromString(body))
       return body;
 
@@ skipping 550 matching lines @@
     scoped_ptr<ServiceWorkerResponse> response,
     scoped_ptr<storage::BlobDataHandle> blob_data_handle) {
   base::WeakPtr<CacheStorage> cache_storage = weak_factory_.GetWeakPtr();
 
   callback.Run(error, response.Pass(), blob_data_handle.Pass());
   if (cache_storage)
     scheduler_->CompleteOperationAndRunNext();
 }
 
 }  // namespace content