Clear disk cache when the cache is not initialized

Clear disk cache when the cache is not initialized

BUG=24765
TEST=unit test, clear browsing data and the cache, media cache will
     be cleared even if a media object was not loaded.

Since the disk cache backend in HttpCache is lazily initialized, clearing
the cache before it receives the first transaction would have no effect.
So initialize the disk cache explicitly when we clear the cache.

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

[rvargas (doing something else), 2009-11-06 21:12:51]

Thanks for doing this.

http://codereview.chromium.org/378015/diff/1/3
File net/http/http_cache.cc (right):

http://codereview.chromium.org/378015/diff/1/3#newcode1730
Line 1730: // was an in-memory cache.
I would keep these comments in the code above, and replace this block with a
call to GetBackend() (if there is no disk_cache_)

http://codereview.chromium.org/378015/diff/1/4
File net/http/http_cache.h (right):

http://codereview.chromium.org/378015/diff/1/4#newcode97
Line 97: // Return the cache backend for this HttpCache instance. If the backend
nit: Returns

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

http://codereview.chromium.org/378015/diff/1/5#newcode803
Line 803: net::HttpCache cache(network_session, path, 0);
Impressive test.

I think a simpler test would be to create a MockHttpCache(NULL),
set_type(MEMORY_CACHE) and verify that the disk_cache() is there after
GetBackend().

You can keep your test if you prefer it, but it introduces a lot of dependencies
to the test.

[Alpha Left Google, 2009-11-06 22:27:05]

http://codereview.chromium.org/378015/diff/1/3
File net/http/http_cache.cc (right):

http://codereview.chromium.org/378015/diff/1/3#newcode1730
Line 1730: // was an in-memory cache.
On 2009/11/06 21:12:51, rvargas wrote:
> I would keep these comments in the code above, and replace this block with a
> call to GetBackend() (if there is no disk_cache_)

Done.

http://codereview.chromium.org/378015/diff/1/4
File net/http/http_cache.h (right):

http://codereview.chromium.org/378015/diff/1/4#newcode97
Line 97: // Return the cache backend for this HttpCache instance. If the backend
On 2009/11/06 21:12:51, rvargas wrote:
> nit: Returns

Done.

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

http://codereview.chromium.org/378015/diff/1/5#newcode803
Line 803: net::HttpCache cache(network_session, path, 0);
I'd prefer keeping the test, so we can assert the state of before and after
calling cache.GetBackend().

[rvargas (doing something else), 2009-11-07 00:17:18]

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

http://codereview.chromium.org/378015/diff/1/5#newcode803
Line 803: net::HttpCache cache(network_session, path, 0);
On 2009/11/06 22:27:05, Alpha wrote:
> I'd prefer keeping the test, so we can assert the state of before and after
> calling cache.GetBackend().
> 

Then how about modifying the HttpCache test constructor to include a path and
adding another constructor to MockHttpCache that sets that path to something non
empty? I would really prefer not adding so many dependencies to this file just
for this test.

Or maybe passing NULL everywhere works? we just need to test one method, not
that the cache works. (and using the second version of the constructor).

[Alpha Left Google, 2009-11-07 01:10:25]

I also removed in_memory_ from HttpCache because it seems redundant.

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

http://codereview.chromium.org/378015/diff/1/5#newcode803
Line 803: net::HttpCache cache(network_session, path, 0);
On 2009/11/07 00:17:18, rvargas wrote:
> On 2009/11/06 22:27:05, Alpha wrote:
> > I'd prefer keeping the test, so we can assert the state of before and after
> > calling cache.GetBackend().
> > 
> 
> Then how about modifying the HttpCache test constructor to include a path and
> adding another constructor to MockHttpCache that sets that path to something
non
> empty? I would really prefer not adding so many dependencies to this file just
> for this test.
> 
> Or maybe passing NULL everywhere works? we just need to test one method, not
> that the cache works. (and using the second version of the constructor).

In this case I think your first suggestion works better. We'll start with a NULL
backend and GetBackend() will give us a concrete instance.

[rvargas (doing something else), 2009-11-07 01:38:12]

LGTM. Thanks again for doing this.