Bug 179263

Summary: WebAssembly: sending module to iframe fails
Product: WebKit Reporter: JF Bastien <jfbastien>
Component: WebAssemblyAssignee: GSkachkov <gskachkov>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, beidson, commit-queue, ews-watchlist, fpizlo, gskachkov, jfbastien, jlewis3, jsbell, keith_miller, mark.lam, msaboff, rmorisset, rniwa, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews101 for mac-elcapitan
none
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews112 for mac-elcapitan
none
Archive of layout-test-results from ews123 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Patch
none
Patch
none
Patch
none
Patch none

JF Bastien
Reported 2017-11-03 12:39:31 PDT
There's a WPT test here: https://github.com/w3c/web-platform-tests/blob/master/wasm/wasm_local_iframe_test.html Which does the following: <!DOCTYPE html> <script src="/resources/testharness.js"></script> <script src="/resources/testharnessreport.js"></script> <script src="resources/load_wasm.js"></script> <iframe src="resources/frame.html" id="iframe"></iframe> <script> promise_test(async function() { var mod = await createWasmModule(); assert_true(mod instanceof WebAssembly.Module); var ans = await new Promise((resolve, reject) => { var iframe = document.getElementById("iframe").contentWindow; iframe.postMessage(mod, '*'); window.addEventListener("message", (reply) => resolve(reply.data), false); }); assert_equals(ans, 43); }, "send wasm module to iframe"); </script> wpt.fyi says that we fail with: Failure message: promise_test: Unhandled rejection with value: object "DataCloneError (DOM Exception 25): The object can not be cloned." It seems like we should succeed? We can definitely postMessage to a Worker. I don't understand iframe enough to know why this fails, we should investigate.
Attachments
Patch (226.41 KB, patch)
2017-12-02 14:01 PST, GSkachkov 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews101 for mac-elcapitan (2.12 MB, application/zip)
2017-12-02 15:01 PST, EWS Watchlist 		no flags
Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (2.56 MB, application/zip)
2017-12-02 15:13 PST, EWS Watchlist 		no flags
Details
Archive of layout-test-results from ews112 for mac-elcapitan (2.97 MB, application/zip)
2017-12-02 15:38 PST, EWS Watchlist 		no flags
Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (20.53 MB, application/zip)
2017-12-02 15:39 PST, EWS Watchlist 		no flags
Details
Patch (223.90 KB, patch)
2017-12-02 15:53 PST, GSkachkov 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (23.51 MB, application/zip)
2017-12-02 17:31 PST, EWS Watchlist 		no flags
Details
Patch (225.27 KB, patch)
2017-12-03 11:03 PST, GSkachkov 		no flags
DetailsFormatted DiffDiff
Patch (225.48 KB, patch)
2017-12-04 23:03 PST, GSkachkov 		no flags
DetailsFormatted DiffDiff
Patch (228.16 KB, patch)
2017-12-06 14:25 PST, GSkachkov 		no flags
DetailsFormatted DiffDiff
Patch (228.50 KB, patch)
2017-12-21 14:19 PST, GSkachkov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.
GSkachkov
Comment 1 2017-12-01 00:53:51 PST
I've checked in Chrome and Firefox and they both pass this test. I'll look to this issue.
JF Bastien
Comment 2 2017-12-01 08:53:41 PST
(In reply to GSkachkov from comment #1) > I've checked in Chrome and Firefox and they both pass this test. I'll look > to this issue. I understand that they pass, I'm just not sure whether they're right or we are and if so why :-)
GSkachkov
Comment 3 2017-12-01 13:11:47 PST
(In reply to JF Bastien from comment #2) > (In reply to GSkachkov from comment #1) > > I've checked in Chrome and Firefox and they both pass this test. I'll look > > to this issue. > > I understand that they pass, I'm just not sure whether they're right or we > are and if so why :-) Ohh. I see what you mean, I'm not expert in iframe :( But here some thoughts: I think we also should allow postMessage to iframe because according to current spec: ``` A Module object is stateless and supports structured cloning which means that the compiled code can be stored in IndexedDB and/or shared between windows and workers via postMessage. ``` However by this https://github.com/WebKit/webkit/blob/master/Source/WebCore/bindings/js/SerializedScriptValue.cpp#L1069 we limited to post messages only to Workers. Also as I can we see SharedArrayBuffer can be shared between window and iframe(https://html.spec.whatwg.org/multipage/webappapis.html#integration-with-the-javascript-agent-cluster-formalism): ``` SharedArrayBuffer objects can be shared. ... A Window object A and the Window object of an iframe element that A created that could be same origin-domain with A. ``` So I would expect that we can do this for window and iframe for WebAssembly.Module
JF Bastien
Comment 4 2017-12-01 13:18:28 PST
I think you're right, I talked to Chris Dumez and he agrees.
Saam Barati
Comment 5 2017-12-01 13:20:20 PST
I also agree. The important thing here is we still prevent storing to indexDB since we don't have any way of doing that yet.
GSkachkov
Comment 6 2017-12-01 13:36:18 PST
Cool! I'll try to prepare fix for this. @Saam Barati, I see that we have test storage/indexeddb/resources/wasm-exceptions.js, that should protect us.
GSkachkov
Comment 7 2017-12-02 14:01:54 PST
Created attachment 328253 [details] Patch Patch uploaded
EWS Watchlist
Comment 8 2017-12-02 15:01:16 PST
Comment on attachment 328253 [details] Patch Attachment 328253 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5469942 New failing tests: workers/sab/postMessage-clones.html
EWS Watchlist
Comment 9 2017-12-02 15:01:17 PST
Created attachment 328256 [details] Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
EWS Watchlist
Comment 10 2017-12-02 15:13:16 PST
Comment on attachment 328253 [details] Patch Attachment 328253 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5469954 New failing tests: workers/sab/postMessage-clones.html
EWS Watchlist
Comment 11 2017-12-02 15:13:18 PST
Created attachment 328257 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
EWS Watchlist
Comment 12 2017-12-02 15:38:03 PST
Comment on attachment 328253 [details] Patch Attachment 328253 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5469975 New failing tests: workers/sab/postMessage-clones.html
EWS Watchlist
Comment 13 2017-12-02 15:38:05 PST
Created attachment 328259 [details] Archive of layout-test-results from ews112 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
EWS Watchlist
Comment 14 2017-12-02 15:39:26 PST
Comment on attachment 328253 [details] Patch Attachment 328253 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5469992 New failing tests: wasm/iframe-postmessage.html workers/sab/postMessage-clones.html wasm/window-postmessage.html
EWS Watchlist
Comment 15 2017-12-02 15:39:28 PST
Created attachment 328260 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
GSkachkov
Comment 16 2017-12-02 15:53:17 PST
Created attachment 328262 [details] Patch Fixed tests
EWS Watchlist
Comment 17 2017-12-02 17:31:29 PST
Comment on attachment 328262 [details] Patch Attachment 328262 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5470679 New failing tests: wasm/iframe-postmessage.html wasm/window-postmessage.html
EWS Watchlist
Comment 18 2017-12-02 17:31:31 PST
Created attachment 328268 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
JF Bastien
Comment 19 2017-12-02 21:20:59 PST
Looks like there are still 2 failures?
GSkachkov
Comment 20 2017-12-03 11:03:09 PST
Created attachment 328297 [details] Patch Skip wasm tests on ios simulator
GSkachkov
Comment 21 2017-12-03 11:05:10 PST
(In reply to JF Bastien from comment #19) > Looks like there are still 2 failures? Yeah, it appears that wasm does not work correctly on --ios-simulator.
JF Bastien
Comment 22 2017-12-04 16:55:26 PST
Comment on attachment 328297 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328297&action=review Looks good other than the iOS tests. > LayoutTests/platform/ios/TestExpectations:2909 > +wasm/window-postmessage.html [ Skip ] This seems wrong, no? We only want to skip on iOS simulator because wasm requires JIT, but not on iOS itself.
GSkachkov
Comment 23 2017-12-04 23:03:43 PST
Created attachment 328441 [details] Patch Fix comments
JF Bastien
Comment 24 2017-12-04 23:35:13 PST
Comment on attachment 328441 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328441&action=review r=me > Source/WebCore/ChangeLog:8 > + Allow use WebAssembly.Module as input paramters for postMessage Typo "paramters"
Saam Barati
Comment 25 2017-12-05 09:51:27 PST
Comment on attachment 328441 [details] Patch LGTM too. Can you also add a test where you postMessage from an iframe to its parent window?
GSkachkov
Comment 26 2017-12-06 14:25:12 PST
Created attachment 328628 [details] Patch Add one more test
JF Bastien
Comment 27 2017-12-07 09:19:56 PST
Comment on attachment 328628 [details] Patch Update lgtm as well.
WebKit Commit Bot
Comment 28 2017-12-07 16:56:23 PST
Comment on attachment 328628 [details] Patch Clearing flags on attachment: 328628 Committed r225656: <https://trac.webkit.org/changeset/225656>
WebKit Commit Bot
Comment 29 2017-12-07 16:56:25 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 30 2017-12-07 17:03:45 PST
<rdar://problem/35923865>
Matt Lewis
Comment 31 2017-12-20 18:07:58 PST
The following layout test is a flaky timeout on macOS: wasm/iframe-postmessage.html Probable cause: Most likely flaky from the time of being added in revision r225656 https://trac.webkit.org/changeset/225656/webkit Flakiness Dashboard: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=wasm%2Fiframe-postmessage.html https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK1%20(Tests)/r226203%20(2204)/results.html --- /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/wasm/iframe-postmessage-expected.txt +++ /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/wasm/iframe-postmessage-actual.txt @@ -1,3 +1,4 @@ +FAIL: Timed out waiting for notifyDone to be called Test that expected Wasm Module can be sent over iframe.postMessage. On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". @@ -6,5 +7,4 @@ PASS successfullyParsed is true TEST COMPLETE -PASS () => value is 35010
Matt Lewis
Comment 32 2017-12-20 18:38:06 PST
Reverted r225656 for reason: The test has been a flaky timout since being added. Committed r226210: <https://trac.webkit.org/changeset/226210>
GSkachkov
Comment 33 2017-12-20 23:35:37 PST
Ohh, I'll look to the issue. I think cause of flakiness is that I did wrong assumption and I use setTimeout in test, but in many cases iframe is not ready in time when we send postMessage :( I'll try to release fix
GSkachkov
Comment 34 2017-12-20 23:48:50 PST
(In reply to Matt Lewis from comment #32) > Reverted r225656 for reason: > > The test has been a flaky timout since being added. > > Committed r226210: <https://trac.webkit.org/changeset/226210> I could not find iframe-postMessage.html in rollback patch, that fails in tests
GSkachkov
Comment 35 2017-12-21 14:19:06 PST
Created attachment 330068 [details] Patch Fix flacky tests & checked with repeat 100 times
JF Bastien
Comment 36 2018-01-02 06:37:52 PST
Comment on attachment 330068 [details] Patch r=me
GSkachkov
Comment 37 2018-01-02 07:30:58 PST
(In reply to JF Bastien from comment #36) > Comment on attachment 330068 [details] > Patch > > r=me Thanks! I'll keep my eye on test https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=wasm%2Fiframe-postmessage.html
WebKit Commit Bot
Comment 38 2018-01-02 07:38:44 PST
Comment on attachment 330068 [details] Patch Clearing flags on attachment: 330068 Committed r226322: <https://trac.webkit.org/changeset/226322>
WebKit Commit Bot
Comment 39 2018-01-02 07:38:47 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.