Skip to content

Commit 4c47bd4

Browse files
Skip IndexedDB cleanup on Safari 14
This PR changes our IndexedDB shutdown to skip all remaining work items on Safari 14. It is believed that writing to IndexedDB during unload can trigger a bug on Safari that prevents IndexedDB from loading during the next page load.
1 parent 02586c9 commit 4c47bd4

File tree

4 files changed

+54
-6
lines changed

4 files changed

+54
-6
lines changed

packages/firestore/src/local/indexeddb_persistence.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { isSafari } from '@firebase/util';
19+
1820
import { User } from '../auth/user';
1921
import { DatabaseId } from '../core/database_info';
2022
import { ListenSequence, SequenceNumberSyncer } from '../core/listen_sequence';
@@ -960,6 +962,14 @@ export class IndexedDbPersistence implements Persistence {
960962
// to make sure it gets a chance to run.
961963
this.markClientZombied();
962964

965+
if (isSafari() && navigator.appVersion.match(`Version/14`)) {
966+
// On Safari 14, we do not run any cleanup actions as it might trigger
967+
// a bug that prevents Safari from re-opening IndexedDB during the
968+
// next page load.
969+
// See https://bugs.webkit.org/show_bug.cgi?id=226547
970+
this.queue.enterRestrictedMode(/* purgeExistingTasks= */ true);
971+
}
972+
963973
this.queue.enqueueAndForget(() => {
964974
// Attempt graceful shutdown (including releasing our primary lease),
965975
// but there's no guarantee it will complete.

packages/firestore/src/util/async_queue.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,13 @@ export interface AsyncQueue {
223223
* Initialize the shutdown of this queue. Once this method is called, the
224224
* only possible way to request running an operation is through
225225
* `enqueueEvenWhileRestricted()`.
226+
*
227+
* @param purgeExistingTasks Whether already enqueued tasked should be
228+
* rejected (unless enqueued wih `enqueueEvenWhileRestricted()`). Defaults
229+
* to false.
226230
*/
227-
enterRestrictedMode(): void;
231+
enterRestrictedMode(purgeExistingTasks?: boolean): void;
232+
228233
/**
229234
* Adds a new operation to the queue. Returns a promise that will be resolved
230235
* when the promise returned by the new operation is (with its value).

packages/firestore/src/util/async_queue_impl.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { debugAssert, fail } from './assert';
2323
import { AsyncQueue, DelayedOperation, TimerId } from './async_queue';
2424
import { FirestoreError } from './error';
2525
import { logDebug, logError } from './log';
26+
import { Deferred } from './promise';
2627

2728
const LOG_TAG = 'AsyncQueue';
2829

@@ -49,6 +50,9 @@ export class AsyncQueueImpl implements AsyncQueue {
4950
// assertion sanity-checks.
5051
private operationInProgress = false;
5152

53+
// Enabled during shutdown on Safari to prevent future access to IndexedDB.
54+
private skipNonRestrictedTasks = false;
55+
5256
// List of TimerIds to fast-forward delays for.
5357
private timerIdsToSkip: TimerId[] = [];
5458

@@ -97,9 +101,10 @@ export class AsyncQueueImpl implements AsyncQueue {
97101
this.enqueueInternal(op);
98102
}
99103

100-
enterRestrictedMode(): void {
104+
enterRestrictedMode(purgeExistingTasks?: boolean): void {
101105
if (!this._isShuttingDown) {
102106
this._isShuttingDown = true;
107+
this.skipNonRestrictedTasks = purgeExistingTasks || false;
103108
const document = getDocument();
104109
if (document && typeof document.removeEventListener === 'function') {
105110
document.removeEventListener(
@@ -114,9 +119,22 @@ export class AsyncQueueImpl implements AsyncQueue {
114119
this.verifyNotFailed();
115120
if (this._isShuttingDown) {
116121
// Return a Promise which never resolves.
117-
return new Promise<T>(resolve => {});
122+
return new Promise<T>(() => {});
118123
}
119-
return this.enqueueInternal(op);
124+
125+
// Create a deferred Promise that we can return to the callee. This
126+
// allows us to return a "hanging Promise" only to the callee and still
127+
// advance the queue even when the operation is not run.
128+
const task = new Deferred<T>();
129+
return this.enqueueInternal<unknown>(() => {
130+
if (this._isShuttingDown && this.skipNonRestrictedTasks) {
131+
// We do not resolve 'task'
132+
return Promise.resolve();
133+
}
134+
135+
op().then(task.resolve, task.reject);
136+
return task.promise;
137+
}).then(() => task.promise);
120138
}
121139

122140
enqueueRetryable(op: () => Promise<void>): void {

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ describe('AsyncQueue', () => {
8787

8888
it('handles failures', () => {
8989
const queue = newAsyncQueue() as AsyncQueueImpl;
90-
const expected = new Error('Firit cestore Test Simulated Error');
90+
const expected = new Error('Firestore Test Simulated Error');
9191

9292
// Disable logging for this test to avoid the assertion being logged
9393
const oldLogLevel = getLogLevel();
@@ -142,7 +142,7 @@ describe('AsyncQueue', () => {
142142
}).to.throw(/already failed:.*Simulated Error/);
143143

144144
// Finally, restore log level.
145-
setLogLevel((oldLogLevel as unknown) as LogLevelString);
145+
setLogLevel(oldLogLevel as unknown as LogLevelString);
146146
});
147147
});
148148

@@ -417,6 +417,21 @@ describe('AsyncQueue', () => {
417417
await queue.drain();
418418
expect(completedSteps).to.deep.equal([1, 2, 4]);
419419
});
420+
421+
it('Does not run existing operations if opted out', async () => {
422+
const queue = newAsyncQueue() as AsyncQueueImpl;
423+
const completedSteps: number[] = [];
424+
const doStep = (n: number): Promise<void> =>
425+
defer(() => {
426+
completedSteps.push(n);
427+
});
428+
429+
queue.enqueueAndForget(() => doStep(1));
430+
queue.enterRestrictedMode(/* purgeExistingTasks =*/ true);
431+
432+
await queue.drain();
433+
expect(completedSteps).to.deep.equal([]);
434+
});
420435
});
421436

422437
function defer<T>(op: () => T): Promise<T> {

0 commit comments

Comments
 (0)