Add hook for deactivating new Indexed DB transactions.

[inexorabletash]

Context: w3c/IndexedDB#87

Nominally, to describe the Indexed DB behavior in browsers implied by the current spec's "When control is returned to the event loop, the implementation must unset the active flag." (here) we'd need a hook akin to the "clean-up jobs list" hook for FileAPI.

The event loop processing model would grow a new step after 6. Microtasks: Perform a microtask checkpoint., something like:

7. Perform each of the jobs in the post-task clean-up jobs list, and then empty the list.

Event loop would then have a definition for post-task clean-up jobs list similar to the global script clean-up jobs list with similar requirements ("a clean-up job cannot run scripts, and cannot be sensitive to the order in which other clean-up jobs are executed").

Indexed DB would invoke with:

8. Add a step to the relevant event loop's post-task clean-up jobs list to unset transaction's active flag

There may be a better approach which is web-compatible, but tossing this out for initial discussion.

[annevk]

So if we need this that means that promises are fundamentally incompatible with Indexed DB's transaction model (I somewhat recall us reaching that conclusion many years ago). Is that acceptable?

Also, reading w3c/IndexedDB#87 (comment) it seems like this "cleanup step" can be associated with the current task. You don't need a hook at the event loop level.

(While reviewing the issue I also found w3c/IndexedDB#140. 😟)

[domenic]

Having read through the linked thread it does seem like there is general agreement on the path forward here and that adding something like what you describe would match most or all browsers. It's a bit sad to add a new after-microtasks primitive, but oh well.

Did you want to work on a PR for this, or shall we? Certainly we could use help getting this web-platform-tested.

[inexorabletash]

I think y'all would wrangle the PR in far less time than I will (although I'm sympathetic to distributing the workload)

I'll definitely wrangle the w-p-t side of things. I'll track down an existing IDB test that verifies this and/or land one. (I'm slammed this week and next so this may stretch out a bit, although honestly I'd rather be specing/testing.)

[domenic]

OK. @annevk, do you want to take on the spec for this? It sounds like you have opinions. I can otherwise.

[inexorabletash]

I missed one of the details pointed out in w3c/IndexedDB#87 - in Blink, WebKit, and future-Gecko the deactivation takes place at the end of invocation in the event dispatch algorithm. So this would need to go into DOM WebIDL to capture this behavior, not HTML.

So... don't spec anything until we've got more clarity.

[inexorabletash]

*sigh* Twisty little maze... event dispatch goes via WebIDL's call a user object’s operation which invokes HTML's clean up after running script

Is there any case where "clean up after running script" does not run after executing user script? If not, then I think that existing hook is sufficient the new hook should go into the "clean up..." steps rather than the "event loop processing steps", but after the microtask checkpoint.

Experts?

[annevk]

Isn't it a problem that "clean up after running script" is also run after each microtask queued by JavaScript's EnqueueJob?

Also, I thought the event dispatch case was something you could handle yourself. Set the flag. Dispatch the event. Unset the flag. There doesn't seem to be a problem there necessarily or am I missing something?

[inexorabletash]

Isn't it a problem that "clean up after running script" is also run after each microtask queued by JavaScript's EnqueueJob?

I believe that complicates things, yes. Obviously it needs to run in some of those cases (e.g. a fetch completes and resolves), but not when we're performing a microtask checkpoint after another script invocation. It may need a guard flag.

Also, I thought the event dispatch case was something you could handle yourself. Set the flag. Dispatch the event. Unset the flag. There doesn't seem to be a problem there necessarily or am I missing something?

You're not missing something but that's "case 2" in the linked bug; an existing transaction is activated, event is dispatched, the transaction is deactivated. That's fine.

The weird bit is "case 1"; we just need to consider how "case 1" applies when occurring during event dispatch. The current vague spec text for "case 1" is where it refers to deactivating a newly created transaction when control returns to the event loop. In practice (Blink, WebKit, future-Gecko) the transaction is deactivated at the end of the invocation - after microtasks but before the next handler/listener is invoked, despite all listener invocations happening within one "task" in the event loop. This is called out in bevis' example in w3c/IndexedDB#87. (Which is slightly confusing to read since it uses one transaction to generate an event seen by multiple listeners, but that's not the transaction we care about.)

[annevk]

When a fetch completes you would get a task (and within that a microtask).

It sounds like we want to do this at the end of "perform a microtask checkpoint" since that happens at the end-of-task or after each listener (when the event is synthetic). (Basically whenever the JavaScript execution context stack is empty.) @domenic knows this better than I do though.

[inexorabletash]

Sounds closer.

when the event is synthetic

Hopefully you mean the reverse? (or either my interpretation of "synthetic" is wrong, or Blink is wrong)

[annevk]

Yes, I meant to write not synthetic. Apologies.

[inexorabletash]

PR for tests: web-platform-tests/wpt#4643

[inexorabletash]

And to recap current thinking, proposed change would be something like:

• In event loop add:

An event loop has a post-microtasks jobs list, which must initially be empty. A post-microtasks job cannot run scripts, and cannot be sensitive to the order in which other post-microtasks jobs are executed.

• Add a new step to the end of perform a microtask checkpoint:

Perform each of the jobs in the event loop's post-microtasks jobs list, and then empty the list.

• Indexed DB would invoke with:

Add a step to the relevant event loop's post-microtask jobs list to unset transaction's active flag

[annevk]

So does it need to be associated with the event loop or just the current task? Can this span multiple tasks?

[inexorabletash]

I believe just the current task is adequate. (I see most hooks associated with a loop rather than a task, e.g. the "perform a microtask checkpoint" steps have an event loop as context, not a task, which is why I was fixated on the event loop)

This should not span multiple tasks to my knowledge. If there are any special cases where multiple tasks have special behavior (e.g. are executed within one "turn" of the event loop, other than microtasks) then we should test and see how implementations behave.

[annevk]

Hmm. The current setup is a little weird indeed in that microtasks are associated with the event loop, rather than a task. Do you know the rationale for that @domenic?

[domenic]

It doesn't seem to make much of a difference, and seems like a simpler model to associate with the event loop. That way you don't need to e.g. always specify the associated task when you queue a microtask. I'd go with the event loop for these hooks too.

Basically, the only reason to associate it with tasks is if you want these steps to run after some tasks but not others. Microtasks we want to run after all tasks, so they are associated with the event loop.

[annevk]

Posted a PR.