-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
WIP: SharedArrayBuffer structured cloning tests #5003
Conversation
Firefox (nightly channel)Testing web-platform-tests at revision 34ef0d4 All results17 tests ran/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/broadcastchannel-success-and-failure.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/broadcastchannel-success.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/identity-not-preserved.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/nested-worker-success-dedicatedworker.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/nested-worker-success-sharedworker.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/no-transferring.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-history.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-idb.any.worker.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-idb.any.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-notifications-api.any.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-notifications-api.any.worker.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-domain-success.sub.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-messagechannel-success.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-serviceworker-failure.https.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-sharedworker-failure.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-similar-but-cross-origin-success.sub.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-simple-success.html
|
Chrome (unstable channel)Testing web-platform-tests at revision 34ef0d4 All results17 tests ran/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/broadcastchannel-success-and-failure.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/broadcastchannel-success.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/identity-not-preserved.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/nested-worker-success-dedicatedworker.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/nested-worker-success-sharedworker.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/no-transferring.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-history.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-idb.any.worker.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-idb.any.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-notifications-api.any.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-notifications-api.any.worker.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-domain-success.sub.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-messagechannel-success.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-serviceworker-failure.https.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-sharedworker-failure.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-similar-but-cross-origin-success.sub.html
/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-simple-success.html
|
64f1136
to
6186300
Compare
These tests all look good. (I rebased them across the landing of css/.) To test the agent cluster boundary between Window objects we need #2669 fixed. But I suppose we can add a test where a Window attempts to share with a SharedWorker and vice versa. As I understand it there is supposed to be such a boundary there. For those tests I'll assume the @lars-t-hansen thoughts on other things to test? |
Moved the test plan to this thread's OP. We can of course land some tests before having exhaustive tests. |
Follows whatwg/html#2530. As explained there, the circumstances in which this would actually fire are not tested in this PR, waiting instead for #5003.
Follows whatwg/html#2530. As explained there, the circumstances in which this would actually be fired by the UA are not tested in this PR, waiting instead for #5003. As a side effect this syncs various IDL snippets with the HTML Standard.
c17e8f6
to
6d79a36
Compare
Tests are nearing completion (and running ahead of the spec a bit). The history/IDB/Notification tests have caught crash bugs in Chrome. In Firefox they are using TypeErrors instead of "DataCloneError" DOMExceptions; the latter seems better and is what I assume the spec will mandate. Note that a new test reveals that both Firefox and Chrome use similar-origin, not same-origin or same-origin-domain, for agent clusters. This is as specced. This revealed that we'll need to do some spec work for messageerror in service workers, as noted in whatwg/html#2530 (comment) . |
So TypeError follows from using IDL, but I guess we can use it for StructuredSerializeForStorage too for that specific case. |
Wait, why would it follow from IDL? history/IDB/Notifications use StructuredSerializeForStorage should do "DataCloneError" IMO. |
Oops, misread. If you pass this to XHR, TextDecoder, or Blob you'll get a TypeError though, I assume. |
Ah, yes. I hear @binji is working on tests for that as part of safelisting SAB for certain APIs. Which I still need to finish speccing, hrm. |
I added a bunch of Blink layout tests to make sure the SAB typed arrays throw. Some of them use the new WPT-like testing infrastructure, so shouldn't be too hard to port, but many others use various ad-hoc testing harnesses, so might be trickier. Didn't realize someone put my name in for doing it... :-) |
Added a BroadcastChannel test broadcasting to different iframes. In a now-familiar pattern, Firefox disallows and Chrome crashes. (It should work.)
I'm not sure such a test is possible. To go outside the agent cluster using BroadcastChannel, we need to use shared/service workers. (BroadcastChannel doesn't work cross-origin, so windows cannot work, #2669 notwithstanding.) But it seems like BroadcastChannel only works with "suspendable workers", and I don't know how to create one of those. |
@domenic you can have two same-origin Window objects that are not in the same cluster, but you need some privileged API to get that automated. |
Follows w3c/ServiceWorker#1130. The actual situations in which this will be called is being tested as part of #5003.
Follows whatwg/html#2530. As explained there, the circumstances in which this would actually be fired by the UA are not tested in this PR, waiting instead for #5003. As a side effect this syncs various IDL snippets with the HTML Standard.
Additionally, define StructuredSerializeForStorage when for serializable objects need to be stored more persistently. Tests: web-platform-tests/wpt#5003. Fixes #2260 (by being the last in a set of fixes). Closes tc39/proposal-ecmascript-sharedmem#144 and closes tc39/proposal-ecmascript-sharedmem#65.
Additionally, define StructuredSerializeForStorage when for serializable objects need to be stored more persistently. Tests: web-platform-tests/wpt#5003. Fixes #2260 (by being the last in a set of fixes). Closes tc39/proposal-ecmascript-sharedmem#144 and closes tc39/proposal-ecmascript-sharedmem#65.
Also, s/serialization-via-notifications-api.any.js/serialization-via-notifications-api-manual.any.js/ since permissions cannot be automated yet? |
LintPassed |
I've been using this for streams, but sure, I see your point. Renaming works fine.
Somehow I still got these tests to crash Chrome and produce an error in Firefox in their current form. I guess because per https://notifications.spec.whatwg.org/#constructors the "create a notification" step runs before the in-parallel permission business.
How are you sure that these two workers are "suspendable workers", per https://html.spec.whatwg.org/#dom-broadcastchannel-postmessage step 7 bullet 1 sub-bullet 2? Assumign the test is OK, can we also test the other properties of the MessageEvent for the messageerror case? |
It's not okay. I guess we can only test it once #2669 is fixed. I wonder though why BroadcastChannel basically cannot work in a shared worker. Seems like a bug of sorts. |
This reverts commit be5acc0.
Yeah, I think you're right that the Notification test is fine as automated, since we don't actually care about what happens with the permission thing. And spawning a permission dialog doesn't violate automation I think. |
I can't figure out how BroadcastChannel could work in workers, basically; I don't understand what a "suspendable worker" is supposed to map to in the real world, or why you might want to broadcast to them. |
I went digging, it was a typo that was introduced when this feature was added. Compare https://www.w3.org/Bugs/Public/show_bug.cgi?id=24414 with whatwg/html@300e7de. |
…ucceeds"" This reverts commit 916775e.
We now have limited coverage for all different views as well, including DataView, and including that they're successfully "cloning" their backing shared buffer. |
These tests are now available on w3c-test.org |
This LGTM to merge and file bugs. |
Additionally, define StructuredSerializeForStorage when for serializable objects need to be stored more persistently. Tests: web-platform-tests/wpt#5003. StructuredSerializeForStorage follow-up: * whatwg/notifications#99 * w3c/IndexedDB#197 Fixes #2260 (by being the last in a set of fixes). Closes tc39/proposal-ecmascript-sharedmem#144 and closes tc39/proposal-ecmascript-sharedmem#65.
Follows w3c/ServiceWorker#1130. The actual situations in which this will be called is being tested as part of #5003.
Additionally, define StructuredSerializeForStorage when for serializable objects need to be stored more persistently. Tests: web-platform-tests/wpt#5003. StructuredSerializeForStorage follow-up: * whatwg/notifications#99 * w3c/IndexedDB#197 Fixes whatwg#2260 (by being the last in a set of fixes). Closes tc39/proposal-ecmascript-sharedmem#144 and closes tc39/proposal-ecmascript-sharedmem#65.
Additionally, define StructuredSerializeForStorage when for serializable objects need to be stored more persistently. Tests: web-platform-tests/wpt#5003. StructuredSerializeForStorage follow-up: * whatwg/notifications#99 * w3c/IndexedDB#197 Fixes whatwg#2260 (by being the last in a set of fixes). Closes tc39/proposal-ecmascript-sharedmem#144 and closes tc39/proposal-ecmascript-sharedmem#65.
Additionally, define StructuredSerializeForStorage when for serializable objects need to be stored more persistently. Tests: web-platform-tests/wpt#5003. StructuredSerializeForStorage follow-up: * whatwg/notifications#99 * w3c/IndexedDB#197 Fixes whatwg#2260 (by being the last in a set of fixes). Closes tc39/proposal-ecmascript-sharedmem#144 and closes tc39/proposal-ecmascript-sharedmem#65.
This is for whatwg/html#2518 which is ready to land.
Test plan (please suggest more!):
Success cases (test that modifying on both sides is reflected on the other):
Failure cases that cause error behavior on the receiver side (messageerror event):
Failure cases that cause synchronous failure:
This change is