From 5755b029ed2276d98a9cf5c0e4e729e7cbbcd320 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Fri, 2 Nov 2018 16:54:28 -0400 Subject: [PATCH 1/2] [wptrunner] Transmit test results via postMessage A recent improvement to wptrunner modified the way data structures are transmitted from the browser to the WebDriver server [1]. One consequence of this change is that the data structures are created in a JavaScript realm which differs from the realm in which the "Execute Script" WebDriver command is running. As of version 17, Microsoft Edge does not correctly serialize cross-realm values, so referenced improvement unintentionally precluded testing in that browser. Transmit data structures between realms via the `postMessage` API to avoid this problem. [1] https://github.com/web-platform-tests/wpt/pull/13419 --- tools/wptrunner/wptrunner/executors/runner.js | 58 +++++++++---------- .../executors/testharness_webdriver.js | 4 -- .../executors/testharness_webdriver_resume.js | 25 +++++++- 3 files changed, 49 insertions(+), 38 deletions(-) diff --git a/tools/wptrunner/wptrunner/executors/runner.js b/tools/wptrunner/wptrunner/executors/runner.js index 8b800036756d01..2bce6473c7506f 100644 --- a/tools/wptrunner/wptrunner/executors/runner.js +++ b/tools/wptrunner/wptrunner/executors/runner.js @@ -1,42 +1,32 @@ document.title = '%(title)s'; -window.addEventListener( - "message", - function(event) { - window.message_queue.push(event); - window.process_next_event(); - }, - false -); +var message_queue = []; - -window.process_next_event = function() { - /* This function handles the next testdriver event. The presence of - window.testdriver_callback is used as a switch; when that function - is present we are able to handle the next event and when is is not - present we must wait. Therefore to drive the event processing, this - function must be called in two circumstances: - * Every time there is a new event that we may be able to handle - * Every time we set the callback function - This function unsets the callback, so no further testdriver actions - will be run until it is reset, which wptrunner does after it has - completed handling the current action. - */ - if (!window.testdriver_callback) { +/** + * In preparation to execute a test, testdriver issues a number of messages + * which should be queued until execution time. During execution, testdriver + * consumes the queued messages by repeatedly "resuming" via `do_testharness`. + */ +window.addEventListener("message", function(event) { + if (!event.data || !event.data.type) { return; } - var event = window.message_queue.shift(); - if (!event) { - return; + + if (event.data.type === "testdriver-resume") { + var next_message = message_queue.shift(); + reply(event.source, next_message); + } else { + message_queue.push(event.data); } - var data = event.data; +}, false); +function reply(source, data) { var payload = undefined; switch(data.type) { case "complete": - var tests = event.data.tests; - var status = event.data.status; + var tests = data.tests; + var status = data.status; var subtest_results = tests.map(function(x) { return [x.name, x.status, x.message, x.stack]; @@ -53,7 +43,11 @@ window.process_next_event = function() { default: return; } - var callback = window.testdriver_callback; - window.testdriver_callback = null; - callback([window.url, data.type, payload]); -}; + + source.postMessage({ + type: "testdriver-next message", + message: [window.url, data.type, payload] + }, + "*" + ); +} diff --git a/tools/wptrunner/wptrunner/executors/testharness_webdriver.js b/tools/wptrunner/wptrunner/executors/testharness_webdriver.js index 37023d81408d32..6640a0df8f6b48 100644 --- a/tools/wptrunner/wptrunner/executors/testharness_webdriver.js +++ b/tools/wptrunner/wptrunner/executors/testharness_webdriver.js @@ -8,10 +8,6 @@ window.win.addEventListener('DOMContentLoaded', (e) => { callback(); }); - -window.message_queue = []; -window.testdriver_callback = null; - if (%(timeout)s != null) { window.timer = setTimeout(function() { window.win.timeout(); diff --git a/tools/wptrunner/wptrunner/executors/testharness_webdriver_resume.js b/tools/wptrunner/wptrunner/executors/testharness_webdriver_resume.js index 279572bfe4d203..ff553f00afe137 100644 --- a/tools/wptrunner/wptrunner/executors/testharness_webdriver_resume.js +++ b/tools/wptrunner/wptrunner/executors/testharness_webdriver_resume.js @@ -1,3 +1,24 @@ var callback = arguments[arguments.length - 1]; -window.opener.testdriver_callback = callback; -window.opener.process_next_event(); + +window.addEventListener("message", function handle(event) { + if (!event.data || event.data.type !== "testdriver-next message") { + return; + } + + window.removeEventListener("message", handle); + + callback(event.data.message); +}); + +/** + * The current window and its opener belong to the same domain, making it + * technically possible for data structures to be shared directly. Some + * browser/WebDriver implementations [1] do not correctly serialize Arrays from + * a foreign realm. Transmitting data structures via the `postMessage` API + * avoids this issue because doing so involves de-serializing values in the + * local realm. + * + * [1] This has been observed in Edge version 17 and/or the corresponding + * release of Edgedriver + */ +window.opener.postMessage({ type: "testdriver-resume" }, "*"); From 47402eac1ec44479c6318943d90ea229cdcd34c6 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Fri, 2 Nov 2018 21:47:51 -0400 Subject: [PATCH 2/2] fixup! [wptrunner] Transmit test results via postMessage --- tools/wptrunner/wptrunner/executors/runner.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tools/wptrunner/wptrunner/executors/runner.js b/tools/wptrunner/wptrunner/executors/runner.js index 2bce6473c7506f..37cc73c5a965dc 100644 --- a/tools/wptrunner/wptrunner/executors/runner.js +++ b/tools/wptrunner/wptrunner/executors/runner.js @@ -1,23 +1,35 @@ document.title = '%(title)s'; var message_queue = []; +var source = null; /** * In preparation to execute a test, testdriver issues a number of messages * which should be queued until execution time. During execution, testdriver * consumes the queued messages by repeatedly "resuming" via `do_testharness`. + * The sequence of these events is variable, so the handler must account for + * cases where "resume" requests arrive before any messages are available (by + * caching the request's source) and where messages arrive before "resume" + * requests (by queuing the messages). */ window.addEventListener("message", function(event) { - if (!event.data || !event.data.type) { + if (!event.data) { return; } if (event.data.type === "testdriver-resume") { - var next_message = message_queue.shift(); - reply(event.source, next_message); + source = event.source; } else { message_queue.push(event.data); } + + if (!source || !message_queue.length) { + return; + } + + reply(source, message_queue[0]); + source = null; + message_queue.shift(); }, false); function reply(source, data) {