Disk Cache: Don't depend on the backend being enabled to

be able to return the key of an open entry.

Whenever a critical corruption is detected by the disk cache,
the backend disables itself and starts failing all requests
until it's able to re-create the backing store.

Key's longer than 928 bytes are not stored inside the entry
itself, so a file object is required to access them. The backend
will reject any request for a file object after it is disabled,
so a user's request for the key of an open entry will also
fail.

Now we keep a pointer to the related file object (if needed) so
that we don't have to ask the backend for it when the user
requests the current key.

BUG=9952
TEST=unittest


Review URL: http://codereview.chromium.org/165030

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

4 files changed, 95 insertions, 21 deletions

diff --git a/net/disk_cache/backend_unittest.cc b/net/disk_cache/backend_unittest.cc
index 0747d04..ec9b61e 100644
--- a/net/disk_cache/backend_unittest.cc
+++ b/net/disk_cache/backend_unittest.cc
@@ -67,6 +67,7 @@ class DiskCacheBackendTest : public DiskCacheTestWithCache {
   void BackendDisable();
   void BackendDisable2();
   void BackendDisable3();
+  void BackendDisable4();
 };
 
 void DiskCacheBackendTest::BackendBasics() {
@@ -1324,6 +1325,71 @@ TEST_F(DiskCacheBackendTest, NewEvictionDisableSuccess3) {
   BackendDisable3();
 }
 
+// If we disable the cache, already open entries should work as far as possible.
+void DiskCacheBackendTest::BackendDisable4() {
+  disk_cache::Entry *entry1, *entry2, *entry3, *entry4;
+  void* iter = NULL;
+  ASSERT_TRUE(cache_->OpenNextEntry(&iter, &entry1));
+
+  char key2[2000];
+  char key3[20000];
+  CacheTestFillBuffer(key2, sizeof(key2), true);
+  CacheTestFillBuffer(key3, sizeof(key3), true);
+  key2[sizeof(key2) - 1] = '\0';
+  key3[sizeof(key3) - 1] = '\0';
+  ASSERT_TRUE(cache_->CreateEntry(key2, &entry2));
+  ASSERT_TRUE(cache_->CreateEntry(key3, &entry3));
+
+  const int kBufSize = 20000;
+  scoped_refptr<net::IOBuffer> buf = new net::IOBuffer(kBufSize);
+  memset(buf->data(), 0, kBufSize);
+  EXPECT_EQ(100, entry2->WriteData(0, 0, buf, 100, NULL, false));
+  EXPECT_EQ(kBufSize, entry3->WriteData(0, 0, buf, kBufSize, NULL, false));
+
+  // This line should disable the cache but not delete it.
+  EXPECT_FALSE(cache_->OpenNextEntry(&iter, &entry4));
+  EXPECT_EQ(4, cache_->GetEntryCount());
+
+  EXPECT_FALSE(cache_->CreateEntry("cache is disabled", &entry4));
+
+  EXPECT_EQ(100, entry2->ReadData(0, 0, buf, 100, NULL));
+  EXPECT_EQ(100, entry2->WriteData(0, 0, buf, 100, NULL, false));
+  EXPECT_EQ(100, entry2->WriteData(1, 0, buf, 100, NULL, false));
+
+  EXPECT_EQ(kBufSize, entry3->ReadData(0, 0, buf, kBufSize, NULL));
+  EXPECT_EQ(kBufSize, entry3->WriteData(0, 0, buf, kBufSize, NULL, false));
+  EXPECT_EQ(kBufSize, entry3->WriteData(1, 0, buf, kBufSize, NULL, false));
+
+  std::string key = entry2->GetKey();
+  EXPECT_EQ(sizeof(key2) - 1, key.size());
+  key = entry3->GetKey();
+  EXPECT_EQ(sizeof(key3) - 1, key.size());
+
+  entry1->Close();
+  entry2->Close();
+  entry3->Close();
+  MessageLoop::current()->RunAllPending();
+
+  EXPECT_EQ(0, cache_->GetEntryCount());
+}
+
+TEST_F(DiskCacheBackendTest, DisableSuccess4) {
+  ASSERT_TRUE(CopyTestCache(L"bad_rankings"));
+  DisableFirstCleanup();
+  SetDirectMode();
+  InitCache();
+  BackendDisable4();
+}
+
+TEST_F(DiskCacheBackendTest, NewEvictionDisableSuccess4) {
+  ASSERT_TRUE(CopyTestCache(L"bad_rankings"));
+  DisableFirstCleanup();
+  SetDirectMode();
+  SetNewEviction();
+  InitCache();
+  BackendDisable4();
+}
+
 TEST_F(DiskCacheTest, Backend_UsageStats) {
   MessageLoopHelper helper;
 
diff --git a/net/disk_cache/entry_impl.cc b/net/disk_cache/entry_impl.cc
index 3493152..0f25684 100644
--- a/net/disk_cache/entry_impl.cc
+++ b/net/disk_cache/entry_impl.cc
@@ -84,6 +84,7 @@ EntryImpl::EntryImpl(BackendImpl* backend, Addr address)
   for (int i = 0; i < kNumStreams; i++) {
     unreported_size_[i] = 0;
   }
+  key_file_ = NULL;
 }
 
 // When an entry is deleted from the cache, we clean up all the data associated
@@ -139,25 +140,29 @@ void EntryImpl::Close() {
 
 std::string EntryImpl::GetKey() const {
   CacheEntryBlock* entry = const_cast<CacheEntryBlock*>(&entry_);
-  if (entry->Data()->key_len > kMaxInternalKeyLength) {
-    Addr address(entry->Data()->long_key);
-    DCHECK(address.is_initialized());
-    COMPILE_ASSERT(kNumStreams == kKeyFileIndex, invalid_key_index);
-    File* file = const_cast<EntryImpl*>(this)->GetBackingFile(address,
-                                                              kKeyFileIndex);
+  if (entry->Data()->key_len <= kMaxInternalKeyLength)
+    return std::string(entry->Data()->key);
 
-    size_t offset = 0;
-    if (address.is_block_file())
-      offset = address.start_block() * address.BlockSize() + kBlockHeaderSize;
+  Addr address(entry->Data()->long_key);
+  DCHECK(address.is_initialized());
+  size_t offset = 0;
+  if (address.is_block_file())
+    offset = address.start_block() * address.BlockSize() + kBlockHeaderSize;
 
-    std::string key;
-    if (!file || !file->Read(WriteInto(&key, entry->Data()->key_len + 1),
-                             entry->Data()->key_len + 1, offset))
-      key.clear();
-    return key;
-  } else {
-    return std::string(entry->Data()->key);
+  if (!key_file_) {
+    // We keep a copy of the file needed to access the key so that we can
+    // always return this object's key, even if the backend is disabled.
+    COMPILE_ASSERT(kNumStreams == kKeyFileIndex, invalid_key_index);
+    key_file_ = const_cast<EntryImpl*>(this)->GetBackingFile(address,
+                                                             kKeyFileIndex);
   }
+
+  std::string key;
+  if (!key_file_ ||
+      !key_file_->Read(WriteInto(&key, entry->Data()->key_len + 1),
+                       entry->Data()->key_len + 1, offset))
+    key.clear();
+  return key;
 }
 
 Time EntryImpl::GetLastUsed() const {
@@ -402,19 +407,19 @@ bool EntryImpl::CreateEntry(Addr node_address, const std::string& key,
       return false;
 
     entry_store->long_key = address.value();
-    File* file = GetBackingFile(address, kKeyFileIndex);
+    key_file_ = GetBackingFile(address, kKeyFileIndex);
 
     size_t offset = 0;
     if (address.is_block_file())
       offset = address.start_block() * address.BlockSize() + kBlockHeaderSize;
 
-    if (!file || !file->Write(key.data(), key.size(), offset)) {
+    if (!key_file_ || !key_file_->Write(key.data(), key.size(), offset)) {
       DeleteData(address, kKeyFileIndex);
       return false;
     }
 
     if (address.is_separate_file())
-      file->SetLength(key.size() + 1);
+      key_file_->SetLength(key.size() + 1);
   } else {
     memcpy(entry_store->key, key.data(), key.size());
     entry_store->key[key.size()] = '\0';
diff --git a/net/disk_cache/entry_impl.h b/net/disk_cache/entry_impl.h
index da82396..4bb4d04 100644
--- a/net/disk_cache/entry_impl.h
+++ b/net/disk_cache/entry_impl.h
@@ -176,8 +176,10 @@ class EntryImpl : public Entry, public base::RefCounted<EntryImpl> {
   CacheRankingsBlock node_;   // Rankings related information for this entry.
   BackendImpl* backend_;      // Back pointer to the cache.
   scoped_array<char> user_buffers_[kNumStreams];  // Store user data.
-  scoped_refptr<File> files_[kNumStreams + 1];    // Files to store external
-                                                  // user data and key.
+  // Files to store external user data and key.
+  scoped_refptr<File> files_[kNumStreams + 1];
+  // Copy of the file used to store the key. We don't own this object.
+  mutable File* key_file_;
   int unreported_size_[kNumStreams];  // Bytes not reported yet to the backend.
   bool doomed_;               // True if this entry was removed from the cache.
   scoped_ptr<SparseControl> sparse_;  // Support for sparse entries.
diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index a0f77ef..df5a058 100644
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -809,6 +809,7 @@ void DiskCacheEntryTest::DoomedEntry() {
   EXPECT_EQ(2000, entry->WriteData(0, 0, buffer1, 2000, NULL, false));
   EXPECT_EQ(2000, entry->ReadData(0, 0, buffer2, 2000, NULL));
   EXPECT_EQ(0, memcmp(buffer1->data(), buffer2->data(), kSize1));
+  EXPECT_EQ(key, entry->GetKey());
   EXPECT_TRUE(initial < entry->GetLastModified());
   EXPECT_TRUE(initial < entry->GetLastUsed());