Skip to content

Commit

Permalink
CORE: prevent unbound growth of suspendedTimeouts and possible NaN va…
Browse files Browse the repository at this point in the history
…lues (#12059)

* prevent unbound growth of suspendedTimeouts and possible NaN values in timeOutOfFocus

* Add tests

* Reintroduce outOfFocusStart default to 0

---------

Co-authored-by: Demetrio Girardi <dgirardi@prebid.org>
  • Loading branch information
olafbuitelaar and dgirardi authored Jul 30, 2024
1 parent cd13fc4 commit a103a2e
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 13 deletions.
23 changes: 17 additions & 6 deletions src/utils/focusTimeout.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/ttlCollection.js
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
39 changes: 33 additions & 6 deletions test/spec/unit/utils/focusTimeout_spec.js
Original file line number Diff line number Diff line change
@@ -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', {
Expand All @@ -9,33 +9,41 @@ export const setDocumentHidden = (hidden) => {
};

describe('focusTimeout', () => {
let clock;
let clock, callback;

beforeEach(() => {
reset()
clock = sinon.useFakeTimers();
callback = sinon.spy();
});

afterEach(() => {
clock.restore();
})

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);
expect(callback.called).to.be.false;
});

it('should defer callback execution when page is hidden', () => {
let callback = sinon.stub();
setFocusTimeout(callback, 4000);
clock.tick(2000);
setDocumentHidden(true);
Expand All @@ -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);
Expand Down

0 comments on commit a103a2e

Please sign in to comment.