Bug 1595285 - Fix TestUtils.waitForCondition to not use setInterval. r=mak

The test that is timing out with these patches does something relatively simple:

  await TestUtils.waitForCondition(async function() {
    let color = await ContentTask.spawn(browserWindow, async function() {
      /* Do stuff... */
    });
    return color == something;
  });

  await closeWindow(browserWindow);

Turns out that this can intermittently leak the window due to waitForCondition
using setInterval. setInterval can schedule multiple tasks while awaiting for
the inner ContentTask.

What this means, is that we may still have a ContentTask awaiting us when we get
to close the window. Closing the window makes the ContentTask not finish, and
thus we leak a promise keeping alive the window in gPromises:

  https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/testing/mochitest/BrowserTestUtils/ContentTask.jsm#24

Which means that we keep alive the window all the way until shutdown.

Fix it by ensuring that we only run one task at a time.

Differential Revision: https://phabricator.services.mozilla.com/D52833

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2019-11-13 18:24:02 +00:00
Родитель b77ab540b1
Коммит 2c2c3c23dd
1 изменённых файлов: 11 добавлений и 8 удалений

Просмотреть файл

@ -21,9 +21,7 @@
var EXPORTED_SYMBOLS = ["TestUtils"];
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
const { clearInterval, setInterval } = ChromeUtils.import(
"resource://gre/modules/Timer.jsm"
);
const { setTimeout } = ChromeUtils.import("resource://gre/modules/Timer.jsm");
var TestUtils = {
executeSoon(callbackFn) {
@ -163,13 +161,15 @@ var TestUtils = {
* @return Promise
* Resolves with the return value of the condition function.
* Rejects if timeout is exceeded or condition ever throws.
*
* NOTE: This is intentionally not using setInterval, using setTimeout
* instead. setInterval is not promise-safe.
*/
waitForCondition(condition, msg, interval = 100, maxTries = 50) {
return new Promise((resolve, reject) => {
let tries = 0;
let intervalID = setInterval(async function() {
async function tryOnce() {
if (tries >= maxTries) {
clearInterval(intervalID);
msg += ` - timed out after ${maxTries} tries.`;
reject(msg);
return;
@ -180,17 +180,20 @@ var TestUtils = {
conditionPassed = await condition();
} catch (e) {
msg += ` - threw exception: ${e}`;
clearInterval(intervalID);
reject(msg);
return;
}
if (conditionPassed) {
clearInterval(intervalID);
resolve(conditionPassed);
return;
}
tries++;
}, interval);
setTimeout(tryOnce, interval);
}
// FIXME(bug 1596165): This could be a direct call, ideally.
setTimeout(tryOnce, interval);
});
},