Skip to content
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

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

Open
asutherland opened this issue Mar 5, 2024 · 3 comments

Comments

@asutherland
Copy link
Collaborator

#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.

#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 web-platform-tests/wpt#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
Copy link
Member

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

PRs welcome!

@evanstade
Copy link

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: whatwg/html#10194 - @domfarolino any insights?

@domfarolino
Copy link
Member

Perhaps related: whatwg/html#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants