Loading from the "most appropiate" appcache.

Plumbing to support loading from the "most appropriate" appcache.

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

[michaeln, 2011-03-17 21:54:46]

Hello, 

Please take a look at the associated bug to get an idea of what's being done
here. Jenn, I'm any insight you might have on the general approach i'm taking
for this would be great. Adam, it would be great if somebody else was somewhat
familiar with this part of things too and you seem to have an interest in the
resource loading machinery so i hope you don't mind reviewing too?

This CL doesn't change any behavior yet, just plumbing info about what host
spawned another into the backend. A later CL will utilize that info to
disambiguate between multiple main resource candidates.

[michaeln, 2011-03-17 21:58:51]

http://codereview.chromium.org/6667057/diff/6001/webkit/appcache/web_applicat...
File webkit/appcache/web_application_cache_host_impl.cc (right):

http://codereview.chromium.org/6667057/diff/6001/webkit/appcache/web_applicat...
webkit/appcache/web_application_cache_host_impl.cc:167: 
i'm gonna run some try jobs with a CHECK(frame) here to see if this ever gets
called w/o one

[adamk, 2011-03-17 23:33:29]

I'm happy to review.  It felt kinda weird that I wasn't reviewing anything!

Mostly nits follow.

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache/appc...
File content/common/appcache/appcache_backend_proxy.cc (right):

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache/appc...
content/common/appcache/appcache_backend_proxy.cc:20: host_id,
spawning_host_id));
nit: indentation off here.

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache_mess...
File content/common/appcache_messages.h (right):

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache_mess...
content/common/appcache_messages.h:46: // Informs the browser about which host
caused another to be created.
grammar nit: s/about/of/

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache_mess...
content/common/appcache_messages.h:47: // This can influence which appcache
should be utlized for the main
s/utlized/utilized/

http://codereview.chromium.org/6667057/diff/6006/webkit/appcache/appcache_bac...
File webkit/appcache/appcache_backend_impl.cc (right):

http://codereview.chromium.org/6667057/diff/6006/webkit/appcache/appcache_bac...
webkit/appcache/appcache_backend_impl.cc:62: // TODO(michaeln): write me
Can this be made any more informative about what is to be done?

http://codereview.chromium.org/6667057/diff/6006/webkit/appcache/web_applicat...
File webkit/appcache/web_application_cache_host_impl.h (right):

http://codereview.chromium.org/6667057/diff/6006/webkit/appcache/web_applicat...
webkit/appcache/web_application_cache_host_impl.h:48: const WebKit::WebFrame*);
Where's the WebKit patch that changes this signature?  I'm curious of where the
caller gets the frame.

[adamk, 2011-03-17 23:34:14]

http://codereview.chromium.org/6667057/diff/6006/webkit/appcache/web_applicat...
File webkit/appcache/web_application_cache_host_impl.h (right):

http://codereview.chromium.org/6667057/diff/6006/webkit/appcache/web_applicat...
webkit/appcache/web_application_cache_host_impl.h:48: const WebKit::WebFrame*);
On 2011/03/17 23:33:29, Adam Klein wrote:
> Where's the WebKit patch that changes this signature?  I'm curious of where
the
> caller gets the frame.

nm, found the link from the bug.

[michaeln, 2011-03-17 23:59:03]

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache/appc...
File content/common/appcache/appcache_backend_proxy.cc (right):

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache/appc...
content/common/appcache/appcache_backend_proxy.cc:20: host_id,
spawning_host_id));
On 2011/03/17 23:33:29, adamk wrote:
> nit: indentation off here.

i was making this consistent with how the indentation is in the methods below

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache_mess...
File content/common/appcache_messages.h (right):

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache_mess...
content/common/appcache_messages.h:46: // Informs the browser about which host
caused another to be created.
On 2011/03/17 23:33:29, adamk wrote:
> grammar nit: s/about/of/

Done.

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache_mess...
content/common/appcache_messages.h:47: // This can influence which appcache
should be utlized for the main
On 2011/03/17 23:33:29, adamk wrote:
> s/utlized/utilized/

Done.

http://codereview.chromium.org/6667057/diff/6006/webkit/appcache/appcache_bac...
File webkit/appcache/appcache_backend_impl.cc (right):

http://codereview.chromium.org/6667057/diff/6006/webkit/appcache/appcache_bac...
webkit/appcache/appcache_backend_impl.cc:62: // TODO(michaeln): write me
On 2011/03/17 23:33:29, adamk wrote:
> Can this be made any more informative about what is to be done?

would the crbug url help? i plan on following up almost immediately

[adamk, 2011-03-18 00:03:28]

LGTM

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache/appc...
File content/common/appcache/appcache_backend_proxy.cc (right):

http://codereview.chromium.org/6667057/diff/6006/content/common/appcache/appc...
content/common/appcache/appcache_backend_proxy.cc:20: host_id,
spawning_host_id));
On 2011/03/17 23:59:04, michaeln wrote:
> On 2011/03/17 23:33:29, adamk wrote:
> > nit: indentation off here.
> 
> i was making this consistent with how the indentation is in the methods below

Ah, ok, I'm not yet used to Rietveld and its folding.  Consistency seems right.

http://codereview.chromium.org/6667057/diff/6006/webkit/appcache/appcache_bac...
File webkit/appcache/appcache_backend_impl.cc (right):

http://codereview.chromium.org/6667057/diff/6006/webkit/appcache/appcache_bac...
webkit/appcache/appcache_backend_impl.cc:62: // TODO(michaeln): write me
On 2011/03/17 23:59:04, michaeln wrote:
> On 2011/03/17 23:33:29, adamk wrote:
> > Can this be made any more informative about what is to be done?
> 
> would the crbug url help? i plan on following up almost immediately

Yeah, the bug number seems sufficient.

[jennb, 2011-03-18 20:49:53]

LGTM

General approach looks fine. Good to see the TODO for choosing between multiple
candidates in the cache will finally get implemented.

Jenn