1134324 - Set credentials and CORS flags on Request object used in onfetch event

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Josh Matthews [:jdm]]

There are currently TODO comments about this in FetchEventRunnable::DispatchFetchEvent.

Comment 1

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

Impossible to test this without header synthesis.

Comment 2

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

Attachment: MozReview Request: bz://1134324/nsm

/r/5409 - Bug 1134324 - Set CORS mode and credentials on Fetch event Request.
/r/5411 - Bug 1134324 - TODO add more tests.

Pull down these commits:

hg pull review -r 369b6e7f65384ba24731c82da55e93d9c1ee069d

Comment 3

[Josh Matthews [:jdm]]

Comment on attachment 8577508 [details]
MozReview Request: bz://1134324/nsm

https://reviewboard.mozilla.org/r/5407/#review4373

::: dom/workers/ServiceWorkerEvents.cpp
(Diff revision 1)
> -  nsRefPtr<InternalHeaders> headers = response->GetInternalHeaders();
> +  nsRefPtr<InternalResponse> ir = response->GetInternalResponse();

This looks unused.

::: dom/workers/ServiceWorkerManager.cpp
(Diff revision 1)
> +    // We have static asserts at the top of this file ensuring the Necko
> +    // constants match RequestMode, so this direct assignment is fine.
> +    mRequestMode = static_cast<RequestMode>(mode);

I might prefer a switch here instead of the direct assignment, since we can include a default branch as well.

::: dom/workers/ServiceWorkerManager.cpp
(Diff revision 1)
> +    //reqInit.mMode.Construct(mRequestMode);

Eh?

Comment 4

[Josh Matthews [:jdm]]

One of the necko peers should sign off on the nsIHTTPChannelInternal changes, too.

Comment 5

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

Comment on attachment 8577508 [details]
MozReview Request: bz://1134324/nsm

/r/5409 - Bug 1134324 - Set CORS mode and credentials on Fetch event Request.

Pull down this commit:

hg pull review -r d381c0b9937528bdda35cf3f4e7aa92d793c1b9c

Comment 6

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

Comment on attachment 8577508 [details]
MozReview Request: bz://1134324/nsm

/r/5409 - Bug 1134324 - Set CORS mode and credentials on Fetch event Request.

Pull down this commit:

hg pull review -r 0b50244fc1f4170ef00a9a69780b4e113595bfd1

Comment 7

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

Comment on attachment 8577508 [details]
MozReview Request: bz://1134324/nsm

sorry, forgot to flip reviewboard flags.

Comment 8

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

Comment on attachment 8577508 [details]
MozReview Request: bz://1134324/nsm

/r/5409 - Bug 1134324 - Set CORS mode and credentials on Fetch event Request.

Pull down this commit:

hg pull review -r 0b50244fc1f4170ef00a9a69780b4e113595bfd1

Comment 9

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

Michal, could you review the changes to nsIHttpChannelInternal? Thanks!

Comment 10

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

I made a small change in the patch. Earlier, if the cast to http channel failed, we would abort, which led to some XHR tests failing that did not use HTTP urls. I made it case directly to http internal channel, and used an if (internalChannel) {} instead of early return.

Comment 11

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/6a639c5517e3

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6a639c5517e3

Comment 14

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

Attachment: MozReview Request: Bug 1134324 - Set CORS mode and credentials on Fetch event Request.

Comment 15

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

Attachment: MozReview Request: Bug 1134324 - TODO add more tests.