Closed
Bug 1312817
Opened 8 years ago
Closed 8 years ago
support postMessage() of WebAssembly.Module
Categories
(Core :: DOM: Workers, defect)
Core
DOM: Workers
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: luke, Assigned: baku)
References
Details
Attachments
(2 files)
17.96 KB,
patch
|
luke
:
review+
qdot
:
review+
|
Details | Diff | Splinter Review |
1.08 KB,
patch
|
bbouvier
:
review+
baku
:
feedback+
|
Details | Diff | Splinter Review |
Bug 1276029 adds a threadsafe refcounted JS::WasmModule interface to JS API that allows WebAssembly.Module objects to be stored in an IDBObjectStore. To support full structured clone, we also need the structure clone integration to allow Module to be sent via postMessage().
Assignee | ||
Comment 1•8 years ago
|
||
There are many postMessages. window to window, window to worker, MessageChannel and BroadcastChannel. We divide them based on the 'contexts': SameProcessSameThread (window to window), SameProcessDifferentThread (workers) and DifferentProcess (MessageChannel and BroadcastChannel). I suspect we want to support the first 2 contexts. Am I right? This should be described in the spec, btw.
Reporter | ||
Comment 2•8 years ago
|
||
I think the first two contexts would be immediately useful and sufficient, so what we should do in this bug, but eventually I think it'd probably be good to allow structured-cloning between all contexts (since we have the ability to serialize). There isn't a fully explicit spec yet, we just have a hand-wavy paragraph saying "it should be structured cloneable, equivalent to if the original bytecode was copied and recompiled": https://github.com/WebAssembly/design/blob/master/JS.md#structured-clone-of-a-webassemblymodule Are there more details that a proper spec would need to fill in to describe? Do you have any links to examples to follow?
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → amarchesini
Assignee | ||
Comment 3•8 years ago
|
||
qdot: are you familiar with StructuredCloneHolder? like: it works \o/ but not window to iframe, because JS::IsWasmModuleObject(aObj) returns false. gdb says that it returns false because instead being a WasmModule, it is a ProxyObject. Should I unwrap something first?
Attachment #8805042 -
Flags: review?(luke)
Attachment #8805042 -
Flags: review?(kyle)
Reporter | ||
Comment 4•8 years ago
|
||
Comment on attachment 8805042 [details] [diff] [review] wasm.patch Review of attachment 8805042 [details] [diff] [review]: ----------------------------------------------------------------- Beautiful! I think we just need a CheckedUnwrap() in JS::IsWasmModuleObject/JS::GetWasmModule to handle the transparent-cross-compartment-wrapper case; I'll post a patch for that real quick and then perhaps you can validate with an iframe testcase?
Attachment #8805042 -
Flags: review?(luke) → review+
Reporter | ||
Comment 5•8 years ago
|
||
Attachment #8805100 -
Flags: review?(bbouvier)
Attachment #8805100 -
Flags: feedback?(amarchesini)
Updated•8 years ago
|
Attachment #8805100 -
Flags: review?(bbouvier) → review+
Assignee | ||
Updated•8 years ago
|
Attachment #8805100 -
Flags: feedback?(amarchesini) → feedback+
Comment 6•8 years ago
|
||
Comment on attachment 8805042 [details] [diff] [review] wasm.patch Review of attachment 8805042 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/StructuredCloneTags.h @@ +56,5 @@ > // This tag is used by both main thread and workers. > SCTAG_DOM_URLSEARCHPARAMS, > > + // Wasm Modules > + SCTAG_DOM_WASM_MODULE, Is this intentionally different than the SCTAG_DOM_WASM :janv added in bug 1311466? (It's up above because IndexedDB needs a stable tag identifier for persistence. Although based on your line numbers I think your checkout is not up-to-date and you need to rebase.)
Comment 7•8 years ago
|
||
Comment on attachment 8805042 [details] [diff] [review] wasm.patch Review of attachment 8805042 [details] [diff] [review]: ----------------------------------------------------------------- Haven't worked with StructuredClone too much, but I read through the patch and the current state of the file and this seems to make sense.
Attachment #8805042 -
Flags: review?(kyle) → review+
Assignee | ||
Comment 8•8 years ago
|
||
Thanks Andrew! Funny thing is that I introduced that tag, but I didn't know janv already landed those patches.
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/19e79becc540 support {window,worker}.postMessage() of WebAssembly.Module, r=qdot, r=luke
Assignee | ||
Comment 10•8 years ago
|
||
Luke, I landed my patch. When you want, can you land yours plus: https://hg.mozilla.org/integration/mozilla-inbound/file/tip/dom/base/test/test_postMessages.html#l371 this set to true?
Flags: needinfo?(luke)
Comment 11•8 years ago
|
||
Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/79635ac69f44 Backed out changeset 19e79becc540 for failures in own test on android
Comment 12•8 years ago
|
||
backed out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=38357030&repo=mozilla-inbound
Flags: needinfo?(amarchesini)
Comment 13•8 years ago
|
||
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/36deeea122f9 support {window,worker}.postMessage() of WebAssembly.Module, r=qdot, r=luke
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(amarchesini)
Comment 14•8 years ago
|
||
Pushed by lwagner@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/dc216bb3f2c7 Handle wrappers in JS::WasmModule APIs, enable {window,iframe}.postMessage() (r=bbouvier)
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/36deeea122f9 https://hg.mozilla.org/mozilla-central/rev/dc216bb3f2c7
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•