simple_db.ts: change setVersionChangeListener to setDatabaseDeletedListener to be more abstract.

Author: dconeybe

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;
 }

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<void>
+    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);
   }
 
   /**

packages/firestore/src/local/persistence.ts

@@ -98,6 +98,8 @@ export interface ReferenceDelegate {
   ): PersistencePromise<void>;
 }
 
+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<void>;
 
   /**
-   * 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<void>
+    databaseDeletedListener: DatabaseDeletedListener
   ): void;
 
   /**

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<void> {
@@ -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<T>(

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;