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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years ago by rvargas (slow to respond)
Modified:
3 years, 9 months ago
Reviewers:
michaeln, eroman
CC:
chromium-reviews, 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) Patch
M chrome/browser/net/chrome_url_request_context.cc View 1 2 3 4 3 chunks +3 lines, -3 lines 0 comments Download
M net/disk_cache/backend_impl.cc View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M net/disk_cache/disk_cache.h View 1 2 3 4 3 chunks +11 lines, -8 lines 0 comments Download
M net/http/http_cache.h View 1 2 3 4 6 chunks +14 lines, -2 lines 0 comments Download
M net/http/http_cache.cc View 1 2 3 4 3 chunks +11 lines, -0 lines 0 comments Download
M webkit/appcache/appcache_disk_cache.cc View 1 2 3 4 1 chunk +3 lines, -1 line 0 comments Download
M webkit/tools/test_shell/test_shell_request_context.cc View 3 4 1 chunk +1 line, -1 line 0 comments Download
Commit: CQ not working?

Messages

Total messages: 11 (0 generated)
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, ...
5 years ago (2010-03-18 06:20:59 UTC) #1
michaeln
LGTM Maybe update the description to indicate that your just putting in the programming interface ...
5 years ago (2010-03-18 19:57:34 UTC) #2
michaeln
oh... i also had this question about the API that could be cleared up in ...
5 years ago (2010-03-18 19:58:21 UTC) #3
rvargas (slow to respond)
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: ...
5 years ago (2010-03-18 20:47:57 UTC) #4
michaeln
> I updated the comment to show that passing NULL is not really supported (except ...
5 years ago (2010-03-18 21:15:21 UTC) #5
michaeln
> Ok, thanx for the clarification. QQ: What if you were to pass in the ...
5 years ago (2010-03-18 21:38:17 UTC) #6
rvargas (slow to respond)
On 2010/03/18 21:38:17, michaeln wrote: > > Ok, thanx for the clarification. > > QQ: ...
5 years ago (2010-03-18 21:50:26 UTC) #7
eroman
I wander if it wouldn't be better to simplify the constructors of HttpCache, and always ...
5 years ago (2010-03-19 20:07:30 UTC) #8
rvargas (slow to respond)
Interesting idea. I think we can collapse the constructors so that they always pass the ...
5 years ago (2010-03-19 21:31:48 UTC) #9
eroman
As far as the issue initializing backend, won't the interface for backend need to be ...
5 years ago (2010-03-19 23:48:32 UTC) #10
rvargas (slow to respond)
4 years, 11 months ago (2010-04-29 19:42:44 UTC) #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 ecdb341-tainted