Disk Cache: Make Stats independent of the backend implementation.

Disk Cache: Make Stats independent of the backend implementation.

Stats is still dependent on block files, but now they don't call
back into BackendImpl

BUG=241277
TEST=none
R=gavinp@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201943

[gavinp, 2013-05-23 15:32:33]

lgtm, only nits which you can take or leave.

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/addr.h
File net/disk_cache/addr.h (right):

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/addr.h#newcode130
net/disk_cache/addr.h:130: static int RequiredBlocks(int size, FileType
file_type) {
This long list of inline functions seems not in keeping with our coding style.

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/stats.cc
File net/disk_cache/stats.cc (right):

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/stats.cc#newco...
net/disk_cache/stats.cc:213: return false;
Hmm. Odd that this is an error return; the only caller DCHECKs on the result.

Would it make more sense to just DCHECK here and return void?

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/stats.h
File net/disk_cache/stats.h (right):

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/stats.h#newcode57
net/disk_cache/stats.h:57: // Initializes this object with the provided |data|
comming from disk.
Spelling, but also I think the grammar is a bit awkward.

How about:

// Initialize |*this| with |data| from disk.

Perhaps with prose instead of |*this| ?

[rvargas (doing something else), 2013-05-23 19:39:42]

Thanks

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/addr.h
File net/disk_cache/addr.h (right):

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/addr.h#newcode130
net/disk_cache/addr.h:130: static int RequiredBlocks(int size, FileType
file_type) {
On 2013/05/23 15:32:33, gavinp wrote:
> This long list of inline functions seems not in keeping with our coding style.

They are all trivial functions and I want to hint the compiler that they should
be inlined.

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/stats.cc
File net/disk_cache/stats.cc (right):

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/stats.cc#newco...
net/disk_cache/stats.cc:213: return false;
On 2013/05/23 15:32:33, gavinp wrote:
> Hmm. Odd that this is an error return; the only caller DCHECKs on the result.
> 
> Would it make more sense to just DCHECK here and return void?

The caller doesn't know what sizeof(OnDiskStats) is, and there is no requirement
that the value returned by StorageSize() should be used here...

DCHECKing across function calls (in effect connecting StorageSize() and this
method) is possible, but I prefer to be more agnostic of the use pattern. DCHECK
here results in memory corruption on release... DCHECK on the caller does not.

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/stats.h
File net/disk_cache/stats.h (right):

https://codereview.chromium.org/15772003/diff/1/net/disk_cache/stats.h#newcode57
net/disk_cache/stats.h:57: // Initializes this object with the provided |data|
comming from disk.
On 2013/05/23 15:32:33, gavinp wrote:
> Spelling, but also I think the grammar is a bit awkward.
> 
> How about:
> 
> // Initialize |*this| with |data| from disk.
> 
> Perhaps with prose instead of |*this| ?

Done.

[rvargas (doing something else), 2013-05-24 00:47:26]

Committed patchset #1 manually as r201943 (presubmit successful).