From 33d52ed26a9dba459619e908b866f700840d0459 Mon Sep 17 00:00:00 2001 From: Aleksandr Guidrevitch Date: Sun, 18 Aug 2024 18:28:51 +0200 Subject: [PATCH 1/3] checkForQuiet bugfix to properly handle setTimeout() --- core/gather/driver/wait-for-condition.js | 48 +++++++++++++++--------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/core/gather/driver/wait-for-condition.js b/core/gather/driver/wait-for-condition.js index ac5b57f18b14..83b5bff22528 100644 --- a/core/gather/driver/wait-for-condition.js +++ b/core/gather/driver/wait-for-condition.js @@ -231,26 +231,37 @@ function waitForCPUIdle(session, waitForCPUQuiet) { /** * @param {ExecutionContext} executionContext - * @param {() => void} resolve * @return {Promise} */ - async function checkForQuiet(executionContext, resolve) { - if (canceled) return; - const timeSinceLongTask = - await executionContext.evaluate( - checkTimeSinceLastLongTaskInPage, {args: [], useIsolation: true}); - if (canceled) return; - - if (typeof timeSinceLongTask === 'number') { - if (timeSinceLongTask >= waitForCPUQuiet) { - log.verbose('waitFor', `CPU has been idle for ${timeSinceLongTask} ms`); - resolve(); - } else { - log.verbose('waitFor', `CPU has been idle for ${timeSinceLongTask} ms`); - const timeToWait = waitForCPUQuiet - timeSinceLongTask; - lastTimeout = setTimeout(() => checkForQuiet(executionContext, resolve), timeToWait); - } + function checkForQuiet(executionContext) { + if (canceled) { + return Promise.resolve(); } + + return executionContext.evaluate( + checkTimeSinceLastLongTaskInPage, {args: [], useIsolation: true}) + .then((timeSinceLongTask) => { + if (canceled) { + return; + } + + if (typeof timeSinceLongTask === 'number') { + if (timeSinceLongTask >= waitForCPUQuiet) { + log.verbose('waitFor', `CPU has been idle for ${timeSinceLongTask} ms`); + return; + } else { + log.verbose('waitFor', `CPU has been idle for ${timeSinceLongTask} ms`); + const timeToWait = waitForCPUQuiet - timeSinceLongTask; + return new Promise((resolve, reject) => { + lastTimeout = setTimeout(() => { + checkForQuiet(executionContext) + .then(resolve) + .catch(reject); + }, timeToWait); + }); + } + } + }); } /** @type {(() => void)} */ @@ -262,7 +273,8 @@ function waitForCPUIdle(session, waitForCPUQuiet) { /** @type {Promise} */ const promise = new Promise((resolve, reject) => { executionContext.evaluate(registerPerformanceObserverInPage, {args: [], useIsolation: true}) - .then(() => checkForQuiet(executionContext, resolve)) + .then(() => checkForQuiet(executionContext)) + .then(resolve) .catch(reject); cancel = () => { if (canceled) return; From 400ce2bf0103b08ef89e317f197726901e9b62f0 Mon Sep 17 00:00:00 2001 From: Aleksandr Guidrevitch Date: Sat, 31 Aug 2024 17:38:30 +0200 Subject: [PATCH 2/3] proper fix --- core/gather/driver/wait-for-condition.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/core/gather/driver/wait-for-condition.js b/core/gather/driver/wait-for-condition.js index 83b5bff22528..2e6d5d34608a 100644 --- a/core/gather/driver/wait-for-condition.js +++ b/core/gather/driver/wait-for-condition.js @@ -225,8 +225,6 @@ function waitForCPUIdle(session, waitForCPUQuiet) { }; } - /** @type {NodeJS.Timeout|undefined} */ - let lastTimeout; let canceled = false; /** @@ -253,7 +251,11 @@ function waitForCPUIdle(session, waitForCPUQuiet) { log.verbose('waitFor', `CPU has been idle for ${timeSinceLongTask} ms`); const timeToWait = waitForCPUQuiet - timeSinceLongTask; return new Promise((resolve, reject) => { - lastTimeout = setTimeout(() => { + setTimeout(() => { + if (canceled) { + resolve(); + return; + } checkForQuiet(executionContext) .then(resolve) .catch(reject); @@ -279,7 +281,6 @@ function waitForCPUIdle(session, waitForCPUQuiet) { cancel = () => { if (canceled) return; canceled = true; - if (lastTimeout) clearTimeout(lastTimeout); reject(new Error('Wait for CPU idle canceled')); }; }); From 8b3b88acb9822363d42667c2846a7552757bd6fd Mon Sep 17 00:00:00 2001 From: Aleksandr Guidrevitch Date: Sat, 31 Aug 2024 20:00:35 +0200 Subject: [PATCH 3/3] removing redunant check for cancelled, which will happen anyway in the next checkForQuiet --- core/gather/driver/wait-for-condition.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/gather/driver/wait-for-condition.js b/core/gather/driver/wait-for-condition.js index 2e6d5d34608a..55207ee63d08 100644 --- a/core/gather/driver/wait-for-condition.js +++ b/core/gather/driver/wait-for-condition.js @@ -252,10 +252,6 @@ function waitForCPUIdle(session, waitForCPUQuiet) { const timeToWait = waitForCPUQuiet - timeSinceLongTask; return new Promise((resolve, reject) => { setTimeout(() => { - if (canceled) { - resolve(); - return; - } checkForQuiet(executionContext) .then(resolve) .catch(reject);