Unable to postMessage a WebAssembly module to a worklet #6596
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
EWS run on previous version of this PR (hash dd09a69)
|
webkit-ews-buildbot
added
the
merging-blocked
cdumez
force-pushed
the
220038_postMessage_wasm_mobule
branch
from
dd09a69
to
affd86c
Compare
|
EWS run on previous version of this PR (hash affd86c)
|
|
EWS run on previous version of this PR (hash affd86c) |
webkit-ews-buildbot
added
the
merging-blocked
cdumez
force-pushed
the
220038_postMessage_wasm_mobule
branch
from
affd86c
to
3f77422
Compare
|
EWS run on previous version of this PR (hash 3f77422) |
cdumez
commented
Nov 18, 2022
| TIMEOUT WebAssembly.Module cannot cross agent clusters, BroadcastChannel edition Test timed out | ||
|
|
||
| Harness Error (FAIL), message = Unhandled rejection: The object can not be cloned. | ||
| Harness Error (TIMEOUT), message = null |
There was a problem hiding this comment.
The failure tests cases are still failing. This is because they rely on the messageerror event which we don't support. When deserialization fails, we fire a regular message event but with data being null.
There was a problem hiding this comment.
Do you plan on adding messageerror?
There was a problem hiding this comment.
Yes, we need to add it. I have a work-in-progress patch for it but haven't gotten around to finish it yet.
There was a problem hiding this comment.
I have verified that with the messageerror event implemented, this test and the one below start passing.
My patch to support the messageerror event is almost ready.
|
EWS run on previous version of this PR (hash 3f77422)
|
cdumez
requested review from
Constellation,
youennf,
darinadler and
achristensen07
darinadler
approved these changes
Nov 18, 2022
| TIMEOUT WebAssembly.Module cannot cross agent clusters, BroadcastChannel edition Test timed out | ||
|
|
||
| Harness Error (FAIL), message = Unhandled rejection: The object can not be cloned. | ||
| Harness Error (TIMEOUT), message = null |
There was a problem hiding this comment.
Do you plan on adding messageerror?
| @@ -31,6 +31,7 @@ | |||
| #include <JavaScriptCore/JSGlobalObject.h> | |||
| #include <JavaScriptCore/JSObjectInlines.h> | |||
| #include <JavaScriptCore/WeakGCMap.h> | |||
| #include <wtf/text/WTFString.h> | |||
There was a problem hiding this comment.
A forward declaration usually suffices to just use something as a return type.
cdumez
force-pushed
the
220038_postMessage_wasm_mobule
branch
from
3f77422
to
284a06e
Compare
|
EWS run on current version of this PR (hash 284a06e) |
|
EWS run on current version of this PR (hash 284a06e) |
🧪 style
|
EWS run on current version of this PR (hash 284a06e) |
https://bugs.webkit.org/show_bug.cgi?id=220038 rdar://72596556 Reviewed by Darin Adler. We were previously unable to send WASM modules (and WASM memory) via message ports, even if the destination was in the same process (e.g. another window on the same page or a dedicated worker). There were two reasons for this: - MessagePort::postMessage() was using the default SerializationContext, which doesn't allow encoding on WASM types. I updated the serialization context to address that. - Such WASM types are not serializable over IPC and despite the changes in 255948@main, messages would often still go over IPC, even when the destination is in the same process. To address this, I now mark MessagePorts as out-of-process when they get sent over IPC, instead of doing it when they get disentangled (which can happen when sending the port to an in-process dedicated worker) * LayoutTests/fast/storage/serialized-script-value.html: * LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-postmessage-wasm-module.https-expected.txt: Added. * LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-postmessage-wasm-module.https.html: Added. * LayoutTests/http/wpt/webaudio/the-audio-api/the-audioworklet-interface/processors/wasm-module-processor.js: Added. (WASMModuleProcessor): (WASMModuleProcessor.prototype.testModule): (WASMModuleProcessor.prototype.handleMessage): (WASMModuleProcessor.prototype.handleMessageError): (WASMModuleProcessor.prototype.process): * LayoutTests/imported/w3c/web-platform-tests/wasm/serialization/module/window-messagechannel-success-expected.txt: * Source/WebCore/bindings/js/JSDOMGlobalObject.cpp: (WebCore::JSDOMGlobalObject::defaultAgentClusterID): (WebCore::JSDOMGlobalObject::agentClusterID const): * Source/WebCore/bindings/js/JSDOMGlobalObject.h: * Source/WebCore/bindings/js/SerializedScriptValue.cpp: (WebCore::agentClusterIDFromGlobalObject): (WebCore::CloneSerializer::dumpIfTerminal): (WebCore::CloneDeserializer::readTerminal): * Source/WebCore/dom/MessagePort.cpp: (WebCore::MessagePort::postMessage): * Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp: (WebKit::WebSWClientConnection::postMessageToServiceWorker): * Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp: (WebKit::WebSWContextManagerConnection::postMessageToServiceWorkerClient): * Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.cpp: (WebKit::WebMessagePortChannelProvider::entangleLocalPortInThisProcessToRemote): (WebKit::WebMessagePortChannelProvider::messagePortDisentangled): (WebKit::WebMessagePortChannelProvider::messagePortSentToRemote): (WebKit::WebMessagePortChannelProvider::postMessageToRemote): * Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.h: Canonical link: https://commits.webkit.org/256853@main
webkit-early-warning-system
force-pushed
the
220038_postMessage_wasm_mobule
branch
from
284a06e
to
a9d61c9
Compare
|
Committed 256853@main (a9d61c9): https://commits.webkit.org/256853@main Reviewed commits have been landed. Closing PR #6596 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
a9d61c9
284a06e
🧪 ios-wk2🧪 mac-wk1🧪 mac-AS-debug-wk2