Re-do close watcher user activation tracking

[domenic]

As noted in #10046, the close watcher anti-abuse measures were overly strict, and would prevent firing the cancel event even when the close watcher was created with user interaction.

Additionally, the "is grouped with previous" infrastructure used to track close watcher groups was buggy, since close watchers could be destroyed and this would cause groups to spuriously collapse.

To fix both of these problems, we re-do the close watcher anti-abuse protections. We now track the groups as a list-of-lists, and explicitly track how many groups should be allowed at a given time. We can then compare them when making decisions about whether to group a new close watcher, or whether to fire a cancel event.

Note that the initial fix proposed in #10046 (and implemented in #10048) is incorrect, as it allows indefinite trapping of the user.

Closes #10046.

• At least two implementers are interested (and none opposed):
  • Chromium
  • Interpreting this comment from @zcorpan as support for this sort of bugfix from Mozilla
• Tests are written and can be reviewed and commented upon at:
  • Re-do close watcher user activation tests web-platform-tests/wpt#44825
• Implementation bugs are filed:
  • Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1512224
  • Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1859702 (general close request/close watcher bug)
  • WebKit: https://bugs.webkit.org/show_bug.cgi?id=263305 (general close request/close watcher bug)
  • Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): N/A
  • Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): N/A
• MDN issue is filed: Not necessary for this edge-case change
• The top of this comment includes a clear commit message to use.

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

---

/interaction.html ( diff )

[lukewarlow]

I've given this a read through and it all seems to make sense to me. I can't think of a way this can be abused to circumvent the protections.

Just to clarify this doesn't increase the total number of back gestures required to escape compared to the previous implementation? (In the case where the user doesn't interact with the page in the mean time)

[domenic]

Thanks so much for your review @lukewarlow!

Just to clarify this doesn't increase the total number of back gestures required to escape compared to the previous implementation? (In the case where the user doesn't interact with the page in the mean time)

Indeed. With much more test coverage now!

[zcorpan]

The "groups" concept seems reasonable, and makes behavior similar to e.g. a fragment navigation for each user activation (adding a close watcher to the same group is like a "replace" navigation). Users can navigate back multiple steps in various browsers, which should also bypass close watchers IIUC.

If abuse becomes a significant problem, maybe browsers can heuristically detect user frustration (repeatedly trying to go back and groups.length > some number) and then close everything or navigate back?

[domenic]

The "groups" concept seems reasonable, and makes behavior similar to e.g. a fragment navigation for each user activation (adding a close watcher to the same group is like a "replace" navigation).

I like this analogy!

Users can navigate back multiple steps in various browsers, which should also bypass close watchers IIUC.

Yep, exactly. It's just kind of a power-user feature, so we don't want to tell people that's their only recourse.

If abuse becomes a significant problem, maybe browsers can heuristically detect user frustration (repeatedly trying to go back and groups.length > some number) and then close everything or navigate back?

For sure, this is the bazooka we can always use. But it hurts web platform predictability a bit if we're not careful, so that's why I've spent a lot of time trying to put deterministic guardrails around this for the spec (and Chromium).

But, I did put in an optional "skip to the end" step as you suggested, since it's useful not only for anti-abuse, but also for other cases. In general I'm in favor of giving browsers more "may" options for browser UI interactions, so thanks for suggesting it.