[Multi-Tab] Merge Enable Persistence Started (#1054)

Author: schmidt-sebastian

packages/firestore/src/local/indexeddb_persistence.ts

@@ -125,7 +125,7 @@ export class IndexedDbPersistence implements Persistence {
   private readonly window: Window;
 
   private simpleDb: SimpleDb;
-  private started: boolean;
+  private _started = false;
   private isPrimary = false;
   private networkEnabled = true;
   private dbName: string;
@@ -184,7 +184,6 @@ export class IndexedDbPersistence implements Persistence {
 
     assert(!this.started, 'IndexedDbPersistence double-started!');
     this.allowTabSynchronization = !!synchronizeTabs;
-    this.started = true;
 
     assert(this.window !== null, "Expected 'window' to be defined");
 
@@ -198,6 +197,9 @@ export class IndexedDbPersistence implements Persistence {
         return this.updateClientMetadataAndTryBecomePrimary().then(() =>
           this.scheduleClientMetadataAndPrimaryLeaseRefreshes()
         );
+      })
+      .then(() => {
+        this._started = true;
       });
   }
 
@@ -391,10 +393,9 @@ export class IndexedDbPersistence implements Persistence {
   }
 
   async shutdown(deleteData?: boolean): Promise<void> {
-    if (!this.started) {
-      return Promise.resolve();
-    }
-    this.started = false;
+    // The shutdown() operations are idempotent and can be called even when
+    // start() aborted (e.g. because it couldn't acquire the persistence lease).
+    this._started = false;
 
     this.markClientZombied();
     if (this.clientMetadataRefresher) {
@@ -434,6 +435,10 @@ export class IndexedDbPersistence implements Persistence {
       .then(() => clientIds);
   }
 
+  get started(): boolean {
+    return this._started;
+  }
+
   getMutationQueue(user: User): MutationQueue {
     return IndexedDbMutationQueue.forUser(user, this.serializer);
   }

packages/firestore/src/local/local_store.ts

@@ -176,6 +176,10 @@ export class LocalStore {
      */
     private sharedClientState: SharedClientState
   ) {
+    assert(
+      persistence.started,
+      'LocalStore was passed an unstarted persistence implementation'
+    );
     this.mutationQueue = persistence.getMutationQueue(initialUser);
     this.remoteDocuments = persistence.getRemoteDocumentCache();
     this.queryCache = persistence.getQueryCache();

packages/firestore/src/local/memory_persistence.ts

@@ -50,20 +50,23 @@ export class MemoryPersistence implements Persistence {
   private remoteDocumentCache = new MemoryRemoteDocumentCache();
   private queryCache = new MemoryQueryCache();
 
-  private started = false;
+  private _started = false;
 
   constructor(private readonly clientId: ClientId) {}
 
   async start(): Promise<void> {
     // No durable state to read on startup.
-    assert(!this.started, 'MemoryPersistence double-started!');
-    this.started = true;
+    assert(!this._started, 'MemoryPersistence double-started!');
+    this._started = true;
   }
 
   async shutdown(deleteData?: boolean): Promise<void> {
     // No durable state to ensure is closed on shutdown.
-    assert(this.started, 'MemoryPersistence shutdown without start!');
-    this.started = false;
+    this._started = false;
+  }
+
+  get started(): boolean {
+    return this._started;
   }
 
   async getActiveClients(): Promise<ClientId[]> {

packages/firestore/src/local/persistence.ts

@@ -84,6 +84,11 @@ export type PrimaryStateListener = (isPrimary: boolean) => Promise<void>;
 // point out (and maybe enforce) when methods cannot safely be used from
 // secondary tabs.
 export interface Persistence {
+  /**
+   * Whether or not this persistence instance has been started.
+   */
+  readonly started: boolean;
+
   /**
    * Starts persistent storage, opening the database or similar.
    *

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

@@ -141,13 +141,6 @@ class MockConnection implements Connection {
   /** A Deferred that is resolved once watch opens. */
   watchOpen = new Deferred<void>();
 
-  reset(): void {
-    this.watchStreamRequestCount = 0;
-    this.writeStreamRequestCount = 0;
-    this.earlyWrites = [];
-    this.activeTargets = [];
-  }
-
   invokeRPC<Req>(rpcName: string, request: Req): never {
     throw new Error('Not implemented!');
   }
@@ -399,23 +392,29 @@ abstract class TestRunner {
       'host',
       false
     );
+
+    // TODO(mrschmidt): During client startup in `firestore_client`, we block
+    // the AsyncQueue from executing any operation. We should mimic this in the
+    // setup of the spec tests.
     this.queue = new AsyncQueue();
     this.serializer = new JsonProtoSerializer(this.databaseInfo.databaseId, {
       useProto3Json: true
     });
-    this.persistence = this.getPersistence(this.serializer);
 
     this.useGarbageCollection = config.useGarbageCollection;
 
-    this.init();
-
     this.expectedLimboDocs = [];
     this.expectedActiveTargets = {};
     this.acknowledgedDocs = [];
     this.rejectedDocs = [];
   }
 
-  private init(): void {
+  async start(): Promise<void> {
+    this.persistence = await this.initPersistence(this.serializer);
+    await this.init();
+  }
+
+  private async init(): Promise<void> {
     const garbageCollector = this.getGarbageCollector();
 
     this.sharedClientState = this.getSharedClientState();
@@ -466,6 +465,16 @@ abstract class TestRunner {
     this.sharedClientState.onlineStateHandler = sharedClientStateOnlineStateChangedHandler;
 
     this.eventManager = new EventManager(this.syncEngine);
+
+    await this.localStore.start();
+    await this.sharedClientState.start();
+    await this.remoteStore.start();
+
+    await this.persistence.setPrimaryStateListener(isPrimary =>
+      this.syncEngine.applyPrimaryState(isPrimary)
+    );
+
+    this.started = true;
   }
 
   private getGarbageCollector(): GarbageCollector {
@@ -474,32 +483,16 @@ abstract class TestRunner {
       : new NoOpGarbageCollector();
   }
 
-  protected abstract getPersistence(
+  protected abstract initPersistence(
     serializer: JsonProtoSerializer
-  ): Persistence;
-
-  protected abstract startPersistence(persistence: Persistence): Promise<void>;
+  ): Promise<Persistence>;
 
   protected abstract getSharedClientState(): SharedClientState;
 
   get isPrimaryClient(): boolean {
     return this.syncEngine.isPrimaryClient;
   }
 
-  async start(): Promise<void> {
-    this.connection.reset();
-    await this.startPersistence(this.persistence);
-    await this.localStore.start();
-    await this.sharedClientState.start();
-    await this.remoteStore.start();
-
-    await this.persistence.setPrimaryStateListener(isPrimary =>
-      this.syncEngine.applyPrimaryState(isPrimary)
-    );
-
-    this.started = true;
-  }
-
   async shutdown(): Promise<void> {
     if (this.started) {
       await this.doShutdown();
@@ -874,19 +867,9 @@ abstract class TestRunner {
     // No local store to shutdown.
     await this.remoteStore.shutdown();
 
-    this.init();
-
     // We have to schedule the starts, otherwise we could end up with
     // interleaved events.
-    await this.queue.enqueue(async () => {
-      await this.localStore.start();
-      await this.remoteStore.start();
-      await this.sharedClientState.start();
-
-      await this.persistence.setPrimaryStateListener(isPrimary =>
-        this.syncEngine.applyPrimaryState(isPrimary)
-      );
-    });
+    await this.queue.enqueue(() => this.init());
   }
 
   private async doApplyClientState(state: SpecClientState): Promise<void> {
@@ -1142,16 +1125,16 @@ abstract class TestRunner {
 }
 
 class MemoryTestRunner extends TestRunner {
-  protected getPersistence(serializer: JsonProtoSerializer): Persistence {
-    return new MemoryPersistence(this.clientId);
-  }
-
   protected getSharedClientState(): SharedClientState {
     return new MemorySharedClientState();
   }
 
-  protected startPersistence(persistence: Persistence): Promise<void> {
-    return persistence.start();
+  protected async initPersistence(
+    serializer: JsonProtoSerializer
+  ): Promise<Persistence> {
+    const persistence = new MemoryPersistence(this.clientId);
+    await persistence.start();
+    return persistence;
   }
 }
 
@@ -1162,16 +1145,6 @@ class MemoryTestRunner extends TestRunner {
 class IndexedDbTestRunner extends TestRunner {
   static TEST_DB_NAME = 'firestore/[DEFAULT]/specs';
 
-  protected getPersistence(serializer: JsonProtoSerializer): Persistence {
-    return new IndexedDbPersistence(
-      IndexedDbTestRunner.TEST_DB_NAME,
-      this.clientId,
-      this.platform,
-      this.queue,
-      serializer
-    );
-  }
-
   protected getSharedClientState(): SharedClientState {
     return new WebStorageSharedClientState(
       this.queue,
@@ -1182,10 +1155,18 @@ class IndexedDbTestRunner extends TestRunner {
     );
   }
 
-  protected startPersistence(persistence: Persistence): Promise<void> {
-    return (persistence as IndexedDbPersistence).start(
-      /*synchronizeTabs=*/ true
+  protected async initPersistence(
+    serializer: JsonProtoSerializer
+  ): Promise<Persistence> {
+    const persistence = new IndexedDbPersistence(
+      IndexedDbTestRunner.TEST_DB_NAME,
+      this.clientId,
+      this.platform,
+      this.queue,
+      serializer
     );
+    await persistence.start(/*synchronizeTabs=*/ true);
+    return persistence;
   }
 
   static destroyPersistence(): Promise<void> {