Use SimpleDb transaction

Author: schmidt-sebastian

packages/firestore/src/local/indexeddb_persistence.ts

@@ -75,7 +75,6 @@ import {
   SimpleDbTransaction
 } from './simple_db';
 import { DocumentLike, WindowLike } from '../util/types';
-import { ignoreIfPrimaryLeaseLoss } from './local_store';
 
 const LOG_TAG = 'IndexedDbPersistence';
 
@@ -681,11 +680,22 @@ export class IndexedDbPersistence implements Persistence {
     }
     this.detachVisibilityHandler();
     this.detachWindowUnloadHook();
-    await this.runTransaction('shutdown', 'readwrite', txn =>
-      this.releasePrimaryLeaseIfHeld(txn).next(() =>
-        this.removeClientMetadata(txn)
-      )
-    ).catch(e => ignoreIfPrimaryLeaseLoss(e));
+
+    // Use `SimpleDb.runTransaction` directly to avoid failing if another tab
+    // has obtained the primary lease.
+    await this.simpleDb.runTransaction(
+      'readwrite',
+      [DbPrimaryClient.store, DbClientMetadata.store],
+      simpleDbTxn => {
+        const persistenceTransaction = new IndexedDbTransaction(
+          simpleDbTxn,
+          ListenSequence.INVALID
+        );
+        return this.releasePrimaryLeaseIfHeld(persistenceTransaction).next(() =>
+          this.removeClientMetadata(persistenceTransaction)
+        );
+      }
+    );
     this.simpleDb.close();
 
     // Remove the entry marking the client as zombied from LocalStorage since

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

@@ -840,12 +840,4 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
       })
       .userUnlistens(query1);
   });
-
-  specTest('Terminate (with recovery)', [], () => {
-    return spec()
-      .failDatabaseTransactions('shutdown')
-      .shutdown({ expectFailure: true })
-      .recoverDatabase()
-      .shutdown();
-  });
 });

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

@@ -436,10 +436,10 @@ export class SpecBuilder {
     return this;
   }
 
-  shutdown(options?: { expectFailure?: boolean }): this {
+  shutdown(): this {
     this.nextStep();
     this.currentStep = {
-      shutdown: options ?? true,
+      shutdown: true,
       expectedState: {
         activeTargets: {},
         activeLimboDocs: [],

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

@@ -370,9 +370,7 @@ abstract class TestRunner {
     } else if ('restart' in step) {
       return this.doRestart();
     } else if ('shutdown' in step) {
-      return typeof step.shutdown === 'object'
-        ? this.doShutdown(step.shutdown)
-        : this.doShutdown();
+      return this.doShutdown();
     } else if ('applyClientState' in step) {
       // PORTING NOTE: Only used by web multi-tab tests.
       return this.doApplyClientState(step.applyClientState!);
@@ -716,22 +714,14 @@ abstract class TestRunner {
     await this.remoteStore.enableNetwork();
   }
 
-  private async doShutdown(options?: {
-    expectFailure?: boolean;
-  }): Promise<void> {
-    try {
-      await this.remoteStore.shutdown();
-      await this.sharedClientState.shutdown();
-      // We don't delete the persisted data here since multi-clients may still
-      // be accessing it. Instead, we manually remove it at the end of the
-      // test run.
-      await this.persistence.shutdown();
-      expect(options?.expectFailure).to.not.be.true;
-
-      this.started = false;
-    } catch (e) {
-      expect(options?.expectFailure).to.be.true;
-    }
+  private async doShutdown(): Promise<void> {
+    await this.remoteStore.shutdown();
+    await this.sharedClientState.shutdown();
+    // We don't delete the persisted data here since multi-clients may still
+    // be accessing it. Instead, we manually remove it at the end of the
+    // test run.
+    await this.persistence.shutdown();
+    this.started = false;
   }
 
   private async doClearPersistence(): Promise<void> {
@@ -1270,8 +1260,7 @@ export type PersistenceAction =
   | 'Get new document changes'
   | 'Synchronize last document change read time'
   | 'updateClientMetadataAndTryBecomePrimary'
-  | 'getHighestListenSequenceNumber'
-  | 'shutdown';
+  | 'getHighestListenSequenceNumber';
 
 /**
  * Union type for each step. The step consists of exactly one `field`
@@ -1351,7 +1340,7 @@ export interface SpecStep {
   restart?: true;
 
   /** Shut down the client and close it network connection. */
-  shutdown?: true | { expectFailure?: boolean };
+  shutdown?: true;
 
   /**
    * Optional list of expected events.