Merge branch 'master' into mrschmidt/pendingwrites

Author: schmidt-sebastian

packages/firestore/src/core/sync_engine.ts

@@ -919,7 +919,7 @@ export class MultiTabSyncEngine extends SyncEngine
   // The primary state is set to `true` or `false` immediately after Firestore
   // startup. In the interim, a client should only be considered primary if
   // `isPrimary` is true.
-  private isPrimary: undefined | boolean = undefined;
+  private _isPrimaryClient: undefined | boolean = undefined;
 
   constructor(
     protected localStore: MultiTabLocalStore,
@@ -938,7 +938,7 @@ export class MultiTabSyncEngine extends SyncEngine
   }
 
   get isPrimaryClient(): boolean {
-    return this.isPrimary === true;
+    return this._isPrimaryClient === true;
   }
 
   enableNetwork(): Promise<void> {
@@ -965,7 +965,7 @@ export class MultiTabSyncEngine extends SyncEngine
     const viewSnapshot = queryView.view.synchronizeWithPersistedState(
       queryResult
     );
-    if (this.isPrimary) {
+    if (this._isPrimaryClient) {
       this.updateTrackedLimbos(queryView.targetId, viewSnapshot.limboChanges);
     }
     return viewSnapshot;
@@ -1032,7 +1032,7 @@ export class MultiTabSyncEngine extends SyncEngine
   }
 
   async applyPrimaryState(isPrimary: boolean): Promise<void> {
-    if (isPrimary === true && this.isPrimary !== true) {
+    if (isPrimary === true && this._isPrimaryClient !== true) {
       // Secondary tabs only maintain Views for their local listeners and the
       // Views internal state may not be 100% populated (in particular
       // secondary tabs don't track syncedDocuments, the set of documents the
@@ -1044,12 +1044,12 @@ export class MultiTabSyncEngine extends SyncEngine
         activeTargets.toArray(),
         /*transitionToPrimary=*/ true
       );
-      this.isPrimary = true;
+      this._isPrimaryClient = true;
       await this.remoteStore.applyPrimaryState(true);
       for (const targetData of activeQueries) {
         this.remoteStore.listen(targetData);
       }
-    } else if (isPrimary === false && this.isPrimary !== false) {
+    } else if (isPrimary === false && this._isPrimaryClient !== false) {
       const activeTargets: TargetId[] = [];
 
       let p = Promise.resolve();
@@ -1074,7 +1074,7 @@ export class MultiTabSyncEngine extends SyncEngine
         /*transitionToPrimary=*/ false
       );
       this.resetLimboDocuments();
-      this.isPrimary = false;
+      this._isPrimaryClient = false;
       await this.remoteStore.applyPrimaryState(false);
     }
   }
@@ -1189,7 +1189,7 @@ export class MultiTabSyncEngine extends SyncEngine
     state: QueryTargetState,
     error?: FirestoreError
   ): Promise<void> {
-    if (this.isPrimary) {
+    if (this._isPrimaryClient) {
       // If we receive a target state notification via WebStorage, we are
       // either already secondary or another tab has taken the primary lease.
       logDebug(LOG_TAG, 'Ignoring unexpected query state notification.');
@@ -1229,7 +1229,7 @@ export class MultiTabSyncEngine extends SyncEngine
     added: TargetId[],
     removed: TargetId[]
   ): Promise<void> {
-    if (!this.isPrimary) {
+    if (!this._isPrimaryClient) {
       return;
     }
 

packages/firestore/src/local/indexeddb_persistence.ts

@@ -100,7 +100,7 @@ const MAX_PRIMARY_ELIGIBLE_AGE_MS = 5000;
 const CLIENT_METADATA_REFRESH_INTERVAL_MS = 4000;
 /** User-facing error when the primary lease is required but not available. */
 const PRIMARY_LEASE_EXCLUSIVE_ERROR_MSG =
-  'Another tab has exclusive access to the persistence layer. ' +
+  'Failed to obtain exclusive access to the persistence layer. ' +
   'To allow shared access, make sure to invoke ' +
   '`enablePersistence()` with `synchronizeTabs:true` in all tabs.';
 const UNSUPPORTED_PLATFORM_ERROR_MSG =
@@ -190,11 +190,12 @@ export class IndexedDbPersistence implements Persistence {
   private readonly document: Document | null;
   private readonly window: Window;
 
-  // Technically these types should be `| undefined` because they are
+  // Technically `simpleDb` should be `| undefined` because it is
   // initialized asynchronously by start(), but that would be more misleading
   // than useful.
   private simpleDb!: SimpleDb;
-  private listenSequence!: ListenSequence;
+
+  private listenSequence: ListenSequence | null = null;
 
   private _started = false;
   private isPrimary = false;
@@ -286,7 +287,15 @@ export class IndexedDbPersistence implements Persistence {
         // having the persistence lock), so it's the first thing we should do.
         return this.updateClientMetadataAndTryBecomePrimary();
       })
-      .then(() => {
+      .then(e => {
+        if (!this.isPrimary && !this.allowTabSynchronization) {
+          // Fail `start()` if `synchronizeTabs` is disabled and we cannot
+          // obtain the primary lease.
+          throw new FirestoreError(
+            Code.FAILED_PRECONDITION,
+            PRIMARY_LEASE_EXCLUSIVE_ERROR_MSG
+          );
+        }
         this.attachVisibilityHandler();
         this.attachWindowUnloadHook();
 
@@ -374,8 +383,10 @@ export class IndexedDbPersistence implements Persistence {
    * primary lease.
    */
   private updateClientMetadataAndTryBecomePrimary(): Promise<void> {
-    return this.simpleDb
-      .runTransaction('readwrite', ALL_STORES, txn => {
+    return this.runTransaction(
+      'updateClientMetadataAndTryBecomePrimary',
+      'readwrite',
+      txn => {
         const metadataStore = clientMetadataStore(txn);
         return metadataStore
           .put(
@@ -408,10 +419,18 @@ export class IndexedDbPersistence implements Persistence {
               return /* canActAsPrimary= */ false;
             }
           });
-      })
+      }
+    )
       .catch(e => {
         if (!this.allowTabSynchronization) {
-          throw e;
+          if (e.name === 'IndexedDbTransactionError') {
+            logDebug(LOG_TAG, 'Failed to extend owner lease: ', e);
+            // Proceed with the existing state. Any subsequent access to
+            // IndexedDB will verify the lease.
+            return this.isPrimary;
+          } else {
+            throw e;
+          }
         }
 
         logDebug(
@@ -432,7 +451,7 @@ export class IndexedDbPersistence implements Persistence {
   }
 
   private verifyPrimaryLease(
-    txn: SimpleDbTransaction
+    txn: PersistenceTransaction
   ): PersistencePromise<boolean> {
     const store = primaryClientStore(txn);
     return store.get(DbPrimaryClient.key).next(primaryClient => {
@@ -441,7 +460,7 @@ export class IndexedDbPersistence implements Persistence {
   }
 
   private removeClientMetadata(
-    txn: SimpleDbTransaction
+    txn: PersistenceTransaction
   ): PersistencePromise<void> {
     const metadataStore = clientMetadataStore(txn);
     return metadataStore.delete(this.clientId);
@@ -535,7 +554,7 @@ export class IndexedDbPersistence implements Persistence {
    * (foreground) client should become leaseholder instead.
    */
   private canActAsPrimary(
-    txn: SimpleDbTransaction
+    txn: PersistenceTransaction
   ): PersistencePromise<boolean> {
     const store = primaryClientStore(txn);
     return store
@@ -644,15 +663,13 @@ export class IndexedDbPersistence implements Persistence {
     }
     this.detachVisibilityHandler();
     this.detachWindowUnloadHook();
-    await this.simpleDb.runTransaction(
-      'readwrite',
-      [DbPrimaryClient.store, DbClientMetadata.store],
-      txn => {
-        return this.releasePrimaryLeaseIfHeld(txn).next(() =>
-          this.removeClientMetadata(txn)
-        );
-      }
-    );
+    await this.runTransaction('shutdown', 'readwrite', txn => {
+      return this.releasePrimaryLeaseIfHeld(txn).next(() =>
+        this.removeClientMetadata(txn)
+      );
+    }).catch(e => {
+      logDebug(LOG_TAG, 'Proceeding with shutdown despite failure: ', e);
+    });
     this.simpleDb.close();
 
     // Remove the entry marking the client as zombied from LocalStorage since
@@ -683,19 +700,15 @@ export class IndexedDbPersistence implements Persistence {
    * PORTING NOTE: This is only used for Web multi-tab.
    */
   getActiveClients(): Promise<ClientId[]> {
-    return this.simpleDb.runTransaction(
-      'readonly',
-      [DbClientMetadata.store],
-      txn => {
-        return clientMetadataStore(txn)
-          .loadAll()
-          .next(clients =>
-            this.filterActiveClients(clients, MAX_CLIENT_AGE_MS).map(
-              clientMetadata => clientMetadata.clientId
-            )
-          );
-      }
-    );
+    return this.runTransaction('getActiveClients', 'readonly', txn => {
+      return clientMetadataStore(txn)
+        .loadAll()
+        .next(clients =>
+          this.filterActiveClients(clients, MAX_CLIENT_AGE_MS).map(
+            clientMetadata => clientMetadata.clientId
+          )
+        );
+    });
   }
 
   static async clearPersistence(persistenceKey: string): Promise<void> {
@@ -766,7 +779,9 @@ export class IndexedDbPersistence implements Persistence {
       .runTransaction(simpleDbMode, ALL_STORES, simpleDbTxn => {
         persistenceTransaction = new IndexedDbTransaction(
           simpleDbTxn,
-          this.listenSequence.next()
+          this.listenSequence
+            ? this.listenSequence.next()
+            : ListenSequence.INVALID
         );
 
         if (mode === 'readwrite-primary') {
@@ -775,12 +790,12 @@ export class IndexedDbPersistence implements Persistence {
           // executing transactionOperation(). This ensures that even if the
           // transactionOperation takes a long time, we'll use a recent
           // leaseTimestampMs in the extended (or newly acquired) lease.
-          return this.verifyPrimaryLease(simpleDbTxn)
+          return this.verifyPrimaryLease(persistenceTransaction)
             .next(holdsPrimaryLease => {
               if (holdsPrimaryLease) {
                 return /* holdsPrimaryLease= */ true;
               }
-              return this.canActAsPrimary(simpleDbTxn);
+              return this.canActAsPrimary(persistenceTransaction);
             })
             .next(holdsPrimaryLease => {
               if (!holdsPrimaryLease) {
@@ -799,14 +814,14 @@ export class IndexedDbPersistence implements Persistence {
               return transactionOperation(persistenceTransaction);
             })
             .next(result => {
-              return this.acquireOrExtendPrimaryLease(simpleDbTxn).next(
-                () => result
-              );
+              return this.acquireOrExtendPrimaryLease(
+                persistenceTransaction
+              ).next(() => result);
             });
         } else {
-          return this.verifyAllowTabSynchronization(simpleDbTxn).next(() =>
-            transactionOperation(persistenceTransaction)
-          );
+          return this.verifyAllowTabSynchronization(
+            persistenceTransaction
+          ).next(() => transactionOperation(persistenceTransaction));
         }
       })
       .then(result => {
@@ -822,7 +837,7 @@ export class IndexedDbPersistence implements Persistence {
   // TODO(b/114226234): Remove this check when `synchronizeTabs` can no longer
   // be turned off.
   private verifyAllowTabSynchronization(
-    txn: SimpleDbTransaction
+    txn: PersistenceTransaction
   ): PersistencePromise<void> {
     const store = primaryClientStore(txn);
     return store.get(DbPrimaryClient.key).next(currentPrimary => {
@@ -835,7 +850,10 @@ export class IndexedDbPersistence implements Persistence {
         !this.isClientZombied(currentPrimary.ownerId);
 
       if (currentLeaseIsValid && !this.isLocalClient(currentPrimary)) {
-        if (!currentPrimary!.allowTabSynchronization) {
+        if (
+          !this.allowTabSynchronization ||
+          !currentPrimary!.allowTabSynchronization
+        ) {
           throw new FirestoreError(
             Code.FAILED_PRECONDITION,
             PRIMARY_LEASE_EXCLUSIVE_ERROR_MSG
@@ -850,7 +868,7 @@ export class IndexedDbPersistence implements Persistence {
    * method does not verify that the client is eligible for this lease.
    */
   private acquireOrExtendPrimaryLease(
-    txn: SimpleDbTransaction
+    txn: PersistenceTransaction
   ): PersistencePromise<void> {
     const newPrimary = new DbPrimaryClient(
       this.clientId,
@@ -886,7 +904,7 @@ export class IndexedDbPersistence implements Persistence {
 
   /** Checks the primary lease and removes it if we are the current primary. */
   private releasePrimaryLeaseIfHeld(
-    txn: SimpleDbTransaction
+    txn: PersistenceTransaction
   ): PersistencePromise<void> {
     const store = primaryClientStore(txn);
     return store.get(DbPrimaryClient.key).next(primaryClient => {
@@ -1051,18 +1069,22 @@ export class IndexedDbPersistence implements Persistence {
  * Helper to get a typed SimpleDbStore for the primary client object store.
  */
 function primaryClientStore(
-  txn: SimpleDbTransaction
+  txn: PersistenceTransaction
 ): SimpleDbStore<DbPrimaryClientKey, DbPrimaryClient> {
-  return txn.store<DbPrimaryClientKey, DbPrimaryClient>(DbPrimaryClient.store);
+  return IndexedDbPersistence.getStore<DbPrimaryClientKey, DbPrimaryClient>(
+    txn,
+    DbPrimaryClient.store
+  );
 }
 
 /**
  * Helper to get a typed SimpleDbStore for the client metadata object store.
  */
 function clientMetadataStore(
-  txn: SimpleDbTransaction
+  txn: PersistenceTransaction
 ): SimpleDbStore<DbClientMetadataKey, DbClientMetadata> {
-  return txn.store<DbClientMetadataKey, DbClientMetadata>(
+  return IndexedDbPersistence.getStore<DbClientMetadataKey, DbClientMetadata>(
+    txn,
     DbClientMetadata.store
   );
 }