Remove compound microtasks

[annevk]

Per https://lists.w3.org/Archives/Public/www-archive/2016Oct/0001.html the rationale for having this concept is the ability for some API to spin the event loop. This used to be the case with showModalDialog(), but that API got removed.

Implementations also do not have this concept and treat this as a regular microtoask that just happens to do quite a few things (going through mutation observer callbacks and dispatching slotchange events).

---

/webappapis.html ( diff )

[annevk]

@domenic pointed out that per @bzbarsky's request https://html.spec.whatwg.org/multipage/webappapis.html#pause allows user agents to spin the event loop (used by alert() et al). Perhaps we should disallow that from microtasks? But also, given that implementations do not really have the compound microtask concept, there either already is a problem with that or it's not such a big deal somehow?

[domenic]

This doesn't seem safe to do unless all uses of "spin the event loop" are also removed from the specification?

[annevk]

1. How are implementations safe if they don't have this concept? (Or perhaps you mean that the Assert I added is not safe, which is probably correct.)
2. It does seem like spin the event loop can be triggered by "pause" and document.close(), both of which could be invoked from a microtask. All other uses cannot be so are not relevant as far as I can tell.

[domenic]

My tentative guess is that implementations have something observably equivalent to this concept, but implemented in a different way.

The way to test this would be to invoke the scenario outlined at https://lists.w3.org/Archives/Public/www-archive/2016Oct/0001.html :

If you were to spin the event loop when the current task is a compound microtask, then you'd end up moving all the subtasks after the first one that spins the event loop into a later full task, which is probably not what you want.

If we get the "probably not what you want" scenario, then indeed implementations don't have this concept. Otherwise, we have something that serves the same purpose, but probably with a very different shape.

[annevk]

So given that "performing a microtask checkpoint flag" already blocks microtasks while we're in nested event loop territory, what exactly would the observable difference be?

[annevk]

I fixed the issue with the initial commit I think. As far as I can tell this is now correct and there's no difference with the status quo setup due to the performing a microtask checkpoint flag.

[domenic]

I'll review, try to convince myself this is no different from the status quo, and even if I can't quite convince myself I'll lean toward trusting you anyway. But, I would also love it if implementers (particularly: @bzbarsky, @smaug----, and @rniwa) were able to confirm that this change was OK.

On the Chrome side I'm not sure who our expert is, but @zetafunction, @jeremyroman, and @yuki3 are good bets; if any of them wants to weigh in I'd love their thoughts too.

[ajklein]

@tzik is perhaps the most-current Chromium contact for Microtasks

[annevk]

I spend some time looking at this again today and some of the above was mistaken. The idea behind subtasks:

If inside a compound microtask subtask you spin the event loop, then that subtask is paused, but the remainder of the compound microtask will then run (due to spin the event loop's "Stop task, allowing whatever algorithm that invoked it to resume"), followed by the microtask checkpoint steps, followed by the general event loop. All this until some goal is met, at which point a task is queued to run the remainder of the subtask that spun the event loop (again defined in spin the event loop).

However, Firefox does not implement subtasks (and based on discussion we also rather not add it). WebKit does not appear to implement subtasks either. Removing that concept therefore still seems like the way to go. Hopefully @tzik can confirm that Chromium matches this.

(Also, spinning the event loop isn't invoked that often; for the case of alert() Firefox uses a more restricted version of spinning the event loop that does not run microtasks. It seems that requires some follow-up to decide what we actually want to allow and do long term. document.close() could also use some more investigation as part of this.)

---

Some historical references:

• 01e1f33#diff-36cd38f49b9afa08222c0dc9ebfe35eb introduced the concept of perform a microtask checkpoint for mutation observers
• 95d6fa9#diff-36cd38f49b9afa08222c0dc9ebfe35eb renamed the reentrency flag to performing a microtask checkpoint flag (custom elements ended up not using this though)
• https://www.w3.org/Bugs/Public/show_bug.cgi?id=20821 discussed the problem of spinning the event loop from a microtask and resulted in the addition of compound microtasks in 9077690#diff-36cd38f49b9afa08222c0dc9ebfe35eb

[tzik]

Chrome's implementation doesn't have the concept of compound microtask. A microtask just blocks other microtasks by setting the checkpoint flag.

[annevk]

@tzik thank you, but could you elaborate a bit? Per the specification, if the event loop is spun during a microtask paused, it would mean that all microtasks newer than paused in the microtask queue get executed, followed by subsequent tasks in the overall event loop, until some goal is met, at which point the remainder of paused is executed as part of a new task.

I think it is clear at this point that this patch (assuming no mistakes) can be merged, as compound microtasks are not a thing. But there's an open question around spinning the event loop and to what extent implementations are observably affected. (Which can be tracked via #3387 and #4439 I think.)

[annevk]

I think we should make this say "This is a task or a microtask." I'm not convinced the remainder adds clarity. Thoughts? (We could perhaps Assert it instead.)

[domenic]

I think the remainder adds some clarity. It could be rephrased as an assert but I'm fine with it as-is.

Editorial nit: "If this is a microtask, then the..."

[annevk]

So the problem with the remainder is that (as happened in this thread) folks might assume the flag remains set.

[domenic]

Hmm. I think assert is pretty clear that it's a point-in-time check, then. Or, you could add a note to the "Perform a microtask checkpoint" step which notes that it unsets the flag. (Or even an assert afterward that it's unset.)

[annevk]

That's not where it gets unset, right? It gets unset after you return to the caller when you stop the microtask but resume the steps that invoked it.

[domenic]

OK here is what I would do. Remove the remainder from here. Instead, in the "Perform a microtask checkpoint" step, add a note specifically saying "This step will be a no-op if task is a microtask, since in that case the performing a microtask checkpoint flag will be set." That seems to capture the actually-interesting part of it being set.

[annevk]

Done.

[domenic]

I guess I find "assert" a bit strong here since it's more of a reminder that microtasks are a type of task. But, I'm OK keeping it like this.

[littledan]

Note, if it's possible to pause a microtask and spin the event loop, this would seem to violate the invariants in line 6170-6172 of https://github.com/tc39/ecma262/pull/735/files , which is @domenic's PR to clean up layering between HTML and JS in this area.

[annevk]

@littledan indeed, I suggest we track that separately. It greatly depends on the outcome of the issues referenced in #4437 (comment).

[tzik]

@tzik thank you, but could you elaborate a bit? Per the specification, if the event loop is spun during a microtask paused, it would mean that all microtasks newer than paused in the microtask queue get executed, followed by subsequent tasks in the overall event loop, until some goal is met, at which point the remainder of paused is executed as part of a new task.

Aren't these microtasks blocked by the performing a microtask checkpoint flag, which is set by the outer microtask? On the Chrome implementation, the nested event loop runs other tasks, but it doesn't run microtasks, that might not be spec compliant.

I think it is clear at this point that this patch (assuming no mistakes) can be merged, as compound microtasks are not a thing. But there's an open question around spinning the event loop and to what extent implementations are observably affected. (Which can be tracked via #3387 and #4439 I think.)

[annevk]

@tzik I believe that matches Firefox. I initially thought that is also what the specification says, but it's not. In particular:

• Say we're executing a microtask Initiator. This means we're at step 2.3 of https://html.spec.whatwg.org/#perform-a-microtask-checkpoint running the steps of Initiator.
• We then come across "spin the event loop" (defined a little lower) in one of the steps.
• "Spin the event loop" stops Initiator, but says to continue the algorithm that invoked it. That means we'll continue "perform a microtask checkpoint" at step 2.4.
• That means subsequent microtasks will now execute and eventually the performing a microtask checkpoint flag will be set to false.
• Then tasks will execute, which are followed by microtasks, etc.
• Until at some point the goal is met that "spin the event loop" talks about at which point another task is queued to execute the remainder of Initiator.

Makes sense?

I haven't found a reliable way to spin the event loop in implementations however and definitely not in a way that is as unconstrained as the specification makes it out to be. I don't think we should follow the specification here necessarily, but instead make it closer in line with what implementations are doing.

Removing compound microtasks seems like a good first step.

Not running microtasks when nesting seems like a good second step, given it's what at least Chrome and Firefox, and likely Safari, do, but I'm not entirely sure yet how to accomplish this wording-wise. (It's also not an architecture TC39 would be happy with, I think.)

[tzik]

Oh... OK, now that makes sense to me. Though I'm not sure we can do it anytime soon.

[bzbarsky]

Not running microtasks when nesting seems like a good second step, given it's what at least Chrome and Firefox, and likely Safari, do

Hmm. So in Firefox and Chrome, if you put up an alert from inside a microtask, no other microtasks run (across what set of globals?) until the alert comes down?

[annevk]

Though I'm not sure we can do it anytime soon.

To be clear, I'm saying maybe you don't have to and we should fix the specification. But also, as @bzbarsky notes, more tests are needed.

[domenic]

LGTM. I guess we should not merge until DOM stops using them?

[domenic]

I think the remainder adds some clarity. It could be rephrased as an assert but I'm fine with it as-is.

Editorial nit: "If this is a microtask, then the..."

[domenic]

Maybe not worth fixing here, but I think indentation could help here (i.e. the modern style that does not "continue running these steps in parallel"). Plus perhaps a note about how this has such drastic effects on the caller. But maybe that's better addressed by just doing more refactorings like #4440.

[annevk]

whatwg/dom#741 should remove the concept from DOM. Feel free to review/land.

[domenic]

I guess I find "assert" a bit strong here since it's more of a reminder that microtasks are a type of task. But, I'm OK keeping it like this.