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

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
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112336

[gavinp, 2011-11-29 15:47:42]

Just a few questions.

http://codereview.chromium.org/8658001/diff/1/net/disk_cache/backend_impl.cc
File net/disk_cache/backend_impl.cc (right):

http://codereview.chromium.org/8658001/diff/1/net/disk_cache/backend_impl.cc#...
net/disk_cache/backend_impl.cc:823: cache_entry->rankings()->Store();
Nice.

http://codereview.chromium.org/8658001/diff/1/net/disk_cache/backend_impl.cc#...
net/disk_cache/backend_impl.cc:1666:
cache_entry->SetDirtyFlag(GetCurrentEntryId());
Why is this moving beyond the two sanity-check blocks?

http://codereview.chromium.org/8658001/diff/1/net/disk_cache/disk_format.h
File net/disk_cache/disk_format.h (right):

http://codereview.chromium.org/8658001/diff/1/net/disk_cache/disk_format.h#ne...
net/disk_cache/disk_format.h:127: uint32      self_hash;          // The hash of
EntryStore up to this point.
Why after the pad, and not before?  What do we expect the pad to usually be?  Is
it because if we later move values out of pad and into use, we'll end up with a
mid-structure self-hash, which would be too confusing?

[rvargas (doing something else), 2011-11-29 19:08:56]

Thanks

http://codereview.chromium.org/8658001/diff/1/net/disk_cache/backend_impl.cc
File net/disk_cache/backend_impl.cc (right):

http://codereview.chromium.org/8658001/diff/1/net/disk_cache/backend_impl.cc#...
net/disk_cache/backend_impl.cc:1666:
cache_entry->SetDirtyFlag(GetCurrentEntryId());
On 2011/11/29 15:47:42, gavinp wrote:
> Why is this moving beyond the two sanity-check blocks?

Because this is reading Rankings; we want the first access to read the hash and
verify it. On the other hand, all code paths end up calling SetDirtyFlag (even
if a future refactor decides to return after one of the sanity checks)

http://codereview.chromium.org/8658001/diff/1/net/disk_cache/disk_format.h
File net/disk_cache/disk_format.h (right):

http://codereview.chromium.org/8658001/diff/1/net/disk_cache/disk_format.h#ne...
net/disk_cache/disk_format.h:127: uint32      self_hash;          // The hash of
EntryStore up to this point.
On 2011/11/29 15:47:42, gavinp wrote:
> Why after the pad, and not before?  What do we expect the pad to usually be? 
Is
> it because if we later move values out of pad and into use, we'll end up with
a
> mid-structure self-hash, which would be too confusing?

Yes, that's it. Pad is set to zero, and the expected behavior is that we'll want
to verify that whatever we use it for is there. The only reason to not keep
going and hash the key is that we cannot have the hash in the middle of the
stuff that we are hashing (besides, there is already a hash of the key).

[gavinp, 2011-11-30 13:53:34]

LGTM!