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: whatwg/html#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: whatwg/html#7267 Bug: 1239274 Change-Id: Ia014e412a2dc8267aac57041672835fb90994d89 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3229181 Commit-Queue: Andrew Williams <[email protected]> Reviewed-by: Ben Kelly <[email protected]> Cr-Commit-Position: refs/heads/main@{#935672}
FrankEnderman · Oct 28, 2021 · 0be41dd · 0be41dd
1 parent ba3b972
commit 0be41dd
Show file tree

Hide file tree

Showing 9 changed files with 477 additions and 49 deletions.
diff --git a/...b_tests/external/wpt/webmessaging/broadcastchannel/detached-iframe.tentative-expected.txt b/...b_tests/external/wpt/webmessaging/broadcastchannel/detached-iframe.tentative-expected.txt
@@ -0,0 +1,9 @@
+This is a testharness.js-based test.
+FAIL BroadcastChannel messages from detached iframe to parent should be ignored (BC created before detaching) promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'postMessage' on 'BroadcastChannel': Channel is closed"
+FAIL BroadcastChannel messages from detached iframe to parent should be ignored (BC created after detaching) assert_equals: expected 1 but got 2
+PASS BroadcastChannel messages from parent to detached iframe should be ignored (BC created before detaching)
+PASS BroadcastChannel messages from parent to detached iframe should be ignored (BC created after detaching)
+FAIL BroadcastChannel messages within detached iframe should be ignored (BCs created before detaching) promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'postMessage' on 'BroadcastChannel': Channel is closed"
+PASS BroadcastChannel messages within detached iframe should be ignored (BCs created after detaching)
+Harness: the test ran to completion.
+
diff --git a/...blink/web_tests/external/wpt/webmessaging/broadcastchannel/detached-iframe.tentative.html b/...blink/web_tests/external/wpt/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/...ty/blink/web_tests/external/wpt/webmessaging/broadcastchannel/resources/service-worker.js b/...ty/blink/web_tests/external/wpt/webmessaging/broadcastchannel/resources/service-worker.js
@@ -9,6 +9,7 @@ bc3.onmessage = e => {
 };
 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/third_party/blink/web_tests/external/wpt/webmessaging/broadcastchannel/resources/worker.js b/third_party/blink/web_tests/external/wpt/webmessaging/broadcastchannel/resources/worker.js
@@ -16,6 +16,12 @@ function handler(e, reply) {
   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/third_party/blink/web_tests/external/wpt/webmessaging/broadcastchannel/workers.html b/third_party/blink/web_tests/external/wpt/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/...blink/web_tests/external/wpt/webmessaging/broadcastchannel/workers.tentative-expected.txt b/...blink/web_tests/external/wpt/webmessaging/broadcastchannel/workers.tentative-expected.txt
@@ -0,0 +1,10 @@
+This is a testharness.js-based test.
+PASS BroadcastChannel created after a worker self.close()
+FAIL BroadcastChannel messages from closed worker to parent should be ignored (BC created before closing) assert_equals: BroadcastChannel message should only be received from the second worker expected "done-worker done" but got "close-after-create-worker done"
+FAIL BroadcastChannel messages from closed worker to parent should be ignored (BC created after closing) assert_equals: BroadcastChannel message should only be received from the second worker expected "done-worker done" but got "close-before-create-worker done"
+PASS BroadcastChannel messages from parent to closed worker should be ignored (BC created before closing)
+PASS BroadcastChannel messages from parent to closed worker should be ignored (BC created after closing)
+PASS BroadcastChannel messages within closed worker should be ignored (BCs created before closing)
+PASS BroadcastChannel messages within closed worker should be ignored (BCs created after closing)
+Harness: the test ran to completion.
+