Closed Bug 1387473 Opened 4 years ago Closed 4 years ago

add test for cross-domain source map loading

Categories

(DevTools :: General, enhancement, P3)

enhancement

Tracking

(firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: tromey, Assigned: tromey)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

In https://github.com/devtools-html/devtools-core/issues/551 I plan to
change the source map worker to use fetch rather than XHR.
There Jason said that this wasn't done to avoid a CORS problem; so
first I plan to land a cross-domain source map test.
Comment on attachment 8893867 [details]
Bug 1387473 - add cross-domain source-map test;

https://reviewboard.mozilla.org/r/164956/#review170362

Thanks for the patch! 

I have a few background questions about what this is really exercizing.
We have a bundle on domain A, and a sourcemap on domain B and we want to check that we can fetch the sourcemap on domain B without any issue.
I guess it's a reasonable thing to check, but as devtools running in the panel, I assume we are fetching the sourcemap using the system principal?
Am I correct here, or do we ask for the sourcemap on behalf of the content page?

Now the reason I'm asking is that you mentioned you discussed it with Jason, and I assume the constraints are different when running inside of a browser tab compared to running in Firefox? Maybe that's why an issue with CORs was brought up? (and in this case I'm not sure having this test will help prevent breakage?)

::: devtools/client/framework/test/browser_source_map-cross-domain.js:32
(Diff revision 1)
> +  // Start with the empty page, then navigate, so that we can properly
> +  // listen for new sources arriving.

Is this comment relevant here?
Attachment #8893867 - Flags: review?(jdescottes) → review+
Comment on attachment 8893867 [details]
Bug 1387473 - add cross-domain source-map test;

https://reviewboard.mozilla.org/r/164956/#review170362

> Am I correct here, or do we ask for the sourcemap on behalf of the content page?

You're correct.  The source map is fetched by a worker running in chrome context.

> I assume the constraints are different when running inside of a browser tab compared to running in Firefox? Maybe that's why an issue with CORs was brought up? (and in this case I'm not sure having this test will help prevent breakage?)

The browser tab case actually already uses fetch.  See https://github.com/devtools-html/devtools-core/blob/master/packages/devtools-utils/src/network-request.js -- AFAIK only the M-C worker uses privileged-network-request.

So, I don't think that can be the issue.

It's not entirely clear what the issue might be.  All I know is: https://github.com/devtools-html/devtools-core/issues/551#issuecomment-320062687

So I thought perhaps testing the basic case would be a good preventative measure.

I hope this helps.  Honestly I'm not certain that this is either necessary or sufficient.  I welcome other suggestions.
Thanks for the clarifications Tom. I agree this is still worth testing, just was unsure about the conversation on GitHub :)
Comment on attachment 8893867 [details]
Bug 1387473 - add cross-domain source-map test;

https://reviewboard.mozilla.org/r/164956/#review170362

> Is this comment relevant here?

Indeed not.
Those eslint failures were from an earlier patch that was on the branch, and they're fixed
in the patch that landed.
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/50efa6725f5d
add cross-domain source-map test; r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/50efa6725f5d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.