Unlink corrupt SimpleCache index files immediately after load.

The crash on startup in the referenced bug is very bad in part because
the corrupt index which caused it is left on disk, so all startups
until user data is cleared will crash. For maximum safety, we should
unlink the index file ASAP after finding it is corrupt.

R=rdsmith@chromium.org
BUG=251465

Review URL: https://codereview.chromium.org/17265007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@207813 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 63 insertions, 1 deletions

diff --git a/net/disk_cache/simple/simple_index_file.cc b/net/disk_cache/simple/simple_index_file.cc
index 529faf15..3193520 100644
--- a/net/disk_cache/simple/simple_index_file.cc
+++ b/net/disk_cache/simple/simple_index_file.cc
@@ -178,10 +178,18 @@ scoped_ptr<SimpleIndex::EntrySet> SimpleIndexFile::LoadFromDisk(
   std::string contents;
   if (!file_util::ReadFileToString(index_filename, &contents)) {
     LOG(WARNING) << "Could not read Simple Index file.";
+    file_util::Delete(index_filename, false);
     return scoped_ptr<SimpleIndex::EntrySet>();
   }
 
-  return SimpleIndexFile::Deserialize(contents.data(), contents.size());
+  scoped_ptr<SimpleIndex::EntrySet> entries =
+      SimpleIndexFile::Deserialize(contents.data(), contents.size());
+  if (!entries) {
+    file_util::Delete(index_filename, false);
+    return scoped_ptr<SimpleIndex::EntrySet>();
+  }
+
+  return entries.Pass();
 }
 
 // static
diff --git a/net/disk_cache/simple/simple_index_file.h b/net/disk_cache/simple/simple_index_file.h
index 44a5c6a..d8cccf4 100644
--- a/net/disk_cache/simple/simple_index_file.h
+++ b/net/disk_cache/simple/simple_index_file.h
@@ -91,6 +91,7 @@ class NET_EXPORT_PRIVATE SimpleIndexFile {
   FRIEND_TEST_ALL_PREFIXES(SimpleIndexFileTest, IsIndexFileCorrupt);
   FRIEND_TEST_ALL_PREFIXES(SimpleIndexFileTest, IsIndexFileStale);
   FRIEND_TEST_ALL_PREFIXES(SimpleIndexFileTest, Serialize);
+  FRIEND_TEST_ALL_PREFIXES(SimpleIndexFileTest, WriteThenLoadIndex);
 
   // Using the mtime of the file and its mtime, detects if the index file is
   // stale.
diff --git a/net/disk_cache/simple/simple_index_file_unittest.cc b/net/disk_cache/simple/simple_index_file_unittest.cc
index 76ecd917..caf3f40 100644
--- a/net/disk_cache/simple/simple_index_file_unittest.cc
+++ b/net/disk_cache/simple/simple_index_file_unittest.cc
@@ -149,6 +149,58 @@ TEST_F(SimpleIndexFileTest, IsIndexFileStale) {
   EXPECT_TRUE(SimpleIndexFile::IsIndexFileStale(index_path));
 }
 
+TEST_F(SimpleIndexFileTest, WriteThenLoadIndex) {
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  const base::FilePath index_path =
+      temp_dir.path().AppendASCII(SimpleIndexFile::kIndexFileName);
+  EXPECT_TRUE(SimpleIndexFile::IsIndexFileStale(index_path));
+
+  SimpleIndex::EntrySet entries;
+  static const uint64 kHashes[] = { 11, 22, 33 };
+  static const size_t kNumHashes = arraysize(kHashes);
+  EntryMetadata metadata_entries[kNumHashes];
+  for (size_t i = 0; i < kNumHashes; ++i) {
+    uint64 hash = kHashes[i];
+    metadata_entries[i] =
+        EntryMetadata(Time::FromInternalValue(hash), hash);
+    SimpleIndex::InsertInEntrySet(hash, metadata_entries[i], &entries);
+  }
+
+  const uint64 kCacheSize = 456U;
+  {
+    SimpleIndexFile simple_index_file(base::MessageLoopProxy::current(),
+                                      base::MessageLoopProxy::current(),
+                                      temp_dir.path());
+    simple_index_file.WriteToDisk(entries, kCacheSize,
+                                  base::TimeTicks(), false);
+    base::RunLoop().RunUntilIdle();
+    EXPECT_TRUE(file_util::PathExists(index_path));
+  }
+
+  SimpleIndexFile simple_index_file(base::MessageLoopProxy::current(),
+                                    base::MessageLoopProxy::current(),
+                                    temp_dir.path());
+  SimpleIndexFile::IndexCompletionCallback callback =
+      base::Bind(&SimpleIndexFileTest::IndexCompletionCallback,
+                 base::Unretained(this));
+  simple_index_file.LoadIndexEntries(base::MessageLoopProxy::current(),
+                                     callback);
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(file_util::PathExists(index_path));
+  ASSERT_TRUE(callback_result());
+  EXPECT_FALSE(callback_result()->force_index_flush);
+  const SimpleIndex::EntrySet* read_entries =
+      callback_result()->index_file_entries.get();
+  ASSERT_TRUE(read_entries);
+
+  EXPECT_EQ(kNumHashes, read_entries->size());
+  for (size_t i = 0; i < kNumHashes; ++i)
+    EXPECT_EQ(1U, read_entries->count(kHashes[i]));
+}
+
 TEST_F(SimpleIndexFileTest, IsIndexFileCorrupt) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
@@ -175,6 +227,7 @@ TEST_F(SimpleIndexFileTest, IsIndexFileCorrupt) {
                                      callback);
   base::RunLoop().RunUntilIdle();
 
+  EXPECT_FALSE(file_util::PathExists(index_path));
   ASSERT_TRUE(callback_result());
   EXPECT_TRUE(callback_result()->index_file_entries);
   EXPECT_TRUE(callback_result()->force_index_flush);