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

SharedArrayBuffer and blob:/data: worker #21146

Merged
merged 4 commits into from Feb 20, 2020
Merged

Conversation

annevk
Copy link
Member

@annevk annevk commented Jan 13, 2020

See whatwg/html#5198 and whatwg/html#4916. blob: has to work, not sure about data: URLs again.

@annevk
Copy link
Member Author

annevk commented Feb 5, 2020

The problem with the data URL case is that it needs to use a blob URL for its nested worker as otherwise the nested worker would not be same-origin with it. (See also whatwg/html#5254 for how it's a bit more complex therefore.)

@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-21146 February 5, 2020 16:49 Inactive
@annevk
Copy link
Member Author

annevk commented Feb 6, 2020

That's now addressed. @hemeryar @ParisMeuleman @camillelamy please review!

@annevk
Copy link
Member Author

annevk commented Feb 6, 2020

If this seems reasonable I could maybe make this more generic to also test blob/data frames this way.

@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-21146 February 6, 2020 17:46 Inactive
Copy link
Contributor

@ParisMeuleman ParisMeuleman left a comment

Choose a reason for hiding this comment

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

Thanks @annevk for this.

Overall lgtm.

Although adding a few comments, especially in the dataWorkerScript could improve readability IMO (e.g. view was not a very clear name to me, likewise for L52 the handling of the 2 messages from the sub-worker) .
I reckon doing the work to make it generic could solve this point.

@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-21146 February 12, 2020 16:33 Inactive
annevk added a commit that referenced this pull request Feb 13, 2020
@annevk
Copy link
Member Author

annevk commented Feb 13, 2020

@ParisMeuleman do you want to have another look?

annevk added a commit to w3c/webappsec-secure-contexts that referenced this pull request Feb 13, 2020
As long as they have a creator, anyway. (Note that data: URLs cannot be opened in a top-level browsing context, such as a popup, except via a user-initiated navigation.)

Tests: web-platform-tests/wpt#21146 and web-platform-tests/wpt#21781.

Fixes #69. Nice!
annevk added a commit that referenced this pull request Feb 13, 2020
mikewest pushed a commit to w3c/webappsec-secure-contexts that referenced this pull request Feb 13, 2020
As long as they have a creator, anyway. (Note that data: URLs cannot be opened in a top-level browsing context, such as a popup, except via a user-initiated navigation.)

Tests: web-platform-tests/wpt#21146 and web-platform-tests/wpt#21781.

Fixes #69. Nice!
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 17, 2020
Automatic update from web-platform-tests
Secure contexts: add data: URLs

See w3c/webappsec-secure-contexts#69. Complements web-platform-tests/wpt#21146.
--

wpt-commits: cc7860faa93b6c404a5767b7635843d274eb7d0a
wpt-pr: 21781
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Feb 18, 2020
Automatic update from web-platform-tests
Secure contexts: add data: URLs

See w3c/webappsec-secure-contexts#69. Complements web-platform-tests/wpt#21146.
--

wpt-commits: cc7860faa93b6c404a5767b7635843d274eb7d0a
wpt-pr: 21781
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Feb 18, 2020
Automatic update from web-platform-tests
Secure contexts: add data: URLs

See w3c/webappsec-secure-contexts#69. Complements web-platform-tests/wpt#21146.
--

wpt-commits: cc7860faa93b6c404a5767b7635843d274eb7d0a
wpt-pr: 21781

UltraBlame original commit: 9168eaebb7a09addc3a9c1700a2b2312bae790c5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Feb 18, 2020
Automatic update from web-platform-tests
Secure contexts: add data: URLs

See w3c/webappsec-secure-contexts#69. Complements web-platform-tests/wpt#21146.
--

wpt-commits: cc7860faa93b6c404a5767b7635843d274eb7d0a
wpt-pr: 21781

UltraBlame original commit: 9168eaebb7a09addc3a9c1700a2b2312bae790c5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Feb 18, 2020
Automatic update from web-platform-tests
Secure contexts: add data: URLs

See w3c/webappsec-secure-contexts#69. Complements web-platform-tests/wpt#21146.
--

wpt-commits: cc7860faa93b6c404a5767b7635843d274eb7d0a
wpt-pr: 21781

UltraBlame original commit: 9168eaebb7a09addc3a9c1700a2b2312bae790c5
@annevk
Copy link
Member Author

annevk commented Feb 19, 2020

@ParisMeuleman ping.

Copy link
Contributor

@ParisMeuleman ParisMeuleman left a comment

Choose a reason for hiding this comment

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

Thanks LGTM!

@annevk annevk merged commit 981d2d5 into master Feb 20, 2020
@annevk annevk deleted the annevk/sab-blob-data-worker branch February 20, 2020 09:31
annevk added a commit that referenced this pull request Apr 14, 2020
#21146 missed a crucial aspect and could use slightly better variable naming.

As the data URL document is (always) cross-origin from the testharness.js resource that resource needs the CORP header with cross-origin as value.
annevk added a commit that referenced this pull request Apr 14, 2020
PR #21146 missed a crucial aspect and could use slightly better variable naming.

As the data URL document is (always) cross-origin from the testharness.js resource that resource needs the CORP header with cross-origin as value.
stephenmcgruer pushed a commit that referenced this pull request Apr 14, 2020
PR #21146 missed a crucial aspect and could use slightly better variable naming.

As the data URL document is (always) cross-origin from the testharness.js resource that resource needs the CORP header with cross-origin as value.
annevk added a commit to whatwg/html that referenced this pull request Apr 21, 2020
Tests:

* There should be a test for this where you create a data URL worker and try to message it SAB and WebAssembly.Module and check that it results in messageerror events.
* web-platform-tests/wpt#21146 and web-platform-tests/wpt#22928 test that inside a data URL worker you can share memory with a further nested data URL worker that is same-origin with the opaque origin of the data URL worker.

Fixes #5254.
annevk added a commit to whatwg/html that referenced this pull request Mar 11, 2023
Tests:

* There should be a test for this where you create a data URL worker and try to message it SAB and WebAssembly.Module and check that it results in messageerror events.
* web-platform-tests/wpt#21146 and web-platform-tests/wpt#22928 test that inside a data URL worker you can share memory with a further nested data URL worker that is same-origin with the opaque origin of the data URL worker.

Fixes #5254.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants