1165256 - use origin for app_cache

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect, P2)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

We implemented app caches for apps on B2G back in Bug 751754 and Bug 786299.
For the new security model we will update nsIPrincipal.origin in Bug 1163254.
So for app_cache it should use the origin, instead of {appId, isBrowser}, for example in 
nsIApplicationCacheService.idl, and nsIOfflineCacheUpdate.idl.

Comment 1

[Honza Bambas (:mayhemer)]

Here applies similar as in bug 1165269.  Also offline cache is an unmaintained code about to be removed.  No dates/more details avail now tho.

Comment 2

[Honza Bambas (:mayhemer)]

See bug 1165269 comment 3.  This may "just work" when we fix that bug.

Comment 3

[Honza Bambas (:mayhemer)]

How can I at [1] get OriginAttributes instead of "just" nsILoadContext?

[1] http://hg.mozilla.org/mozilla-central/annotate/f61c3cc0eb8b/dom/offline/nsDOMOfflineResourceList.cpp#l809

Comment 4

[Honza Bambas (:mayhemer)]

(The motivation is to covert nsApplicationCacheService::BuildGroupIDForApp to take OriginAttributes, or maybe be removed/renamed to something else working with OA)

Comment 5

[Honza Bambas (:mayhemer)]

This bug should mainly fix:
- nsIApplicationCacheService.buildGroupIDForApp to take either directly OriginAttributes (as a jsval?) or be removed completely and convert existing code to use the nsILoadContextInfo variant "buildGroupID".
- nsIOfflineCacheUpdate* methods taking appid/inbrowser to accept OriginAttributes

As I understand, I can always obtain OriginAttributes from nsIPrincipal.

Comment 6

[Honza Bambas (:mayhemer)]

One more question, how should I use/obtain OriginAttributes at [1]?

[1] http://hg.mozilla.org/mozilla-central/annotate/56a6c786bdb0/dom/apps/Webapps.jsm#l1955

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Honza Bambas (not reviewing) (:mayhemer) from comment #3)
> How can I at [1] get OriginAttributes instead of "just" nsILoadContext?
> 
> [1]
> http://hg.mozilla.org/mozilla-central/annotate/f61c3cc0eb8b/dom/offline/
> nsDOMOfflineResourceList.cpp#l809

I am adding OriginAttributes into nsILoadContext in Bug 1165466.

(In reply to Honza Bambas (not reviewing) (:mayhemer) from comment #6)
> One more question, how should I use/obtain OriginAttributes at [1]?
> 
> [1]
> http://hg.mozilla.org/mozilla-central/annotate/56a6c786bdb0/dom/apps/Webapps.
> jsm#l1955

If aApp is an instance of mozIApplication, it has a 'principal' attribute. (bug 1168783).
If not we usually pass the dictionaty of OriginAttributes.
{ appId: aApp.localId }

Comment 8

[Honza Bambas (:mayhemer)]

Thanks!

Comment 10

[Honza Bambas (:mayhemer)]

Attachment: wip1 (backup)

Comment 11

[Honza Bambas (:mayhemer)]

adding bug 1199295 to the dep list since it changes the offline app APIs in prefetch to pass down principal, will base a much simpler patch on it here.  OriginAttributes can be extracted from principals (with bug 1165466).

Comment 12

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

mark this also blocks Bug 1168777, for AppCacheClearDataObserver in nsApplicationCacheService.cpp should also listen to clear-origin-data and clear the cache accordingly.

Comment 13

[Honza Bambas (:mayhemer)]

Attachment: wip2 (backup)

- rebased
- builds
- fails

Comment 14

[Honza Bambas (:mayhemer)]

Attachment: wip3 (backup)

- pattern match
- builds
- no more data..

Comment 15

[Tanvi Vyas[:tanvi]]

Honza, any updates here?

Comment 16

[Honza Bambas (:mayhemer)]

(In reply to Tanvi Vyas [:tanvi] from comment #15)
> Honza, any updates here?

In two weeks I may have a production quality patch.

Comment 17

[Honza Bambas (:mayhemer)]

Attachment: v1

goal was to make appcache code respect origin attributes suffix string for isolation.  to obtain origin attrs in nsOfflineCacheUpdate I use the loading principal which is identical source for appid/inbrowser in the current code.  In the cache code itself further I use nsILoadContextInfo (from cache2 apis) or internally directly the suffix string as appropriate.  the patch also impls clear-origin-data properly.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bae9785efd4c

Comment 18

[Jason Duell]

Comment on attachment 8709191 [details] [diff] [review]
v1

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

Thanks for doing all this fun stuff Honza :)

Comment 19

[Honza Bambas (:mayhemer)]

One more detail left to fix:

chrome://mochitests/content/browser/browser/base/content/test/general/browser_sanitizeDialog.js:557 - TypeError: appcacheserv.buildGroupID is not a function

Comment 20

[Honza Bambas (:mayhemer)]

Attachment: v1.1

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bae9785efd4c - where BC7 is FIXED in the v1.1 patch.

Comment 21

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/56582e4322f6

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/56582e4322f6