BroadcastChannel: Add WPTs for detached iframes, closed workers
This CL adds tentative WPTs that test whether BroadcastChannels in
detached iframes and closed workers behave according to proposed
changes to the specification. For more details, see:
https://github.com/whatwg/html/issues/7219
Note that the WPT CI found the existing closing/re-opening test
(and two of the new tests) to be flaky in Chrome. From
investigating further it seems there's a race condition when
instantiating and sending BroadcastChannel messages between
a worker and its parent such that using postMessage from either
side to indicate BroadcastChannel readiness is insufficient.
This CL also adds extra logic to mitigate this in the existing
test case (and the new ones). For more info, see:
https://github.com/whatwg/html/issues/7267
Bug: 1239274
Change-Id: Ia014e412a2dc8267aac57041672835fb90994d89
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3229181
Commit-Queue: Andrew Williams <awillia@google.com>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#935672}
diff --git a/webmessaging/broadcastchannel/detached-iframe.tentative.html b/webmessaging/broadcastchannel/detached-iframe.tentative.html
new file mode 100644
index 0000000..b9b06c3
--- /dev/null
+++ b/webmessaging/broadcastchannel/detached-iframe.tentative.html
@@ -0,0 +1,174 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/common/get-host-info.sub.js"></script>
+<!-- Pull in the with_iframe helper function from the service worker tests -->
+<script src="/service-workers/service-worker/resources/test-helpers.sub.js"></script>
+<body>
+<script>
+const TEST_IFRAME_CLASS_NAME = 'test-iframe';
+const events = [];
+var bc1;
+const DONE_MSG = 'done';
+
+function DetachedIframeTestCheckForOneMessage(t) {
+ return new Promise((resolve) => {
+ bc1.onmessage = t.step_func(e => {
+ events.push(e);
+ if (e.data == DONE_MSG) {
+ assert_equals(events.length, 1);
+ resolve();
+ }
+ });
+ });
+}
+
+const IframeAction = {
+ REMOVE_BEFORE_CREATION: 'remove-before-creation',
+ REMOVE_AFTER_CREATION: 'remove-after-creation',
+};
+
+async function doMessageSentTest(t, channelName, action) {
+ await with_iframe('about:blank');
+ const iframe = document.getElementsByClassName(TEST_IFRAME_CLASS_NAME)[0];
+ const iframe_BroadcastChannel = iframe.contentWindow.BroadcastChannel;
+
+ if (action === IframeAction.REMOVE_BEFORE_CREATION) {
+ iframe.remove();
+ }
+
+ events.length = 0;
+
+ bc1 = new BroadcastChannel(channelName);
+ const bc2 = new BroadcastChannel(channelName);
+ const iframe_bc = new iframe_BroadcastChannel(channelName);
+
+ if (action === IframeAction.REMOVE_AFTER_CREATION) {
+ iframe.remove();
+ }
+
+ const testResultsPromise = DetachedIframeTestCheckForOneMessage(t);
+
+ iframe_bc.postMessage('test');
+ bc2.postMessage(DONE_MSG);
+
+ bc2.close();
+ iframe_bc.close();
+ t.add_cleanup(() => bc1.close());
+
+ return testResultsPromise;
+}
+
+promise_test(async t => {
+ return doMessageSentTest(
+ t, 'postMessage-from-detached-iframe-pre',
+ IframeAction.REMOVE_AFTER_CREATION);
+}, 'BroadcastChannel messages from detached iframe to parent should be ignored (BC created before detaching)');
+
+promise_test(async t => {
+ return doMessageSentTest(
+ t, 'postMessage-from-detached-iframe-post',
+ IframeAction.REMOVE_BEFORE_CREATION);
+}, 'BroadcastChannel messages from detached iframe to parent should be ignored (BC created after detaching)');
+
+
+async function doMessageReceivedTest(t, channelName, action) {
+ await with_iframe('about:blank');
+ const iframe = document.getElementsByClassName(TEST_IFRAME_CLASS_NAME)[0];
+ const iframe_BroadcastChannel = iframe.contentWindow.BroadcastChannel;
+
+ if (action === IframeAction.REMOVE_BEFORE_CREATION) {
+ iframe.remove();
+ }
+
+ events.length = 0;
+
+ // `iframe_bc` must be created first so that it receives messages before
+ // `bc1`. That way we can tell whether `iframe_bc` received a message by
+ // inspecting `events` in the `bc1` message handler.
+ const iframe_bc = new iframe_BroadcastChannel(channelName);
+ iframe_bc.onmessage = e => {
+ events.push(e)
+ };
+ bc1 = new BroadcastChannel(channelName);
+ const bc2 = new BroadcastChannel(channelName);
+
+ if (action === IframeAction.REMOVE_AFTER_CREATION) {
+ iframe.remove();
+ }
+
+ const testResultsPromise = DetachedIframeTestCheckForOneMessage(t);
+ bc2.postMessage(DONE_MSG);
+
+ bc2.close();
+ iframe_bc.close();
+ t.add_cleanup(() => bc1.close());
+}
+
+promise_test(async t => {
+ return doMessageReceivedTest(
+ t, 'postMessage-to-detached-iframe-pre',
+ IframeAction.REMOVE_AFTER_CREATION);
+}, 'BroadcastChannel messages from parent to detached iframe should be ignored (BC created before detaching)');
+
+promise_test(async t => {
+ return doMessageReceivedTest(
+ t, 'postMessage-to-detached-iframe-post',
+ IframeAction.REMOVE_BEFORE_CREATION);
+}, 'BroadcastChannel messages from parent to detached iframe should be ignored (BC created after detaching)');
+
+
+async function doMessageSendReceiveTest(t, channelName, action) {
+ await with_iframe('about:blank');
+ const iframe = document.getElementsByClassName(TEST_IFRAME_CLASS_NAME)[0];
+ const iframe_BroadcastChannel = iframe.contentWindow.BroadcastChannel;
+
+ if (action === IframeAction.REMOVE_BEFORE_CREATION) {
+ iframe.remove();
+ }
+
+ const iframe_bc1 = new iframe_BroadcastChannel(channelName);
+ const iframe_bc2 = new iframe_BroadcastChannel(channelName);
+ iframe_bc1.onmessage = t.unreached_func(
+ 'Detached iframe BroadcastChannel instance received message unexpectedly');
+
+ if (action === IframeAction.REMOVE_AFTER_CREATION) {
+ iframe.remove();
+ }
+
+ iframe_bc2.postMessage(DONE_MSG);
+
+ iframe_bc2.close();
+ t.add_cleanup(() => iframe_bc1.close());
+
+ // To avoid calling t.step_timeout here, instead just create two new
+ // BroadcastChannel instances and complete the test when a message is passed
+ // between them. Per the spec, all "BroadcastChannel objects whose relevant
+ // agents are the same" must have messages delivered to them in creation
+ // order, so if we get this message then it's safe to assume the earlier
+ // message would have been delivered if it was going to be.
+ const bc1 = new BroadcastChannel(channelName);
+ const bc2 = new BroadcastChannel(channelName);
+ return new Promise((resolve) => {
+ bc1.onmessage = t.step_func(e => {
+ resolve();
+ });
+ bc2.postMessage(DONE_MSG);
+ });
+}
+
+promise_test(async t => {
+ return doMessageSendReceiveTest(
+ t, 'postMessage-within-detached-iframe-pre',
+ IframeAction.REMOVE_AFTER_CREATION);
+}, 'BroadcastChannel messages within detached iframe should be ignored (BCs created before detaching)');
+
+promise_test(async t => {
+ return doMessageSendReceiveTest(
+ t, 'postMessage-within-detached-iframe-post',
+ IframeAction.REMOVE_BEFORE_CREATION);
+}, 'BroadcastChannel messages within detached iframe should be ignored (BCs created after detaching)');
+
+</script>
+</body>
diff --git a/webmessaging/broadcastchannel/resources/service-worker.js b/webmessaging/broadcastchannel/resources/service-worker.js
index fc37aede..a3d17b9 100644
--- a/webmessaging/broadcastchannel/resources/service-worker.js
+++ b/webmessaging/broadcastchannel/resources/service-worker.js
@@ -9,6 +9,7 @@
};
bc3.postMessage('from worker');
+// Ensure that the worker stays alive for the duration of the test
self.addEventListener('install', evt => {
evt.waitUntil(promise);
});
diff --git a/webmessaging/broadcastchannel/resources/worker.js b/webmessaging/broadcastchannel/resources/worker.js
index 9364c60..df23072 100644
--- a/webmessaging/broadcastchannel/resources/worker.js
+++ b/webmessaging/broadcastchannel/resources/worker.js
@@ -16,6 +16,12 @@
c = new BroadcastChannel(e.data.channel);
let messages = [];
c.onmessage = e => {
+ if (e.data === 'ready') {
+ // Ignore any 'ready' messages from the other thread since there could
+ // be some race conditions between this BroadcastChannel instance
+ // being created / ready to receive messages and the message being sent.
+ return;
+ }
messages.push(e.data);
if (e.data == 'done')
reply(messages);
diff --git a/webmessaging/broadcastchannel/workers.html b/webmessaging/broadcastchannel/workers.html
index 1d05feb..16fea06 100644
--- a/webmessaging/broadcastchannel/workers.html
+++ b/webmessaging/broadcastchannel/workers.html
@@ -80,11 +80,14 @@
let worker = new Worker('resources/worker.js');
worker.onmessage = t.step_func(e => {
- assert_array_equals(events, ['c1: from worker', 'c2: echo'],
+ assert_array_equals(events,
+ ['c1: from worker', 'c2: ready', 'c2: echo'],
'messages in document');
assert_array_equals(e.data, ['done'], 'messages in worker');
t.done();
});
+ worker.onmessagerror =
+ t.unreached_func('Worker\'s onmessageerror handler called');
c.addEventListener('message', e => {
if (e.data == 'from worker') {
@@ -94,48 +97,30 @@
let c2 = new BroadcastChannel('worker-close');
c2.onmessage = e => {
events.push('c2: ' + e.data);
- c2.postMessage('done');
+ if (e.data === 'ready') {
+ worker.postMessage({ping: 'echo'});
+ } else {
+ c2.postMessage('done');
+ c2.close();
+ }
};
- worker.postMessage({ping: 'echo'});
+ // For some implementations there may be a race condition between
+ // when the BroadcastChannel instance above is created / ready to
+ // receive messages and when the worker calls postMessage on it's
+ // BroadcastChannel instance. To avoid this, confirm that our
+ // instance can receive a message before indicating to the other
+ // thread that we are ready. For more details, see:
+ // https://github.com/whatwg/html/issues/7267
+ let c3 = new BroadcastChannel('worker-close');
+ c3.postMessage('ready');
+ c3.close();
}, 1);
}
});
worker.postMessage({channel: 'worker-close'});
+ t.add_cleanup(() => worker.terminate());
}, 'Closing and re-opening a channel works.');
-async_test(t => {
- function workerCode() {
- close();
- var bc = new BroadcastChannel('worker-test');
- postMessage(true);
- }
-
- var workerBlob = new Blob([workerCode.toString() + ";workerCode();"], {type:"application/javascript"});
-
- var w = new Worker(URL.createObjectURL(workerBlob));
- w.onmessage = function(e) {
- assert_true(e.data, "BroadcastChannel created on worker shutdown.");
- t.done();
- }
-}, 'BroadcastChannel created after a worker self.close()');
-
-async_test(t => {
- function workerCode() {
- close();
- var bc = new BroadcastChannel('worker-test-after-close');
- bc.postMessage(true);
- }
-
- var bc = new BroadcastChannel('worker-test-after-close');
- bc.onmessage = function(e) {
- assert_true(e.data, "BroadcastChannel created on worker shutdown.");
- t.done();
- }
-
- var workerBlob = new Blob([workerCode.toString() + ";workerCode();"], {type:"application/javascript"});
- new Worker(URL.createObjectURL(workerBlob));
-}, 'BroadcastChannel used after a worker self.close()');
-
</script>
diff --git a/webmessaging/broadcastchannel/workers.tentative.html b/webmessaging/broadcastchannel/workers.tentative.html
new file mode 100644
index 0000000..e72fa36
--- /dev/null
+++ b/webmessaging/broadcastchannel/workers.tentative.html
@@ -0,0 +1,256 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+
+async_test(t => {
+ function workerCode() {
+ close();
+ try {
+ var bc = new BroadcastChannel('worker-create-after-close');
+ } catch (e) {
+ postMessage(e);
+ return;
+ }
+ postMessage(true);
+ }
+
+ var workerBlob = new Blob(
+ [workerCode.toString() + ';workerCode();'],
+ {type: 'application/javascript'});
+
+ var w = new Worker(URL.createObjectURL(workerBlob));
+ w.onmessage = t.step_func_done(function(e) {
+ assert_equals(
+ e.data, true,
+ 'BroadcastChannel creation in closed worker triggered exception: ' +
+ e.data.message);
+ });
+ t.add_cleanup(() => w.terminate());
+}, 'BroadcastChannel created after a worker self.close()');
+
+
+function postMessageFromWorkerWorkerCode(workerName, channelName) {
+ if (workerName === 'close-before-create-worker') {
+ close();
+ }
+ let bc = new BroadcastChannel(channelName);
+ if (workerName === 'close-after-create-worker') {
+ close();
+ }
+ bc.postMessage(workerName + ' done');
+ postMessage(true);
+}
+
+function doPostMessageFromWorkerTest(t, workerName, channelName) {
+ var bc = new BroadcastChannel(channelName);
+ bc.onmessage = t.step_func_done(function(e) {
+ assert_equals(
+ e.data, 'done-worker done',
+ 'BroadcastChannel message should only be received from the second worker');
+ });
+ t.add_cleanup(() => bc.close());
+
+ var testMessageHandler = t.step_func(function(e) {
+ assert_equals(
+ e.data, true,
+ 'Worker sent postMessage indicating it sent a BroadcastChannel message');
+
+ var w = createWorker(
+ postMessageFromWorkerWorkerCode, 'done-worker', channelName);
+ t.add_cleanup(() => w.terminate());
+ });
+ createWorker(
+ postMessageFromWorkerWorkerCode, workerName, channelName,
+ testMessageHandler);
+
+ // To avoid calling t.step_timeout here, have the worker postMessage(true)
+ // once it is finished and then we'll instantiate another worker that
+ // performs the same test steps but doesn't close. By the time the
+ // BroadcastChannel message in that worker gets sent successfully it should
+ // be safe to assume that any BroadcastChannel messages from the previous
+ // worker would have been sent if they were going to be.
+}
+
+function createWorker(workerCode, workerName, channelName, handler = null) {
+ var workerCodeStr = workerCode.toString() +
+ `;${workerCode.name}("${workerName}", "${channelName}");`;
+ var workerBlob = new Blob([workerCodeStr], {type: 'application/javascript'});
+ var w = new Worker(URL.createObjectURL(workerBlob));
+ if (handler !== null) {
+ w.onmessage = handler;
+ }
+ return w;
+}
+
+async_test(t => {
+ const workerName = 'close-after-create-worker';
+ const channelName = workerName + '-postmessage-from-worker';
+ doPostMessageFromWorkerTest(t, workerName, channelName);
+}, 'BroadcastChannel messages from closed worker to parent should be ignored (BC created before closing)');
+
+async_test(t => {
+ const workerName = 'close-before-create-worker';
+ const channelName = workerName + '-postmessage-from-worker';
+ doPostMessageFromWorkerTest(t, workerName, channelName);
+}, 'BroadcastChannel messages from closed worker to parent should be ignored (BC created after closing)');
+
+
+function postMessageToWorkerWorkerCode(workerName, channelName) {
+ self.addEventListener('message', () => {
+ if (workerName === 'close-before-create-worker') {
+ close();
+ }
+ try {
+ let bc1 = new BroadcastChannel(channelName);
+ bc1.onmessage = e => {
+ if (e.data === 'ready') {
+ postMessage(e.data);
+ } else if (e.data === 'test') {
+ postMessage(workerName + ' done');
+ }
+ };
+ bc1.onmessageerror = () => {
+ postMessage('onmessageerror called from worker BroadcastChannel');
+ };
+ if (workerName === 'close-after-create-worker') {
+ close();
+ }
+ } catch (e) {
+ postMessage(e);
+ return;
+ }
+
+ if (workerName === 'done-worker') {
+ // For some implementations there may be a race condition between when
+ // the BroadcastChannel instance above is created / ready to receive
+ // messages and when the parent calls postMessage on it's
+ // BroadcastChannel instance. To avoid this, confirm that our instance
+ // can receive a message before indicating to the other thread that we
+ // are ready. For more details, see:
+ // https://github.com/whatwg/html/issues/7267
+ let bc2 = new BroadcastChannel(channelName);
+ bc2.postMessage('ready');
+ bc2.close();
+ } else {
+ // Since the worker has closed, it's not expected that the
+ // BroadcastChannel will receive messages (there's a separate test for
+ // that), so just indicate directly that it's ready to test receiving
+ // a message from the parent dispite the possibility of a race condition.
+ postMessage('ready');
+ }
+ });
+ self.addEventListener('messageerror', () => {
+ postMessage('onmessageerror called from worker');
+ });
+}
+
+function doPostMessageToWorkerTest(t, workerName, channelName) {
+ var bc = new BroadcastChannel(channelName);
+ t.add_cleanup(() => bc.close());
+
+ var doneMessageHandler = t.step_func(function(e) {
+ if (e.data === 'ready') {
+ bc.postMessage('test');
+ } else if (e.data === 'done-worker done') {
+ t.done();
+ } else {
+ assert_unreached(
+ 'BroadcastChannel.postMessage triggered exception within second worker: ' +
+ e.data.message);
+ }
+ });
+ var testMessageHandler = t.step_func(function(e) {
+ assert_equals(
+ e.data, 'ready',
+ 'Worker sent postMessage indicating its BroadcastChannel instance is ready');
+ bc.postMessage('test');
+
+ var doneWorker = createWorker(
+ postMessageToWorkerWorkerCode, 'done-worker', channelName,
+ doneMessageHandler);
+ t.add_cleanup(() => {
+ doneWorker.terminate();
+ });
+ doneWorker.postMessage('start');
+ });
+ var testWorker = createWorker(
+ postMessageToWorkerWorkerCode, workerName, channelName,
+ testMessageHandler);
+ testWorker.postMessage('start');
+}
+
+async_test(t => {
+ const workerName = 'close-after-create-worker';
+ const channelName = workerName + '-postmessage-to-worker';
+ doPostMessageToWorkerTest(t, workerName, channelName);
+}, 'BroadcastChannel messages from parent to closed worker should be ignored (BC created before closing)');
+
+async_test(t => {
+ const workerName = 'close-before-create-worker';
+ const channelName = workerName + '-postmessage-to-worker';
+ doPostMessageToWorkerTest(t, workerName, channelName);
+}, 'BroadcastChannel messages from parent to closed worker should be ignored (BC created after closing)');
+
+
+function postMessageWithinWorkerWorkerCode(workerName, channelName) {
+ if (workerName === 'close-before-create-worker') {
+ close();
+ }
+ try {
+ let bc1 = new BroadcastChannel(channelName);
+ let bc2 = new BroadcastChannel(channelName);
+ bc1.onmessage = e => {
+ postMessage(workerName + ' done')
+ };
+ if (workerName === 'close-after-create-worker') {
+ close();
+ }
+ bc2.postMessage(true);
+ postMessage(true);
+ } catch (e) {
+ postMessage(e);
+ }
+}
+
+function doPostMessageWithinWorkerTest(t, workerName, channelName) {
+ var doneMessageHandler = t.step_func(function(e) {
+ if (e.data === true) {
+ // Done worker has finished - no action needed
+ } else if (e.data === 'done-worker done') {
+ t.done();
+ } else {
+ assert_unreached(
+ 'BroadcastChannel.postMessage triggered exception within second worker: ' +
+ e.data.message);
+ }
+ });
+ var testMessageHandler = t.step_func(function(e) {
+ assert_equals(
+ e.data, true,
+ 'Worker indicated that the test procedures were executed successfully');
+
+ var w = createWorker(
+ postMessageWithinWorkerWorkerCode, 'done-worker', channelName,
+ doneMessageHandler);
+ t.add_cleanup(() => w.terminate());
+ });
+ createWorker(
+ postMessageWithinWorkerWorkerCode, workerName, channelName,
+ testMessageHandler);
+}
+
+async_test(t => {
+ const workerName = 'close-after-create-worker';
+ const channelName = workerName + '-postmessage-within-worker';
+ doPostMessageWithinWorkerTest(t, workerName, channelName);
+}, 'BroadcastChannel messages within closed worker should be ignored (BCs created before closing)');
+
+async_test(t => {
+ const workerName = 'close-before-create-worker';
+ const channelName = workerName + '-postmessage-within-worker';
+ doPostMessageWithinWorkerTest(t, workerName, channelName);
+}, 'BroadcastChannel messages within closed worker should be ignored (BCs created after closing)');
+
+</script>
