Bug 1426467: Part 5: Mochitest: pause/unpause window while worker sends messages, without breaking run-to-completion. r=baku
authorJim Blandy <jimb@mozilla.com>
Mon, 22 Oct 2018 15:35:15 +0000
changeset 491173 7790c75ca3911352162feecf5b61dbe29233b259
parent 491172 1564ebaca63f8f796ababdcb61c7df09c189b3a9
child 491174 798b3777f30f4170fb094e1acce33eb70dcbd44a
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersbaku
bugs1426467
milestone65.0a1
Bug 1426467: Part 5: Mochitest: pause/unpause window while worker sends messages, without breaking run-to-completion. r=baku Detailed comments in the test itself. Differential Revision: https://phabricator.services.mozilla.com/D9221
devtools/server/tests/mochitest/chrome.ini
devtools/server/tests/mochitest/suspendTimeouts_content.html
devtools/server/tests/mochitest/suspendTimeouts_content.js
devtools/server/tests/mochitest/suspendTimeouts_worker.js
devtools/server/tests/mochitest/test_suspendTimeouts.html
devtools/server/tests/mochitest/test_suspendTimeouts.js
--- a/devtools/server/tests/mochitest/chrome.ini
+++ b/devtools/server/tests/mochitest/chrome.ini
@@ -21,19 +21,23 @@ support-files =
   inspector-search-data.html
   inspector-styles-data.css
   inspector-styles-data.html
   inspector-template.html
   inspector-traversal-data.html
   large-image.jpg
   memory-helpers.js
   nonchrome_unsafeDereference.html
+  suspendTimeouts_content.html
+  suspendTimeouts_content.js
+  suspendTimeouts_worker.js
   small-image.gif
   setup-in-child.js
   setup-in-parent.js
+  test_suspendTimeouts.js
   webconsole-helpers.js
   webextension-helpers.js
 [test_animation_actor-lifetime.html]
 [test_animation-type-longhand.html]
 [test_connection-manager.html]
 skip-if = (verify && debug && (os == 'win'))
 [test_connectToFrame.html]
 [test_css-logic.html]
@@ -108,8 +112,9 @@ skip-if = (verify && debug && (os == 'wi
 [test_styles-modify.html]
 [test_styles-svg.html]
 [test_unsafeDereference.html]
 [test_webconsole-node-grip.html]
 [test_webextension-addon-debugging-connect.html]
 disabled = true # Bug 1467313: broken, needs repair or removal
 [test_webextension-addon-debugging-reload.html]
 disabled = true # Bug 1467313: broken, needs repair or removal
+[test_suspendTimeouts.html]
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/mochitest/suspendTimeouts_content.html
@@ -0,0 +1,1 @@
+<script src='suspendTimeouts_content.js'></script>
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/mochitest/suspendTimeouts_content.js
@@ -0,0 +1,71 @@
+"use strict";
+
+// To make it easier to follow, this code is arranged so that the functions are
+// arranged in the order they are called.
+
+const worker = new Worker("suspendTimeouts_worker.js");
+worker.onerror = error => {
+  const message =
+      `error from worker: ${error.filename}:${error.lineno}: ${error.message}`;
+  throw new Error(message);
+};
+
+// Create a message channel. Send one end to the worker, and return the other to
+// the mochitest.
+/* exported create_channel */
+function create_channel() {
+  const { port1, port2 } = new MessageChannel();
+  info(`sending port to worker`);
+  worker.postMessage({ mochitestPort: port1 }, [ port1 ]);
+  return port2;
+}
+
+// Provoke the worker into sending us a message, and then refuse to receive said
+// message, causing it to be delayed for later delivery.
+//
+// The worker will also post a message to the MessagePort we sent it earlier.
+// That message should not be delayed, as it is handled by the mochitest window,
+// not the content window. Its receipt signals that the test can assume that the
+// runnable for step 2) is in the main thread's event queue, so the test can
+// prepare for step 3).
+/* exported start_worker */
+function start_worker() {
+  worker.onmessage = handle_echo;
+
+  // This should prevent worker.onmessage from being called, until
+  // resumeTimeouts is called.
+  //
+  // This function is provided by test_suspendTimeouts.js.
+  // eslint-disable-next-line no-undef
+  suspendTimeouts();
+
+  // The worker should echo this message back to us and to the mochitest.
+  worker.postMessage("HALLOOOOOO"); // suitable message for echoing
+  info(`posted message to worker`);
+}
+
+var resumeTimeouts_has_returned = false;
+
+// Resume timeouts. After this call, the worker's message should not be
+// delivered to our onmessage handler until control returns to the event loop.
+/* exported resume_timeouts */
+function resume_timeouts() {
+  // This function is provided by test_suspendTimeouts.js.
+  // eslint-disable-next-line no-undef
+  resumeTimeouts(); // onmessage handlers should not run from this call.
+
+  resumeTimeouts_has_returned = true;
+
+  // When this JavaScript invocation returns to the main thread's event loop,
+  // only then should onmessage handlers be invoked.
+}
+
+// The buggy code calls this handler from the resumeTimeouts call, before the
+// main thread returns to the event loop. The correct code calls this only once
+// the JavaScript invocation that called resumeTimeouts has run to completion.
+function handle_echo({data}) {
+  ok(resumeTimeouts_has_returned, "worker message delivered from main event loop");
+
+  // Finish the mochitest.
+  finish();
+}
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/mochitest/suspendTimeouts_worker.js
@@ -0,0 +1,12 @@
+"use strict";
+
+// Once content sends us a port connected to the mochitest, we simply echo every
+// message we receive back to content and the mochitest.
+onmessage = ({ data: { mochitestPort }}) => {
+  onmessage = ({ data }) => {
+    // Send a message to both content and the mochitest, which the main thread's
+    // event loop will attempt to deliver as step 2).
+    postMessage(`worker echo to content: ${data}`);
+    mochitestPort.postMessage(`worker echo to port: ${data}`);
+  };
+};
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/mochitest/test_suspendTimeouts.html
@@ -0,0 +1,20 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1426467
+
+When we use windowUtils.resumeTimeouts to resume timeouts in a window, that call
+should not immediately dispatch `onmessage` handlers for messages from workers.
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Mozilla Bug 1426467</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+</head>
+<body>
+<pre id="test">
+<script src='test_suspendTimeouts.js'></script>
+</pre>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/mochitest/test_suspendTimeouts.js
@@ -0,0 +1,136 @@
+"use strict";
+
+// The debugger uses nsIDOMWindowUtils::suspendTimeouts and ...::resumeTimeouts
+// to ensure that content event handlers do not run while a JavaScript
+// invocation is stepping or paused at a breakpoint. If a worker thread sends
+// messages to the content while the content is paused, those messages must not
+// run until the JavaScript invocation interrupted by the debugger has completed.
+//
+// Bug 1426467 is that calling nsIDOMWindowUtils::resumeTimeouts actually
+// delivers deferred messages itself, calling the content's 'onmessage' handler.
+// But the debugger calls suspend/resume around each individual interruption of
+// the debuggee -- each step, say -- meaning that hitting the "step into" button
+// causes you to step from the debuggee directly into an onmessage handler,
+// since the onmessage handler is the next function call the debugger sees.
+//
+// In other words, delivering deferred messages from resumeTimeouts, as it is
+// used by the debugger, breaks the run-to-completion rule. They must not be
+// delivered until after the JavaScript invocation at hand is complete. That's
+// what this test checks.
+//
+// For this test to detect the bug, the following steps must take place in
+// order:
+//
+// 1) The content page must call suspendTimeouts.
+// 2) A runnable conveying a message from the worker thread must attempt to
+//    deliver the message, see that the content page has suspended such things,
+//    and hold the message for later delivery.
+// 3) The content page must call resumeTimeouts.
+//
+// In a correct implementation, the message from the worker thread is delivered
+// only after the main thread returns to the event loop after calling
+// resumeTimeouts in step 3). In the buggy implementation, the onmessage handler
+// is called directly from the call to resumeTimeouts, so that the onmessage
+// handlers run in the midst of whatever JavaScript invocation resumed timeouts
+// (say, stepping in the debugger), in violation of the run-to-completion rule.
+//
+// In this specific bug, the handlers are called from resumeTimeouts, but
+// really, running them any time before that invocation returns to the main
+// event loop would be a bug.
+//
+// Posting the message and calling resumeTimeouts take place in different
+// threads, but if 2) and 3) don't occur in that order, the worker's message
+// will never be delayed and the test will pass spuriously. But the worker
+// can't communicate with the content page directly, to let it know that it
+// should proceed with step 3): the purpose of suspendTimeouts is to pause
+// all such communication.
+//
+// So instead, the content page creates a MessageChannel, and passes one
+// MessagePort to the worker and the other to this mochitest (which has its
+// own window, separate from the one calling suspendTimeouts). The worker
+// notifies the mochitest when it has posted the message, and then the
+// mochitest calls into the content to carry out step 3).
+
+// To help you follow all the callbacks and event handlers, this code pulls out
+// event handler functions so that control flows from top to bottom.
+
+window.onload = function() {
+  // This mochitest is not complete until we call SimpleTest.finish. Don't just
+  // exit as soon as we return to the main event loop.
+  SimpleTest.waitForExplicitFinish();
+
+  const iframe = document.createElement("iframe");
+  iframe.src = "http://mochi.test:8888/chrome/devtools/server/tests/mochitest/suspendTimeouts_content.html";
+  iframe.onload = iframe_onload_handler;
+  document.body.appendChild(iframe);
+
+  function iframe_onload_handler() {
+    const content = iframe.contentWindow.wrappedJSObject;
+
+    const windowUtils = iframe.contentWindow.windowUtils;
+
+    // Hand over the suspend and resume functions to the content page, along
+    // with some testing utilities.
+    content.suspendTimeouts = function() {
+      SimpleTest.info("test_suspendTimeouts", "calling suspendTimeouts");
+      windowUtils.suspendTimeouts();
+    };
+    content.resumeTimeouts = function() {
+      windowUtils.resumeTimeouts();
+      SimpleTest.info("test_suspendTimeouts", "resumeTimeouts called");
+    };
+    content.info = function(message) {
+      SimpleTest.info("suspendTimeouts_content.js", message);
+    };
+    content.ok = SimpleTest.ok;
+    content.finish = finish;
+
+    SimpleTest.info("Disappointed with National Tautology Day? Well, it is what it is.");
+
+    // Once the worker has sent a message to its parent (which should get delayed),
+    // it sends us a message directly on this channel.
+    const workerPort = content.create_channel();
+    workerPort.onmessage = handle_worker_echo;
+
+    // Have content send the worker a message that it should echo back to both
+    // content and us. The echo to content should get delayed; the echo to us
+    // should cause our handle_worker_echo to be called.
+    content.start_worker();
+
+    function handle_worker_echo({ data }) {
+      info(`mochitest received message from worker: ${data}`);
+
+      // As it turns out, it's not correct to assume that, if the worker posts a
+      // message to its parent via the global `postMessage` function, and then
+      // posts a message to the mochitest via the MessagePort, those two
+      // messages will be delivered in the order they were sent.
+      //
+      // - Messages sent via the worker's global's postMessage go through two
+      //   ThrottledEventQueues (one in the worker, and one on the parent), and
+      //   eventually find their way into the thread's primary event queue,
+      //   which is a PrioritizedEventQueue.
+      //
+      // - Messages sent via a MessageChannel whose ports are owned by different
+      //   threads are passed as IPDL messages.
+      //
+      // There's basically no reliable way to ensure that delivery to content
+      // has been attempted and the runnable deferred; there are too many
+      // variables affecting the order in which things are processed. Delaying
+      // for a second is the best I could think of.
+      //
+      // Fortunately, this tactic failing can only cause spurious test passes
+      // (the runnable never gets deferred, so things work by accident), not
+      // spurious failures. Without some kind of trustworthy notification that
+      // the runnable has been deferred, perhaps via some special white-box
+      // testing API, we can't do better.
+      setTimeout(() => {
+        content.resume_timeouts();
+      }, 1000);
+    }
+
+    function finish(message) {
+      SimpleTest.info("suspendTimeouts_content.js", "called finish");
+      SimpleTest.finish();
+    }
+  }
+};