Fix http/tests/appcache/foreign-fallback.html

Fix for WKBug 47000: a failure mode given bad content (in an unlikely form). The error occurs when an html page is put in an appcache as a fallback resource (so it's listed in a fallback section), but it contains a manifest attribute that refers to a different manifest file. The system should mark the resource as foreign and exclude it from main resource loads, but it was failing to do so. The fix is to do that.

BUG=WK47000
TEST=http/tests/appcache/foreign-fallback.html



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

[kinuko, 2010-11-02 07:47:22]

LGTM

http://codereview.chromium.org/3529009/diff/68004/48006
File webkit/appcache/appcache_host_unittest.cc (right):

http://codereview.chromium.org/3529009/diff/68004/48006#newcode218
webkit/appcache/appcache_host_unittest.cc:218:
EXPECT_FALSE(host.is_selection_pending());
Do we need to test all of them in both ForeignEntry and ForeignFallbackEntry
cases?

http://codereview.chromium.org/3529009/diff/68004/48007
File webkit/appcache/appcache_request_handler.cc (right):

http://codereview.chromium.org/3529009/diff/68004/48007#newcode100
webkit/appcache/appcache_request_handler.cc:100: // 6.9.6, step 4: If this
results in a redirect to another origin,
(not a review comment) I thought it would be nice if an URL or some pointer to
the spec that has this '6.9.6' is given somewhere...

http://codereview.chromium.org/3529009/diff/68004/48008
File webkit/appcache/appcache_request_handler.h (right):

http://codereview.chromium.org/3529009/diff/68004/48008#newcode57
webkit/appcache/appcache_request_handler.h:57: const GURL& fallback_url);
nit: per recent addition to the coding style guide, it'd be nice to put each
argument on a separate line for function definitions (if you think changing the
format would fit in this cl)

http://www.chromium.org/developers/coding-style#Code_Formatting

http://codereview.chromium.org/3529009/diff/68004/48012
File webkit/appcache/appcache_storage_impl.cc (right):

http://codereview.chromium.org/3529009/diff/68004/48012#newcode641
webkit/appcache/appcache_storage_impl.cc:641: continue;
do we want to check this before looking up a group record? (just wondering)

[kinuko, 2010-11-03 00:25:18]

http://codereview.chromium.org/3529009/diff/68004/48006
File webkit/appcache/appcache_host_unittest.cc (right):

http://codereview.chromium.org/3529009/diff/68004/48006#newcode218
webkit/appcache/appcache_host_unittest.cc:218:
EXPECT_FALSE(host.is_selection_pending());
On 2010/11/02 20:25:36, michaeln wrote:
> On 2010/11/02 07:47:23, Kinuko Yasuda wrote:
> > Do we need to test all of them in both ForeignEntry and ForeignFallbackEntry
> > cases?
> 
> It does seem like some of these are redundant. How 'bout I remove the block of
> checks under the "response as if" comment?

sgtm!

http://codereview.chromium.org/3529009/diff/68004/48007
File webkit/appcache/appcache_request_handler.cc (right):

http://codereview.chromium.org/3529009/diff/68004/48007#newcode100
webkit/appcache/appcache_request_handler.cc:100: // 6.9.6, step 4: If this
results in a redirect to another origin,
On 2010/11/02 20:25:36, michaeln wrote:
> On 2010/11/02 07:47:23, Kinuko Yasuda wrote:
> > (not a review comment) I thought it would be nice if an URL or some pointer
to
> > the spec that has this '6.9.6' is given somewhere...
> 
> Comments that refer section number in the HTML5 draft are are pretty common in
a
> few of the .cc files in the /appcache directory. Most of the numbers are
> probably stale by now since the draft changes and the numbers change with
those
> changes.

That's true. I just wasn't able to find a relevant section whose number is
6.9.6.  Might be nice if section / subsection title was given together with the
numbers for important ones?  (Again, this is not a review comment.)