1458342 - IndexedDB bfcache behavior is broken: window will successfully go in bfcache but active transactions will be silently aborted

Status: NEW

(Core :: Storage: IndexedDB, defect, P2)

Description

[Andrew Sutherland [:asuth] (he/him)]

When looking at https://github.com/w3c/IndexedDB/issues/237#issuecomment-385713287 about IndexedDB commit() in the face of proposed life-cycle standardization, I noticed our bfcache situation is a little broken.

Right now it appears the only explicit bfcache handling IndexedDB has is that if an BackgroundDatabaseChild receives a "VersionChange" message, it will AbortTransactions() and Close() at https://searchfox.org/mozilla-central/rev/8837610b6c999451435695e800f38d4acbc0a644/dom/indexedDB/ActorsChild.cpp#2326.  This is sufficient to avoid blocking pages not in bfcache.

It also raises the question of what happens to IDB transactions that are actively processing when they go into bfcache.

Based on very brief testing with a helper I made at https://idb-browser-shutdown.glitch.me/ we leave the database alive, but the transaction ends up getting aborted because the "success" event fails to dispatch on the frozen/suspended global.  This is consistent with my other understandings of how we've been handling shutdown.

The problem with that is that the abort event will also fail to dispatch, so when we pull the page out of bfcache:
- the database will still be open and alive and viable to create new transactions
- the transaction will have its "AbortError" error state so if you wanted to poll on every "pageshow" event, you could hackily attempt a workaround.
- Content code will never have been able to process the error event.

In bug 618590 back in 2011 we addressed this problem by not putting IDB databases in the bfcache that had active transactions via a check in nsDocument::CanSavePresentation.  That seems correct.  It evolved away, though[1]:
- Bug 767944 introduced QuotaManager and migrated the check to be against QM.
- Bug 994190 was the PBackground migration and it removed the check.  I think this just got lost.

We should fix this.

1: https://hg.mozilla.org/mozilla-central/log/8d558e07caf4/content/base/src/nsDocument.cpp?patch=&linerange=7641:7646&descend=

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

(This sounds rather bad, but P3 for now)

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Oof, embarrassing. Nice catch!