767258 - offline-apps.allow_by_default prevents a webpage from using AppCache on Firefox

Status: RESOLVED WONTFIX

(Core :: Networking: Cache, defect)

Description

[(not reading bugmail) Nick Desaulniers [:\n]]

Creating the preference name 'offline-apps.allow_by_default' with a value of 'true' prevents the appCache prompt from appearing.  But then, the usual addition to:

Preferences -> Advanced -> The following websites are allowed to store data for offline use

does not contain the site in the list, so it cannot be cleared.

Comment 1

[Honza Bambas (:mayhemer)]

With what Gecko based application do you see this (Firefox, Fennec, B2G) ?

Comment 2

[Anant Narayanan [:anant]]

(In reply to Honza Bambas (:mayhemer) from comment #1)
> With what Gecko based application do you see this (Firefox, Fennec, B2G) ?

Firefox Desktop (Windows, Mac & Linux).

Comment 3

[Anant Narayanan [:anant]]

The prompt not appearing is expected behavior (this looks like the relevant code lines: https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#5762) - but this bug is about the cache not working at all if the pref is set to true.

Comment 4

[Martijn Wargers (dead)]

Anant, do you have the browser.tabs.remote pref set to true, by any chance?

This is called when a manifest is found (I found this while reading bug 892488, comment 9):
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentUtils.cpp#1413
When offline-apps.allow_by_default=true, then the offline-app permission is set to allow for that site (btw, it seems really strange to me, to add a permission when this pref is true).
This doesn't seem to work when oop is turned on, because the permission isn't set in the chrome process.

The prefetch is only happening when the offline-app permission is there:
http://mxr.mozilla.org/mozilla-central/source/uriloader/prefetch/nsOfflineCacheUpdateService.cpp#689

So in the case of oop turned on, you need to manually add the offline-app permission, still.
(I might be totally wrong in these, please correct me if I'm wrong)

Comment 5

[Martijn Wargers (dead)]

Attachment: 767258.diff

This seems to fix it for me.
This patch makes fixing bug 871323 at least simpler, because I don't really have to wonder anymore if the "offline-app" permission is set or not.

Comment 6

[Honza Bambas (:mayhemer)]

Comment on attachment 814239 [details] [diff] [review]
767258.diff

Review of attachment 814239 [details] [diff] [review]:
-----------------------------------------------------------------

No, no and no, absolutely no.

You must add the permission when we hit the manifest attribute.  Otherwise the page won't load from the offline cache even though offline cache has been downloaded.  And allowing offline-app perm for anything (by modifying OfflineAppPermForURI) will cause a huge performance issue for ALL page loads since we will be checking app cache for all domains.  You must not do that!

We have tests for offline cache, I was also carefully testing the allow-by-default preference.  It works well in single process model.

You either do something wrong, or I don't have all the information (like what you do and what you expect, what is your setting) to understand what the problem is.

Comment 7

[Martijn Wargers (dead)]

Ok, thanks for the explanation, I didn't know about the performance issue.

The problem is that adding the offline-app permission doesn't work inside nsContentUtils::MaybeAllowOfflineAppByDefault in the multi process model.
I tested this by adding this code to a test:
dump('*********'+SpecialPowers.testPermission("offline-app", Ci.nsIPermissionManager.ALLOW_ACTION, document)+'******\n\n\n');
This returns false in the multi process model, and true when run in single process.
The attached patch made the tests work, but I can fix this issue also from inside the tests, if needed.

Comment 8

[Martijn Wargers (dead)]

I'm wondering though, with the 'offline-apps.allow_by_default' pref set to true, it seems that when a manifest is found on the page, the offline-app permission is set on that page. Is that correct?
When a user then sets the 'offline-apps.allow_by_default' pref to false, the page would still have offline-app permissions.

Comment 9

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #8)
> I'm wondering though, with the 'offline-apps.allow_by_default' pref set to
> true, it seems that when a manifest is found on the page, the offline-app
> permission is set on that page. Is that correct?
> When a user then sets the 'offline-apps.allow_by_default' pref to false, the
> page would still have offline-app permissions.

Ah, never mind, bug 892488, comment 9 confirms this:
"
- when offline-apps.allow_by_default is true and we hit a web page with a manifest, we automatically grant the offline-app privilege to the origin during the cache selection algorithm (call to new nsContentUtils::MaybeAllowOfflineAppByDefault helper form nsContentSink)
"

And I also could have read about the performance issue in bug 892488, comment 9.

Comment 10

[Honza Bambas (:mayhemer)]

See bug 918880 comment 1.

Comment 11

[Patrick McManus [:mcmanus]]

appcache is going away