[CacheStorage] Give cache directories unique names

content/browser/cache_storage/cache_storage_manager_unittest.cc

Index: content/browser/cache_storage/cache_storage_manager_unittest.cc
diff --git a/content/browser/cache_storage/cache_storage_manager_unittest.cc b/content/browser/cache_storage/cache_storage_manager_unittest.cc
index 2a5541fc2314e4cb342d1d9f369f77c74e44cd56..c753b72d09a900ca0f2172005f5cad41a25545e6 100644
--- a/content/browser/cache_storage/cache_storage_manager_unittest.cc
+++ b/content/browser/cache_storage/cache_storage_manager_unittest.cc
@@ -8,8 +8,11 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/run_loop.h"
+#include "base/sha1.h"
 #include "base/stl_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/thread_task_runner_handle.h"
+#include "content/browser/cache_storage/cache_storage.pb.h"
 #include "content/browser/cache_storage/cache_storage_quota_client.h"
 #include "content/browser/fileapi/chrome_blob_storage_context.h"
 #include "content/browser/quota/mock_quota_manager_proxy.h"
@@ -685,6 +688,91 @@ TEST_F(CacheStorageMigrationTest, MoveFailure) {
   EXPECT_EQ(expected_keys, callback_strings_);
 }
 
+// Tests that legacy caches created via a hash of the cache name instead of a
+// random name continue to function in a new world of random cache names.
+class LegacyCacheDirectoryNameTest : public CacheStorageManagerTest {
+ protected:
+  LegacyCacheDirectoryNameTest()
+      : legacy_cache_name_("foo"), stored_url_("http://example.com/foo") {}
+
+  void SetUp() override {
+    CacheStorageManagerTest::SetUp();
+
+    // Create a cache that is stored on disk with the legacy naming scheme (hash
+    // of the name) and without a directory name in the index.
+    base::FilePath origin_path = CacheStorageManager::ConstructOriginPath(
+        cache_manager_->root_path(), origin1_);
+
+    // Populate a cache.
+    ASSERT_TRUE(Open(origin1_, legacy_cache_name_));
+    EXPECT_TRUE(CachePut(callback_cache_, stored_url_));
+    base::FilePath new_path = callback_cache_->path();
+
+    // Close the cache.
+    callback_cache_ = nullptr;
+    base::RunLoop().RunUntilIdle();
+
+    // Legacy index files didn't have the cache directory, so remove it from the
+    // index.
+    base::FilePath index_path = origin_path.AppendASCII("index.txt");
+    std::string index_contents;
+    base::ReadFileToString(index_path, &index_contents);
+    CacheStorageIndex index;
+    ASSERT_TRUE(index.ParseFromString(index_contents));
+    ASSERT_EQ(1, index.cache_size());
+    index.mutable_cache(0)->release_cache_dir();
+    ASSERT_TRUE(index.SerializeToString(&index_contents));
+    index.ParseFromString(index_contents);
+    ASSERT_EQ(index_contents.size(),
+              (uint32_t)base::WriteFile(index_path, index_contents.c_str(),
+                                        index_contents.size()));
+
+    // Move the cache to the legacy location.
+    legacy_path_ = origin_path.AppendASCII(HexedHash(legacy_cache_name_));
+    ASSERT_FALSE(base::DirectoryExists(legacy_path_));
+    ASSERT_TRUE(base::Move(new_path, legacy_path_));
+
+    // Create a new manager to reread the index file.
+    quota_manager_proxy_->SimulateQuotaManagerDestroyed();
+    cache_manager_ = CacheStorageManager::Create(cache_manager_.get());
+  }
+
+  static std::string HexedHash(const std::string& value) {
+    std::string value_hash = base::SHA1HashString(value);
+    std::string valued_hexed_hash = base::ToLowerASCII(
+        base::HexEncode(value_hash.c_str(), value_hash.length()));
+    return valued_hexed_hash;
+  }
+
+  base::FilePath legacy_path_;
+  const std::string legacy_cache_name_;
+  const GURL stored_url_;
+
+  DISALLOW_COPY_AND_ASSIGN(LegacyCacheDirectoryNameTest);
+};
+
+TEST_F(LegacyCacheDirectoryNameTest, LegacyCacheWorks) {
+  EXPECT_TRUE(Open(origin1_, legacy_cache_name_));
+  EXPECT_TRUE(CacheMatch(callback_cache_, stored_url_));
+  EXPECT_TRUE(CachePut(callback_cache_, GURL("http://example.com/foo2")));
+  EXPECT_TRUE(CacheMatch(callback_cache_, GURL("http://example.com/foo2")));
+}
+
+TEST_F(LegacyCacheDirectoryNameTest, RandomDirectoryCacheSideBySideWithLegacy) {
+  EXPECT_TRUE(Open(origin1_, legacy_cache_name_));
+  EXPECT_TRUE(Open(origin1_, "bar"));
+  EXPECT_TRUE(CachePut(callback_cache_, stored_url_));
+  EXPECT_TRUE(CacheMatch(callback_cache_, stored_url_));
+}
+
+TEST_F(LegacyCacheDirectoryNameTest, DeleteLegacyCacheAndRecreateNew) {
+  EXPECT_TRUE(Delete(origin1_, legacy_cache_name_));

[jsbell, 2015/10/22 16:38:52]

This exercises the failure seen on Windows, right?

SetUp() implicitly opens the cache, held in callback_cache_
Delete() deletes it but callback_cache_ still keeps it alive
Open() will only overwrite callback_cache_ after it succeeds

Can you add a note or an EXPECT/ASSERT after the Delete() that callback_cache_
is still populated?

[jkarlin, 2015/10/23 17:23:09]

No, but I've since added the test StorageReuseCacheName which does.
I don't believe that callback_cache_ keeps it alive, as the simplecache doesn't
leave any handles open unless an entry is open. StorageReuseCacheName does
exactly that.
This doesn't seem necessary given the above.

+  EXPECT_TRUE(Open(origin1_, legacy_cache_name_));
+  EXPECT_FALSE(CacheMatch(callback_cache_, stored_url_));
+  EXPECT_TRUE(CachePut(callback_cache_, GURL("http://example.com/foo2")));
+  EXPECT_TRUE(CacheMatch(callback_cache_, GURL("http://example.com/foo2")));
+}
+
 class CacheStorageQuotaClientTest : public CacheStorageManagerTest {
  protected:
   CacheStorageQuotaClientTest() {}