Bug 1828084 - Relax waitForMultiple ordering constraints. r=dom-worker-reviewers,jstutte

test_WorkerDebugger_frozen.xhtml had 2 bad things happening:
- It wasn't using `add_task` so thrown exceptions would not propagate to
  the test framework; the test is now modernized to use add_task.
- waitForMultiple was requiring an ordering which did not hold and the
  test had even been changed in a place to use `Promise.all()` already
  because of this.  waitForMultiple has been changed to Promise.all()
  because this seems to have been a systemic error.  In the cases where
  the ordering dependency was fine like pairs of waitForUnregister()
  and waitForDebuggerClose(), the specific ordering was meaningless.
  - I did not change all call-sites for blame reasons and to avoid
    perturbing the linter further.

Differential Revision: https://phabricator.services.mozilla.com/D191822
This commit is contained in:
Andrew Sutherland 2023-10-28 00:29:52 +00:00
parent 572daab0e8
commit 07c2f306cf
3 changed files with 12 additions and 33 deletions

View File

@ -82,7 +82,6 @@ skip-if = ["true"] # Frequent intermittent on QR platforms Bug 1454935
["test_WorkerDebugger_console.xhtml"]
["test_WorkerDebugger_frozen.xhtml"]
skip-if = ["debug"] # Bug 1828084
["test_WorkerDebugger_promise.xhtml"]

View File

@ -147,25 +147,10 @@ function waitForWorkerMessage(worker, message) {
}
function waitForMultiple(promises) {
return new Promise(function (resolve) {
let values = [];
for (let i = 0; i < promises.length; ++i) {
let index = i;
promises[i].then(function (value) {
is(
index + 1,
values.length + 1,
"Promise " +
(values.length + 1) +
" out of " +
promises.length +
" should be resolved."
);
values.push(value);
if (values.length === promises.length) {
resolve(values);
}
});
}
});
// There used to be old logic which expects promises to be resolved in
// succession, but where it seems like this was an incorrect assumption.
// Assuming this change sticks, bug 1861778 tracks removing this method
// entirely in favor of Promise.all at the call-sites or transform the callers
// into explicitly documented awaited sequences.
return Promise.all(promises);
}

View File

@ -4,8 +4,7 @@
http://creativecommons.org/publicdomain/zero/1.0/
-->
<window title="Test for WorkerDebugger with frozen workers"
xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
onload="test();">
xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
<script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
<script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"/>
@ -20,11 +19,9 @@
const WORKER1_URL = "WorkerDebugger_frozen_worker1.js";
const WORKER2_URL = "WorkerDebugger_frozen_worker2.js";
function test() {
(async function() {
SimpleTest.waitForExplicitFinish();
SpecialPowers.pushPrefEnv({set:
add_task(
async function runTest() {
await SpecialPowers.pushPrefEnv({set:
[["browser.sessionhistory.max_total_viewers", 10]]});
let promise = waitForMultiple([
@ -60,10 +57,8 @@
"debugger for worker on page 1 should not be closed");
is(dbg2.isClosed, true,
"debugger for worker on page 2 should be closed");
SimpleTest.finish();
})();
}
}
);
]]>
</script>