179263 – WebAssembly: sending module to iframe fails

RESOLVED FIXED 179263

WebAssembly: sending module to iframe fails

https://bugs.webkit.org/show_bug.cgi?id=179263

Summary WebAssembly: sending module to iframe fails

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	Details Formatted Diff Diff
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	Details Formatted Diff Diff
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	Details Formatted Diff Diff
Patch (225.48 KB, patch) 2017-12-04 23:03 PST, GSkachkov	no flags	Details Formatted Diff Diff
Patch (228.16 KB, patch) 2017-12-06 14:25 PST, GSkachkov	no flags	Details Formatted Diff Diff
Patch (228.50 KB, patch) 2017-12-21 14:19 PST, GSkachkov	no flags	Details Formatted Diff Diff
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.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component WebAssembly

Assignee

GSkachkov

Reported

2017-11-03 12:39 PDT

Modified

2018-01-02 07:38 PST History

CC List

16 users Show

URL

Keywords InRadar

Depends on

Blocks