From 951f9caf60f2edc9efc7f0e183f7f7efe80e6b8f Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Mon, 10 May 2021 12:54:37 -0400 Subject: [PATCH 1/5] Use 'pagehide' for page termination by default. --- .../src/local/indexeddb_persistence.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 782155898be..d90a0fe3343 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -942,6 +942,14 @@ export class IndexedDbPersistence implements Persistence { } } + /** + * Returns the page termination event to listen to. 'pagehide' is recommended + * if available, it falls back to the less reliable 'unload'. + */ + private terminationEvent(): string { + return 'onpagehide' in this.window! ? 'pagehide' : 'unload'; + } + /** * Attaches a window.unload handler that will synchronously write our * clientId to a "zombie client id" location in LocalStorage. This can be used @@ -966,7 +974,10 @@ export class IndexedDbPersistence implements Persistence { return this.shutdown(); }); }; - this.window.addEventListener('unload', this.windowUnloadHandler); + this.window.addEventListener( + this.terminationEvent(), + this.windowUnloadHandler + ); } } @@ -976,7 +987,10 @@ export class IndexedDbPersistence implements Persistence { typeof this.window?.removeEventListener === 'function', "Expected 'window.removeEventListener' to be a function" ); - this.window!.removeEventListener('unload', this.windowUnloadHandler); + this.window!.removeEventListener( + this.terminationEvent(), + this.windowUnloadHandler + ); this.windowUnloadHandler = null; } } From 044f9aef89e00c1c38fe27f6952970c441ff5120 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Mon, 10 May 2021 13:20:34 -0400 Subject: [PATCH 2/5] More places using 'unload' --- .../firestore/src/local/indexeddb_persistence.ts | 14 +++----------- .../firestore/src/local/shared_client_state.ts | 6 ++++-- packages/firestore/src/util/types.ts | 8 ++++++++ 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index d90a0fe3343..8e3b8b520c2 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -23,7 +23,7 @@ import { debugAssert } from '../util/assert'; import { AsyncQueue, DelayedOperation, TimerId } from '../util/async_queue'; import { Code, FirestoreError } from '../util/error'; import { logDebug, logError } from '../util/log'; -import { DocumentLike, WindowLike } from '../util/types'; +import { DocumentLike, WindowLike, terminationEvent } from '../util/types'; import { BundleCache } from './bundle_cache'; import { IndexManager } from './index_manager'; @@ -942,14 +942,6 @@ export class IndexedDbPersistence implements Persistence { } } - /** - * Returns the page termination event to listen to. 'pagehide' is recommended - * if available, it falls back to the less reliable 'unload'. - */ - private terminationEvent(): string { - return 'onpagehide' in this.window! ? 'pagehide' : 'unload'; - } - /** * Attaches a window.unload handler that will synchronously write our * clientId to a "zombie client id" location in LocalStorage. This can be used @@ -975,7 +967,7 @@ export class IndexedDbPersistence implements Persistence { }); }; this.window.addEventListener( - this.terminationEvent(), + terminationEvent(this.window), this.windowUnloadHandler ); } @@ -988,7 +980,7 @@ export class IndexedDbPersistence implements Persistence { "Expected 'window.removeEventListener' to be a function" ); this.window!.removeEventListener( - this.terminationEvent(), + terminationEvent(this.window), this.windowUnloadHandler ); this.windowUnloadHandler = null; diff --git a/packages/firestore/src/local/shared_client_state.ts b/packages/firestore/src/local/shared_client_state.ts index f310e286461..7405a899cd5 100644 --- a/packages/firestore/src/local/shared_client_state.ts +++ b/packages/firestore/src/local/shared_client_state.ts @@ -32,7 +32,7 @@ import { logError, logDebug } from '../util/log'; import { primitiveComparator } from '../util/misc'; import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; -import { isSafeInteger, WindowLike } from '../util/types'; +import { isSafeInteger, terminationEvent, WindowLike } from '../util/types'; import { CLIENT_STATE_KEY_PREFIX, @@ -613,7 +613,9 @@ export class WebStorageSharedClientState implements SharedClientState { // Register a window unload hook to remove the client metadata entry from // WebStorage even if `shutdown()` was not called. - this.window.addEventListener('unload', () => this.shutdown()); + this.window.addEventListener(terminationEvent(this.window), () => + this.shutdown() + ); this.started = true; } diff --git a/packages/firestore/src/util/types.ts b/packages/firestore/src/util/types.ts index be547455983..f6a34293923 100644 --- a/packages/firestore/src/util/types.ts +++ b/packages/firestore/src/util/types.ts @@ -59,6 +59,14 @@ export interface WindowLike { removeEventListener(type: string, listener: EventListener): void; } +/** + * Returns the page termination event to listen to. 'pagehide' is recommended + * if available, it falls back to the less reliable 'unload'. + */ +export function terminationEvent(window: WindowLike | null): string { + return 'onpagehide' in window! ? 'pagehide' : 'unload'; +} + /** The subset of the browser's Document interface used by the SDK. */ export interface DocumentLike { readonly visibilityState: VisibilityState; From 8c4e1919adaac1f0c017e8a6926ac31e061cc503 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Mon, 10 May 2021 20:17:27 -0400 Subject: [PATCH 3/5] 'pagehide' all the way --- .../firestore/src/local/indexeddb_persistence.ts | 12 +++--------- packages/firestore/src/local/shared_client_state.ts | 6 ++---- packages/firestore/src/util/types.ts | 8 -------- 3 files changed, 5 insertions(+), 21 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 8e3b8b520c2..859505f8dcf 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -23,7 +23,7 @@ import { debugAssert } from '../util/assert'; import { AsyncQueue, DelayedOperation, TimerId } from '../util/async_queue'; import { Code, FirestoreError } from '../util/error'; import { logDebug, logError } from '../util/log'; -import { DocumentLike, WindowLike, terminationEvent } from '../util/types'; +import { DocumentLike, WindowLike } from '../util/types'; import { BundleCache } from './bundle_cache'; import { IndexManager } from './index_manager'; @@ -966,10 +966,7 @@ export class IndexedDbPersistence implements Persistence { return this.shutdown(); }); }; - this.window.addEventListener( - terminationEvent(this.window), - this.windowUnloadHandler - ); + this.window.addEventListener('pagehide', this.windowUnloadHandler); } } @@ -979,10 +976,7 @@ export class IndexedDbPersistence implements Persistence { typeof this.window?.removeEventListener === 'function', "Expected 'window.removeEventListener' to be a function" ); - this.window!.removeEventListener( - terminationEvent(this.window), - this.windowUnloadHandler - ); + this.window!.removeEventListener('pagehide', this.windowUnloadHandler); this.windowUnloadHandler = null; } } diff --git a/packages/firestore/src/local/shared_client_state.ts b/packages/firestore/src/local/shared_client_state.ts index 7405a899cd5..a3deb3c5179 100644 --- a/packages/firestore/src/local/shared_client_state.ts +++ b/packages/firestore/src/local/shared_client_state.ts @@ -32,7 +32,7 @@ import { logError, logDebug } from '../util/log'; import { primitiveComparator } from '../util/misc'; import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; -import { isSafeInteger, terminationEvent, WindowLike } from '../util/types'; +import { isSafeInteger, WindowLike } from '../util/types'; import { CLIENT_STATE_KEY_PREFIX, @@ -613,9 +613,7 @@ export class WebStorageSharedClientState implements SharedClientState { // Register a window unload hook to remove the client metadata entry from // WebStorage even if `shutdown()` was not called. - this.window.addEventListener(terminationEvent(this.window), () => - this.shutdown() - ); + this.window.addEventListener('pagehide', () => this.shutdown()); this.started = true; } diff --git a/packages/firestore/src/util/types.ts b/packages/firestore/src/util/types.ts index f6a34293923..be547455983 100644 --- a/packages/firestore/src/util/types.ts +++ b/packages/firestore/src/util/types.ts @@ -59,14 +59,6 @@ export interface WindowLike { removeEventListener(type: string, listener: EventListener): void; } -/** - * Returns the page termination event to listen to. 'pagehide' is recommended - * if available, it falls back to the less reliable 'unload'. - */ -export function terminationEvent(window: WindowLike | null): string { - return 'onpagehide' in window! ? 'pagehide' : 'unload'; -} - /** The subset of the browser's Document interface used by the SDK. */ export interface DocumentLike { readonly visibilityState: VisibilityState; From 4aacca78b3ad29b97e151ad57699653a4a191c4a Mon Sep 17 00:00:00 2001 From: wu-hui <53845758+wu-hui@users.noreply.github.com> Date: Tue, 11 May 2021 10:25:12 -0400 Subject: [PATCH 4/5] Create perfect-comics-march.md --- .changeset/perfect-comics-march.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/perfect-comics-march.md diff --git a/.changeset/perfect-comics-march.md b/.changeset/perfect-comics-march.md new file mode 100644 index 00000000000..c7e0f24da3c --- /dev/null +++ b/.changeset/perfect-comics-march.md @@ -0,0 +1,5 @@ +--- +"@firebase/firestore": patch +--- + +Use 'pagehide' for page termination by default. From 044c5a251cb2f098eb408a0f84f27f6a27d74af6 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Tue, 11 May 2021 13:11:40 -0400 Subject: [PATCH 5/5] Update test platform --- packages/firestore/test/util/test_platform.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/util/test_platform.ts b/packages/firestore/test/util/test_platform.ts index 27249def42c..88a71c09a44 100644 --- a/packages/firestore/test/util/test_platform.ts +++ b/packages/firestore/test/util/test_platform.ts @@ -58,7 +58,7 @@ export class FakeWindow implements WindowLike { case 'storage': this.storageListeners.push(listener); break; - case 'unload': + case 'pagehide': case 'visibilitychange': // The spec tests currently do not rely on `unload`/`visibilitychange` // listeners.