Disk cache: Update the disk cache tools and tests to use...

Disk cache: Update the disk cache tools and tests to use
the new interface (provide a cache thread and callback).

BUG=26730
TEST=unit tests



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

[Nicolas Sylvain, 2010-06-12 22:35:38]

c:\b\slave\win\build\src\net\http\http_cache.cc(47) : error C2039:
'CreateInMemoryCacheBackend' : is not a member of 'disk_cache'
c:\b\slave\win\build\src\net\http\http_cache.cc(47) : error C3861:
'CreateInMemoryCacheBackend': identifier not found
c:\b\slave\win\build\src\net\http\http_cache.cc(49) : error C2660:
'disk_cache::CreateCacheBackend' : function does not take 4 arguments

i cant tell you my nits because app engine is read only.


Nicolas



On Fri, Jun 11, 2010 at 2:59 PM, <rvargas@chromium.org> wrote:

> Reviewers: Nicolas Sylvain,
>
> Description:
> Disk cache: Update the disk cache tools and tests to use
> the new interface (provide a cache thread and callback).
>
> BUG=26730
> TEST=current tests
>
>
>
> Please review this at http://codereview.chromium.org/2739007/show
>
> SVN Base: svn://chrome-svn/chrome/trunk/src/
>
> Affected files:
>  M     net/disk_cache/backend_impl.h
>  M     net/disk_cache/backend_impl.cc
>  M     net/disk_cache/backend_unittest.cc
>  M     net/disk_cache/disk_cache.h
>  M     net/disk_cache/disk_cache_perftest.cc
>  M     net/disk_cache/disk_cache_test_base.h
>  M     net/disk_cache/disk_cache_test_base.cc
>  M     net/disk_cache/disk_cache_test_util.cc
>  M     net/disk_cache/mem_backend_impl.h
>  M     net/disk_cache/mem_backend_impl.cc
>  M     net/disk_cache/stress_cache.cc
>  M     net/tools/crash_cache/crash_cache.cc
>  M     net/tools/dump_cache/cache_dumper.h
>  M     net/tools/dump_cache/cache_dumper.cc
>  M     net/tools/dump_cache/upgrade.cc
>
>
>

[rvargas (doing something else), 2010-06-14 19:12:23]

Thanks. The failures are caused by this CL depending on
http://codereview.chromium.org/2776007 landing (I landed it again today).

On 2010/06/12 22:35:38, Nicolas Sylvain wrote:
> c:\b\slave\win\build\src\net\http\http_cache.cc(47) : error C2039:
> 'CreateInMemoryCacheBackend' : is not a member of 'disk_cache'
> c:\b\slave\win\build\src\net\http\http_cache.cc(47) : error C3861:
> 'CreateInMemoryCacheBackend': identifier not found
> c:\b\slave\win\build\src\net\http\http_cache.cc(49) : error C2660:
> 'disk_cache::CreateCacheBackend' : function does not take 4 arguments
> 
> i cant tell you my nits because app engine is read only.
> 
> 
> Nicolas
> 
> 
> 
> On Fri, Jun 11, 2010 at 2:59 PM, <mailto:rvargas@chromium.org> wrote:
> 
> > Reviewers: Nicolas Sylvain,
> >
> > Description:
> > Disk cache: Update the disk cache tools and tests to use
> > the new interface (provide a cache thread and callback).
> >
> > BUG=26730
> > TEST=current tests
> >
> >
> >
> > Please review this at http://codereview.chromium.org/2739007/show
> >
> > SVN Base: svn://chrome-svn/chrome/trunk/src/
> >
> > Affected files:
> >  M     net/disk_cache/backend_impl.h
> >  M     net/disk_cache/backend_impl.cc
> >  M     net/disk_cache/backend_unittest.cc
> >  M     net/disk_cache/disk_cache.h
> >  M     net/disk_cache/disk_cache_perftest.cc
> >  M     net/disk_cache/disk_cache_test_base.h
> >  M     net/disk_cache/disk_cache_test_base.cc
> >  M     net/disk_cache/disk_cache_test_util.cc
> >  M     net/disk_cache/mem_backend_impl.h
> >  M     net/disk_cache/mem_backend_impl.cc
> >  M     net/disk_cache/stress_cache.cc
> >  M     net/tools/crash_cache/crash_cache.cc
> >  M     net/tools/dump_cache/cache_dumper.h
> >  M     net/tools/dump_cache/cache_dumper.cc
> >  M     net/tools/dump_cache/upgrade.cc
> >
> >
> >
>

[Nicolas Sylvain, 2010-06-14 20:10:18]

lgtm, you should submit another try job for it.

Are the tools compiled by default?

http://codereview.chromium.org/2739007/diff/1/3
File net/disk_cache/backend_impl.h (right):

http://codereview.chromium.org/2739007/diff/1/3#newcode38
net/disk_cache/backend_impl.h:38: BackendImpl(const FilePath& path,
base::MessageLoopProxy* cache_thread)
I haven't looked at code like this in a long time, but for someone who knows
better, is it normal to have a "MessageLoopProxy" called "cache_thread".  Both
names seem to imply different things.

http://codereview.chromium.org/2739007/diff/1/4
File net/disk_cache/backend_unittest.cc (right):

http://codereview.chromium.org/2739007/diff/1/4#newcode223
net/disk_cache/backend_unittest.cc:223: disk_cache::Backend* cache;
you could = NULL and ASSERT_NE that it's not null after the call.

[rvargas (doing something else), 2010-06-14 21:41:40]

> lgtm, you should submit another try job for it.

Sure, I was waiting for the try bots to update :)

> Are the tools compiled by default?

yes.

http://codereview.chromium.org/2739007/diff/1/3
File net/disk_cache/backend_impl.h (right):

http://codereview.chromium.org/2739007/diff/1/3#newcode38
net/disk_cache/backend_impl.h:38: BackendImpl(const FilePath& path,
base::MessageLoopProxy* cache_thread)
On 2010/06/14 20:10:18, Nicolas Sylvain wrote:
> I haven't looked at code like this in a long time, but for someone who knows
> better, is it normal to have a "MessageLoopProxy" called "cache_thread".  Both
> names seem to imply different things.

Yeah, I know, looks weird. Pretty much all the code that passes this value
around call it cache_thread (It makes more sense to me, but it may have been
better to name it something else).

http://codereview.chromium.org/2739007/diff/1/4
File net/disk_cache/backend_unittest.cc (right):

http://codereview.chromium.org/2739007/diff/1/4#newcode223
net/disk_cache/backend_unittest.cc:223: disk_cache::Backend* cache;
On 2010/06/14 20:10:18, Nicolas Sylvain wrote:
> you could = NULL and ASSERT_NE that it's not null after the call.

Good point. I added a dedicated test for this interface.