Merge 3663f6f into 02586c9

Author: schmidt-sebastian

.changeset/nine-needles-compare.md

@@ -0,0 +1,5 @@
+---
+"@firebase/firestore": patch
+---
+
+The SDK no longer accesses IndexedDB during a page unload event on Safari 14. This aims to reduce the occurrence of an IndexedDB bug in Safari (https://bugs.webkit.org/show_bug.cgi?id=226547).

packages/firestore/src/local/indexeddb_persistence.ts

@@ -15,6 +15,8 @@
  * limitations under the License.
  */
 
+import { isSafari } from '@firebase/util';
+
 import { User } from '../auth/user';
 import { DatabaseId } from '../core/database_info';
 import { ListenSequence, SequenceNumberSyncer } from '../core/listen_sequence';
@@ -960,6 +962,14 @@ export class IndexedDbPersistence implements Persistence {
         // to make sure it gets a chance to run.
         this.markClientZombied();
 
+        if (isSafari() && navigator.appVersion.match(`Version/14`)) {
+          // On Safari 14, we do not run any cleanup actions as it might trigger
+          // a bug that prevents Safari from re-opening IndexedDB during the
+          // next page load.
+          // See https://bugs.webkit.org/show_bug.cgi?id=226547
+          this.queue.enterRestrictedMode(/* purgeExistingTasks= */ true);
+        }
+
         this.queue.enqueueAndForget(() => {
           // Attempt graceful shutdown (including releasing our primary lease),
           // but there's no guarantee it will complete.

packages/firestore/src/util/async_queue.ts

@@ -223,8 +223,13 @@ export interface AsyncQueue {
    * Initialize the shutdown of this queue. Once this method is called, the
    * only possible way to request running an operation is through
    * `enqueueEvenWhileRestricted()`.
+   *
+   * @param purgeExistingTasks Whether already enqueued tasked should be
+   * rejected (unless enqueued wih `enqueueEvenWhileRestricted()`). Defaults
+   * to false.
    */
-  enterRestrictedMode(): void;
+  enterRestrictedMode(purgeExistingTasks?: boolean): void;
+
   /**
    * Adds a new operation to the queue. Returns a promise that will be resolved
    * when the promise returned by the new operation is (with its value).

packages/firestore/src/util/async_queue_impl.ts

@@ -23,6 +23,7 @@ import { debugAssert, fail } from './assert';
 import { AsyncQueue, DelayedOperation, TimerId } from './async_queue';
 import { FirestoreError } from './error';
 import { logDebug, logError } from './log';
+import { Deferred } from './promise';
 
 const LOG_TAG = 'AsyncQueue';
 
@@ -49,6 +50,9 @@ export class AsyncQueueImpl implements AsyncQueue {
   // assertion sanity-checks.
   private operationInProgress = false;
 
+  // Enabled during shutdown on Safari to prevent future access to IndexedDB.
+  private skipNonRestrictedTasks = false;
+
   // List of TimerIds to fast-forward delays for.
   private timerIdsToSkip: TimerId[] = [];
 
@@ -97,9 +101,10 @@ export class AsyncQueueImpl implements AsyncQueue {
     this.enqueueInternal(op);
   }
 
-  enterRestrictedMode(): void {
+  enterRestrictedMode(purgeExistingTasks?: boolean): void {
     if (!this._isShuttingDown) {
       this._isShuttingDown = true;
+      this.skipNonRestrictedTasks = purgeExistingTasks || false;
       const document = getDocument();
       if (document && typeof document.removeEventListener === 'function') {
         document.removeEventListener(
@@ -114,9 +119,22 @@ export class AsyncQueueImpl implements AsyncQueue {
     this.verifyNotFailed();
     if (this._isShuttingDown) {
       // Return a Promise which never resolves.
-      return new Promise<T>(resolve => {});
+      return new Promise<T>(() => {});
     }
-    return this.enqueueInternal(op);
+
+    // Create a deferred Promise that we can return to the callee. This
+    // allows us to return a "hanging Promise" only to the callee and still
+    // advance the queue even when the operation is not run.
+    const task = new Deferred<T>();
+    return this.enqueueInternal<unknown>(() => {
+      if (this._isShuttingDown && this.skipNonRestrictedTasks) {
+        // We do not resolve 'task'
+        return Promise.resolve();
+      }
+
+      op().then(task.resolve, task.reject);
+      return task.promise;
+    }).then(() => task.promise);
   }
 
   enqueueRetryable(op: () => Promise<void>): void {

packages/firestore/test/unit/util/async_queue.test.ts

@@ -87,7 +87,7 @@ describe('AsyncQueue', () => {
 
   it('handles failures', () => {
     const queue = newAsyncQueue() as AsyncQueueImpl;
-    const expected = new Error('Firit cestore Test Simulated Error');
+    const expected = new Error('Firestore Test Simulated Error');
 
     // Disable logging for this test to avoid the assertion being logged
     const oldLogLevel = getLogLevel();
@@ -142,7 +142,7 @@ describe('AsyncQueue', () => {
       }).to.throw(/already failed:.*Simulated Error/);
 
       // Finally, restore log level.
-      setLogLevel((oldLogLevel as unknown) as LogLevelString);
+      setLogLevel(oldLogLevel as unknown as LogLevelString);
     });
   });
 
@@ -417,6 +417,22 @@ describe('AsyncQueue', () => {
     await queue.drain();
     expect(completedSteps).to.deep.equal([1, 2, 4]);
   });
+
+  it('Does not run existing operations if opted out', async () => {
+    const queue = newAsyncQueue() as AsyncQueueImpl;
+    const completedSteps: number[] = [];
+    const doStep = (n: number): Promise<void> =>
+      defer(() => {
+        completedSteps.push(n);
+      });
+
+    queue.enqueueAndForget(() => doStep(1));
+    queue.enqueueAndForgetEvenWhileRestricted(() => doStep(2));
+    queue.enterRestrictedMode(/* purgeExistingTasks =*/ true);
+
+    await queue.drain();
+    expect(completedSteps).to.deep.equal([2]);
+  });
 });
 
 function defer<T>(op: () => T): Promise<T> {