Disk cache: Add a hash to the entry's internal data.

EntryStore and RankingsNode now have a new member that
verifies that whatever we read from disk is what we
wrote before.

BUG=100125
TEST=none
Review URL: http://codereview.chromium.org/8658001

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

7 files changed, 37 insertions, 57 deletions

diff --git a/net/disk_cache/backend_impl.cc b/net/disk_cache/backend_impl.cc
index d482386..14a2722 100644
--- a/net/disk_cache/backend_impl.cc
+++ b/net/disk_cache/backend_impl.cc
@@ -819,8 +819,8 @@ EntryImpl* BackendImpl::CreateEntryImpl(const std::string& key) {
   open_entries_[entry_address.value()] = cache_entry;
 
   // Save the entry.
-  block_files_.GetFile(entry_address)->Store(cache_entry->entry());
-  block_files_.GetFile(node_address)->Store(cache_entry->rankings());
+  cache_entry->entry()->Store();
+  cache_entry->rankings()->Store();
   IncreaseNumEntries();
   entry_count_++;
 
@@ -1643,9 +1643,6 @@ int BackendImpl::NewEntry(Addr address, EntryImpl** entry) {
   if (!cache_entry->LoadNodeAddress())
     return ERR_READ_FAILURE;
 
-  // Prevent overwriting the dirty flag on the destructor.
-  cache_entry->SetDirtyFlag(GetCurrentEntryId());
-
   if (!rankings_.SanityCheck(cache_entry->rankings(), false)) {
     STRESS_NOTREACHED();
     cache_entry->SetDirtyFlag(0);
@@ -1665,6 +1662,9 @@ int BackendImpl::NewEntry(Addr address, EntryImpl** entry) {
     cache_entry->FixForDelete();
   }
 
+  // Prevent overwriting the dirty flag on the destructor.
+  cache_entry->SetDirtyFlag(GetCurrentEntryId());
+
   if (cache_entry->dirty()) {
     Trace("Dirty entry 0x%p 0x%x", reinterpret_cast<void*>(cache_entry.get()),
           address.value());
@@ -2219,8 +2219,7 @@ bool BackendImpl::CheckEntry(EntryImpl* cache_entry) {
     }
   }
 
-  RankingsNode* rankings = cache_entry->rankings()->Data();
-  return ok && !rankings->dummy;
+  return ok && cache_entry->rankings()->VerifyHash();
 }
 
 int BackendImpl::MaxBuffersSize() {
diff --git a/net/disk_cache/backend_unittest.cc b/net/disk_cache/backend_unittest.cc
index b102193..c19ca68 100644
--- a/net/disk_cache/backend_unittest.cc
+++ b/net/disk_cache/backend_unittest.cc
@@ -58,7 +58,6 @@ class DiskCacheBackendTest : public DiskCacheTestWithCache {
   void BackendInvalidEntry10(bool eviction);
   void BackendInvalidEntry11(bool eviction);
   void BackendTrimInvalidEntry12();
-  void BackendNotMarkedButDirty(const std::string& name);
   void BackendDoomAll();
   void BackendDoomAll2();
   void BackendInvalidRankings();
@@ -699,7 +698,7 @@ void DiskCacheBackendTest::BackendInvalidEntryWithLoad() {
 
   for (int i = 0; i < kNumEntries / 2; i++) {
     disk_cache::Entry* entry;
-    EXPECT_EQ(net::OK, OpenEntry(keys[i], &entry));
+    ASSERT_EQ(net::OK, OpenEntry(keys[i], &entry));
     entry->Close();
   }
 
@@ -1843,36 +1842,6 @@ TEST_F(DiskCacheBackendTest, NewEvictionTrimInvalidEntry12) {
   BackendTrimInvalidEntry12();
 }
 
-// We want to be able to deal with abnormal dirty entries.
-void DiskCacheBackendTest::BackendNotMarkedButDirty(const std::string& name) {
-  ASSERT_TRUE(CopyTestCache(name));
-  DisableFirstCleanup();
-  InitCache();
-
-  disk_cache::Entry *entry1, *entry2;
-  ASSERT_EQ(net::OK, OpenEntry("the first key", &entry1));
-  EXPECT_NE(net::OK, OpenEntry("some other key", &entry2));
-  entry1->Close();
-}
-
-TEST_F(DiskCacheBackendTest, NotMarkedButDirty) {
-  BackendNotMarkedButDirty("dirty_entry");
-}
-
-TEST_F(DiskCacheBackendTest, NewEvictionNotMarkedButDirty) {
-  SetNewEviction();
-  BackendNotMarkedButDirty("dirty_entry");
-}
-
-TEST_F(DiskCacheBackendTest, NotMarkedButDirty2) {
-  BackendNotMarkedButDirty("dirty_entry2");
-}
-
-TEST_F(DiskCacheBackendTest, NewEvictionNotMarkedButDirty2) {
-  SetNewEviction();
-  BackendNotMarkedButDirty("dirty_entry2");
-}
-
 // We want to be able to deal with messed up entries on disk.
 void DiskCacheBackendTest::BackendInvalidRankings2() {
   ASSERT_TRUE(CopyTestCache("bad_rankings"));
diff --git a/net/disk_cache/disk_format.h b/net/disk_cache/disk_format.h
index fd36d34..35b7030 100644
--- a/net/disk_cache/disk_format.h
+++ b/net/disk_cache/disk_format.h
@@ -123,7 +123,8 @@ struct EntryStore {
   int32       data_size[4];       // We can store up to 4 data streams for each
   CacheAddr   data_addr[4];       // entry.
   uint32      flags;              // Any combination of EntryFlags.
-  int32       pad[5];
+  int32       pad[4];
+  uint32      self_hash;          // The hash of EntryStore up to this point.
   char        key[256 - 24 * 4];  // null terminated
 };
 
@@ -153,7 +154,7 @@ struct RankingsNode {
   CacheAddr   prev;             // LRU list.
   CacheAddr   contents;         // Address of the EntryStore.
   int32       dirty;            // The entry is being modifyied.
-  int32       dummy;            // Old files may have a pointer here.
+  uint32      self_hash;        // RankingsNode's hash.
 };
 #pragma pack(pop)
 
diff --git a/net/disk_cache/entry_impl.cc b/net/disk_cache/entry_impl.cc
index 0a6a2d2..7e13029 100644
--- a/net/disk_cache/entry_impl.cc
+++ b/net/disk_cache/entry_impl.cc
@@ -544,11 +544,6 @@ bool EntryImpl::Update() {
 
 void EntryImpl::SetDirtyFlag(int32 current_id) {
   DCHECK(node_.HasData());
-  // We are checking if the entry is valid or not. If there is a pointer here,
-  // we should not be checking the entry.
-  if (node_.Data()->dummy)
-    dirty_ = true;
-
   if (node_.Data()->dirty && current_id != node_.Data()->dirty)
     dirty_ = true;
 
@@ -558,7 +553,6 @@ void EntryImpl::SetDirtyFlag(int32 current_id) {
 
 void EntryImpl::SetPointerForInvalidEntry(int32 new_id) {
   node_.Data()->dirty = new_id;
-  node_.Data()->dummy = 0;
   node_.Store();
 }
 
@@ -571,6 +565,9 @@ bool EntryImpl::LeaveRankingsBehind() {
 // Basically, even if there is something wrong with this entry, we want to see
 // if it is possible to load the rankings node and delete them together.
 bool EntryImpl::SanityCheck() {
+  if (!entry_.VerifyHash())
+    return false;
+
   EntryStore* stored = entry_.Data();
   if (!stored->rankings_node || stored->key_len <= 0)
     return false;
diff --git a/net/disk_cache/rankings.cc b/net/disk_cache/rankings.cc
index d478d92..4d936f3 100644
--- a/net/disk_cache/rankings.cc
+++ b/net/disk_cache/rankings.cc
@@ -494,6 +494,9 @@ int Rankings::SelfCheck() {
 }
 
 bool Rankings::SanityCheck(CacheRankingsBlock* node, bool from_list) const {
+  if (!node->VerifyHash())
+    return false;
+
   const RankingsNode* data = node->Data();
 
   if ((!data->next && data->prev) || (data->next && !data->prev))
@@ -572,17 +575,14 @@ bool Rankings::GetRanking(CacheRankingsBlock* rankings) {
 
   backend_->OnEvent(Stats::OPEN_RANKINGS);
 
-  // "dummy" is the old "pointer" value, so it has to be 0.
-  if (!rankings->Data()->dirty && !rankings->Data()->dummy)
+  if (!rankings->Data()->dirty)
     return true;
 
   EntryImpl* entry = backend_->GetOpenEntry(rankings);
   if (!entry) {
     // We cannot trust this entry, but we cannot initiate a cleanup from this
-    // point (we may be in the middle of a cleanup already). Just get rid of
-    // the invalid pointer and continue; the entry will be deleted when detected
-    // from a regular open/create path.
-    rankings->Data()->dummy = 0;
+    // point (we may be in the middle of a cleanup already). The entry will be
+    // deleted when detected from a regular open/create path.
     rankings->Data()->dirty = backend_->GetCurrentEntryId() - 1;
     if (!rankings->Data()->dirty)
       rankings->Data()->dirty--;
@@ -623,7 +623,6 @@ void Rankings::CompleteTransaction() {
   if (!node.Load())
     return;
 
-  node.Data()->dummy = 0;
   node.Store();
 
   Addr& my_head = heads_[control_data_->operation_list];
@@ -720,7 +719,7 @@ void Rankings::RevertRemove(CacheRankingsBlock* node) {
 }
 
 bool Rankings::CheckEntry(CacheRankingsBlock* rankings) {
-  if (!rankings->Data()->dummy)
+  if (rankings->VerifyHash())
     return true;
 
   // If this entry is not dirty, it is a serious problem.
diff --git a/net/disk_cache/storage_block-inl.h b/net/disk_cache/storage_block-inl.h
index e2f4b3c..614b143 100644
--- a/net/disk_cache/storage_block-inl.h
+++ b/net/disk_cache/storage_block-inl.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -9,6 +9,7 @@
 #include "net/disk_cache/storage_block.h"
 
 #include "base/logging.h"
+#include "net/disk_cache/hash.h"
 #include "net/disk_cache/trace.h"
 
 namespace disk_cache {
@@ -98,6 +99,11 @@ template<typename T> bool StorageBlock<T>::HasData() const {
   return (NULL != data_);
 }
 
+template<typename T> bool StorageBlock<T>::VerifyHash() const {
+  uint32 hash = CalculateHash();
+  return (!data_->self_hash || data_->self_hash == hash);
+}
+
 template<typename T> bool StorageBlock<T>::own_data() const {
   return own_data_;
 }
@@ -123,6 +129,7 @@ template<typename T> bool StorageBlock<T>::Load() {
 
 template<typename T> bool StorageBlock<T>::Store() {
   if (file_ && data_) {
+    data_->self_hash = CalculateHash();
     if (file_->Store(this)) {
       modified_ = false;
       return true;
@@ -156,6 +163,10 @@ template<typename T> void StorageBlock<T>::DeleteData() {
   }
 }
 
+template<typename T> uint32 StorageBlock<T>::CalculateHash() const {
+  return Hash(reinterpret_cast<char*>(data_), offsetof(T, self_hash));
+}
+
 }  // namespace disk_cache
 
 #endif  // NET_DISK_CACHE_STORAGE_BLOCK_INL_H_
diff --git a/net/disk_cache/storage_block.h b/net/disk_cache/storage_block.h
index 49694c6..2cf3c3a 100644
--- a/net/disk_cache/storage_block.h
+++ b/net/disk_cache/storage_block.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -61,6 +61,9 @@ class StorageBlock : public FileBlock {
   // Returns true if there is data associated with this object.
   bool HasData() const;
 
+  // Returns true if the internal hash is correct.
+  bool VerifyHash() const;
+
   // Returns true if this object owns the data buffer, false if it is shared.
   bool own_data() const;
 
@@ -73,6 +76,7 @@ class StorageBlock : public FileBlock {
  private:
   void AllocateData();
   void DeleteData();
+  uint32 CalculateHash() const;
 
   T* data_;
   MappedFile* file_;