Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COEP reporting for workers: fix which variable to use for 'owner' #6525

Merged
merged 1 commit into from
Mar 31, 2021

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Mar 23, 2021

Fixes #6518

cc @whatwg/cross-origin-isolation

(See WHATWG Working Mode: Changes for more details.)


/workers.html ( diff )

Copy link
Member

@mikewest mikewest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a bug fix that doesn't change intended behavior. LGTM!

@zcorpan
Copy link
Member Author

zcorpan commented Mar 23, 2021

Looks like a relevant test here was added in https://chromium-review.googlesource.com/c/chromium/src/+/2650015 (reporting-to-owner.https.html) but was then reverted in https://chromium-review.googlesource.com/c/chromium/src/+/2717382

Reporting for workers in workers doesn't seem to be covered in that test though.

@yutakahirano
Copy link
Member

cc: @d0iasm

@zcorpan
Copy link
Member Author

zcorpan commented Mar 26, 2021

Test written here: web-platform-tests/wpt#28261

@d0iasm
Copy link

d0iasm commented Mar 31, 2021

Thank you for updating the test and the spec!
Let me clarify the behavior.

  • when a document creates a worker, reports are sent to the document with the invalid COEP worker loading.
  • when a worker (A) creates a child worker (B), reports are sent to the worker (A) with the invalid worker (B) loading.

Is my understanding correct?

@zcorpan
Copy link
Member Author

zcorpan commented Mar 31, 2021

That matches my understanding, though which document "the document" is depends on how the Worker constructor was invoked.

https://html.spec.whatwg.org/multipage/workers.html#dom-worker

Let outside settings be the current settings object.

I'll try to submit a test for this today for COEP report for worker, but essentially, the environment settings object that the Worker constructor algorithm uses for parsing scriptURL is the same as the one that should be used for the COEP report.

@zcorpan
Copy link
Member Author

zcorpan commented Mar 31, 2021

OK, test that exercises "entry environment settings object" vs "incumbent environment settings object" vs "current environment settings object": web-platform-tests/wpt#28313

@domenic
Copy link
Member

domenic commented Mar 31, 2021

Thanks for going the extra mile on testing!!

@domenic domenic merged commit 7c06bc4 into main Mar 31, 2021
@domenic domenic deleted the bocoup/coep-reporting-worker-owner branch March 31, 2021 15:37
@domenic domenic added topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal. topic: workers labels Mar 31, 2021
zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Apr 7, 2021
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 15, 2021
…t' for Worker() URL parsing and COEP report, a=testonly

Automatic update from web-platform-tests
Test 'current environment settings object' for Worker() URL parsing and COEP report

See whatwg/html#6525
--

wpt-commits: 25dba84cf7cae753d192f470d82c457bfcdb4290
wpt-pr: 28313
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal. topic: workers
5 participants