If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

use origin for app_cache

RESOLVED FIXED in Firefox 46

Status

()

Core
Networking: Cache
P2
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: allstars, Assigned: mayhemer)

Tracking

Trunk
FxOS-S8 (02Oct)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 affected, firefox46 fixed)

Details

(Whiteboard: must land on 43)

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

3 years ago
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.
(Assignee)

Comment 1

2 years ago
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.
No longer depends on: 1163254
Blocks: 1179985
(Assignee)

Comment 2

2 years ago
See bug 1165269 comment 3.  This may "just work" when we fix that bug.
Assignee: nobody → honzab.moz
Blocks: 1153435
(Assignee)

Comment 3

2 years ago
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
Flags: needinfo?(jonas)
(Assignee)

Comment 4

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

Comment 5

2 years ago
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.
(Assignee)

Comment 6

2 years ago
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
(Assignee)

Updated

2 years ago
Depends on: 1168777
(Reporter)

Comment 7

2 years ago
(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 }
Flags: needinfo?(jonas)
(Assignee)

Comment 8

2 years ago
Thanks!
Depends on: 1165466
Priority: -- → P2
Target Milestone: --- → FxOS-S8 (02Oct)
(Assignee)

Updated

2 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1197093
(Assignee)

Comment 10

2 years ago
Created attachment 8656047 [details] [diff] [review]
wip1 (backup)
(Assignee)

Comment 11

2 years ago
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).
Depends on: 1199295
(Assignee)

Updated

2 years ago
Depends on: 1165269
(Assignee)

Updated

2 years ago
No longer depends on: 1168777
(Assignee)

Updated

2 years ago
Whiteboard: must land on 43
(Reporter)

Comment 12

2 years ago
mark this also blocks Bug 1168777, for AppCacheClearDataObserver in nsApplicationCacheService.cpp should also listen to clear-origin-data and clear the cache accordingly.
Blocks: 1168777
No longer blocks: 1153435

Updated

2 years ago
QA Whiteboard: [COM=NSec]
(Assignee)

Comment 13

2 years ago
Created attachment 8681363 [details] [diff] [review]
wip2 (backup)

- rebased
- builds
- fails
Attachment #8656047 - Attachment is obsolete: true
(Assignee)

Comment 14

2 years ago
Created attachment 8681383 [details] [diff] [review]
wip3 (backup)

- pattern match
- builds
- no more data..
Attachment #8681363 - Attachment is obsolete: true

Comment 15

2 years ago
Honza, any updates here?
(Assignee)

Comment 16

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

In two weeks I may have a production quality patch.
(Assignee)

Comment 17

2 years ago
Created attachment 8709191 [details] [diff] [review]
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
Attachment #8681383 - Attachment is obsolete: true
Attachment #8709191 - Flags: review?(jduell.mcbugs)
Comment on attachment 8709191 [details] [diff] [review]
v1

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

Thanks for doing all this fun stuff Honza :)
Attachment #8709191 - Flags: review?(jduell.mcbugs) → review+
(Assignee)

Comment 19

2 years ago
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
(Assignee)

Comment 20

2 years ago
Created attachment 8710015 [details] [diff] [review]
v1.1

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bae9785efd4c - where BC7 is FIXED in the v1.1 patch.
Attachment #8709191 - Attachment is obsolete: true
Attachment #8710015 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 21

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/56582e4322f6
Keywords: checkin-needed

Comment 22

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/56582e4322f6
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.