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

Issue 983007: Http cache: Add support for a dedicated cache thread. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 1 month ago by rvargas
Modified:
2 years, 10 months ago
Reviewers:
michaeln, eroman
CC:
chromium-reviews_chromium.org, darin-cc_chromium.org, ben+cc_chromium.org
Visibility:
Public.

Description

Http cache: Add support for a dedicated cache thread.

This is an interface-only change, nothing is really moving to another thread yet.

BUG=26730
TEST=none



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

Patch Set 1 #

Total comments: 1

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 2

Patch Set 4 : '' #

Total comments: 2

Patch Set 5 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -17 lines) Lint Patch
M chrome/browser/net/chrome_url_request_context.cc View 1 2 3 4 3 chunks +3 lines, -3 lines 0 comments 0 errors Download
M net/disk_cache/backend_impl.cc View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments 0 errors Download
M net/disk_cache/disk_cache.h View 1 2 3 4 3 chunks +11 lines, -8 lines 0 comments 0 errors Download
M net/http/http_cache.h View 1 2 3 4 6 chunks +14 lines, -2 lines 0 comments 0 errors Download
M net/http/http_cache.cc View 1 2 3 4 3 chunks +11 lines, -0 lines 0 comments 0 errors Download
M webkit/appcache/appcache_disk_cache.cc View 1 2 3 4 1 chunk +3 lines, -1 line 0 comments 0 errors Download
M webkit/tools/test_shell/test_shell_request_context.cc View 3 4 1 chunk +1 line, -1 line 0 comments 0 errors Download
Commit:

Messages

Total messages: 11
michaeln
i'll look closer tomorrow http://codereview.chromium.org/983007/diff/1/13 File webkit/appcache/appcache_disk_cache.cc (right): http://codereview.chromium.org/983007/diff/1/13#newcode108 webkit/appcache/appcache_disk_cache.cc:108: cache_type, cache_directory, cache_size, force, NULL, ...
4 years, 1 month ago #1
michaeln
LGTM Maybe update the description to indicate that your just putting in the programming interface ...
4 years, 1 month ago #2
michaeln
oh... i also had this question about the API that could be cleared up in ...
4 years, 1 month ago #3
rvargas
Michael, thanks for the review. Eric, your turn :) http://codereview.chromium.org/983007/diff/20001/21005 File net/disk_cache/disk_cache.h (right): http://codereview.chromium.org/983007/diff/20001/21005#newcode61 net/disk_cache/disk_cache.h:61: ...
4 years, 1 month ago #4
michaeln
> I updated the comment to show that passing NULL is not really supported (except ...
4 years, 1 month ago #5
michaeln
> Ok, thanx for the clarification. QQ: What if you were to pass in the ...
4 years, 1 month ago #6
rvargas
On 2010/03/18 21:38:17, michaeln wrote: > > Ok, thanx for the clarification. > > QQ: ...
4 years, 1 month ago #7
eroman
I wander if it wouldn't be better to simplify the constructors of HttpCache, and always ...
4 years, 1 month ago #8
rvargas
Interesting idea. I think we can collapse the constructors so that they always pass the ...
4 years, 1 month ago #9
eroman
As far as the issue initializing backend, won't the interface for backend need to be ...
4 years, 1 month ago #10
rvargas
3 years, 12 months ago #11
I finally checked in this cl.

To answer your last question, yes, the backend interface for creating the cache
is async, and all methods that could block are also async.

The flow goes like this:

1. ChromeURLRequestContext creates the http synchronously at startup.
2. With the first request, the http cache starts the creation of the backend
(when creating the HttpTransaction). It uses an internal completion callback.
3. All transactions that reach Start before the backend is created will be
queued by the http cache (seems safer than just bypassing the cache).
4. When the callback is notified, resume processing of queued TXs. Note that a
possible outcome of this step is that there is no backend at all (and the
session will proceed without a cache).

The downside is that anybody accessing directly the backend must implement a
callback to deal with the async creation. That's the case of the browser data
cleaner and the view-cache implementation.

Of course, this is yet to be implemented.

Thanks again.

On 2010/03/19 23:48:32, eroman wrote:
> As far as the issue initializing backend, won't the interface for backend need
> to be async, so that initialization could happen lazily during one of the
async
> calls?
> 
> Anyhow, I won't block this change from happening, just voicing my ideas.
> 
> LGTM.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6