Bug 1145201: Fix xpcshell tests not to mix the test's own microtasks with the debuggee's. r=jlast
authorJim Blandy <jimb@mozilla.com>
Tue, 12 Feb 2019 08:20:29 +0000
changeset 458925 9a2412c232a5
parent 458924 8df956c34c94
child 458926 b745576a1639
push id35551
push usershindli@mozilla.com
push dateWed, 13 Feb 2019 21:34:09 +0000
treeherdermozilla-central@08f794a4928e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlast
bugs1145201
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1145201: Fix xpcshell tests not to mix the test's own microtasks with the debuggee's. r=jlast Evaluation of debuggee code should always begin with an empty microtask queue. In xpcshell tests, this is not guaranteed, as it is in the web platform. This patch changes those devtools server xpcshell tests that break this rule in a detectable way to run the debuggee code as a separate HTML task. In an actual browser environment, debuggee JavaScript runs as an HTML task. Since HTML requires a microtask checkpoint at the end of each task, this means that a debuggee task begins execution with an empty microtask queue, free of microtasks from other tabs or the browser machinery itself. Hence, while the debugger is pausing debuggee code, it is safe for it to save the debuggee's microtask queue, so that those jobs do not make progress. (Which is fortunate, because it *must* do so, lest the debuggee's microtasks run during the pause!) In an xpcshell test, however, there is no guarantee that debuggee code begins execution with a fresh microtask queue: the test may call `eval` or `evalInSandbox` at any time. If such an evaluation hits a breakpoint, `debugger` statement, etc. that invokes a Debugger hook, supervisory microtasks from the test harness code may be set aside along with the debuggee's microtasks. If the hook code then blocks waiting for those microtasks to run, the test will hang. Differential Revision: https://phabricator.services.mozilla.com/D18904
devtools/server/tests/unit/test_breakpoint-01.js
devtools/server/tests/unit/test_objectgrips-20.js
--- a/devtools/server/tests/unit/test_breakpoint-01.js
+++ b/devtools/server/tests/unit/test_breakpoint-01.js
@@ -32,18 +32,25 @@ add_task(threadClientTest(async ({ threa
 
     info("Check that the breakpoint worked.");
     Assert.equal(debuggee.a, 1);
     Assert.equal(debuggee.b, undefined);
 
     await threadClient.resume();
   })();
 
-  /* eslint-disable */
-  Cu.evalInSandbox(
-    "var line0 = Error().lineNumber;\n" +
-    "debugger;\n" +   // line0 + 1
-    "var a = 1;\n" +  // line0 + 2
-    "var b = 2;\n",   // line0 + 3
-     debuggee
-  );
-  /* eslint-enable */
+  /*
+   * Be sure to run debuggee code in its own HTML 'task', so that when we call
+   * the onDebuggerStatement hook, the test's own microtasks don't get suspended
+   * along with the debuggee's.
+   */
+  do_timeout(0, () => {
+    /* eslint-disable */
+    Cu.evalInSandbox(
+      "var line0 = Error().lineNumber;\n" +
+        "debugger;\n" +   // line0 + 1
+        "var a = 1;\n" +  // line0 + 2
+        "var b = 2;\n",   // line0 + 3
+        debuggee
+    );
+    /* eslint-enable */
+  });
 }));
--- a/devtools/server/tests/unit/test_objectgrips-20.js
+++ b/devtools/server/tests/unit/test_objectgrips-20.js
@@ -15,17 +15,17 @@ registerCleanupFunction(() => {
   Services.prefs.clearUserPref("security.allow_eval_with_system_principal");
 });
 
 add_task(threadClientTest(async ({ threadClient, debuggee, client }) => {
   debuggee.eval(function stopMe(arg1) {
     debugger;
   }.toString());
 
-  await Promise.all([{
+  const testCases = [{
     evaledObject: { a: 10 },
     expectedIndexedProperties: [],
     expectedNonIndexedProperties: [["a", 10]],
   }, {
     evaledObject: { length: 10 },
     expectedIndexedProperties: [],
     expectedNonIndexedProperties: [["length", 10]],
   }, {
@@ -161,19 +161,21 @@ add_task(threadClientTest(async ({ threa
   }, {
     evaledObject: `(() => {
       x = new Int32Array([1, 2]);
       Object.setPrototypeOf(x, null);
       return x;
     })()`,
     expectedIndexedProperties: [["0", 1], ["1", 2]],
     expectedNonIndexedProperties: [],
-  }].map(async (testData) => {
-    await test_object_grip(debuggee, client, threadClient, testData);
-  }));
+  }];
+
+  for (const test of testCases) {
+    await test_object_grip(debuggee, client, threadClient, test);
+  }
 }));
 
 async function test_object_grip(debuggee, dbgClient, threadClient, testData = {}) {
   const {
     evaledObject,
     expectedIndexedProperties,
     expectedNonIndexedProperties,
   } = testData;
@@ -199,23 +201,28 @@ async function test_object_grip(debuggee
 
       response = await objClient.enumProperties({ ignoreIndexedProperties: true });
       await check_enum_properties(response, expectedNonIndexedProperties);
 
       await threadClient.resume();
       resolve();
     });
 
-    debuggee.eval(`
-      stopMe(${
-        typeof evaledObject === "string"
-          ? evaledObject
-          : JSON.stringify(evaledObject)
-      });
-    `);
+    // Be sure to run debuggee code in its own HTML 'task', so that when we call
+    // the onDebuggerStatement hook, the test's own microtasks don't get suspended
+    // along with the debuggee's.
+    do_timeout(0, () => {
+      debuggee.eval(`
+        stopMe(${
+          typeof evaledObject === "string"
+            ? evaledObject
+            : JSON.stringify(evaledObject)
+        });
+      `);
+    });
   });
 }
 
 async function check_enum_properties(response, expected = []) {
   ok(response && Object.getOwnPropertyNames(response).includes("iterator"),
     "The response object has an iterator property");
 
   const {iterator} = response;