Skip to content

Fix bug in AsyncQueue visibility handler #3586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tidy-elephants-beam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@firebase/firestore": patch
---

Fixed a bug that caused slow retries for IndexedDB operations even when a webpage re-entered the foreground.
4 changes: 2 additions & 2 deletions packages/firestore/src/local/indexeddb_persistence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,8 @@ export class IndexedDbPersistence implements Persistence {
debugAssert(!this.started, 'IndexedDbPersistence double-started!');
debugAssert(this.window !== null, "Expected 'window' to be defined");

// NOTE: This is expected to fail sometimes (in the case of another tab
// already having the persistence lock), so it's the first thing we should
// NOTE: This is expected to fail sometimes (in the case of another tab
// already having the persistence lock), so it's the first thing we should
// do.
return this.updateClientMetadataAndTryBecomePrimary()
.then(() => {
Expand Down
29 changes: 21 additions & 8 deletions packages/firestore/src/util/async_queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { logDebug, logError } from './log';
import { Deferred } from './promise';
import { ExponentialBackoff } from '../remote/backoff';
import { isIndexedDbTransactionError } from '../local/simple_db';
import { getWindow } from '../platform/dom';
import { getDocument } from '../platform/dom';

const LOG_TAG = 'AsyncQueue';

Expand Down Expand Up @@ -235,12 +235,22 @@ export class AsyncQueue {
// Visibility handler that triggers an immediate retry of all retryable
// operations. Meant to speed up recovery when we regain file system access
// after page comes into foreground.
private visibilityHandler = (): void => this.backoff.skipBackoff();
private visibilityHandler: () => void = () => {
const document = getDocument();
if (document) {
logDebug(
LOG_TAG,
'Visibility state changed to ',
document.visibilityState
);
}
this.backoff.skipBackoff();
};

constructor() {
const window = getWindow();
if (window && typeof window.addEventListener === 'function') {
window.addEventListener('visibilitychange', this.visibilityHandler);
const document = getDocument();
if (document && typeof document.addEventListener === 'function') {
document.addEventListener('visibilitychange', this.visibilityHandler);
}
}

Expand Down Expand Up @@ -293,9 +303,12 @@ export class AsyncQueue {
this.verifyNotFailed();
if (!this._isShuttingDown) {
this._isShuttingDown = true;
const window = getWindow();
if (window) {
window.removeEventListener('visibilitychange', this.visibilityHandler);
const document = getDocument();
if (document && typeof document.removeEventListener === 'function') {
document.removeEventListener(
'visibilitychange',
this.visibilityHandler
);
}
await this.enqueueEvenAfterShutdown(op);
}
Expand Down