Bug 23165 – Add support for application cache dynamic entries

• Component: WebCore Misc.
• Status: RESOLVED INVALID

[Alexey Proskuryakov]

We need to support dynamic application cache entries from HTML5.

[Alexey Proskuryakov]

Created attachment 26496 [details]
Part1 - implement DOMStringList

[Darin Adler]

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

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

Comment on attachment 26496 [details]
Part1 - implement DOMStringList

Clearing review flag from a landed patch.

[Darin Adler]

(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.

[Alexey Proskuryakov]

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).

[Alexey Proskuryakov]

<rdar://problem/5946450>

[Alexey Proskuryakov]

Created attachment 26601 [details]
Work in progress

Some code I had in my tree.

[Alexey Proskuryakov]

Created attachment 26602 [details]
Work in progress

Also added tests.

[Alexey Proskuryakov]

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.

[Darin Adler]

Comment on attachment 26603 [details]
Disable existing code

r=me

[Alexey Proskuryakov]

Comment on attachment 26603 [details]
Disable existing code

Committed revision 39816.

[Michael Nordman]

> 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.

[Alexey Proskuryakov]

Since this is no longer in spec, and is unlikely to come back in original form, marking as INVALID.