Summary: | Add support for application cache dynamic entries | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||||||
Component: | WebCore Misc. | Assignee: | Alexey Proskuryakov <ap> | ||||||||||
Status: | RESOLVED INVALID | ||||||||||||
Severity: | Enhancement | CC: | darin, ian, michaeln, pknight | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2009-01-07 10:46:31 PST
Created attachment 26496 [details]
Part1 - implement DOMStringList
Comment on attachment 26496 [details] Part1 - implement DOMStringList > + static PassRefPtr<StaticStringList> create(const Vector<String>& strings) > + { > + return adoptRef(new StaticStringList(strings)); > + } It'd be more efficient to make this adopt a vector rather than copying a vector. You'd then probably call it adopt rather than create. And you could implement it by doing a swap and then a clear. Although that's not the perfect name, I think it's pretty good. > Index: WebCore/loader/appcache/ApplicationCacheGroup.cpp > =================================================================== > --- WebCore/loader/appcache/ApplicationCacheGroup.cpp (revision 39671) > +++ WebCore/loader/appcache/ApplicationCacheGroup.cpp (working copy) Should revert the change to this file. r=me Committed revision 39699. (In reply to comment #2) > It'd be more efficient to make this adopt a vector rather than copying a > vector. You'd then probably call it adopt rather than create. And you could > implement it by doing a swap and then a clear. I added an adopt() method, and used it in DOMApplicationCache. I didn't understand why use clear() though. Comment on attachment 26496 [details]
Part1 - implement DOMStringList
Clearing review flag from a landed patch.
(In reply to comment #3) > > It'd be more efficient to make this adopt a vector rather than copying a > > vector. You'd then probably call it adopt rather than create. And you could > > implement it by doing a swap and then a clear. > > I added an adopt() method, and used it in DOMApplicationCache. I didn't > understand why use clear() though. My mistake. The clear function is for cases when we already have a vector and want to adopt a replacement. It's better to clear after swapping than to export the old values in the vector. Not at all relevant to this patch. After further investigation and some discussion with Hixie, I believe that the spec is not in a state that we should implement, and it's not yet clear how to fix it. 1) Dynamic entry lifetime is unclear. The UA can silently drop dynamic entries when updating the cache, so an application cannot rely on the resources being present. 2) The model for propagation of resources added to an old cache in not mature yet. I.e., if window.applicationCache is associated with a cache that isn't newest, then add/remove calls will only manipulate that cache, not its newer versions. There is currently a requirement that changes the behavior a bit when cache update is in progress (adds/removes should be stored and replayed when update is finished), but that's not general enough. 3) The API is too easy to misuse - since online whitelist can not be manipulated dynamically, clients would add dynamic entries for external resources they want to access, easily blowing up the cache. 4) WebKit doesn't have quotas for application cache yet, and I think that it's a pre-requisite for letting JS code add resources dynamically (the latter is not an issue with the spec, of course). More generally, I think that while appcache holds application code and other resources, dynamic entries are really for application data, and it's quite questionable that they should be mixed up like that. Existing local storage APIs looks like they may be better suited for application data storage (if we design a way for loader to access these directly). Created attachment 26601 [details]
Work in progress
Some code I had in my tree.
Created attachment 26602 [details]
Work in progress
Also added tests.
Created attachment 26603 [details]
Disable existing code
Disable existing stub code, to make feature checks easier. I wouldn't go as far as removing all existing code as long as the feature is in HTML5.
Comment on attachment 26603 [details]
Disable existing code
r=me
Comment on attachment 26603 [details]
Disable existing code
Committed revision 39816.
> More generally, I think that while appcache holds application code and other
> resources, dynamic entries are really for application data, and it's quite
> questionable that they should be mixed up like that. Existing local storage
> APIs looks like they may be better suited for application data storage (if we
> design a way for loader to access these directly).
+1 this line of thinking
I can tell you from working with Gears that applications will not be happy with a rigid cache for the http resources that fall into that "application data" category.
For example, gmail message attachments are referenced via urls that contain two ids (messageID + attachmentID). But each unique draft of a message has a unique messageID. To carry attachments forward is a PITA with a fixed url scheme. Additionally, if messages with attachments are to be composed 'offline', these resources have to go somewhere at that time.
Additionally, there is more than one way (and url) to access each attachment, 'download it' vs 'view it'. In many cases the only difference in the response is the addition or absence of a Content-Disposition header. In other cases, an HTML page is synthesized to dress up a 'viewed' image (for example).
My dream on this dimension is allow application script to execute in response to a resource load which would generate/compose a response (based on the contents of local storage among other things).
The 'dynamic entries' stuff was sorely lacking.
Since this is no longer in spec, and is unlikely to come back in original form, marking as INVALID. |