Bug 55505 - ApplicationCache - doesn't pick the "most appropriate" cache when there are multiple candidates
Summary: ApplicationCache - doesn't pick the "most appropriate" cache when there are m...
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-01 14:06 PST by Michael Nordman
Modified: 2011-04-15 16:22 PDT (History)
4 users (show)

See Also:


Attachments
frameParam (3.48 KB, patch)
2011-03-14 18:42 PDT, Michael Nordman
no flags Details | Formatted Diff | Diff
frameParam (3.48 KB, patch)
2011-03-14 19:30 PDT, Michael Nordman
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Nordman 2011-03-01 14:06:05 PST
Please see http://code.google.com/p/chromium/issues/detail?id=68479 for some history.

-- some comments from the chromium bug ---

URLs?
http://dictionarymid.sourceforge.net/htmlApp/playground/EngLat_IDP1/
http://dictionarymid.sourceforge.net/htmlApp/playground/EngLat_IDP2/

Other browsers tested:
Add OK or FAIL after other browsers where you have tested this issue:
     Safari 5: FAIL
  Firefox 3.x: FAIL 
  Firefox 4  : OK
       IE 7/8: N/A [does not support Application Cache]

What steps will reproduce the problem?
1. Open http://dictionarymid.sourceforge.net/htmlApp/playground/EngLat_IDP1/
2. Wait for application cache to be loaded (total of 127 files; press "cache info" and wait for status 'IDLE')
3. Open http://dictionarymid.sourceforge.net/htmlApp/playground/EngLat_IDP2/

What is the expected result?
The URL from step 3. is correctly loaded, no error occurs.

What happens instead?
An error window pops up: "XMLHttpRequestSend Exception 101"


Both manifests contain the same URLs from the ../htmlApp directory

#
# HTML files for mini_hmi
# 
.
../htmlApp/mini_hmi/OptionWindow.html
#
# Files generated by GWT 
# 
../htmlApp/translationlayergwt/0DFD596AF51F4CC7A00293FBD296E92A.cache.html
../htmlApp/translationlayergwt/688A00B4C9356DCDA436BE1DA13D5CBA.cache.html
../htmlApp/translationlayergwt/720FBED404E45C61EFEA4E4AD451AB8F.cache.html
../htmlApp/translationlayergwt/8EC125F3977CC8A7FC592AE673DBFB17.cache.html
../htmlApp/translationlayergwt/clear.cache.gif
../htmlApp/translationlayergwt/hosted.html
../htmlApp/translationlayergwt/translationlayergwt.nocache.js

When one of these duplicate resources is loaded as a document into a frame, the system has to pick one of the two instances to load. The spec is squishy about which to pick (it isn't specified), but our impl clearly chooses the wrong one in this case. Both apps are loading 8EC125F3977CC8A7FC592AE673DBFB17.cache.html into a subframe. Since the parent frame's appcache contains the resource its pretty clear that the system should load the resource from the parent frames manifest... but its not doing that.

Similarly when mini_hmi/OptionWindow.html is opened in a seperate window, that should load the resource from the invoking frame's appcache (if present in that cache), but currently its not guaranteed to do that.

To fix this I'll think we'll need to inform the appcache about which document (ultimately the appcache in use by that document) caused the creation of the document being loaded. In the case of a subframe that's the parent document, and in the case of a pop out window it's the opener document. And in the case of starting a shared worker, the invoking document.

Ideally basic <a> tag navigations should also be sensitive to which document initiated the navigation.
Comment 1 Alexey Proskuryakov 2011-03-01 22:36:56 PST
Specifically, HTML5 says:

-----------------------------------
Multiple application caches in different application cache groups can contain the same resource, e.g. if the manifests all reference that resource. If the user agent is to select an application cache from a list of relevant application caches that contain a resource, the user agent must use the application cache that the user most likely wants to see the resource from, taking into account the following:

which application cache was most recently updated,
which application cache was being used to display the resource from which the user decided to look at the new resource, and
which application cache the user prefers.
-----------------------------------

Cross platform WebKit code makes no attempt to implement that yet, indeed.
Comment 2 Michael Nordman 2011-03-02 12:02:14 PST
> Cross platform WebKit code makes no attempt to implement that yet, indeed.

Neither does chrome, but this seems like a pretty big rough edge that i'm interested in smoothing out. Doing so may involve changes to some of the code that we do share...

"which application cache was being used to display the resource from which the user decided to look at the new resource"

... arranging for that input in particular when choosing which cached main resource to load.
Comment 3 Michael Nordman 2011-03-14 18:42:21 PDT
Created attachment 85753 [details]
frameParam

Some groundwork for fixing this in chromium. An additional webFrame parameter to a webkit api so chromium's impl has enough info to figure out the "most appropiate".
Comment 4 WebKit Review Bot 2011-03-14 18:44:58 PDT
Attachment 85753 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1

Source/WebKit/chromium/public/WebApplicationCacheHost.h:80:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Michael Nordman 2011-03-14 19:30:09 PDT
Created attachment 85762 [details]
frameParam

fix style'ism
Comment 6 Alexey Proskuryakov 2011-03-14 21:06:13 PDT
Comment on attachment 85762 [details]
frameParam

OK. I would suggest fixing cross-platform code behavior first if possible though. As this is an observable behavior, it's something we should maintain parity at, and working on code that has more parties interested in would guarantee fewer surprises than backporting from Chromium branch.
Comment 7 WebKit Commit Bot 2011-03-15 15:29:54 PDT
The commit-queue encountered the following flaky tests while processing attachment 85762 [details]:

transitions/interrupted-accelerated-transition.html bug 56242 (authors: simon.fraser@apple.com and tonyg@chromium.org)
The commit-queue is continuing to process your patch.
Comment 8 WebKit Commit Bot 2011-03-15 15:31:50 PDT
Comment on attachment 85762 [details]
frameParam

Clearing flags on attachment: 85762

Committed r81183: <http://trac.webkit.org/changeset/81183>
Comment 9 WebKit Commit Bot 2011-03-15 15:31:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Michael Nordman 2011-03-15 15:33:00 PDT
reopening, the initial patch was just some groundwork
Comment 11 WebKit Commit Bot 2011-03-15 15:36:55 PDT
The commit-queue encountered the following flaky tests while processing attachment 85762 [details]:

transitions/interrupted-accelerated-transition.html bug 56242 (authors: simon.fraser@apple.com and tonyg@chromium.org)
The commit-queue is continuing to process your patch.
Comment 12 Michael Nordman 2011-03-22 18:55:46 PDT
My plan for this in chrome is to modify the storage.FindResponseForMainRequest method to accept an additional input about which manifest_url the caller would like a bias towards, and to provide that input based on the manifest used by the opener|parent. Ties would go to the resource from the preferred manifest_url.

I've got a start at this here...
http://codereview.chromium.org/6727006/

A further refinement could be to have a bias towards the manifest used by the document currently residing in the frame into which a new document is being loaded, but for now i'm just starting with what the opener|parent has to say.

I have not looked at webcore's impl in any detail, but breaking ties in the same way should be possible there too.
Comment 13 Michael Nordman 2011-04-15 16:22:23 PDT
I've landed changes in the chromium along these lines, here's the CL description.

Select a more appropiate appcache based on the opener or the parent or the target frame of the new document. The change determines a 'preferred manifest url' in these cases:
* <iframe> loading, we prefer the parent frame's manifest url
* window.open() loading, we prefer the opener frame's manifest url
* href clicking to navigate a frame in place, we prefer the manifest url of the document in the frame when the navigation starts

More details can be found here.
http://code.google.com/p/chromium/issues/detail?id=68479