Chromium Code Reviews
Help | Chromium Project | Sign in
(45)

Issue 2739007: Disk cache: Update the disk cache tools and tests to use... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 8 months ago by rvargas
Modified:
3 years, 8 months ago
Reviewers:
Nicolas Sylvain
CC:
chromium-reviews, darin-cc_chromium.org, cbentzel+watch_chromium.org, pam+watch_chromium.org, PaweĊ‚ Hajdan Jr.
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 4

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+413 lines, -190 lines) Patch
M net/disk_cache/backend_impl.h View 1 3 chunks +8 lines, -6 lines 0 comments Download
M net/disk_cache/backend_impl.cc View 1 3 chunks +29 lines, -23 lines 0 comments Download
M net/disk_cache/backend_unittest.cc View 1 8 chunks +102 lines, -23 lines 0 comments Download
M net/disk_cache/disk_cache.h View 1 1 chunk +0 lines, -21 lines 0 comments Download
M net/disk_cache/disk_cache_perftest.cc View 1 4 chunks +18 lines, -8 lines 0 comments Download
M net/disk_cache/disk_cache_test_base.h View 1 3 chunks +6 lines, -2 lines 0 comments Download
M net/disk_cache/disk_cache_test_base.cc View 1 3 chunks +19 lines, -6 lines 0 comments Download
M net/disk_cache/disk_cache_test_util.cc View 1 3 chunks +4 lines, -2 lines 0 comments Download
M net/disk_cache/mem_backend_impl.h View 1 1 chunk +7 lines, -0 lines 0 comments Download
M net/disk_cache/mem_backend_impl.cc View 1 3 chunks +3 lines, -4 lines 0 comments Download
M net/disk_cache/stress_cache.cc View 1 4 chunks +24 lines, -10 lines 0 comments Download
M net/tools/crash_cache/crash_cache.cc View 1 10 chunks +75 lines, -39 lines 0 comments Download
M net/tools/dump_cache/cache_dumper.h View 1 5 chunks +19 lines, -14 lines 0 comments Download
M net/tools/dump_cache/cache_dumper.cc View 1 5 chunks +16 lines, -14 lines 0 comments Download
M net/tools/dump_cache/upgrade.cc View 1 14 chunks +83 lines, -18 lines 0 comments Download
Trybot results:
Commit:

Messages

Total messages: 5 (0 generated)
rvargas
4 years, 8 months ago (2010-06-11 21:59:45 UTC) #1
Nicolas Sylvain
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 ...
4 years, 8 months ago (2010-06-12 22:35:38 UTC) #2
rvargas
Thanks. The failures are caused by this CL depending on http://codereview.chromium.org/2776007 landing (I landed it ...
4 years, 8 months ago (2010-06-14 19:12:23 UTC) #3
Nicolas Sylvain
lgtm, you should submit another try job for it. Are the tools compiled by default? ...
4 years, 8 months ago (2010-06-14 20:10:18 UTC) #4
rvargas
4 years, 8 months ago (2010-06-14 21:41:40 UTC) #5
> 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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87e6a26