Skip to content

Commit 44d336b

Browse files
authored
fix(auth): remove visibilitychange listener from IndexedDBLocalPersistence (#10300)
Fixes #10264. PR #10123 introduced pagehide, pageshow, and visibilitychange listeners to prevent null hydration during page reload. However, visibilitychange fires with document.visibilityState === 'hidden' whenever focus shifts to an OAuth popup window (e.g. signInWithPopup) or background tab. This caused IndexedDB to close prematurely and throw 'Database is closing/hidden' when the popup returned credentials to the opener. This change: 1. Removes the visibilitychange listener from IndexedDBLocalPersistence, relying solely on pagehide/pageshow for true page teardown lifecycle events. 2. Renames isHiding -> isClosing to accurately reflect its purpose. 3. Updates unit tests to assert that visibilitychange does not close the DB or interrupt persistence writes.
1 parent 19693de commit 44d336b

3 files changed

Lines changed: 28 additions & 46 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@firebase/auth': patch
3+
---
4+
5+
Fix issue where `signInWithPopup` and other background tab operations fail with "Database is closing/hidden" by removing the `visibilitychange` listener from `IndexedDBLocalPersistence`.

‎packages/auth/src/platform_browser/persistence/indexed_db.test.ts‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,7 @@ describe('platform_browser/persistence/indexed_db', () => {
439439
clock = sinon.useFakeTimers();
440440
callback = sinon.spy();
441441
// Ensure we start fresh
442-
(persistence as any).isHiding = false;
442+
(persistence as any).isClosing = false;
443443
(persistence as any).dbPromise = null;
444444
});
445445

@@ -452,10 +452,12 @@ describe('platform_browser/persistence/indexed_db', () => {
452452
it('should register event listeners when first listener is added and unregister when last is removed', () => {
453453
const addSpy = sinon.spy(window, 'addEventListener');
454454
const removeSpy = sinon.spy(window, 'removeEventListener');
455+
const docAddSpy = sinon.spy(document, 'addEventListener');
455456

456457
persistence._addListener(key, callback);
457458
expect(addSpy).to.have.been.calledWith('pagehide');
458459
expect(addSpy).to.have.been.calledWith('pageshow');
460+
expect(docAddSpy).not.to.have.been.calledWith('visibilitychange');
459461

460462
persistence._removeListener(key, callback);
461463
expect(removeSpy).to.have.been.calledWith('pagehide');
@@ -468,7 +470,7 @@ describe('platform_browser/persistence/indexed_db', () => {
468470

469471
// Trigger pagehide
470472
window.dispatchEvent(new Event('pagehide'));
471-
expect((persistence as any).isHiding).to.be.true;
473+
expect((persistence as any).isClosing).to.be.true;
472474
expect((persistence as any).pollTimer).to.be.null;
473475
expect((persistence as any).dbPromise).to.be.null;
474476

@@ -479,7 +481,7 @@ describe('platform_browser/persistence/indexed_db', () => {
479481

480482
// Trigger pageshow
481483
window.dispatchEvent(new Event('pageshow'));
482-
expect((persistence as any).isHiding).to.be.false;
484+
expect((persistence as any).isClosing).to.be.false;
483485
expect((persistence as any).pollTimer).not.to.be.null;
484486

485487
// Modify DB in background, ensure polling picks it up after pageshow
@@ -488,23 +490,20 @@ describe('platform_browser/persistence/indexed_db', () => {
488490
expect(callback).to.have.been.calledWith('new-value');
489491
});
490492

491-
it('should handle visibilitychange hidden and visible', () => {
493+
it('should not close DB or set isClosing on visibilitychange', async () => {
492494
persistence._addListener(key, callback);
495+
await persistence._set(key, value);
493496

494-
// Mock document.visibilityState to 'hidden'
497+
// Mock document.visibilityState to 'hidden' and dispatch visibilitychange
495498
sinon.stub(document, 'visibilityState').get(() => 'hidden');
496499
document.dispatchEvent(new Event('visibilitychange'));
497500

498-
expect((persistence as any).isHiding).to.be.true;
499-
expect((persistence as any).pollTimer).to.be.null;
500-
501-
// Mock document.visibilityState to 'visible'
502-
sinon.restore(); // Restore stub
503-
sinon.stub(document, 'visibilityState').get(() => 'visible');
504-
document.dispatchEvent(new Event('visibilitychange'));
505-
506-
expect((persistence as any).isHiding).to.be.false;
501+
expect((persistence as any).isClosing).to.be.false;
507502
expect((persistence as any).pollTimer).not.to.be.null;
503+
504+
// Persistence writes should continue to succeed while document is hidden
505+
await persistence._set(key, 'another-value');
506+
expect(await persistence._get(key)).to.eq('another-value');
508507
});
509508

510509
it('should discard in-flight poll results if pagehide occurs before poll completes', async () => {

‎packages/auth/src/platform_browser/persistence/indexed_db.ts‎

Lines changed: 10 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ class IndexedDBLocalPersistence implements InternalPersistence {
166166
// setTimeout return value is platform specific
167167
// eslint-disable-next-line @typescript-eslint/no-explicit-any
168168
private pollTimer: any | null = null;
169-
private isHiding = false;
169+
private isClosing = false;
170170
private pendingWrites = 0;
171171

172172
private receiver: Receiver | null = null;
@@ -177,7 +177,7 @@ class IndexedDBLocalPersistence implements InternalPersistence {
177177
readonly _workerInitializationPromise: Promise<void>;
178178

179179
private readonly onPageHide = (): void => {
180-
this.isHiding = true;
180+
this.isClosing = true;
181181
this.stopPolling();
182182
if (this.dbPromise) {
183183
this.dbPromise.then(db => db.close()).catch(() => {});
@@ -186,24 +186,14 @@ class IndexedDBLocalPersistence implements InternalPersistence {
186186
};
187187

188188
private readonly onPageShow = (): void => {
189-
if (this.isHiding) {
190-
this.isHiding = false;
189+
if (this.isClosing) {
190+
this.isClosing = false;
191191
if (Object.keys(this.listeners).length > 0) {
192192
this.startPolling();
193193
}
194194
}
195195
};
196196

197-
private readonly onVisibilityChange = (): void => {
198-
if (typeof document !== 'undefined') {
199-
if (document.visibilityState === 'hidden') {
200-
this.onPageHide();
201-
} else if (document.visibilityState === 'visible') {
202-
this.onPageShow();
203-
}
204-
}
205-
};
206-
207197
private registerLifecycleListeners(): void {
208198
if (
209199
typeof window !== 'undefined' &&
@@ -212,12 +202,6 @@ class IndexedDBLocalPersistence implements InternalPersistence {
212202
window.addEventListener('pagehide', this.onPageHide);
213203
window.addEventListener('pageshow', this.onPageShow);
214204
}
215-
if (
216-
typeof document !== 'undefined' &&
217-
typeof document.addEventListener === 'function'
218-
) {
219-
document.addEventListener('visibilitychange', this.onVisibilityChange);
220-
}
221205
}
222206

223207
private unregisterLifecycleListeners(): void {
@@ -228,12 +212,6 @@ class IndexedDBLocalPersistence implements InternalPersistence {
228212
window.removeEventListener('pagehide', this.onPageHide);
229213
window.removeEventListener('pageshow', this.onPageShow);
230214
}
231-
if (
232-
typeof document !== 'undefined' &&
233-
typeof document.removeEventListener === 'function'
234-
) {
235-
document.removeEventListener('visibilitychange', this.onVisibilityChange);
236-
}
237215
}
238216

239217
constructor() {
@@ -246,8 +224,8 @@ class IndexedDBLocalPersistence implements InternalPersistence {
246224
}
247225

248226
async _openDb(): Promise<IDBDatabase> {
249-
if (this.isHiding) {
250-
throw new Error('Database is closing/hidden');
227+
if (this.isClosing) {
228+
throw new Error('Database is closing');
251229
}
252230
if (this.dbPromise) {
253231
return this.dbPromise;
@@ -267,7 +245,7 @@ class IndexedDBLocalPersistence implements InternalPersistence {
267245
const db = await this._openDb();
268246
return await op(db);
269247
} catch (e) {
270-
if (this.isHiding) {
248+
if (this.isClosing) {
271249
throw e;
272250
}
273251
if (numAttempts++ > _TRANSACTION_RETRY_COUNT) {
@@ -425,7 +403,7 @@ class IndexedDBLocalPersistence implements InternalPersistence {
425403
}
426404

427405
private async _poll(): Promise<string[]> {
428-
if (this.isHiding) {
406+
if (this.isClosing) {
429407
return [];
430408
}
431409
try {
@@ -435,7 +413,7 @@ class IndexedDBLocalPersistence implements InternalPersistence {
435413
return new DBPromise<DBObject[] | null>(getAllRequest).toPromise();
436414
});
437415

438-
if (this.isHiding) {
416+
if (this.isClosing) {
439417
return [];
440418
}
441419

@@ -469,7 +447,7 @@ class IndexedDBLocalPersistence implements InternalPersistence {
469447
}
470448
return keys;
471449
} catch (e) {
472-
if (!this.isHiding) {
450+
if (!this.isClosing) {
473451
_logWarn(`Firebase Auth cross-tab polling failed with error: ${e}`);
474452
}
475453
return [];

0 commit comments

Comments
 (0)