Disk cache: Interface for the sparse cache support....

Disk cache: Interface for the sparse cache support.

This is just the interface declaration plus a mocked
implementation of the interface.

BUG=12258
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=17669

[wtc, 2009-06-04 01:09:07]

LGTM.  Nice documentation of the API.

I have a little trouble understanding the return value of
GetAvailableRange without an example, but it's OK.

http://codereview.chromium.org/119072/diff/1/3
File net/base/net_error_list.h (right):

http://codereview.chromium.org/119072/diff/1/3#newcode254
Line 254: NET_ERROR(CACHE_INVALID_REQUEST, -403)
Nit: should we name this error code CACHE_OPERATION_NOT_SUPPORTED?

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

http://codereview.chromium.org/119072/diff/1/6#newcode160
Line 160: // track of which parts of the entry have been written before, and
returns 0
What does the return value 0 mean here?

http://codereview.chromium.org/119072/diff/1/6#newcode175
Line 175: // or with lengths not aligned with 1 KB may result in the first or
last part
Nit: lengths don't need to be aligned.  How about
    lengths not a multiple of 1 KB
?

http://codereview.chromium.org/119072/diff/1/6#newcode205
Line 205: virtual int GetAvailableRange(int64 offset, int len, int64* start) =
0;
Please document that this return an error code on failure.

http://codereview.chromium.org/119072/diff/1/2
File net/http/http_cache_unittest.cc (right):

http://codereview.chromium.org/119072/diff/1/2#newcode173
Line 173: return 0;
Should we save some reasonable value in *start before
returning 0?

http://codereview.chromium.org/119072/diff/1/2#newcode181
Line 181: *start = real_offset;
How do we convince ourselves that *start is always set?

[rvargas (doing something else), 2009-06-04 01:49:34]

Thanks, I think I addressed all your comments.

The purpose of GetAvailableRange is to tell the http cache which part is not
stored on the cache (so it has to be fetched from the net), and what size to use
for a read when the data is stored. The size of the read is not that important
because we just make a second disk request for more data, but the size of the
initial hole has to be used to ask that range to the server.

On 2009/06/04 01:09:07, wtc wrote:
> LGTM.  Nice documentation of the API.
> 
> I have a little trouble understanding the return value of
> GetAvailableRange without an example, but it's OK.
> 
> http://codereview.chromium.org/119072/diff/1/3
> File net/base/net_error_list.h (right):
> 
> http://codereview.chromium.org/119072/diff/1/3#newcode254
> Line 254: NET_ERROR(CACHE_INVALID_REQUEST, -403)
> Nit: should we name this error code CACHE_OPERATION_NOT_SUPPORTED?
> 
> http://codereview.chromium.org/119072/diff/1/6
> File net/disk_cache/disk_cache.h (right):
> 
> http://codereview.chromium.org/119072/diff/1/6#newcode160
> Line 160: // track of which parts of the entry have been written before, and
> returns 0
> What does the return value 0 mean here?
> 
> http://codereview.chromium.org/119072/diff/1/6#newcode175
> Line 175: // or with lengths not aligned with 1 KB may result in the first or
> last part
> Nit: lengths don't need to be aligned.  How about
>     lengths not a multiple of 1 KB
> ?
> 
> http://codereview.chromium.org/119072/diff/1/6#newcode205
> Line 205: virtual int GetAvailableRange(int64 offset, int len, int64* start) =
> 0;
> Please document that this return an error code on failure.
> 
> http://codereview.chromium.org/119072/diff/1/2
> File net/http/http_cache_unittest.cc (right):
> 
> http://codereview.chromium.org/119072/diff/1/2#newcode173
> Line 173: return 0;
> Should we save some reasonable value in *start before
> returning 0?
> 
> http://codereview.chromium.org/119072/diff/1/2#newcode181
> Line 181: *start = real_offset;
> How do we convince ourselves that *start is always set?

[wtc, 2009-06-04 02:01:04]

LGTM.

http://codereview.chromium.org/119072/diff/14/16
File net/base/net_error_list.h (right):

http://codereview.chromium.org/119072/diff/14/16#newcode254
Line 254: NET_ERROR(CACHE_OPERATION_NOT_SUPPORTED, -403)
You were so fast :-)  I was going to suggest that you could
also use a more specific error code name such as
CACHE_NOT_SPARSE_ENTRY.

[rvargas (doing something else), 2009-06-04 02:07:14]

On 2009/06/04 02:01:04, wtc wrote:
> LGTM.
> 
> http://codereview.chromium.org/119072/diff/14/16
> File net/base/net_error_list.h (right):
> 
> http://codereview.chromium.org/119072/diff/14/16#newcode254
> Line 254: NET_ERROR(CACHE_OPERATION_NOT_SUPPORTED, -403)
> You were so fast :-)  I was going to suggest that you could
> also use a more specific error code name such as
> CACHE_NOT_SPARSE_ENTRY.

Thanks. I'll probably end up using the same error code to fail a regular
operation after the entry has been set to be sparse.