The patch in bug 442806 disables offline cache size selection in the UI. We need to fix that.

To be clear, we still show the size of globalStorage in that dialog, but the size of the cache items isn't included.

Marking as a P2 blocker since I don't think this needs to block beta 3

Comment on attachment 350941 [details] [diff] [review] v1 Are you sure of this line? "var groups = cacheService.getGroups({});". getGroups is defined as void with two out parameters. Shouldn't it be "var groups = {}; cacheService.getGroups(groups, {});" ?

Comment on attachment 350941 [details] [diff] [review] v1 >diff -r 8c541b023937 browser/base/content/browser.js > _checkUsage: function(aURI) { >+ var cacheService = Components.classes["@mozilla.org/network/application-cache-service;1"]. >+ getService(Components.interfaces.nsIApplicationCacheService); >+ var groups = cacheService.getGroups({}); >+ var usage = this._getOfflineAppUsage(aURI.asciiHost, groups); Wouldn't it make more sense to just let _getOfflineAppUsage get the groups, since it's already using nsIApplicationCacheService anyways? Same comment applies to the version in browser/components/preferences/advanced.js . It also doesn't appear to be possible to have multiple groups for a given host, so I'm not sure why the code tries to handle it. I'm not really familiar with the application cache stuff, but having to create nsIURIs for all groups just to get/clear the cache for a given host seems inefficient. Is there a reason we can't rather have an API more like: nsIApplicationCacheService.getUsage(in string group) nsIApplicationCacheService.discardCachesForGroup(in string group) and just call them with makeURI(host).asciiHost as the group? Canceling r? to get these questions answered, feel free to re-request if this is still what we want to go with.

(In reply to comment #4) > Are you sure of this line? "var groups = cacheService.getGroups({});". > getGroups is defined as void with two out parameters. Shouldn't it be "var > groups = {}; cacheService.getGroups(groups, {});" ? Arrays are handled differently. The second array outparam becomes the return value when called from JS, so the patch is correct. See e.g. http://weblogs.mozillazine.org/weirdal/XPCOM/XPCOM_CheatSheet.xml#pass-array

(In reply to comment #5) There can be more then just a single group (group = manifest URI) for a single host. For example: http://example.com/apps/app1.manifest and http://example.com/apps/app2.manifest. Those are two different groups on the same host. To select all groups belonging to a single host we have to parse each group URI to get the host from it (what the patch already does) or store the host to the database in a separate column. It would then have to be updated during every manifest update (which might be often) to safe makeURI during delete operation invoked by user ones a year. Just my opinion.

OK, so the "groups" you get from getGroups() are really just URI specs pointing to cache manifests? None of the interface docs really describe what a "group" is so that wasn't clear. I retract my naive API comments! First comment still applies though, looks OK otherwise.

Comment on attachment 350941 [details] [diff] [review] v1 For me this is OK.

Comment on attachment 350941 [details] [diff] [review] v1 Looks fine with the IIDs on those interfaces changed.

dcamp: any progress on getting this checked in? is it late-compat?

Yeah, late-compat. Gavin wanted a change to one of the methods, I'll put a new one up today.

Fixed the iid stuff, and made groups an optional argument to getOfflineAppUsage(). I prefer to keep passing groups in for the prefs dialog, because we call that once per host with the permission, and getGroups() hits the DB. I'll post an interdiff of the getOfflineAppUsage changes.

Comment on attachment 361408 [details] [diff] [review] v2 (In reply to comment #13) > I prefer to keep passing groups in for the prefs dialog, because we call that > once per host with the permission, and getGroups() hits the DB. Ah, makes sense - I hadn't noticed that caller was in a loop.

http://hg.mozilla.org/mozilla-central/rev/ad173cf77112 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d2d01a91f8e0