940254 - Make "storage" permission in the database not grant "offline-app"

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Currently we grant "offline-app" permission in nsIPermissionManager for all apps that enumerate "storage" as a permission in the manifest.

This has the downside that we start looking for appcached resources in the appcache database, which slows down pageload.

A simple fix to this would be to simply not grant "offline-app" permission when a manifest enumerates "storage". Instead we could create a separate manifest permission specifically for appcache.

That way any apps that doesn't use appcache, including all our gaia apps, can simply avoid asking for the appcache permission.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Mike, can you make sure that this performance bug gets some attention. It should be as simple as modifying the permission in [1] to not grant "offline-app" for the "storage" permission, and instead create a separate permission.

However we also need to coordinate with the marketplace team to make sure that any app which uses appcache also uses the new permission.

Oh, and we need to remove the "offline-app" permission from any already-installed apps as appropriate. That's probably trickier.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/PermissionsTable.jsm

Comment 2

[Jason Smith [:jsmith]]

We really need to check what marketplace apps are currently using the storage permission to identify any compatibility fallout before moving forward here.

Comment 3

[[:fabrice] Fabrice Desré]

From the dump in bug 940682 we can see that 268 application declare the "storage" permission. Among these, only 27 also have an appcache_path declared in their manifest.

Jonas, what about doing what you say in comment #0 and automatically grand the offline-cache (and pin-app) permission to apps that declare an appcache in their manifest?

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

That sounds like a great idea!

Comment 5

[[:fabrice] Fabrice Desré]

Attachment: appcache-permission.patch

Comment 6

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/b2g-inbound/rev/fe10dc4545c6

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/b2g-inbound/rev/f9b88a4cec04 for breaking xpcshell tests: 

https://tbpl.mozilla.org/php/getParsedLog.php?id=32732490&tree=B2g-Inbound

Comment 8

[[:fabrice] Fabrice Desré]

Pushed to try with a couple of tests fixed:
https://tbpl.mozilla.org/?tree=Try&rev=f72b8ec86141

Comment 9

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/b2g-inbound/rev/aed7f997fb00

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/aed7f997fb00