Merge 013d821 into 0f891d8

Author: dconeybe

packages/firestore/src/core/firestore_client.ts

@@ -33,7 +33,11 @@ import {
   localStoreReadDocument,
   localStoreSetIndexAutoCreationEnabled
 } from '../local/local_store_impl';
-import { Persistence } from '../local/persistence';
+import {
+  Persistence,
+  DatabaseDeletedListenerAbortResult,
+  DatabaseDeletedListenerContinueResult
+} from '../local/persistence';
 import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { FieldIndex } from '../model/field_index';
@@ -232,9 +236,17 @@ 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(reason => {
+    // eslint-disable-next-line @typescript-eslint/no-floating-promises
+    client.terminate();
+    if (reason === 'site data cleared') {
+      return new DatabaseDeletedListenerAbortResult(
+        'protecting against database corruption'
+      );
+    } else {
+      return new DatabaseDeletedListenerContinueResult();
+    }
+  });
 
   client._offlineComponents = offlineComponentProvider;
 }

packages/firestore/src/local/indexeddb_persistence.ts

@@ -58,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 {
+  Persistence,
+  PrimaryStateListener,
+  DatabaseDeletedListener
+} from './persistence';
 import { PersistencePromise } from './persistence_promise';
 import {
   PersistenceTransaction,
@@ -324,20 +328,18 @@ 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 database receives an
+   * event indicating that it has deleted. This could be, for example, another
+   * tab in multi-tab persistence mode having its `clearIndexedDbPersistence()`
+   * function called, or a user manually clicking "Clear Site Data" in a
+   * browser.
    *
    * 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,25 @@ export interface ReferenceDelegate {
   ): PersistencePromise<void>;
 }
 
+export type DatabaseDeletedReason = 'persistence cleared' | 'site data cleared';
+
+export class DatabaseDeletedListenerContinueResult {
+  readonly type = 'continue' as const;
+}
+
+export class DatabaseDeletedListenerAbortResult {
+  readonly type = 'abort' as const;
+  constructor(readonly abortReason: string) {}
+}
+
+export type DatabaseDeletedListenerResult =
+  | DatabaseDeletedListenerContinueResult
+  | DatabaseDeletedListenerAbortResult;
+
+export type DatabaseDeletedListener = (
+  reason: DatabaseDeletedReason
+) => DatabaseDeletedListenerResult;
+
 /**
  * Persistence is the lowest-level shared interface to persistent storage in
  * Firestore.
@@ -151,13 +170,16 @@ 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 database receives an
+   * event indicating that it has deleted. This could be, for example, another
+   * tab in multi-tab persistence mode having its `clearIndexedDbPersistence()`
+   * function called, or a user manually clicking "Clear Site Data" in a
+   * browser.
    *
    * PORTING NOTE: This is only used for Web multi-tab.
    */
   setDatabaseDeletedListener(
-    databaseDeletedListener: () => Promise<void>
+    databaseDeletedListener: DatabaseDeletedListener
   ): void;
 
   /**

packages/firestore/src/local/simple_db.ts

@@ -19,9 +19,10 @@ 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 { type DatabaseDeletedListener } from './persistence';
 import { PersistencePromise } from './persistence_promise';
 
 // References to `indexedDB` are guarded by SimpleDb.isAvailable() and getGlobal()
@@ -159,7 +160,7 @@ export class SimpleDbTransaction {
 export class SimpleDb {
   private db?: IDBDatabase;
   private lastClosedDbVersion: number | null = null;
-  private versionchangelistener?: (event: IDBVersionChangeEvent) => void;
+  private databaseDeletedListener?: DatabaseDeletedListener;
 
   /** Deletes the specified database. */
   static delete(name: string): Promise<void> {
@@ -352,19 +353,36 @@ export class SimpleDb {
             this.lastClosedDbVersion !== null &&
             this.lastClosedDbVersion !== event.oldVersion
           ) {
-            // This thrown error will get passed to the `onerror` callback
-            // registered above, and will then be propagated correctly.
-            throw new Error(
-              `refusing to open IndexedDB database due to potential ` +
-                `corruption of the IndexedDB database data; this corruption ` +
-                `could be caused by clicking the "clear site data" button in ` +
-                `a web browser; try reloading the web page to re-initialize ` +
-                `the IndexedDB database: ` +
+            logWarn(
+              `IndexedDB onupgradeneeded indicates that the ` +
+                `database contents may have been cleared, such as by clicking ` +
+                `the "clear site data" button in a browser. This _could_ cause ` +
+                `corruption of the IndexeDB database data if the clear ` +
+                `operation happened in the middle of Firestore operations. (` +
+                `db.name=${db.name}, ` +
+                `db.version=${db.version}, ` +
                 `lastClosedDbVersion=${this.lastClosedDbVersion}, ` +
                 `event.oldVersion=${event.oldVersion}, ` +
-                `event.newVersion=${event.newVersion}, ` +
-                `db.version=${db.version}`
+                `event.newVersion=${event.newVersion}` +
+                `)`
             );
+            if (this.databaseDeletedListener) {
+              const listenerResult =
+                this.databaseDeletedListener('site data cleared');
+              if (listenerResult.type !== 'continue') {
+                throw new Error(
+                  `Refusing to open IndexedDB database after having been ` +
+                    `cleared, such as by clicking the "clear site data" button ` +
+                    `in a web browser: ${listenerResult.abortReason} (` +
+                    `db.name=${db.name}, ` +
+                    `db.version=${db.version}, ` +
+                    `lastClosedDbVersion=${this.lastClosedDbVersion}, ` +
+                    `event.oldVersion=${event.oldVersion}, ` +
+                    `event.newVersion=${event.newVersion}` +
+                    `)`
+                );
+              }
+            }
           }
           this.schemaConverter
             .createOrUpgrade(
@@ -387,27 +405,64 @@ export class SimpleDb {
         event => {
           const db = event.target as IDBDatabase;
           this.lastClosedDbVersion = db.version;
+          logWarn(
+            `IndexedDB "close" event received, indicating abnormal database ` +
+              `closure. The database contents may have been cleared, such as ` +
+              `by clicking the "clear site data" button in a browser. ` +
+              `Re-opening the IndexedDB database may fail to avoid IndexedDB ` +
+              `database data corruption (` +
+              `db.name=${db.name}, ` +
+              `db.version=${db.version}` +
+              `)`
+          );
         },
         { passive: true }
       );
     }
 
-    if (this.versionchangelistener) {
-      this.db.onversionchange = event => this.versionchangelistener!(event);
-    }
+    this.db.addEventListener(
+      'versionchange',
+      event => {
+        const db = event.target as IDBDatabase;
+        if (event.newVersion !== null) {
+          return;
+        }
+
+        logDebug(
+          `IndexedDB "versionchange" event with newVersion===null received; ` +
+            `this is likely because clearIndexedDbPersistence() was called, ` +
+            `possibly in another tab if multi-tab persistence is enabled.`
+        );
+        if (this.databaseDeletedListener) {
+          const listenerResult = this.databaseDeletedListener(
+            'persistence cleared'
+          );
+          if (listenerResult.type !== 'continue') {
+            logWarn(
+              `Closing IndexedDB database "${db.name}" in response to ` +
+                `"versionchange" event with newVersion===null: ` +
+                `${listenerResult.abortReason}`
+            );
+            db.close();
+            if (db === this.db) {
+              this.db = undefined;
+            }
+          }
+        }
+      },
+      { 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('setOnDatabaseDeletedListener() has already been called');
     }
+    this.databaseDeletedListener = databaseDeletedListener;
   }
 
   async runTransaction<T>(

packages/firestore/test/unit/specs/spec_test_runner.ts

@@ -85,6 +85,7 @@ import { LocalStore } from '../../../src/local/local_store';
 import { localStoreConfigureFieldIndexes } from '../../../src/local/local_store_impl';
 import { LruGarbageCollector } from '../../../src/local/lru_garbage_collector';
 import { MemoryLruDelegate } from '../../../src/local/memory_persistence';
+import { DatabaseDeletedListenerContinueResult } from '../../../src/local/persistence';
 import {
   ClientId,
   SharedClientState
@@ -365,8 +366,10 @@ abstract class TestRunner {
     this.eventManager.onLastRemoteStoreUnlisten =
       triggerRemoteStoreUnlisten.bind(null, this.syncEngine);
 
-    await this.persistence.setDatabaseDeletedListener(async () => {
-      await this.shutdown();
+    this.persistence.setDatabaseDeletedListener(() => {
+      // eslint-disable-next-line @typescript-eslint/no-floating-promises
+      this.shutdown();
+      return new DatabaseDeletedListenerContinueResult();
     });
 
     this.started = true;