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

RESOLVED FIXED in Firefox 39

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jdm, Assigned: nsm)

Tracking

unspecified
mozilla39
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
There are currently TODO comments about this in FetchEventRunnable::DispatchFetchEvent.
(Reporter)

Updated

4 years ago
Blocks: 1059784
Impossible to test this without header synthesis.
Depends on: 1134462
Created attachment 8577508 [details]
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
Attachment #8577508 - Flags: review?(josh)
(Reporter)

Comment 3

4 years ago
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?
Attachment #8577508 - Flags: review?(josh)
(Reporter)

Comment 4

4 years ago
One of the necko peers should sign off on the nsIHTTPChannelInternal changes, too.
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
Attachment #8577508 - Flags: review?(honzab.moz)
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 on attachment 8577508 [details]
MozReview Request: bz://1134324/nsm

sorry, forgot to flip reviewboard flags.
Attachment #8577508 - Flags: review?(bkelly)
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
Attachment #8577508 - Flags: review?(honzab.moz) → review?(michal.novotny)
Michal, could you review the changes to nsIHttpChannelInternal? Thanks!
Attachment #8577508 - Flags: review?(michal.novotny) → review+
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.
https://hg.mozilla.org/mozilla-central/rev/6a639c5517e3
Assignee: nobody → nsm.nikhil
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox39: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment on attachment 8577508 [details]
MozReview Request: bz://1134324/nsm
Attachment #8577508 - Attachment is obsolete: true
Attachment #8619513 - Flags: review+
Attachment #8619514 - Flags: review+
Created attachment 8619513 [details]
MozReview Request: Bug 1134324 - Set CORS mode and credentials on Fetch event Request.
Created attachment 8619514 [details]
MozReview Request: Bug 1134324 - TODO add more tests.
You need to log in before you can comment on or make changes to this bug.