Rework media element removal steps

[annevk]

As per #2882 "await a stable state" is a problematic concept as it queues a microtask without encompassing task.

This replaces its usage here by queuing a task as the media element paused member does not change upon element removal directly.

(It's unclear if you can observe the paused member changing before the pause event. As written this seems possible if you are (un)lucky. Potentially this means that the internal pause steps are not correct for this caller. They are correct for at least one other caller: pause().)

---

Even the simplest case turned out to be not simple. Here's the test case I played with on Live DOM Viewer:

<script>
a=document.createElement("audio")
document.body.appendChild(a)
a.addEventListener("pause", () => w(a.paused))
a.play()
w(a.paused)
a.remove()
w(a.paused)
queueMicrotask(() => w(a.paused))
</script>

A follow-up issue for the internal pause steps seems reasonable as they were already broken before this change for this caller. Would love some independent analysis of these things.

• At least two implementers are interested (and none opposed):
  • …
  • …
• Tests are written and can be reviewed and commented upon at:
  • …
• Implementation bugs are filed:
  • Chrome: …
  • Firefox: …
  • Safari: …
  • Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): …
  • Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): …

(See WHATWG Working Mode: Changes for more details.)

---

/media.html ( diff )

[domenic]

(Preexisting issue: this algorithm is not shadow DOM aware.)

So the intent of the current spec seems to be to queue a microtask from within the task that removes the element. I don't think it's possible to remove elements without being in a task, so the current spec doesn't seem problematic to me.

So this is a normative change. A version of this patch that removes "await a stable state" but isn't a normative change would be something like:

1. Assert: the surrounding agent's event loop's currently running task is not null.
2. Queue a microtask to perform the following steps: [...]

Now, is this normative change closer to implementations? Your test case seems to be asking that question. I fleshed it out a bit and added a click gesture so you can call play() successfully: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=10260 . The answer appears to be "yes", your change is closer to implementations; all three log "false" in the queued microtask and "true" after 1 second, so it seems like it's some sort of queued task, not a microtask.

[annevk]

The current spec is problematic because of this:

When an algorithm running in parallel is to await a stable state, ...

It's UB when the algorithm is not running in parallel, as is the case here.

[domenic]

Well, that's easy to fix, by just allowing any algorithm to await a stable state.

Anyway, per the above, this is a good change either way. So we should write up some WPTs based on the above samples and then merge this.

[zcorpan]

Should "in a document" be used here?

[annevk]

This probably needs to be "connected", but I'd consider that a separate change. I'd be willing to make it at the same time though if someone figures out the tests.