From a3427492f8640e8332fd8d41ac59391d4f7e92e1 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 22 Jun 2022 17:56:51 -0300 Subject: [PATCH 1/5] decouple flushData and stopSync logic, and flush data on pagehide and visibilitychange events --- src/listeners/browser.ts | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/listeners/browser.ts b/src/listeners/browser.ts index d50d4bed..18593a5e 100644 --- a/src/listeners/browser.ts +++ b/src/listeners/browser.ts @@ -14,7 +14,8 @@ import { ISyncManager } from '../sync/types'; import { isConsentGranted } from '../consent'; import { telemetryCacheStatsAdapter } from '../sync/submitters/telemetrySubmitter'; -// 'unload' event is used instead of 'beforeunload', since 'unload' is not a cancelable event, so no other listeners can stop the event from occurring. +const VISIBILITYCHANGE_EVENT = 'visibilitychange'; +const PAGEHIDE_EVENT = 'pagehide'; const UNLOAD_DOM_EVENT = 'unload'; const EVENT_NAME = 'for unload page event.'; @@ -32,6 +33,8 @@ export class BrowserSignalListener implements ISignalListener { private serviceApi: ISplitApi, ) { this.flushData = this.flushData.bind(this); + this.flushDataIfHidden = this.flushDataIfHidden.bind(this); + this.stopSync = this.stopSync.bind(this); this.fromImpressionsCollector = fromImpressionsCollector.bind(undefined, settings.core.labelsEnabled); } @@ -43,7 +46,14 @@ export class BrowserSignalListener implements ISignalListener { start() { if (typeof window !== 'undefined' && window.addEventListener) { this.settings.log.debug(CLEANUP_REGISTERING, [EVENT_NAME]); - window.addEventListener(UNLOAD_DOM_EVENT, this.flushData); + + // Flush data whenever the page is hidden or unloaded. + window.addEventListener(VISIBILITYCHANGE_EVENT, this.flushDataIfHidden); + // Some browsers like Safari does not fire the `visibilitychange` event when the page is being unloaded. So we also flush data in the `pagehide` event. + // If both events are triggered, the last one will find the storage empty, so no duplicated data will be submitted. + window.addEventListener(PAGEHIDE_EVENT, this.flushData); + // Stop streaming on 'unload' event. Used instead of 'beforeunload', because 'unload' is not a cancelable event, so no other listeners can stop the event from occurring. + window.addEventListener(UNLOAD_DOM_EVENT, this.stopSync); } } @@ -55,10 +65,17 @@ export class BrowserSignalListener implements ISignalListener { stop() { if (typeof window !== 'undefined' && window.removeEventListener) { this.settings.log.debug(CLEANUP_DEREGISTERING, [EVENT_NAME]); - window.removeEventListener(UNLOAD_DOM_EVENT, this.flushData); + window.removeEventListener(VISIBILITYCHANGE_EVENT, this.flushDataIfHidden); + window.removeEventListener(PAGEHIDE_EVENT, this.flushData); + window.removeEventListener(UNLOAD_DOM_EVENT, this.stopSync); } } + stopSync() { + // Close streaming connection + if (this.syncManager && this.syncManager.pushManager) this.syncManager.pushManager.stop(); + } + /** * flushData method. * Called when unload event is triggered. It flushed remaining impressions and events to the backend, @@ -86,9 +103,10 @@ export class BrowserSignalListener implements ISignalListener { const telemetryCacheAdapter = telemetryCacheStatsAdapter(this.storage.telemetry, this.storage.splits, this.storage.segments); this._flushData(telemetryUrl + '/v1/metrics/usage/beacon', telemetryCacheAdapter, this.serviceApi.postMetricsUsage); } + } - // Close streaming connection - if (this.syncManager.pushManager) this.syncManager.pushManager.stop(); + flushDataIfHidden() { + if (typeof document !== 'undefined' && document.visibilityState === 'hidden') this.flushData(); // On a 'visibilitychange' event, flush data if state is hidden } private _flushData(url: string, cache: IRecorderCacheProducerSync, postService: (body: string) => Promise, fromCacheToPayload?: (cacheData: T) => any, extraMetadata?: {}) { From c3a44e2d9812f21a587c84cce68e067ab14b13c6 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 22 Jun 2022 21:36:19 -0300 Subject: [PATCH 2/5] unit tests --- src/listeners/__tests__/browser.spec.ts | 83 ++++++++++++++++--------- src/listeners/browser.ts | 8 +-- 2 files changed, 58 insertions(+), 33 deletions(-) diff --git a/src/listeners/__tests__/browser.spec.ts b/src/listeners/__tests__/browser.spec.ts index b33bd709..c2aa6088 100644 --- a/src/listeners/__tests__/browser.spec.ts +++ b/src/listeners/__tests__/browser.spec.ts @@ -77,27 +77,40 @@ const fakeSplitApi = { postMetricsUsage: jest.fn(() => Promise.resolve()), } as ISplitApi; +const VISIBILITYCHANGE_EVENT = 'visibilitychange'; +const PAGEHIDE_EVENT = 'pagehide'; const UNLOAD_DOM_EVENT = 'unload'; -const unloadEventListeners = new Set<() => any>(); + +const eventListeners: any = { + [VISIBILITYCHANGE_EVENT]: new Set<() => any>(), + [PAGEHIDE_EVENT]: new Set<() => any>(), + [UNLOAD_DOM_EVENT]: new Set<() => any>() +}; // mock window and navigator objects beforeAll(() => { Object.defineProperty(global, 'window', { value: { addEventListener: jest.fn((event, cb) => { - if (event === UNLOAD_DOM_EVENT) unloadEventListeners.add(cb); + if (eventListeners[event]) eventListeners[event].add(cb); }), removeEventListener: jest.fn((event, cb) => { - if (event === UNLOAD_DOM_EVENT) unloadEventListeners.delete(cb); + if (eventListeners[event]) eventListeners[event].delete(cb); }), navigator: { sendBeacon: jest.fn(() => true) + }, + document: { + visibilityState: 'visible' } } }); Object.defineProperty(global, 'navigator', { value: global.window.navigator }); + Object.defineProperty(global, 'document', { + value: global.window.document + }); }); // clear mocks @@ -105,7 +118,8 @@ beforeEach(() => { (global.window.addEventListener as jest.Mock).mockClear(); (global.window.removeEventListener as jest.Mock).mockClear(); if (global.window.navigator.sendBeacon) (global.window.navigator.sendBeacon as jest.Mock).mockClear(); - Object.values(fakeSplitApi).forEach(method => method.mockClear()); + Object.values(fakeSplitApi).forEach(method => method.mockClear()); // @ts-expect-error + global.document.visibilityState = 'visible'; }); // delete mocks from global @@ -115,8 +129,9 @@ afterAll(() => { delete global.navigator; }); -function triggerUnloadEvent() { - unloadEventListeners.forEach(cb => cb()); +function triggerEvent(event: string, visibilityState?: string) { // @ts-expect-error + if (visibilityState) global.document.visibilityState = visibilityState; + eventListeners[event].forEach((cb: () => any) => cb()); } /* Mocks end */ @@ -128,9 +143,12 @@ test('Browser JS listener / consumer mode', () => { listener.start(); // Assigned right function to right signal. - expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); + const eventListeners = [[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden], [PAGEHIDE_EVENT, listener.flushData], [UNLOAD_DOM_EVENT, listener.stopSync]]; + expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual(eventListeners); - triggerUnloadEvent(); + triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden'); + triggerEvent(PAGEHIDE_EVENT); + triggerEvent(UNLOAD_DOM_EVENT); // Unload event was triggered, but sendBeacon and post services should not be called expect(global.window.navigator.sendBeacon).toBeCalledTimes(0); @@ -143,7 +161,7 @@ test('Browser JS listener / consumer mode', () => { listener.stop(); // removed correct listener from correct signal on stop. - expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); + expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual(eventListeners); }); test('Browser JS listener / standalone mode / Impressions optimized mode with telemetry', () => { @@ -155,11 +173,12 @@ test('Browser JS listener / standalone mode / Impressions optimized mode with te listener.start(); // Assigned right function to right signal. - expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); + const eventListeners = [[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden], [PAGEHIDE_EVENT, listener.flushData], [UNLOAD_DOM_EVENT, listener.stopSync]]; + expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual(eventListeners); - triggerUnloadEvent(); + triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden'); - // Unload event was triggered. Thus sendBeacon method should have been called four times. + // Visibility change event was triggered. Thus sendBeacon method should be called four times. expect(global.window.navigator.sendBeacon).toBeCalledTimes(4); // Http post services should have not been called @@ -172,7 +191,7 @@ test('Browser JS listener / standalone mode / Impressions optimized mode with te listener.stop(); // removed correct listener from correct signal on stop. - expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); + expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual(eventListeners); }); test('Browser JS listener / standalone mode / Impressions debug mode', () => { @@ -184,17 +203,26 @@ test('Browser JS listener / standalone mode / Impressions debug mode', () => { listener.start(); // Assigned right function to right signal. - expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); + const eventListeners = [[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden], [PAGEHIDE_EVENT, listener.flushData], [UNLOAD_DOM_EVENT, listener.stopSync]]; + expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual(eventListeners); + triggerEvent(VISIBILITYCHANGE_EVENT, 'visible'); + + // If visibility changes to visible, do nothing expect(syncManagerMockWithPushManager.pushManager.stop).not.toBeCalled(); + expect(global.window.navigator.sendBeacon).not.toBeCalled(); - triggerUnloadEvent(); + triggerEvent(PAGEHIDE_EVENT); - // Unload event was triggered. Thus sendBeacon method should have been called twice. + // Pagehide event was triggered. Thus sendBeacon method should be called twice. expect(global.window.navigator.sendBeacon).toBeCalledTimes(2); + expect(syncManagerMockWithPushManager.pushManager.stop).not.toBeCalled(); - // And since we passed a syncManager with a pushManager, its stop method should have been called. + triggerEvent(UNLOAD_DOM_EVENT); + + // Unload event was triggered and pushManager is defined, so it must be stopped. expect(syncManagerMockWithPushManager.pushManager.stop).toBeCalledTimes(1); + expect(global.window.navigator.sendBeacon).toBeCalledTimes(2); // Http post services should have not been called expect(fakeSplitApi.postTestImpressionsBulk).not.toBeCalled(); @@ -206,7 +234,7 @@ test('Browser JS listener / standalone mode / Impressions debug mode', () => { listener.stop(); // removed correct listener from correct signal on stop. - expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); + expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual(eventListeners); }); test('Browser JS listener / standalone mode / Impressions debug mode without sendBeacon API', () => { @@ -221,14 +249,13 @@ test('Browser JS listener / standalone mode / Impressions debug mode without sen listener.start(); // Assigned right function to right signal. - expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); + const eventListeners = [[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden], [PAGEHIDE_EVENT, listener.flushData], [UNLOAD_DOM_EVENT, listener.stopSync]]; + expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual(eventListeners); - triggerUnloadEvent(); + triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden'); - // Unload event was triggered. Thus sendBeacon method should have been called twice. + // Visibility has changed to hidden and sendBeacon API is not available, so http post services should be called expect(sendBeacon).not.toBeCalled(); - - // Http post services should have not been called expect(fakeSplitApi.postTestImpressionsBulk).toBeCalledTimes(1); expect(fakeSplitApi.postEventsBulk).toBeCalledTimes(1); expect(fakeSplitApi.postTestImpressionsCount).not.toBeCalled(); @@ -238,7 +265,7 @@ test('Browser JS listener / standalone mode / Impressions debug mode without sen listener.stop(); // removed correct listener from correct signal on stop. - expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); + expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual(eventListeners); // restore sendBeacon API global.navigator.sendBeacon = sendBeacon; @@ -254,9 +281,9 @@ test('Browser JS listener / standalone mode / user consent status', () => { listener.start(); settings.userConsent = 'UNKNOWN'; - triggerUnloadEvent(); + triggerEvent(PAGEHIDE_EVENT); settings.userConsent = 'DECLINED'; - triggerUnloadEvent(); + triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden'); // Unload event was triggered when user consent was unknown and declined. Thus sendBeacon and post services should be called only for telemetry expect(global.window.navigator.sendBeacon).toBeCalledTimes(2); @@ -266,9 +293,9 @@ test('Browser JS listener / standalone mode / user consent status', () => { (global.window.navigator.sendBeacon as jest.Mock).mockClear(); settings.userConsent = 'GRANTED'; - triggerUnloadEvent(); + triggerEvent(PAGEHIDE_EVENT); settings.userConsent = undefined; - triggerUnloadEvent(); + triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden'); // Unload event was triggered when user consent was granted and undefined. Thus sendBeacon should be called 8 times (4 times per event in optimized mode with telemetry). expect(global.window.navigator.sendBeacon).toBeCalledTimes(8); diff --git a/src/listeners/browser.ts b/src/listeners/browser.ts index 18593a5e..b35b0042 100644 --- a/src/listeners/browser.ts +++ b/src/listeners/browser.ts @@ -20,7 +20,7 @@ const UNLOAD_DOM_EVENT = 'unload'; const EVENT_NAME = 'for unload page event.'; /** - * We'll listen for 'unload' event over the window object, since it's the standard way to listen page reload and close. + * We'll listen for events over the window object. */ export class BrowserSignalListener implements ISignalListener { @@ -40,8 +40,7 @@ export class BrowserSignalListener implements ISignalListener { /** * start method. - * Called when SplitFactory is initialized. - * We add a handler on unload events. The handler flushes remaining impressions and events to the backend. + * Called when SplitFactory is initialized, it adds event listeners to close streaming and flush impressions and events. */ start() { if (typeof window !== 'undefined' && window.addEventListener) { @@ -59,8 +58,7 @@ export class BrowserSignalListener implements ISignalListener { /** * stop method. - * Called when client is destroyed. - * We need to remove the handler for unload events, since it can break if called when Split context was destroyed. + * Called when client is destroyed, it removes event listeners. */ stop() { if (typeof window !== 'undefined' && window.removeEventListener) { From c08979eac60c7cbb8e0837e8b0f12466a688448e Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 27 Jun 2022 16:40:15 -0300 Subject: [PATCH 3/5] update rc --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index d6aa7e29..dbc42c12 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.4.2-rc.1", + "version": "1.4.2-rc.4", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 3a036085..791a2ef5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.4.2-rc.1", + "version": "1.4.2-rc.4", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", From 5e5d0de4519d09565400140e10dc68916a4829cb Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 29 Jun 2022 18:20:06 -0300 Subject: [PATCH 4/5] update changelog --- CHANGES.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index aa1f7b44..edaa0869 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,6 @@ +1.5.1 (July XX, 2022) +- Updated browser listener to push remaining impressions and events on 'visibilitychange' and 'pagehide' DOM events, instead of 'unload', which is not reliable in mobile and modern Web browsers (See https://developer.chrome.com/blog/page-lifecycle-api/#legacy-lifecycle-apis-to-avoid). + 1.5.0 (June 29, 2022) - Added a new config option to control the tasks that listen or poll for updates on feature flags and segments, via the new config sync.enabled . Running online Split will always pull the most recent updates upon initialization, this only affects updates fetching on a running instance. Useful when a consistent session experience is a must or to save resources when updates are not being used. - Updated telemetry logic to track the anonymous config for user consent flag set to declined or unknown. From 926b878dd5ebe6d43c3558a2c1d71e83931523d5 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 5 Jul 2022 17:54:54 -0300 Subject: [PATCH 5/5] fix issue with visibilitychange event --- package-lock.json | 2 +- src/listeners/__tests__/browser.spec.ts | 68 ++++++++++++------------- src/listeners/browser.ts | 26 ++++++---- 3 files changed, 50 insertions(+), 46 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2c6c6e5f..b618f717 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4842,7 +4842,7 @@ "lodash.flatten": { "version": "4.4.0", "resolved": "https://registry.npmjs.org/lodash.flatten/-/lodash.flatten-4.4.0.tgz", - "integrity": "sha1-8xwiIlqWMtK7+OSt2+8kCqdlph8=", + "integrity": "sha512-C5N2Z3DgnnKr0LOpv/hKCgKdb7ZZwafIrsesve6lmzvZIRZRGaZ/l6Q8+2W7NaT+ZwO3fFlSCzCzrDCFdJfZ4g==", "dev": true }, "lodash.isarguments": { diff --git a/src/listeners/__tests__/browser.spec.ts b/src/listeners/__tests__/browser.spec.ts index c2aa6088..304ba8a5 100644 --- a/src/listeners/__tests__/browser.spec.ts +++ b/src/listeners/__tests__/browser.spec.ts @@ -79,12 +79,12 @@ const fakeSplitApi = { const VISIBILITYCHANGE_EVENT = 'visibilitychange'; const PAGEHIDE_EVENT = 'pagehide'; -const UNLOAD_DOM_EVENT = 'unload'; +const UNLOAD_EVENT = 'unload'; const eventListeners: any = { [VISIBILITYCHANGE_EVENT]: new Set<() => any>(), [PAGEHIDE_EVENT]: new Set<() => any>(), - [UNLOAD_DOM_EVENT]: new Set<() => any>() + [UNLOAD_EVENT]: new Set<() => any>() }; // mock window and navigator objects @@ -101,7 +101,13 @@ beforeAll(() => { sendBeacon: jest.fn(() => true) }, document: { - visibilityState: 'visible' + visibilityState: 'visible', + addEventListener: jest.fn((event, cb) => { + if (eventListeners[event]) eventListeners[event].add(cb); + }), + removeEventListener: jest.fn((event, cb) => { + if (eventListeners[event]) eventListeners[event].delete(cb); + }), } } }); @@ -117,6 +123,8 @@ beforeAll(() => { beforeEach(() => { (global.window.addEventListener as jest.Mock).mockClear(); (global.window.removeEventListener as jest.Mock).mockClear(); + (global.document.addEventListener as jest.Mock).mockClear(); + (global.document.removeEventListener as jest.Mock).mockClear(); if (global.window.navigator.sendBeacon) (global.window.navigator.sendBeacon as jest.Mock).mockClear(); Object.values(fakeSplitApi).forEach(method => method.mockClear()); // @ts-expect-error global.document.visibilityState = 'visible'; @@ -125,8 +133,7 @@ beforeEach(() => { // delete mocks from global afterAll(() => { // @ts-expect-error - delete global.window; // @ts-expect-error - delete global.navigator; + delete global.window; delete global.navigator; delete global.document; }); function triggerEvent(event: string, visibilityState?: string) { // @ts-expect-error @@ -134,6 +141,18 @@ function triggerEvent(event: string, visibilityState?: string) { // @ts-expect-e eventListeners[event].forEach((cb: () => any) => cb()); } +function assertStart(listener: BrowserSignalListener) { + // Assigned right function to right signal. + expect((global.document.addEventListener as jest.Mock).mock.calls).toEqual([[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden]]); + expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual([[PAGEHIDE_EVENT, listener.flushData], [UNLOAD_EVENT, listener.stopSync]]); +} + +function assertStop(listener: BrowserSignalListener) { + // removed correct listener from correct signal on stop. + expect((global.document.removeEventListener as jest.Mock).mock.calls).toEqual([[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden]]); + expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[PAGEHIDE_EVENT, listener.flushData], [UNLOAD_EVENT, listener.stopSync]]); +} + /* Mocks end */ test('Browser JS listener / consumer mode', () => { @@ -141,14 +160,11 @@ test('Browser JS listener / consumer mode', () => { const listener = new BrowserSignalListener(undefined, fullSettings, fakeStorageOptimized as IStorageSync, fakeSplitApi); listener.start(); - - // Assigned right function to right signal. - const eventListeners = [[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden], [PAGEHIDE_EVENT, listener.flushData], [UNLOAD_DOM_EVENT, listener.stopSync]]; - expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual(eventListeners); + assertStart(listener); triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden'); triggerEvent(PAGEHIDE_EVENT); - triggerEvent(UNLOAD_DOM_EVENT); + triggerEvent(UNLOAD_EVENT); // Unload event was triggered, but sendBeacon and post services should not be called expect(global.window.navigator.sendBeacon).toBeCalledTimes(0); @@ -160,8 +176,7 @@ test('Browser JS listener / consumer mode', () => { expect(global.window.removeEventListener).not.toBeCalled(); listener.stop(); - // removed correct listener from correct signal on stop. - expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual(eventListeners); + assertStop(listener); }); test('Browser JS listener / standalone mode / Impressions optimized mode with telemetry', () => { @@ -171,10 +186,7 @@ test('Browser JS listener / standalone mode / Impressions optimized mode with te const listener = new BrowserSignalListener(syncManagerMock, fullSettings, fakeStorageOptimized as IStorageSync, fakeSplitApi); listener.start(); - - // Assigned right function to right signal. - const eventListeners = [[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden], [PAGEHIDE_EVENT, listener.flushData], [UNLOAD_DOM_EVENT, listener.stopSync]]; - expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual(eventListeners); + assertStart(listener); triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden'); @@ -189,9 +201,7 @@ test('Browser JS listener / standalone mode / Impressions optimized mode with te // pre-check and call stop expect(global.window.removeEventListener).not.toBeCalled(); listener.stop(); - - // removed correct listener from correct signal on stop. - expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual(eventListeners); + assertStop(listener); }); test('Browser JS listener / standalone mode / Impressions debug mode', () => { @@ -201,10 +211,7 @@ test('Browser JS listener / standalone mode / Impressions debug mode', () => { const listener = new BrowserSignalListener(syncManagerMockWithPushManager, fullSettings, fakeStorageDebug as IStorageSync, fakeSplitApi); listener.start(); - - // Assigned right function to right signal. - const eventListeners = [[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden], [PAGEHIDE_EVENT, listener.flushData], [UNLOAD_DOM_EVENT, listener.stopSync]]; - expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual(eventListeners); + assertStart(listener); triggerEvent(VISIBILITYCHANGE_EVENT, 'visible'); @@ -218,7 +225,7 @@ test('Browser JS listener / standalone mode / Impressions debug mode', () => { expect(global.window.navigator.sendBeacon).toBeCalledTimes(2); expect(syncManagerMockWithPushManager.pushManager.stop).not.toBeCalled(); - triggerEvent(UNLOAD_DOM_EVENT); + triggerEvent(UNLOAD_EVENT); // Unload event was triggered and pushManager is defined, so it must be stopped. expect(syncManagerMockWithPushManager.pushManager.stop).toBeCalledTimes(1); @@ -232,9 +239,7 @@ test('Browser JS listener / standalone mode / Impressions debug mode', () => { // pre-check and call stop expect(global.window.removeEventListener).not.toBeCalled(); listener.stop(); - - // removed correct listener from correct signal on stop. - expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual(eventListeners); + assertStop(listener); }); test('Browser JS listener / standalone mode / Impressions debug mode without sendBeacon API', () => { @@ -247,10 +252,7 @@ test('Browser JS listener / standalone mode / Impressions debug mode without sen const listener = new BrowserSignalListener(syncManagerMockWithoutPushManager, fullSettings, fakeStorageDebug as IStorageSync, fakeSplitApi); listener.start(); - - // Assigned right function to right signal. - const eventListeners = [[VISIBILITYCHANGE_EVENT, listener.flushDataIfHidden], [PAGEHIDE_EVENT, listener.flushData], [UNLOAD_DOM_EVENT, listener.stopSync]]; - expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual(eventListeners); + assertStart(listener); triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden'); @@ -263,9 +265,7 @@ test('Browser JS listener / standalone mode / Impressions debug mode without sen // pre-check and call stop expect(global.window.removeEventListener).not.toBeCalled(); listener.stop(); - - // removed correct listener from correct signal on stop. - expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual(eventListeners); + assertStop(listener); // restore sendBeacon API global.navigator.sendBeacon = sendBeacon; diff --git a/src/listeners/browser.ts b/src/listeners/browser.ts index b35b0042..6989e853 100644 --- a/src/listeners/browser.ts +++ b/src/listeners/browser.ts @@ -16,7 +16,7 @@ import { telemetryCacheStatsAdapter } from '../sync/submitters/telemetrySubmitte const VISIBILITYCHANGE_EVENT = 'visibilitychange'; const PAGEHIDE_EVENT = 'pagehide'; -const UNLOAD_DOM_EVENT = 'unload'; +const UNLOAD_EVENT = 'unload'; const EVENT_NAME = 'for unload page event.'; /** @@ -43,16 +43,17 @@ export class BrowserSignalListener implements ISignalListener { * Called when SplitFactory is initialized, it adds event listeners to close streaming and flush impressions and events. */ start() { - if (typeof window !== 'undefined' && window.addEventListener) { - this.settings.log.debug(CLEANUP_REGISTERING, [EVENT_NAME]); - + this.settings.log.debug(CLEANUP_REGISTERING, [EVENT_NAME]); + if (typeof document !== 'undefined' && document.addEventListener) { // Flush data whenever the page is hidden or unloaded. - window.addEventListener(VISIBILITYCHANGE_EVENT, this.flushDataIfHidden); + document.addEventListener(VISIBILITYCHANGE_EVENT, this.flushDataIfHidden); + } + if (typeof window !== 'undefined' && window.addEventListener) { // Some browsers like Safari does not fire the `visibilitychange` event when the page is being unloaded. So we also flush data in the `pagehide` event. // If both events are triggered, the last one will find the storage empty, so no duplicated data will be submitted. window.addEventListener(PAGEHIDE_EVENT, this.flushData); // Stop streaming on 'unload' event. Used instead of 'beforeunload', because 'unload' is not a cancelable event, so no other listeners can stop the event from occurring. - window.addEventListener(UNLOAD_DOM_EVENT, this.stopSync); + window.addEventListener(UNLOAD_EVENT, this.stopSync); } } @@ -61,11 +62,13 @@ export class BrowserSignalListener implements ISignalListener { * Called when client is destroyed, it removes event listeners. */ stop() { + this.settings.log.debug(CLEANUP_DEREGISTERING, [EVENT_NAME]); + if (typeof document !== 'undefined' && document.removeEventListener) { + document.removeEventListener(VISIBILITYCHANGE_EVENT, this.flushDataIfHidden); + } if (typeof window !== 'undefined' && window.removeEventListener) { - this.settings.log.debug(CLEANUP_DEREGISTERING, [EVENT_NAME]); - window.removeEventListener(VISIBILITYCHANGE_EVENT, this.flushDataIfHidden); window.removeEventListener(PAGEHIDE_EVENT, this.flushData); - window.removeEventListener(UNLOAD_DOM_EVENT, this.stopSync); + window.removeEventListener(UNLOAD_EVENT, this.stopSync); } } @@ -76,7 +79,7 @@ export class BrowserSignalListener implements ISignalListener { /** * flushData method. - * Called when unload event is triggered. It flushed remaining impressions and events to the backend, + * Called when pagehide event is triggered. It flushed remaining impressions and events to the backend, * using beacon API if possible, or falling back to regular post transport. */ flushData() { @@ -104,7 +107,8 @@ export class BrowserSignalListener implements ISignalListener { } flushDataIfHidden() { - if (typeof document !== 'undefined' && document.visibilityState === 'hidden') this.flushData(); // On a 'visibilitychange' event, flush data if state is hidden + // Precondition: document defined + if (document.visibilityState === 'hidden') this.flushData(); // On a 'visibilitychange' event, flush data if state is hidden } private _flushData(url: string, cache: IRecorderCacheProducerSync, postService: (body: string) => Promise, fromCacheToPayload?: (cacheData: T) => any, extraMetadata?: {}) {