Mutating the close watcher stack during a close request

[keithamus]

What is the issue with the HTML Standard?

https://html.spec.whatwg.org/multipage/interaction.html#close-watcher-destroy

https://html.spec.whatwg.org/#process-close-watchers

I'm trying to implement CloseWatcher in gecko. For the destroy process steps, the spec states that to destroy process a close watcher the last group is iterated over and for each item RequestClose() is called, until the list is either empty or a cancel event prevents default. However if in a cancel event, a new CloseWatcher() is created, or an existing CloseWatcher is destroyed, then it seems the list is altered?

Are there any mitigations in place to cover this? What happens in these scenarios?

/cc @domenic @lukewarlow @emilio

(Meta: should we add a topic: closewatcher label?)

[domenic]

For the destory steps, the spec states that to destroy a close watcher the last group is iterated over and for each item RequestClose() is called

That's not what I see in https://html.spec.whatwg.org/#close-watcher-destroy ? Can you quote the line you're referring to?

[keithamus]

Sorry you're right I meant the Process steps: https://html.spec.whatwg.org/#process-close-watchers

[domenic]

I see.

So I think the ultimate problem here is whatwg/infra#396.

I think there are two possible patches.

Make a clone

Make a clone of group between steps 2.1 and 2.2.

After that, I think everything works relatively fine. If you destroy a close watcher in group then "requesting to close" will just bail. If you add a new close watcher, then we'll just ignore it, which seems OK-ish?

Switch to popping

Change step 2 to something like "While group is not empty, let closeWatcher be the last item in group."

This again works fine with deletes. And if you add a new close watcher to the group, we'll process it.

---

I think of these I prefer "switch to popping".

I also considered reevaluating "let group be the last item in window's close watcher manager's groups" each time. But that seems bad. Processing half of one group then switching to another is weird.

We could force all close watchers created inside of close watcher callbacks to be grouped, which would prevent that situation from happening. I'm not sure it buys us much.

---

The hardest part of fixing this will be adding WPTs, because with this sort of edge case behavior there are a lot of them. Some suggestions:

• Close watcher destroys itself while it's being processed
• Close watcher destroys previous close watcher in same group while it's being processed
• Close watcher destroys subsequent close watcher in same group while it's being processed
• Close watcher destroys a close watcher in another group while it's being processed
• Close watcher adds a new close watcher in the same group while it's being processed
• Close watcher adds a new close watcher in a new group while it's being processed

Not all would be mandatory to land a fix here but they'd certainly be appreciated :)