From b0e82534aa4be8d3e8335f4bc9e81494ea40556e Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 9 Jun 2025 19:32:47 +0000 Subject: [PATCH 1/3] simple_db.ts: change setVersionChangeListener to setDatabaseDeletedListener to be more abstract. --- .../firestore/src/core/firestore_client.ts | 8 ++--- .../src/local/indexeddb_persistence.ts | 24 ++++++++------ packages/firestore/src/local/persistence.ts | 18 +++++++++-- packages/firestore/src/local/simple_db.ts | 32 ++++++++++++------- .../test/unit/specs/spec_test_runner.ts | 4 +-- 5 files changed, 56 insertions(+), 30 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index e2aa19aaba8..93e458696a0 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -230,11 +230,9 @@ export async function setOfflineComponentProvider( } }); - // When a user calls clearPersistence() in one client, all other clients - // need to be terminated to allow the delete to succeed. - offlineComponentProvider.persistence.setDatabaseDeletedListener(() => - client.terminate() - ); + offlineComponentProvider.persistence.setDatabaseDeletedListener(() => { + client.terminate(); + }); client._offlineComponents = offlineComponentProvider; } diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 57c26ea5baa..61c815bdde4 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -31,6 +31,7 @@ import { BundleCache } from './bundle_cache'; import { DocumentOverlayCache } from './document_overlay_cache'; import { GlobalsCache } from './globals_cache'; import { IndexManager } from './index_manager'; +import { DatabaseDeletedListener } from './persistence'; import { IndexedDbBundleCache } from './indexeddb_bundle_cache'; import { IndexedDbDocumentOverlayCache } from './indexeddb_document_overlay_cache'; import { IndexedDbGlobalsCache } from './indexeddb_globals_cache'; @@ -324,20 +325,25 @@ export class IndexedDbPersistence implements Persistence { } /** - * Registers a listener that gets called when the database receives a - * version change event indicating that it has deleted. + * Registers a listener that gets called when the underlying database receives + * an event indicating that it either has been deleted or is pending deletion + * and must be closed. + * + * For example, this callback will be called in the case that multi-tab + * IndexedDB persistence is in use and another tab calls + * clearIndexedDbPersistence(). In that case, this Firestore instance must + * close its IndexedDB connection in order to allow the deletion initiated by + * the other tab to proceed. + * + * This method may only be called once; subsequent invocations will result in + * an exception, refusing to supersede the previously-registered listener. * * PORTING NOTE: This is only used for Web multi-tab. */ setDatabaseDeletedListener( - databaseDeletedListener: () => Promise + databaseDeletedListener: DatabaseDeletedListener ): void { - this.simpleDb.setVersionChangeListener(async event => { - // Check if an attempt is made to delete IndexedDB. - if (event.newVersion === null) { - await databaseDeletedListener(); - } - }); + this.simpleDb.setDatabaseDeletedListener(databaseDeletedListener); } /** diff --git a/packages/firestore/src/local/persistence.ts b/packages/firestore/src/local/persistence.ts index b014a6479ac..113efe7b7d3 100644 --- a/packages/firestore/src/local/persistence.ts +++ b/packages/firestore/src/local/persistence.ts @@ -98,6 +98,8 @@ export interface ReferenceDelegate { ): PersistencePromise; } +export type DatabaseDeletedListener = () => void; + /** * Persistence is the lowest-level shared interface to persistent storage in * Firestore. @@ -151,13 +153,23 @@ export interface Persistence { shutdown(): Promise; /** - * Registers a listener that gets called when the database receives a - * version change event indicating that it has deleted. + * Registers a listener that gets called when the underlying database receives + * an event indicating that it either has been deleted or is pending deletion + * and must be closed. + * + * For example, this callback will be called in the case that multi-tab + * IndexedDB persistence is in use and another tab calls + * clearIndexedDbPersistence(). In that case, this Firestore instance must + * close its IndexedDB connection in order to allow the deletion initiated by + * the other tab to proceed. + * + * This method may only be called once; subsequent invocations will result in + * an exception, refusing to supersede the previously-registered listener. * * PORTING NOTE: This is only used for Web multi-tab. */ setDatabaseDeletedListener( - databaseDeletedListener: () => Promise + databaseDeletedListener: DatabaseDeletedListener ): void; /** diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index 6d27702e725..3ac1583462a 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -23,6 +23,7 @@ import { logDebug, logError } from '../util/log'; import { Deferred } from '../util/promise'; import { PersistencePromise } from './persistence_promise'; +import { DatabaseDeletedListener } from './persistence'; // References to `indexedDB` are guarded by SimpleDb.isAvailable() and getGlobal() /* eslint-disable no-restricted-globals */ @@ -158,8 +159,8 @@ export class SimpleDbTransaction { */ export class SimpleDb { private db?: IDBDatabase; + private databaseDeletedListener?: DatabaseDeletedListener; private lastClosedDbVersion: number | null = null; - private versionchangelistener?: (event: IDBVersionChangeEvent) => void; /** Deletes the specified database. */ static delete(name: string): Promise { @@ -392,22 +393,31 @@ export class SimpleDb { ); } - if (this.versionchangelistener) { - this.db.onversionchange = event => this.versionchangelistener!(event); - } + this.db.addEventListener( + 'versionchange', + event => { + // Notify the listener if another tab attempted to delete the IndexedDb + // database, such as by calling clearIndexedDbPersistence(). + if (event.newVersion === null) { + this.databaseDeletedListener?.(); + } + }, + { passive: true } + ); return this.db; } - setVersionChangeListener( - versionChangeListener: (event: IDBVersionChangeEvent) => void + setDatabaseDeletedListener( + databaseDeletedListener: DatabaseDeletedListener ): void { - this.versionchangelistener = versionChangeListener; - if (this.db) { - this.db.onversionchange = (event: IDBVersionChangeEvent) => { - return versionChangeListener(event); - }; + if (this.databaseDeletedListener) { + throw new Error( + 'setDatabaseDeletedListener() may only be called once, ' + + 'and it has already been called' + ); } + this.databaseDeletedListener = databaseDeletedListener; } async runTransaction( diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 51d2229b8a1..3c7c21f0804 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -365,8 +365,8 @@ abstract class TestRunner { this.eventManager.onLastRemoteStoreUnlisten = triggerRemoteStoreUnlisten.bind(null, this.syncEngine); - await this.persistence.setDatabaseDeletedListener(async () => { - await this.shutdown(); + this.persistence.setDatabaseDeletedListener(() => { + this.shutdown(); }); this.started = true; From afd769b68c0a1ab5b1b56d87b14b24b5d956f2a8 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 9 Jun 2025 19:35:06 +0000 Subject: [PATCH 2/3] changeset added --- .changeset/long-pets-sell.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/long-pets-sell.md diff --git a/.changeset/long-pets-sell.md b/.changeset/long-pets-sell.md new file mode 100644 index 00000000000..d340f7da82c --- /dev/null +++ b/.changeset/long-pets-sell.md @@ -0,0 +1,5 @@ +--- +'@firebase/firestore': patch +--- + +Internal listener registration change for IndexedDB "versionchange" events. From d76d2af9310842c1a1000f9c9451cd7d6d9a51d4 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 9 Jun 2025 19:43:29 +0000 Subject: [PATCH 3/3] fix lint errors --- packages/firestore/src/core/firestore_client.ts | 16 +++++++++++++++- .../firestore/src/local/indexeddb_persistence.ts | 7 +++++-- packages/firestore/src/local/simple_db.ts | 8 ++++++-- .../test/unit/specs/spec_test_runner.ts | 8 +++++++- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 93e458696a0..e43060d229d 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -231,7 +231,21 @@ export async function setOfflineComponentProvider( }); offlineComponentProvider.persistence.setDatabaseDeletedListener(() => { - client.terminate(); + logWarn('Terminating Firestore due to IndexedDb database deletion'); + client + .terminate() + .then(() => { + logDebug( + 'Terminating Firestore due to IndexedDb database deletion ' + + 'completed successfully' + ); + }) + .catch(error => { + logWarn( + 'Terminating Firestore due to IndexedDb database deletion failed', + error + ); + }); }); client._offlineComponents = offlineComponentProvider; diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 61c815bdde4..0ec2baabfe4 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -31,7 +31,6 @@ import { BundleCache } from './bundle_cache'; import { DocumentOverlayCache } from './document_overlay_cache'; import { GlobalsCache } from './globals_cache'; import { IndexManager } from './index_manager'; -import { DatabaseDeletedListener } from './persistence'; import { IndexedDbBundleCache } from './indexeddb_bundle_cache'; import { IndexedDbDocumentOverlayCache } from './indexeddb_document_overlay_cache'; import { IndexedDbGlobalsCache } from './indexeddb_globals_cache'; @@ -59,7 +58,11 @@ import { IndexedDbTargetCache } from './indexeddb_target_cache'; import { getStore, IndexedDbTransaction } from './indexeddb_transaction'; import { LocalSerializer } from './local_serializer'; import { LruParams } from './lru_garbage_collector'; -import { Persistence, PrimaryStateListener } from './persistence'; +import { + DatabaseDeletedListener, + Persistence, + PrimaryStateListener +} from './persistence'; import { PersistencePromise } from './persistence_promise'; import { PersistenceTransaction, diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index 3ac1583462a..1e315c5dae6 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -19,11 +19,11 @@ import { getGlobal, getUA, isIndexedDBAvailable } from '@firebase/util'; import { debugAssert } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; -import { logDebug, logError } from '../util/log'; +import { logDebug, logError, logWarn } from '../util/log'; import { Deferred } from '../util/promise'; -import { PersistencePromise } from './persistence_promise'; import { DatabaseDeletedListener } from './persistence'; +import { PersistencePromise } from './persistence_promise'; // References to `indexedDB` are guarded by SimpleDb.isAvailable() and getGlobal() /* eslint-disable no-restricted-globals */ @@ -399,6 +399,10 @@ export class SimpleDb { // Notify the listener if another tab attempted to delete the IndexedDb // database, such as by calling clearIndexedDbPersistence(). if (event.newVersion === null) { + logWarn( + `Received "versionchange" event with newVersion===null; ` + + 'notifying the registered DatabaseDeletedListener, if any' + ); this.databaseDeletedListener?.(); } }, diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 3c7c21f0804..daa513edb68 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -366,7 +366,13 @@ abstract class TestRunner { triggerRemoteStoreUnlisten.bind(null, this.syncEngine); this.persistence.setDatabaseDeletedListener(() => { - this.shutdown(); + this.shutdown().catch(error => { + console.warn( + 'WARNING: this.shutdown() failed in callback ' + + 'specified to persistence.setDatabaseDeletedListener', + error + ); + }); }); this.started = true;