Bug 49292

Summary: ApplicationCache - network namespace should take precedence over the fallback namespace.
Product: WebKit Reporter: Michael Nordman <michaeln>
Component: WebCore Misc.Assignee: Michael Nordman <michaeln>
Severity: Normal CC: abarth, ap, commit-queue, dmikurube, eric, fishd, ian, michaeln, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Description Flags
michaeln: commit-queue-
codeChange&layoutTest none

Description Michael Nordman 2010-11-09 17:35:01 PST

See 6.6.6 Changes to the networking model, step 3
So URLs that fall into an online whitelist should not be subject to fallback'ing.

There are also some comments about relative priorities in section Parsing cache manifests

"If a resource is listed in the explicit section or as a fallback entry in the fallback section, the resource will always be taken from the cache, regardless of any other matching entries in the fallback namespaces or online whitelist namespaces.

When a fallback namespace and an online whitelist namespace overlap, the online whitelist namespace has priority.

The online whitelist wildcard flag is applied last, only for URLs that match neither the online whitelist namespace nor the fallback namespace and that are not listed in the explicit section."

Although it looks like section 6.5.1 Navigating across documents, step 17 doesn't reflect those priorities. That omission feels like an oversight. For consistency i think the same priorities should be applied to main resource loads.
Comment 1 Michael Nordman 2010-11-09 17:50:52 PST
Chrome and Firefox 3.6 have the same bug so i suspect this was a (more) recent spec change.
Comment 2 Alexey Proskuryakov 2010-11-09 18:21:01 PST
Interesting. I guess we should find out if there was a good reason for spec change then.
Comment 3 Michael Nordman 2010-11-09 18:26:08 PST
(In reply to comment #2)
> Interesting. I guess we should find out if there was a good reason for spec change then.

The change seems like a good one. Developers are asking for this change.

One example... it allows a broad fallback namespace to be created with smaller holes in it for use with XHRs that should never be satisfied from the cache.
Comment 4 Alexey Proskuryakov 2010-11-09 19:19:04 PST
Yes, I agree with that. But can you find a public-html e-mail announcing the change?
Comment 5 Michael Nordman 2010-11-09 19:41:25 PST
(In reply to comment #4)
> Yes, I agree with that. But can you find a public-html e-mail announcing the change?

Ha... change history... good luck :)

There was a brief discussion of the relative priorities over a year ago.

I didn't see any notice of the change either.
Comment 7 Michael Nordman 2010-11-11 12:37:03 PST
> 6.5.1 Navigating across documents, step 17 doesn't reflect those priorities

Mail has been sent to the whatwg list about the discrepancy between main vs sub resource loading.
Comment 8 Michael Nordman 2010-11-11 13:13:01 PST
Created attachment 73644 [details]
Comment 9 Michael Nordman 2010-11-11 13:14:15 PST
(In reply to comment #8)
> Created an attachment (id=73644) [details]
> deflake

ooops... i attached this patch to the wrong bug :(
Comment 10 Michael Nordman 2010-11-17 15:44:47 PST
Created attachment 74167 [details]

This is just the layout test. The logic in webcore is not yet updated to handle this case so it shouldn't be committed just yet.  There's a chromium code change out for review that this test applies to... http://codereview.chromium.org/4807001/

Not sure of the best way to handle this patch with just the test? 

I guess we have a couple of options...

1. Wait until webcore implements this behavior too.
2. Put it in the appropriate skip lists until webcore implements this behavior.
Comment 11 Alexey Proskuryakov 2010-11-17 15:53:42 PST
>1. Wait until webcore implements this behavior too.

I believe it is a responsibility of whichever port decides to fork webkit.org sources to bring such improvements back to trunk. That's a precondition for accepting fork support code into webkit.org repository - and we have a massive amount of fork support code for application cache.
Comment 12 Alexey Proskuryakov 2010-11-17 15:55:22 PST
I've been alarmed by the word "wait", but I'm not actually sure if your intention is to work on both sides of the fix.
Comment 13 Alexey Proskuryakov 2010-11-17 15:58:50 PST
The test looks great, except for svn nits - please add newlines, and remove the executable bit.

\ No newline at end of file

Added: svn:executable
   + *
Comment 14 Michael Nordman 2010-11-17 16:07:32 PST
Created attachment 74173 [details]

Fixed the svn-props (why did cygwin's svn do that) and the line endings. Also fixed up a couple of references to file names to match the actual file names.

(In reply to comment #12)
> I've been alarmed by the word "wait", but I'm not actually sure if your intention is to work on both sides of the fix.

About who does the webcore fix, its probably either you or me. I'm out for the next couple of weeks, so the soonest i could get to it would be then.
Comment 15 Michael Nordman 2010-11-17 16:14:07 PST
Created attachment 74174 [details]

actually with the newline added this time
Comment 16 Alexey Proskuryakov 2010-11-17 16:24:57 PST
Comment on attachment 74174 [details]

Looks good. I don't think there's a good reason to land this before code fixes land.
Comment 17 Michael Nordman 2010-11-18 19:14:15 PST
fyi: chromium change is landed
Comment 18 Eric Seidel (no email) 2010-12-14 01:40:41 PST
I've certainly landed failing tests before the fix. :)  Makes the fix more dramatic!

Unclear what the current state of this bug is...
Comment 19 Alexey Proskuryakov 2010-12-14 12:03:09 PST
I'm assuming that Michael will fix WebCore side - please do tell if that's a wrong assumption.
Comment 20 Eric Seidel (no email) 2010-12-14 15:13:51 PST
Attachment 74174 [details] was posted by a committer and has review+, assigning to Michael Nordman for commit.
Comment 21 Michael Nordman 2011-02-28 17:57:25 PST
Created attachment 84165 [details]
Comment 22 WebKit Commit Bot 2011-03-01 13:57:36 PST
Comment on attachment 84165 [details]

Clearing flags on attachment: 84165

Committed r80036: <http://trac.webkit.org/changeset/80036>
Comment 23 WebKit Commit Bot 2011-03-01 13:57:42 PST
All reviewed patches have been landed.  Closing bug.
Comment 24 WebKit Review Bot 2011-03-01 14:08:54 PST
http://trac.webkit.org/changeset/80036 might have broken Leopard Intel Release (Build) and SnowLeopard Intel Release (Build)