From a103a2e1df4e1c0568c5cf6994b15b7e49bdd08d Mon Sep 17 00:00:00 2001 From: olafbuitelaar Date: Tue, 30 Jul 2024 21:04:15 +0200 Subject: [PATCH] CORE: prevent unbound growth of suspendedTimeouts and possible NaN values (#12059) * prevent unbound growth of suspendedTimeouts and possible NaN values in timeOutOfFocus * Add tests * Reintroduce outOfFocusStart default to 0 --------- Co-authored-by: Demetrio Girardi --- src/utils/focusTimeout.js | 23 +++++++++---- src/utils/ttlCollection.js | 2 +- test/spec/unit/utils/focusTimeout_spec.js | 39 +++++++++++++++++++---- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/utils/focusTimeout.js b/src/utils/focusTimeout.js index 0ba66cc4efc..0c54bacec97 100644 --- a/src/utils/focusTimeout.js +++ b/src/utils/focusTimeout.js @@ -1,16 +1,27 @@ -let outOfFocusStart; +let outOfFocusStart = null; // enforce null otherwise it could be undefined and the callback wouldn't execute let timeOutOfFocus = 0; let suspendedTimeouts = []; -document.addEventListener('visibilitychange', () => { +function trackTimeOutOfFocus() { if (document.hidden) { outOfFocusStart = Date.now() } else { - timeOutOfFocus += Date.now() - outOfFocusStart - suspendedTimeouts.forEach(({ callback, startTime, setTimerId }) => setTimerId(setFocusTimeout(callback, timeOutOfFocus - startTime)())) + timeOutOfFocus += Date.now() - (outOfFocusStart ?? 0); // when the page is loaded in hidden state outOfFocusStart is undefined, which results in timeoutOffset being NaN outOfFocusStart = null; + suspendedTimeouts.forEach(({ callback, startTime, setTimerId }) => setTimerId(setFocusTimeout(callback, timeOutOfFocus - startTime)())); + suspendedTimeouts = []; } -}); +} + +document.addEventListener('visibilitychange', trackTimeOutOfFocus); + +export function reset() { + outOfFocusStart = null; + timeOutOfFocus = 0; + suspendedTimeouts = []; + document.removeEventListener('visibilitychange', trackTimeOutOfFocus); + document.addEventListener('visibilitychange', trackTimeOutOfFocus); +} /** * Wraps native setTimeout function in order to count time only when page is focused @@ -19,7 +30,7 @@ document.addEventListener('visibilitychange', () => { * @param {number} [milliseconds] - Minimum duration (in milliseconds) that the callback will be executed after * @returns {function(*): (number)} - Getter function for current timer id */ -export default function setFocusTimeout(callback, milliseconds) { +export function setFocusTimeout(callback, milliseconds) { const startTime = timeOutOfFocus; let timerId = setTimeout(() => { if (timeOutOfFocus === startTime && outOfFocusStart == null) { diff --git a/src/utils/ttlCollection.js b/src/utils/ttlCollection.js index b6e0a5198df..ffc11433d06 100644 --- a/src/utils/ttlCollection.js +++ b/src/utils/ttlCollection.js @@ -1,6 +1,6 @@ import {GreedyPromise} from './promise.js'; import {binarySearch, logError, timestamp} from '../utils.js'; -import setFocusTimeout from './focusTimeout.js'; +import {setFocusTimeout} from './focusTimeout.js'; /** * Create a set-like collection that automatically forgets items after a certain time. diff --git a/test/spec/unit/utils/focusTimeout_spec.js b/test/spec/unit/utils/focusTimeout_spec.js index ed7b1c0c2f3..3fcc4af18fe 100644 --- a/test/spec/unit/utils/focusTimeout_spec.js +++ b/test/spec/unit/utils/focusTimeout_spec.js @@ -1,4 +1,4 @@ -import setFocusTimeout from '../../../../src/utils/focusTimeout'; +import {setFocusTimeout, reset} from '../../../../src/utils/focusTimeout'; export const setDocumentHidden = (hidden) => { Object.defineProperty(document, 'hidden', { @@ -9,10 +9,12 @@ export const setDocumentHidden = (hidden) => { }; describe('focusTimeout', () => { - let clock; + let clock, callback; beforeEach(() => { + reset() clock = sinon.useFakeTimers(); + callback = sinon.spy(); }); afterEach(() => { @@ -20,14 +22,21 @@ describe('focusTimeout', () => { }) it('should invoke callback when page is visible', () => { - let callback = sinon.stub(); setFocusTimeout(callback, 2000); clock.tick(2000); expect(callback.called).to.be.true; }); + it('should not choke if page starts hidden', () => { + setDocumentHidden(true); + reset(); + setDocumentHidden(false); + setFocusTimeout(callback, 1000); + clock.tick(1000); + sinon.assert.called(callback); + }) + it('should not invoke callback if page was hidden', () => { - let callback = sinon.stub(); setFocusTimeout(callback, 2000); setDocumentHidden(true); clock.tick(3000); @@ -35,7 +44,6 @@ describe('focusTimeout', () => { }); it('should defer callback execution when page is hidden', () => { - let callback = sinon.stub(); setFocusTimeout(callback, 4000); clock.tick(2000); setDocumentHidden(true); @@ -46,8 +54,27 @@ describe('focusTimeout', () => { expect(callback.called).to.be.true; }); + it('should not execute deferred callbacks again', () => { + setDocumentHidden(true); + setFocusTimeout(callback, 1000); + clock.tick(2000); + [false, true, false].forEach(setDocumentHidden); + clock.tick(2000); + sinon.assert.calledOnce(callback); + }); + + it('should run callbacks that expire while page is hidden', () => { + setFocusTimeout(callback, 1000); + clock.tick(500); + setDocumentHidden(true); + clock.tick(1000); + setDocumentHidden(false); + sinon.assert.notCalled(callback); + clock.tick(1000); + sinon.assert.called(callback); + }) + it('should return updated timerId after page was showed again', () => { - let callback = sinon.stub(); const getCurrentTimerId = setFocusTimeout(callback, 4000); const oldTimerId = getCurrentTimerId(); clock.tick(2000);