Closed
Bug 1160138
Opened 9 years ago
Closed 9 years ago
CacheStorage should provide a ChromeConstructor to allow attaching to any principal's storage
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: bkelly, Assigned: bkelly)
References
Details
Attachments
(3 files, 3 obsolete files)
2.53 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
3.21 KB,
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
4.80 KB,
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
As discussed in bug 1003860, devtools needs a way to access the CacheStorage for a principal from chrome js. I think the best way to do this is with a [ChromeConstructor]. So the js code would do: Cu.import("resource://gre/modules/Services.jsm"); var url = 'http://gaiamobile.org/fm/app/views/main/index.html'; var uri = Services.io.newURI(url, null, null); var principal = Services.scriptSecurityManager.getNoAppCodebasePrincipal(uri); var c1 = new CacheStorage(principal, 'content'); var c2 = new CacheStorage(principal, 'chrome');
Assignee | ||
Comment 1•9 years ago
|
||
I'll try to work on this soonish. If someone else wants to take it, feel free to steal.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•9 years ago
|
||
Before adding a new chrome-only webidl constructor, lets update the webidl to the latest spec form. Also, use [NewObject] instead of [Throws] as its more correct. Spec links: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#cache https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#cache-storage I also wrote a spec bug to use [NewObject] in the spec: https://github.com/slightlyoff/ServiceWorker/issues/692
Attachment #8601060 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 4•9 years ago
|
||
Does this patch (along with P1 in this bug) work for devtools and the tool to pre-populate the cache?
Attachment #8601099 -
Flags: feedback?(poirot.alex)
Attachment #8601099 -
Flags: feedback?(21)
Comment 5•9 years ago
|
||
Comment on attachment 8601099 [details] [diff] [review] P2 Add a [ChromeConstructor] to CacheStorage to support devtools. r=ehsan Sounds awesome. Much better than having to rely on the hidden window. I do think this will help with the 2 use cases that has been mentioned.
Attachment #8601099 -
Flags: feedback?(21) → feedback+
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8601099 -
Attachment is obsolete: true
Attachment #8601099 -
Flags: feedback?(poirot.alex)
Attachment #8601658 -
Flags: review?(ehsan)
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8601659 -
Flags: review?(ehsan)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8601060 [details] [diff] [review] P1 Update CacheStorage and Cache webidl to latest spec. r=bz Ehsan, this patch is just bringing the style of the Cache webidl up-to-date with the spec. (They fixed the indentation in the spec.) Also, now that we don't need [Throws] if a promise-returning method has [NewObject], I converted these gecko-specific annotations to [NewObject]. I opened a spec issue to get the annotations into the spec: https://github.com/slightlyoff/ServiceWorker/issues/692 Boris suggested not waiting for that, though, and just converting them now.
Attachment #8601060 -
Flags: review?(bzbarsky) → review?(ehsan)
Comment 9•9 years ago
|
||
Comment on attachment 8601060 [details] [diff] [review] P1 Update CacheStorage and Cache webidl to latest spec. r=bz Review of attachment 8601060 [details] [diff] [review]: ----------------------------------------------------------------- (Note: the commit message says r=bz, make sure to ask him for review if you specifically wanted him to look at this...)
Attachment #8601060 -
Flags: review?(ehsan) → review+
Comment 10•9 years ago
|
||
Comment on attachment 8601658 [details] [diff] [review] P2 Add a [ChromeConstructor] to CacheStorage to support devtools. r=ehsan Review of attachment 8601658 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/cache/CacheStorage.h @@ +6,5 @@ > > #ifndef mozilla_dom_cache_CacheStorage_h > #define mozilla_dom_cache_CacheStorage_h > > +#include "mozilla/dom/CacheStorageBinding.h" It would be nice if you forward-declared the enum and avoiding including this in the header. ::: dom/webidl/CacheStorage.webidl @@ +27,5 @@ > [NewObject] > Promise<sequence<DOMString>> keys(); > }; > + > +// chrome-only Nit: please also make it super clear this is Mozilla specific in the comment.
Attachment #8601658 -
Flags: review?(ehsan) → review+
Updated•9 years ago
|
Attachment #8601659 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8601060 -
Attachment is obsolete: true
Attachment #8601734 -
Flags: review+
Assignee | ||
Comment 12•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f040538c9f84
Attachment #8601658 -
Attachment is obsolete: true
Attachment #8601735 -
Flags: review+
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cf7a121e011 https://hg.mozilla.org/integration/mozilla-inbound/rev/9c174e93d620 https://hg.mozilla.org/integration/mozilla-inbound/rev/e89d092c5f47
Comment 14•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4cf7a121e011 https://hg.mozilla.org/mozilla-central/rev/9c174e93d620 https://hg.mozilla.org/mozilla-central/rev/e89d092c5f47
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•