diff --git a/.changeset/hungry-icons-dream.md b/.changeset/hungry-icons-dream.md new file mode 100644 index 00000000000..29137c772fd --- /dev/null +++ b/.changeset/hungry-icons-dream.md @@ -0,0 +1,5 @@ +--- +"@firebase/firestore": patch +--- + +Fixed a regression where the SDK did not re-connect to IndexedDb after disconnect (#9087) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index d04f37a3d7b..39bb8dd4eba 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -231,23 +231,11 @@ export async function setOfflineComponentProvider( } }); - offlineComponentProvider.persistence.setDatabaseDeletedListener(() => { - 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 - ); - }); - }); + // 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() + ); client._offlineComponents = offlineComponentProvider; } diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 0ec2baabfe4..57c26ea5baa 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -58,11 +58,7 @@ import { IndexedDbTargetCache } from './indexeddb_target_cache'; import { getStore, IndexedDbTransaction } from './indexeddb_transaction'; import { LocalSerializer } from './local_serializer'; import { LruParams } from './lru_garbage_collector'; -import { - DatabaseDeletedListener, - Persistence, - PrimaryStateListener -} from './persistence'; +import { Persistence, PrimaryStateListener } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { PersistenceTransaction, @@ -328,25 +324,20 @@ export class IndexedDbPersistence implements Persistence { } /** - * 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. + * Registers a listener that gets called when the database receives a + * version change event indicating that it has deleted. * * PORTING NOTE: This is only used for Web multi-tab. */ setDatabaseDeletedListener( - databaseDeletedListener: DatabaseDeletedListener + databaseDeletedListener: () => Promise ): void { - this.simpleDb.setDatabaseDeletedListener(databaseDeletedListener); + this.simpleDb.setVersionChangeListener(async event => { + // Check if an attempt is made to delete IndexedDB. + if (event.newVersion === null) { + await databaseDeletedListener(); + } + }); } /** diff --git a/packages/firestore/src/local/persistence.ts b/packages/firestore/src/local/persistence.ts index 113efe7b7d3..b014a6479ac 100644 --- a/packages/firestore/src/local/persistence.ts +++ b/packages/firestore/src/local/persistence.ts @@ -98,8 +98,6 @@ export interface ReferenceDelegate { ): PersistencePromise; } -export type DatabaseDeletedListener = () => void; - /** * Persistence is the lowest-level shared interface to persistent storage in * Firestore. @@ -153,23 +151,13 @@ export interface Persistence { shutdown(): Promise; /** - * 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. + * Registers a listener that gets called when the database receives a + * version change event indicating that it has deleted. * * PORTING NOTE: This is only used for Web multi-tab. */ setDatabaseDeletedListener( - databaseDeletedListener: DatabaseDeletedListener + databaseDeletedListener: () => Promise ): void; /** diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index e284c3c46ee..1958d853690 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -19,10 +19,9 @@ import { getGlobal, getUA, isIndexedDBAvailable } from '@firebase/util'; import { debugAssert } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; -import { logDebug, logError, logWarn } from '../util/log'; +import { logDebug, logError } from '../util/log'; import { Deferred } from '../util/promise'; -import { DatabaseDeletedListener } from './persistence'; import { PersistencePromise } from './persistence_promise'; // References to `indexedDB` are guarded by SimpleDb.isAvailable() and getGlobal() @@ -159,7 +158,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 { @@ -365,35 +365,22 @@ export class SimpleDb { }); } - 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) { - logWarn( - `Received "versionchange" event with newVersion===null; ` + - 'notifying the registered DatabaseDeletedListener, if any' - ); - this.databaseDeletedListener?.(); - } - }, - { passive: true } - ); + if (this.versionchangelistener) { + this.db.onversionchange = event => this.versionchangelistener!(event); + } return this.db; } - setDatabaseDeletedListener( - databaseDeletedListener: DatabaseDeletedListener + setVersionChangeListener( + versionChangeListener: (event: IDBVersionChangeEvent) => void ): void { - if (this.databaseDeletedListener) { - throw new Error( - 'setDatabaseDeletedListener() may only be called once, ' + - 'and it has already been called' - ); + this.versionchangelistener = versionChangeListener; + if (this.db) { + this.db.onversionchange = (event: IDBVersionChangeEvent) => { + return versionChangeListener(event); + }; } - 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 daa513edb68..51d2229b8a1 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -365,14 +365,8 @@ abstract class TestRunner { this.eventManager.onLastRemoteStoreUnlisten = triggerRemoteStoreUnlisten.bind(null, this.syncEngine); - this.persistence.setDatabaseDeletedListener(() => { - this.shutdown().catch(error => { - console.warn( - 'WARNING: this.shutdown() failed in callback ' + - 'specified to persistence.setDatabaseDeletedListener', - error - ); - }); + await this.persistence.setDatabaseDeletedListener(async () => { + await this.shutdown(); }); this.started = true;