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

Raise the bar for SharedArrayBuffer via postMessage() #4734

Merged
merged 5 commits into from
Jul 8, 2020

Conversation

annevk
Copy link
Member

@annevk annevk commented Jun 25, 2019

@annevk annevk added do not merge yet Pull request must not be merged per rationale in comment needs tests Moving the issue forward requires someone to write tests topic: serialize and transfer labels Jun 25, 2019
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@annevk annevk mentioned this pull request Jul 11, 2019
source Outdated Show resolved Hide resolved
annevk added a commit to web-platform-tests/wpt that referenced this pull request Jul 12, 2019
@annevk annevk added needs tests Moving the issue forward requires someone to write tests and removed needs tests Moving the issue forward requires someone to write tests labels Jul 19, 2019
annevk added a commit to web-platform-tests/wpt that referenced this pull request Jul 19, 2019
annevk added a commit to web-platform-tests/wpt that referenced this pull request Jul 19, 2019
The original test could not work. This should be able to work and also adds COOP/COEP headers for future proofing. No browser passes still.

For whatwg/html#4734.
annevk added a commit to web-platform-tests/wpt that referenced this pull request Jul 19, 2019
@annevk
Copy link
Member Author

annevk commented Jul 19, 2019

I've updated OP with a number of tests of which some are landed. I made the decision to land test changes as this is a large undertaking that has multi-implementer buy-in and landing it all at once would it a lot more likely we'd miss important things. The changes should minimally disrupt existing coverage. I'll try to continue to keep OP up-to-date.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 31, 2019
…ffer success case revamp, a=testonly

Automatic update from web-platform-tests
HTML: BroadcastChannel and SharedArrayBuffer success case revamp

The original test could not work. This should be able to work and also adds COOP/COEP headers for future proofing. No browser passes still.

For whatwg/html#4734.
--

wpt-commits: dedb45f5f20e0ac884ac59634cceb8659cdcff7d
wpt-pr: 17760
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Jul 31, 2019
…ffer success case revamp, a=testonly

Automatic update from web-platform-tests
HTML: BroadcastChannel and SharedArrayBuffer success case revamp

The original test could not work. This should be able to work and also adds COOP/COEP headers for future proofing. No browser passes still.

For whatwg/html#4734.
--

wpt-commits: dedb45f5f20e0ac884ac59634cceb8659cdcff7d
wpt-pr: 17760
annevk added a commit to web-platform-tests/wpt that referenced this pull request Aug 6, 2019
annevk added a commit to web-platform-tests/wpt that referenced this pull request Aug 6, 2019
source Outdated Show resolved Hide resolved
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 9, 2019
…djustments, a=testonly

Automatic update from web-platform-tests
HTML: SharedArrayBuffer infrastructure adjustments

For whatwg/html#4734.
--

wpt-commits: a140f239fa9dc4fc3db593b664064b65340e7d4b
wpt-pr: 17761


--HG--
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-iframe.html.headers => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/resources/echo-iframe.html.headers
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-iframe.html.headers => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/resources/echo-worker.js.headers
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/broadcastchannel-success-and-failure.html => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/broadcastchannel-success-and-failure.https.html
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/identity-not-preserved.html => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/identity-not-preserved.https.html
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/no-transferring.html => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/no-transferring.https.html
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-iframe.html.headers => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-sharedworker.js.headers
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-iframe.html.headers => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-worker.js.headers
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-iframe.html.headers => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/incrementer-worker-with-channel.js.headers
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-iframe.html.headers => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/incrementer-worker.js.headers
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-iframe.html.headers => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/nested-worker-success.js.headers
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-iframe.html.headers => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/serviceworker-failure.js.headers
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/broadcastchannel-iframe.html.headers => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/sharedworker-failure.js.headers
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-history.html => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-history.https.html
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-domain-success.sub.html => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-domain-success.https.sub.html
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-iframe-messagechannel-success.html => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-iframe-messagechannel-success.https.html
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-messagechannel-success.html => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-messagechannel-success.https.html
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-sharedworker-failure.html => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-sharedworker-failure.https.html
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-similar-but-cross-origin-success.sub.html => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-similar-but-cross-origin-success.https.sub.html
rename : testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-simple-success.html => testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-simple-success.https.html
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 9, 2019
…er throws without COOP+COEP, a=testonly

Automatic update from web-platform-tests
HTML: ensure serializing SharedArrayBuffer throws without COOP+COEP

For whatwg/html#4734.
--

wpt-commits: aecedb53e28abad5432227e293dd016cfc46d4b7
wpt-pr: 17802
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Aug 10, 2019
…djustments, a=testonly

Automatic update from web-platform-tests
HTML: SharedArrayBuffer infrastructure adjustments

For whatwg/html#4734.
--

wpt-commits: a140f239fa9dc4fc3db593b664064b65340e7d4b
wpt-pr: 17761
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Aug 10, 2019
…er throws without COOP+COEP, a=testonly

Automatic update from web-platform-tests
HTML: ensure serializing SharedArrayBuffer throws without COOP+COEP

For whatwg/html#4734.
--

wpt-commits: aecedb53e28abad5432227e293dd016cfc46d4b7
wpt-pr: 17802
@yutakahirano

This comment has been minimized.

@annevk
Copy link
Member Author

annevk commented Jun 26, 2020

I would be okay with the [SecureContext] restriction. I don't think Firefox currently has that, but we could add it. On the other hand it might be useful to have available always as then you don't need to check isSecureContext first.

@domenic
Copy link
Member

domenic commented Jun 26, 2020

Sorry, but I think this will need rebasing now that 0221df8 got merged. I'll try to review anyway.

I asked around internally for [SecureContext] and @arturjanc seemed slightly in favor of exposing it in non-secure contexts. I'm torn; [SecureContext] seems a bit more logical since the whole feature is restricted to secure contexts, but maybe we should make an exception for feature-detection getters. (And then there's the whole line of thinking that all new APIs should be [SecureContext], heh...)

I do think the equivalent getter for origin isolation should follow whatever pattern we choose here.

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated
true.</p>

<p class="XXX">This really ought to be set when the agent cluster is created, which requires a
redesign of this section.</p>
Copy link
Member

Choose a reason for hiding this comment

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

It's not clear to me whether this will ever be possible. Fetches require fetch clients (ESOs). So we need to create the ESO/realm/global object before doing the fetch. And we should put all realms into agents, so we need to create the agent and its corresponding agent cluster before the fetch.

I think this is OK as-is, and we should delete the XXX. It seems similar to how various properties of the WorkerGlobalScope (and thus, the values returned by the ESO algorithms) are only set after the fetch. It's OK, as long as no code runs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why wouldn't it use outside settings? Note that for security reasons browsers would not be able to implement this approach as it would not properly process-isolate the worker (using a cross-origin isolated process that is).

Copy link
Member

Choose a reason for hiding this comment

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

You're right; the fetch client is outside settings. Ugh, this would be a painful redesign; we'd need to split up fetching the scripts (doable before creating the global) from creating the scripts (doable only after the global exists). Alright, this XXX should stay, I concede.

Copy link
Member

@domenic domenic Jul 6, 2020

Choose a reason for hiding this comment

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

It would be particularly bad for modules, because parsing requires a realm to exist, but you need to parse before you can continue fetching dependencies. So we'd need to split it up into something like

  1. Fetch top-level source text. (Technically all we need are the headers.... maybe stop at getting the response.)
  2. Create the realm
  3. Create top-level script
  4. Fetch and create dependency scripts

Funnnnn.

A top-level navigation response with Cross-Origin-Opener-Policy set to same-origin and Cross-Origin-Embedder-Policy set to require-corp will create a cross-origin isolated browsing context group. And all agent clusters therein will be cross-origin isolated as well (shared and service workers can still not be, as they sit on the side).

This change also:

* Gates SharedArrayBuffer exposure behind that primitive for web compatibility reasons.
* Gates SharedArrayBuffer sharing behind that primitive.
* Exposes it through self.crossOriginIsolated.
* Makes document.domain return before it mutates the origin.
* Makes agent clusters keyed on origin.

Tests:

* web-platform-tests/wpt#17719
* web-platform-tests/wpt#17760
* web-platform-tests/wpt#17761
* web-platform-tests/wpt#17802
* web-platform-tests/wpt#17909
* web-platform-tests/wpt#18543
* web-platform-tests/wpt#20116
* web-platform-tests/wpt#22358

Closes #4732. Closes #5122. Closes #5444.

Follow-up: #5435.
@annevk
Copy link
Member Author

annevk commented Jun 29, 2020

I squashed and then rebased and there were no issues. Somewhat strange. I'll work on some fixups now.

@annevk annevk removed the do not merge yet Pull request must not be merged per rationale in comment label Jun 29, 2020
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM with nits, hooray

source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
@annevk
Copy link
Member Author

annevk commented Jul 8, 2020

Oh wait, given the wording in tc39/ecma262#1903 I guess it's solely up to the host, not the implementation. So this is all good.

@annevk annevk mentioned this pull request Jul 8, 2020
@annevk annevk merged commit c9d8983 into master Jul 8, 2020
@annevk annevk deleted the annevk/postmessage-sab branch July 8, 2020 09:05
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request Jun 1, 2024
…ffer success case revamp, a=testonly

Automatic update from web-platform-tests
HTML: BroadcastChannel and SharedArrayBuffer success case revamp

The original test could not work. This should be able to work and also adds COOP/COEP headers for future proofing. No browser passes still.

For whatwg/html#4734.
--

wpt-commits: dedb45f5f20e0ac884ac59634cceb8659cdcff7d
wpt-pr: 17760
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: cross-origin-opener-policy Issues and ideas around the new "inverse of rel=noopener" header. topic: serialize and transfer
Development

Successfully merging this pull request may close these issues.

SharedArrayBuffer and postMessage()
4 participants