-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Conversation
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.) |
That's now addressed. @hemeryar @ParisMeuleman @camillelamy please review! |
If this seems reasonable I could maybe make this more generic to also test blob/data frames this way. |
There was a problem hiding this 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.
See w3c/webappsec-secure-contexts#69. Complements #21146.
@ParisMeuleman do you want to have another look? |
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!
See w3c/webappsec-secure-contexts#69. Complements #21146.
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!
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
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
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
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
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
@ParisMeuleman ping. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks LGTM!
#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.
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.
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.
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.
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.
See whatwg/html#5198 and whatwg/html#4916.
blob:
has to work, not sure aboutdata:
URLs again.