From 065140b9d1bb0b7b7902ba3b9c420dde4c3a2639 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt?= Date: Mon, 10 Jan 2022 17:29:24 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20[RUMF-1143]=20make=20sure=20to?= =?UTF-8?q?=20drop=20LCP=20timings=20if=20the=20page=20was=20previously=20?= =?UTF-8?q?hidden=20(#1259)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../view/trackFirstHidden.spec.ts | 78 +++++++++++++------ .../view/trackFirstHidden.ts | 19 +++-- 2 files changed, 65 insertions(+), 32 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackFirstHidden.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackFirstHidden.spec.ts index 8841f46519..850bd10213 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackFirstHidden.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackFirstHidden.spec.ts @@ -8,41 +8,69 @@ describe('trackFirstHidden', () => { restorePageVisibility() }) - it('should return Infinity if the page was not hidden yet', () => { - expect(trackFirstHidden().timeStamp).toBe(Infinity as RelativeTime) - }) + describe('the page is initially hidden', () => { + it('should return 0', () => { + setPageVisibility('hidden') + expect(trackFirstHidden().timeStamp).toBe(0 as RelativeTime) + }) + + it('should ignore events', () => { + setPageVisibility('hidden') + const emitter = document.createElement('div') + const firstHidden = trackFirstHidden(emitter) + + emitter.dispatchEvent(createNewEvent(DOM_EVENT.PAGE_HIDE, { timeStamp: 100 })) + emitter.dispatchEvent(createNewEvent(DOM_EVENT.VISIBILITY_CHANGE, { timeStamp: 100 })) - it('should return 0 if the page was hidden when executing trackFirstHidden', () => { - setPageVisibility('hidden') - expect(trackFirstHidden().timeStamp).toBe(0 as RelativeTime) + expect(firstHidden.timeStamp).toBe(0 as RelativeTime) + }) }) - it('should stay to 0 if the page was hidden when executing trackFirstHidden and a pagehide occurs', () => { - setPageVisibility('hidden') - const emitter = document.createElement('div') - const firstHidden = trackFirstHidden(emitter) + describe('the page is initially visible', () => { + it('should return Infinity if the page was not hidden yet', () => { + expect(trackFirstHidden().timeStamp).toBe(Infinity as RelativeTime) + }) - emitter.dispatchEvent(createNewEvent(DOM_EVENT.PAGE_HIDE, { timeStamp: 100 })) + it('should return the timestamp of the first pagehide event', () => { + const emitter = document.createElement('div') + const firstHidden = trackFirstHidden(emitter) - expect(firstHidden.timeStamp).toBe(0 as RelativeTime) - }) + emitter.dispatchEvent(createNewEvent(DOM_EVENT.PAGE_HIDE, { timeStamp: 100 })) - it('should return the timestamp of the first pagehide event', () => { - const emitter = document.createElement('div') - const firstHidden = trackFirstHidden(emitter) + expect(firstHidden.timeStamp).toBe(100 as RelativeTime) + }) - emitter.dispatchEvent(createNewEvent(DOM_EVENT.PAGE_HIDE, { timeStamp: 100 })) + it('should return the timestamp of the first visibilitychange event if the page is hidden', () => { + const emitter = document.createElement('div') + const firstHidden = trackFirstHidden(emitter) - expect(firstHidden.timeStamp).toBe(100 as RelativeTime) - }) + setPageVisibility('hidden') + emitter.dispatchEvent(createNewEvent(DOM_EVENT.VISIBILITY_CHANGE, { timeStamp: 100 })) + + expect(firstHidden.timeStamp).toBe(100 as RelativeTime) + }) + + it('should ignore visibilitychange event if the page is visible', () => { + const emitter = document.createElement('div') + const firstHidden = trackFirstHidden(emitter) + + emitter.dispatchEvent(createNewEvent(DOM_EVENT.VISIBILITY_CHANGE, { timeStamp: 100 })) + + expect(firstHidden.timeStamp).toBe(Infinity as RelativeTime) + }) + + it('should ignore subsequent events', () => { + const emitter = document.createElement('div') + const firstHidden = trackFirstHidden(emitter) - it('should stay to the first value if multiple pagehide event occurs', () => { - const emitter = document.createElement('div') - const firstHidden = trackFirstHidden(emitter) + emitter.dispatchEvent(createNewEvent(DOM_EVENT.PAGE_HIDE, { timeStamp: 100 })) - emitter.dispatchEvent(createNewEvent(DOM_EVENT.PAGE_HIDE, { timeStamp: 100 })) - emitter.dispatchEvent(createNewEvent(DOM_EVENT.PAGE_HIDE, { timeStamp: 200 })) + // Subsequent events: + emitter.dispatchEvent(createNewEvent(DOM_EVENT.PAGE_HIDE, { timeStamp: 200 })) + setPageVisibility('hidden') + emitter.dispatchEvent(createNewEvent(DOM_EVENT.VISIBILITY_CHANGE, { timeStamp: 200 })) - expect(firstHidden.timeStamp).toBe(100 as RelativeTime) + expect(firstHidden.timeStamp).toBe(100 as RelativeTime) + }) }) }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackFirstHidden.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackFirstHidden.ts index 7871bf2d4b..d3f3fae772 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackFirstHidden.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackFirstHidden.ts @@ -1,4 +1,4 @@ -import { addEventListener, DOM_EVENT, EventEmitter, RelativeTime } from '@datadog/browser-core' +import { addEventListeners, DOM_EVENT, EventEmitter, RelativeTime } from '@datadog/browser-core' let trackFirstHiddenSingleton: { timeStamp: RelativeTime } | undefined let stopListeners: (() => void) | undefined @@ -6,18 +6,23 @@ let stopListeners: (() => void) | undefined export function trackFirstHidden(emitter: EventEmitter = window) { if (!trackFirstHiddenSingleton) { if (document.visibilityState === 'hidden') { - trackFirstHiddenSingleton = { timeStamp: 0 as RelativeTime } + trackFirstHiddenSingleton = { + timeStamp: 0 as RelativeTime, + } } else { trackFirstHiddenSingleton = { timeStamp: Infinity as RelativeTime, } - ;({ stop: stopListeners } = addEventListener( + ;({ stop: stopListeners } = addEventListeners( emitter, - DOM_EVENT.PAGE_HIDE, - ({ timeStamp }) => { - trackFirstHiddenSingleton!.timeStamp = timeStamp as RelativeTime + [DOM_EVENT.PAGE_HIDE, DOM_EVENT.VISIBILITY_CHANGE], + (event) => { + if (event.type === 'pagehide' || document.visibilityState === 'hidden') { + trackFirstHiddenSingleton!.timeStamp = event.timeStamp as RelativeTime + stopListeners!() + } }, - { capture: true, once: true } + { capture: true } )) } }