w3c / IndexedDB

Indexed Database API
https://w3c.github.io/IndexedDB/
Other
240 stars 62 forks source link

Clarify interaction of IndexedDB lifecycles with relation to document and worker lifecycle mechanics #416

Open asutherland opened 7 months ago

asutherland commented 7 months ago

https://github.com/w3c/IndexedDB/issues/381 mentions https://w3c.github.io/IndexedDB/#database-connection which says:

If the execution context where the connection was created is destroyed (for example due to the user navigating away from that page), the connection is closed.

https://github.com/w3c/IndexedDB/issues/412 ("Specify behavior when document is not fully active") is also relevant.

The motivation for raising this issue was looking into IndexedDB/ready-state-destroyed-execution-context.html WPT from https://github.com/web-platform-tests/wpt/pull/30260 (sync of https://chromium-review.googlesource.com/c/chromium/src/+/3125334) for our Firefox bug on fixing our test failure there.

const openRequest = iframe.contentWindow.indexedDB.open(dbname);
assert_equals(openRequest.readyState, 'pending');
iframe.remove();
assert_equals(openRequest.readyState, 'done');

I think the motivation for the test was primarily to verify that readyState returned a value rather than throwing, rather than trying to dictate that the state would be "done".

Digging into the iframe spec stuff here, it looks like:

Notably, as I read it, destroy a document explicitly is happening asynchronously, so it seems like any calls made after an iframe removal can only be reacting to not being fully active since IDB-specific hooks would not have been able to run yet.

Potential clarification

I think clarifying the existing spec text would basically look like add a hook to unloading document cleanup steps that calls close a database connection but where we need something other than the "forced flag" because the forced flag would (correctly for its use case) abort transactions where commit() has been called, but where we still want to abort the transactions.

inexorabletash commented 7 months ago

Spec suggestion SGTM, and agreed that the WPT is asserting more than it should.

PRs welcome!

evanstade commented 6 months ago

After the update to the WPT, Blink is failing the test. This is because Blink does not dispatch any events after the iframe is detached, so the test hangs waiting for an error event. At least that is part of it. IDB would also need to explicitly send an error event in this case, it's just that doing so currently has no effect due to that other behavior of Blink, which is implemented at a fairly core level (i.e. changing this would have widespread effects beyond IDB).

Perhaps related: https://github.com/whatwg/html/issues/10194 - @domfarolino any insights?

domfarolino commented 5 months ago

Perhaps related: https://github.com/whatwg/html/issues/10194 - @domfarolino any insights?

The issue you link to is about iframes getting unloaded, and their event firing in the removal path essentially being another instance of legacy mutation events which are bad.

But the first part of your message — about firing events in detached contexts — is generally desired because running script in a detached document has caused security bugs in the past. So we explicitly prevent events from being fired in those contexts as that's a very common way of running script. For example, note that:

const iframe = document.createElement('iframe');
document.body.append(iframe);
const iframeWindow = iframe.contentWindow;
iframeWindow.addEventListener('foo', e => console.warn(e));
iframe.remove();
iframeWindow.dispatchEvent(new Event('foo'));

Does not lead to the event handler running in all of: Chrome, Safari, and Firefox.