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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

Attachments

(3 files, 3 obsolete files)

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');
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
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)
Main change from the spec in comment 2 was indentation.
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 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+
Attachment #8601099 - Attachment is obsolete: true
Attachment #8601099 - Flags: feedback?(poirot.alex)
Attachment #8601658 - Flags: review?(ehsan)
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 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 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+
Attachment #8601659 - Flags: review?(ehsan) → review+
Depends on: 1162487
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.