975362 - Support HTTP cache v2 API in the UI

Status: RESOLVED FIXED

(SeaMonkey :: Page Info, defect)

Description

[[e-mail]]

The HTTP cache v2 provides a new API. Calls to the new API are transparently redirected to the old cache while it is still enabled, but also work with the new cache. Calls to the old API fail if the new cache is enabled.

Comment 1

[[e-mail]]

Attachment: Proposed patch

Sorry, I'm not going to bother fixing the tests.

Rather fortunately, one of the callbacks has the same name, and the callback info even has the same dataSize attribute, which is why there aren't as many changes as you might imagine.

I wanted to avoid using Services.cache2 in case it gets renamed in future.

Comment 2

[Frank Wein [:mcsmurf]]

For reference: Bug 913807 introduced the new cache API.

Comment 3

[Philip Chee]

(In reply to [e-mail] from comment #1)
> Sorry, I'm not going to bother fixing the tests.
Could you at least disable the relevant tests?

Comment 4

[[e-mail]]

The tests won't break until the HTTP cache v2 is turned on by default.

Comment 5

[Frank Wein [:mcsmurf]]

Neil: Should about:cache also use the new backend then? I'm wondering a bit as with browser.cache.use_new_backend set to 1 about:cache says:
"Disk cache device
Number of entries: 	10817
Maximum storage size: 	358400 KiB
Storage in use: 	340196 KiB

Offline cache device
Number of entries: 	0
Maximum storage size: 	512000 KiB
Storage in use: 	0 KiB"


And the cache pref pane says:
"Your cache is currently using 2,4 GB of disk space"

Does that number include more than what's listed on about:cache? Not sure if this "problem" is really related to the new cache backend though.

Comment 6

[[e-mail]]

(In reply to Frank Wein from comment #5)
> Neil: Should about:cache also use the new backend then?

That's bug 916052.

Comment 7

[Frank Wein [:mcsmurf]]

I'm a bit puzzled now, in Page Info it seems to say for all pictures (media) on a website "Not cached", with the old backend it said "Disk cache". I'll check with Firefox now what it says.

Comment 8

[Frank Wein [:mcsmurf]]

Ok, Firefox has no direct cache entry in Page Info, but if it's cached, it will display the file size in KB and that seems to work here. So I think there might be some bug with the patch, I used http://www.seamonkey-project.org/start/ to test this.

Comment 9

[Frank Wein [:mcsmurf]]

Attachment: Screenshot

Here you see what I mean: On the left side Firefox, on the right side SeaMonkey (both built locally and both with the new backend pref set to 1). On the left you can see Firefox knows about the file size of the image, on the right side nothing about the file size of the image is known.

Comment 10

[[e-mail]]

Attachment: Fixed patch

The old cache reads an uninitialised variable, and this happened to work for me. The new cache doesn't, so I have to explicitly return the correct value ;-)

Comment 11

[Frank Wein [:mcsmurf]]

Comment on attachment 8383602 [details] [diff] [review]
Fixed patch

Patch looks good and works fine. One thing I noticed though, I got this error when looking at the offline apps pref panel in my test profile:
Timestamp: 07.03.2014 12:04:03
Error: NS_ERROR_NOT_IMPLEMENTED: Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsICacheStorage.asyncVisitStorage]
Source File: chrome://communicator/content/pref/pref-offlineapps.js
Line: 62

Not sure why, does one need to explicitly enable offline apps storage with the new cache? With the old backend this error does not seem to happen.
Actually I found the answer ;) http://hg.mozilla.org/mozilla-central/annotate/b0e7f63c2138/netwerk/cache2/AppCacheStorage.cpp#l148 it's not implemented...yet?! So currently one cannot look at the app cache storage looks like.

Comment 12

[Philip Chee]

Comment on attachment 8383602 [details] [diff] [review]
Fixed patch

> +++ b/suite/browser/metadata.js
> +        Components.utils.import("resource://gre/modules/NetUtil.jsm");
We already import this module in makeHrefAbsolute(). How about moving the import to the top of the file?

> +++ b/suite/browser/pageinfo/pageInfo.js
> +const diskCacheStorage =
> +    Components.classes["@mozilla.org/netwerk/cache-storage-service;1"]
> +              .getService(Components.interfaces.nsICacheStorageService)
> +              .diskCacheStorage({ isPrivate: opener.gPrivate }, false);
Services.cache2.diskCacheStorage(...);

> +++ b/suite/common/pref/pref-cache.js
> +  Components.classes["@mozilla.org/netwerk/cache-storage-service;1"]
> +            .getService(Components.interfaces.nsICacheStorageService).clear();
Services.cache2.clear()

> -  Services.cache.visitEntries(visitor);
> +  Components.classes["@mozilla.org/netwerk/cache-storage-service;1"]
> +            .getService(Components.interfaces.nsICacheStorageService)
> +            .appCacheStorage({}, null).asyncVisitStorage(visitor, false);
Services.cache2.appCacheStorage(...)

> +++ b/suite/common/pref/pref-offlineapps.js
See pref-cache.js

> +++ b/suite/modules/Sanitizer.jsm
Services.jsm is imported so we can use Services.cache2

Comment 13

[[e-mail]]

Pushed comm-central changeset babfac82821d.

Comment 14

[[e-mail]]

Attachment: Accidental check-in

Unfortunately I accidentally landed these changes from bug 514280 along with this patch, so here's hoping I get post-facto review.

Comment 15

[[e-mail]]

(In reply to Frank Wein from comment #11)
> http://hg.mozilla.org/mozilla-central/annotate/b0e7f63c2138/netwerk/cache2/
> AppCacheStorage.cpp#l148 it's not implemented...yet?! So currently one
> cannot look at the app cache storage looks like.

Oops, I hadn't realised this. Do you want me to back out the offline app changes on 2.27 and later?

Comment 16

[[e-mail]]

(Bah, I didn't see where my focus had gone and accidentally submitted by mistake.)

(In reply to Frank Wein from comment #11)
> http://hg.mozilla.org/mozilla-central/annotate/b0e7f63c2138/netwerk/cache2/
> AppCacheStorage.cpp#l148 it's not implemented...yet?! So currently one
> cannot look at the app cache storage looks like.

Oops, I hadn't realised this. Do you want me to back out the offline app changes on 2.27 and later?

Comment 17

[Frank Wein [:mcsmurf]]

Looks like Bug 916052 fixed this for Gecko 32/SeaMonkey 2.29. So yes, back out the changes to pref-offlineapps.js on beta and aurora. Trunk is fine. And on beta and aurora we don't use the new backend anyway (by default).

Comment 18

[[e-mail]]

(In reply to Frank Wein from comment #17)
> back out the changes to pref-offlineapps.js on beta and aurora.

Sorry I don't think this happened, but it's too late now.