1124638 - Allow JS chrome code to set the request context

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Marcos Caceres [:marcosc]]

It would be helpful if chrome code could set the request context for a fetch.

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Marcos, would you be ok with something like:

var r = new Request(...);
r.setContext("manifest");
fetch(r);

setContext() would be [ChromeOnly]. This does mean that you can't just call fetch() with a URL and params like so, and have the context set:

fetch("url", { ..., context: "manifest" })

will not work.

If you are fine with this proposal, it is very quick to implement.

Comment 2

[Marcos Caceres [:marcosc]]

Nikhil, setContext() would be great!

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Could you also let me know how you want this behaving internally? If it is a direct mapping to a CSP type, we'll need to add a CSP type. Right now the fetch spec does not enforce any CSP requirements on manifest.

Comment 4

[Marcos Caceres [:marcosc]]

Yeah, we need to add a new "manifest" type to CSP (it's part of the CSP2 spec). Please see: https://bugzilla.mozilla.org/show_bug.cgi?id=1089255

Comment 5

[Marcos Caceres [:marcosc]]

Nikhil, um... I kinda realized that I also need to be able to change the CORS mode to "no-cors" (as I'm emulating the fetch behavior of a <link> element, which by default is "no-cors"). 

It's actually letting me set "no-cors" when I instantiate a Request, but it's obviously going to ignore that when the fetch is made:

```
const reqInit = {mode: 'no-cors'};
const req = new contentWindow.Request("http://other-domain.com", reqInit);
dump(req.mode); // prints 'no-cors'
fetch(req);//resolves response of type error.  
```

Will obviously need to file a different bug for the above, but just wanted to give you heads up and to see if you had any thoughts about it.

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to Marcos Caceres [:marcosc] from comment #5)
> Nikhil, um... I kinda realized that I also need to be able to change the
> CORS mode to "no-cors" (as I'm emulating the fetch behavior of a <link>
> element, which by default is "no-cors"). 
> 
> It's actually letting me set "no-cors" when I instantiate a Request, but
> it's obviously going to ignore that when the fetch is made:
> 
> ```
> const reqInit = {mode: 'no-cors'};
> const req = new contentWindow.Request("http://other-domain.com", reqInit);
> dump(req.mode); // prints 'no-cors'
> fetch(req);//resolves response of type error.  
> ```
> 
> Will obviously need to file a different bug for the above, but just wanted
> to give you heads up and to see if you had any thoughts about it.

I think it might be easier to add a 3-arg form of fetch(), where the 3rd arg is a dictionary of chrome only settings. Then make the 3-arg form [ChromeOnly]. That way fetch could directly override some settings of Request.

Comment 7

[Marcos Caceres [:marcosc]]

> I think it might be easier to add a 3-arg form of fetch(), where the 3rd arg is a dictionary of chrome only settings. Then make the 3-arg form [ChromeOnly]. That way fetch could directly override some settings of Request.

That sounds good. Would you also do that for .setContext()?

Comment 8

[Marcos Caceres [:marcosc]]

(I mean, drop .setContext() in favor of the using the third argument as dictionary).

Comment 9

[Marcos Caceres [:marcosc]]

For bug 1089255, I put together a tiny patch to add support for manifest-src CSP:
https://bug1089255.bugzilla.mozilla.org/attachment.cgi?id=8558915

Comment 10

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7fde8710208a

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/7fde8710208a

Comment 12

[Marcos Caceres [:marcosc]]

Nikhil, setContext is working great :) The proposal in #6 sounds great too. Would you like me to file a separate bug for the above? The last outstanding thing I have for fetching manifest is overriding "cors" from chrome code :)

Comment 13

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to Marcos Caceres [:marcosc] from comment #12)
> Nikhil, setContext is working great :) The proposal in #6 sounds great too.
> Would you like me to file a separate bug for the above? The last outstanding
> thing I have for fetching manifest is overriding "cors" from chrome code :)

Separate bug would be nice. Please make "dom-fetch-api" a blocker for that (don't block dom-fetch-api on it thought :)), just to keep it on my mind. Would you like to take a stab at implementing it? It will be C++, but pretty much parameter twiddling and all documented at https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings

Comment 14

[Marcos Caceres [:marcosc]]

(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #13)
> Separate bug would be nice. Please make "dom-fetch-api" a blocker for that
> (don't block dom-fetch-api on it thought :)), just to keep it on my mind.
> Would you like to take a stab at implementing it?

done. Bug 1130924. 

> It will be C++, but pretty
> much parameter twiddling and all documented at
> https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings

Sounds doable. Will give it a shot.