Clean up IndexedDb initialization (#2163)

Author: schmidt-sebastian

packages/firestore/src/core/firestore_client.ts

@@ -328,7 +328,7 @@ export class FirestoreClient {
   ): Promise<LruGarbageCollector> {
     // TODO(http://b/33384523): For now we just disable garbage collection
     // when persistence is enabled.
-    const storagePrefix = IndexedDbPersistence.buildStoragePrefix(
+    const persistenceKey = IndexedDbPersistence.buildStoragePrefix(
       this.databaseInfo
     );
     // Opt to use proto3 JSON in case the platform doesn't support Uint8Array.
@@ -347,36 +347,31 @@ export class FirestoreClient {
         );
       }
 
-      let persistence: IndexedDbPersistence;
       const lruParams = settings.lruParams();
-      if (settings.synchronizeTabs) {
-        this.sharedClientState = new WebStorageSharedClientState(
-          this.asyncQueue,
-          this.platform,
-          storagePrefix,
-          this.clientId,
-          user
-        );
-        persistence = await IndexedDbPersistence.createMultiClientIndexedDbPersistence(
-          storagePrefix,
-          this.clientId,
-          this.platform,
-          this.asyncQueue,
+
+      this.sharedClientState = settings.synchronizeTabs
+        ? new WebStorageSharedClientState(
+            this.asyncQueue,
+            this.platform,
+            persistenceKey,
+            this.clientId,
+            user
+          )
+        : new MemorySharedClientState();
+
+      const persistence = await IndexedDbPersistence.createIndexedDbPersistence(
+        {
+          allowTabSynchronization: settings.synchronizeTabs,
+          persistenceKey,
+          clientId: this.clientId,
+          platform: this.platform,
+          queue: this.asyncQueue,
           serializer,
           lruParams,
-          { sequenceNumberSyncer: this.sharedClientState }
-        );
-      } else {
-        this.sharedClientState = new MemorySharedClientState();
-        persistence = await IndexedDbPersistence.createIndexedDbPersistence(
-          storagePrefix,
-          this.clientId,
-          this.platform,
-          this.asyncQueue,
-          serializer,
-          lruParams
-        );
-      }
+          sequenceNumberSyncer: this.sharedClientState
+        }
+      );
+
       this.persistence = persistence;
       return persistence.referenceDelegate.garbageCollector;
     });

packages/firestore/src/local/indexeddb_persistence.ts

@@ -168,9 +168,6 @@ export class IndexedDbTransaction extends PersistenceTransaction {
  * TODO(b/114226234): Remove `synchronizeTabs` section when multi-tab is no
  * longer optional.
  */
-export interface MultiClientParams {
-  sequenceNumberSyncer: SequenceNumberSyncer;
-}
 export class IndexedDbPersistence implements Persistence {
   static getStore<Key extends IDBValidKey, Value>(
     txn: PersistenceTransaction,
@@ -191,43 +188,32 @@ export class IndexedDbPersistence implements Persistence {
    */
   static MAIN_DATABASE = 'main';
 
-  static async createIndexedDbPersistence(
-    persistenceKey: string,
-    clientId: ClientId,
-    platform: Platform,
-    queue: AsyncQueue,
-    serializer: JsonProtoSerializer,
-    lruParams: LruParams
-  ): Promise<IndexedDbPersistence> {
-    const persistence = new IndexedDbPersistence(
-      persistenceKey,
-      clientId,
-      platform,
-      queue,
-      serializer,
-      lruParams
-    );
-    await persistence.start();
-    return persistence;
-  }
+  static async createIndexedDbPersistence(options: {
+    allowTabSynchronization: boolean;
+    persistenceKey: string;
+    clientId: ClientId;
+    platform: Platform;
+    lruParams: LruParams;
+    queue: AsyncQueue;
+    serializer: JsonProtoSerializer;
+    sequenceNumberSyncer: SequenceNumberSyncer;
+  }): Promise<IndexedDbPersistence> {
+    if (!IndexedDbPersistence.isAvailable()) {
+      throw new FirestoreError(
+        Code.UNIMPLEMENTED,
+        UNSUPPORTED_PLATFORM_ERROR_MSG
+      );
+    }
 
-  static async createMultiClientIndexedDbPersistence(
-    persistenceKey: string,
-    clientId: ClientId,
-    platform: Platform,
-    queue: AsyncQueue,
-    serializer: JsonProtoSerializer,
-    lruParams: LruParams,
-    multiClientParams: MultiClientParams
-  ): Promise<IndexedDbPersistence> {
     const persistence = new IndexedDbPersistence(
-      persistenceKey,
-      clientId,
-      platform,
-      queue,
-      serializer,
-      lruParams,
-      multiClientParams
+      options.allowTabSynchronization,
+      options.persistenceKey,
+      options.clientId,
+      options.platform,
+      options.lruParams,
+      options.queue,
+      options.serializer,
+      options.sequenceNumberSyncer
     );
     await persistence.start();
     return persistence;
@@ -262,9 +248,6 @@ export class IndexedDbPersistence implements Persistence {
   /** The last time we garbage collected the Remote Document Changelog. */
   private lastGarbageCollectionTime = Number.NEGATIVE_INFINITY;
 
-  /** Whether to allow shared multi-tab access to the persistence layer. */
-  private allowTabSynchronization: boolean;
-
   /** A listener to notify on primary state changes. */
   private primaryStateListener: PrimaryStateListener = _ => Promise.resolve();
 
@@ -274,29 +257,20 @@ export class IndexedDbPersistence implements Persistence {
   private readonly webStorage: Storage;
   readonly referenceDelegate: IndexedDbLruDelegate;
 
-  // Note that `multiClientParams` must be present to enable multi-client support while multi-tab
-  // is still experimental. When multi-client is switched to always on, `multiClientParams` will
-  // no longer be optional.
   private constructor(
+    private readonly allowTabSynchronization: boolean,
     private readonly persistenceKey: string,
     private readonly clientId: ClientId,
     platform: Platform,
+    lruParams: LruParams,
     private readonly queue: AsyncQueue,
     serializer: JsonProtoSerializer,
-    lruParams: LruParams,
-    private readonly multiClientParams?: MultiClientParams
+    private readonly sequenceNumberSyncer: SequenceNumberSyncer
   ) {
-    if (!IndexedDbPersistence.isAvailable()) {
-      throw new FirestoreError(
-        Code.UNIMPLEMENTED,
-        UNSUPPORTED_PLATFORM_ERROR_MSG
-      );
-    }
     this.referenceDelegate = new IndexedDbLruDelegate(this, lruParams);
     this.dbName = persistenceKey + IndexedDbPersistence.MAIN_DATABASE;
     this.serializer = new LocalSerializer(serializer);
     this.document = platform.document;
-    this.allowTabSynchronization = multiClientParams !== undefined;
     this.queryCache = new IndexedDbQueryCache(
       this.referenceDelegate,
       this.serializer
@@ -353,12 +327,9 @@ export class IndexedDbPersistence implements Persistence {
           txn => {
             return getHighestListenSequenceNumber(txn).next(
               highestListenSequenceNumber => {
-                const sequenceNumberSyncer = this.multiClientParams
-                  ? this.multiClientParams.sequenceNumberSyncer
-                  : undefined;
                 this.listenSequence = new ListenSequence(
                   highestListenSequenceNumber,
-                  sequenceNumberSyncer
+                  this.sequenceNumberSyncer
                 );
               }
             );

packages/firestore/test/unit/local/indexeddb_persistence.test.ts

@@ -119,26 +119,16 @@ async function withCustomPersistence(
     PlatformSupport.getPlatform(),
     new SharedFakeWebStorage()
   );
-  const persistence = await (multiClient
-    ? IndexedDbPersistence.createMultiClientIndexedDbPersistence(
-        TEST_PERSISTENCE_PREFIX,
-        clientId,
-        platform,
-        queue,
-        serializer,
-        LruParams.DEFAULT,
-        {
-          sequenceNumberSyncer: MOCK_SEQUENCE_NUMBER_SYNCER
-        }
-      )
-    : IndexedDbPersistence.createIndexedDbPersistence(
-        TEST_PERSISTENCE_PREFIX,
-        clientId,
-        platform,
-        queue,
-        serializer,
-        LruParams.DEFAULT
-      ));
+  const persistence = await IndexedDbPersistence.createIndexedDbPersistence({
+    allowTabSynchronization: multiClient,
+    persistenceKey: TEST_PERSISTENCE_PREFIX,
+    clientId,
+    platform,
+    queue,
+    serializer,
+    lruParams: LruParams.DEFAULT,
+    sequenceNumberSyncer: MOCK_SEQUENCE_NUMBER_SYNCER
+  });
 
   await fn(persistence, platform, queue);
   await persistence.shutdown();

packages/firestore/test/unit/local/persistence_test_helpers.ts

@@ -108,24 +108,16 @@ export async function testIndexedDbPersistence(
     await SimpleDb.delete(prefix + IndexedDbPersistence.MAIN_DATABASE);
   }
   const platform = PlatformSupport.getPlatform();
-  return options.synchronizeTabs
-    ? IndexedDbPersistence.createMultiClientIndexedDbPersistence(
-        TEST_PERSISTENCE_PREFIX,
-        clientId,
-        platform,
-        queue,
-        JSON_SERIALIZER,
-        lruParams,
-        { sequenceNumberSyncer: MOCK_SEQUENCE_NUMBER_SYNCER }
-      )
-    : IndexedDbPersistence.createIndexedDbPersistence(
-        TEST_PERSISTENCE_PREFIX,
-        clientId,
-        platform,
-        queue,
-        JSON_SERIALIZER,
-        lruParams
-      );
+  return IndexedDbPersistence.createIndexedDbPersistence({
+    allowTabSynchronization: !!options.synchronizeTabs,
+    persistenceKey: TEST_PERSISTENCE_PREFIX,
+    clientId,
+    platform,
+    queue,
+    serializer: JSON_SERIALIZER,
+    lruParams,
+    sequenceNumberSyncer: MOCK_SEQUENCE_NUMBER_SYNCER
+  });
 }
 
 /** Creates and starts a MemoryPersistence instance for testing. */

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

@@ -1216,15 +1216,16 @@ class IndexedDbTestRunner extends TestRunner {
     gcEnabled: boolean
   ): Promise<Persistence> {
     // TODO(gsoltis): can we or should we disable this test if gc is enabled?
-    return IndexedDbPersistence.createMultiClientIndexedDbPersistence(
-      TEST_PERSISTENCE_PREFIX,
-      this.clientId,
-      this.platform,
-      this.queue,
+    return IndexedDbPersistence.createIndexedDbPersistence({
+      allowTabSynchronization: true,
+      persistenceKey: TEST_PERSISTENCE_PREFIX,
+      clientId: this.clientId,
+      platform: this.platform,
+      queue: this.queue,
       serializer,
-      LruParams.DEFAULT,
-      { sequenceNumberSyncer: this.sharedClientState }
-    );
+      lruParams: LruParams.DEFAULT,
+      sequenceNumberSyncer: this.sharedClientState
+    });
   }
 
   static destroyPersistence(): Promise<void> {