From f239744191e76402414f6e8f07fdc59863d7a6c8 Mon Sep 17 00:00:00 2001
From: Domenic Denicola <d@domenic.me>
Date: Mon, 22 Apr 2024 15:22:43 +0900
Subject: [PATCH] Always fire CloseWatcher/dialog cancel events

Previously, the anti-abuse mechanisms would sometimes skip these events. However, as discussed in #10047, it is more useful to fire them, and have the anti-abuse mechanisms set cancelable to false for them when appropriate. This allows observing close requests vs. other types of closes, by looking for the cancel event.

Closes #10047.
---
 source | 84 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 46 insertions(+), 38 deletions(-)

diff --git a/source b/source
index 51cd7b38a87..26243bd5b3c 100644
--- a/source
+++ b/source
@@ -61402,10 +61402,12 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> {
     <span>this</span>'s <span>relevant global object</span>, with:</p>
 
     <ul>
-     <li><p><i data-x="create-close-watcher-cancelAction">cancelAction</i> being to return the
-     result of <span data-x="concept-event-fire">firing an event</span> named <code
+     <li><p><i data-x="create-close-watcher-cancelAction">cancelAction</i> given
+     <var>canPreventClose</var> being to return the result of <span
+     data-x="concept-event-fire">firing an event</span> named <code
      data-x="event-cancel">cancel</code> at <span>this</span>, with the <code
-     data-x="dom-Event-cancelable">cancelable</code> attribute initialized to true.</p></li>
+     data-x="dom-Event-cancelable">cancelable</code> attribute initialized to
+     <var>canPreventClose</var>.</p></li>
 
      <li><p><i data-x="create-close-watcher-closeAction">closeAction</i> being to <span>close the
      dialog</span> given <span>this</span> and null.</p></li>
@@ -82102,13 +82104,16 @@ body { display:none }
   <ul>
    <li><p>A <dfn data-x="close-watcher-window">window</dfn>, a <code>Window</code>.</p></li>
 
-   <li><p>A <dfn data-x="close-watcher-cancel-action">cancel action</dfn>, a list of steps. These
-   steps can never throw an exception, and return either true (to indicate that the caller will
-   proceed to the <span data-x="close-watcher-close-action">close action</span>) or false (to
-   indicate that the caller will bail out).</p></li>
+   <li><p>A <dfn data-x="close-watcher-cancel-action">cancel action</dfn>, an algorithm accepting a
+   boolean argument and returning a boolean. The argument indicates whether or not the cancel
+   action algorithm can prevent the close request from proceeding via the algorithm's return value.
+   If the boolean argument is true, then the algorithm can return either true to indicate that the
+   caller will proceed to the <span data-x="close-watcher-close-action">close action</span>, or
+   false to indicate that the caller will bail out. If the argument is false, then the return value
+   is always false. This algorithm can never throw an exception.</p></li>
 
-   <li><p>A <dfn data-x="close-watcher-close-action">close action</dfn>, a list of steps. These
-   steps can never throw an exception.</p></li>
+   <li><p>A <dfn data-x="close-watcher-close-action">close action</dfn>, an algorithm accepting no
+   arguments and returning nothing. This algorithm can never throw an exception.</p></li>
 
    <li><p>An <dfn data-x="close-watcher-is-running-cancel">is running cancel action</dfn>
    boolean.</p></li>
@@ -82193,38 +82198,37 @@ body { display:none }
    <li><p>If <var>window</var>'s <span data-x="concept-document-window">associated
    <code>Document</code></span> is not <span>fully active</span>, then return true.</p></li>
 
-   <li>
-    <p>If <var>window</var>'s <span>close watcher manager</span>'s <span data-x="close watcher
-    manager groups">groups</span>'s <span data-x="list size">size</span> is less than
-    <var>window</var>'s <span>close watcher manager</span>'s <span data-x="close watcher manager
-    allowed number of groups">allowed number of groups</span>, and <var>window</var> has
-    <span>history-action activation</span>, then:</p>
+   <li><p>Let <var>canPreventClose</var> be true if <var>window</var>'s <span>close watcher
+   manager</span>'s <span data-x="close watcher manager groups">groups</span>'s <span data-x="list
+   size">size</span> is less than <var>window</var>'s <span>close watcher manager</span>'s <span
+   data-x="close watcher manager allowed number of groups">allowed number of groups</span>, and
+   <var>window</var> has <span>history-action activation</span>; otherwise false.</p></li>
 
-    <ol>
-     <li><p>Set <var>closeWatcher</var>'s <span data-x="close-watcher-is-running-cancel">is running
-     cancel action</span> to true.</p></li>
+   <li><p>Set <var>closeWatcher</var>'s <span data-x="close-watcher-is-running-cancel">is running
+   cancel action</span> to true.</p></li>
 
-     <li><p>Let <var>shouldContinue</var> be the result of running <var>closeWatcher</var>'s <span
-     data-x="close-watcher-cancel-action">cancel action</span>.</p></li>
+   <li><p>Let <var>shouldContinue</var> be the result of running <var>closeWatcher</var>'s <span
+   data-x="close-watcher-cancel-action">cancel action</span> given
+   <var>canPreventClose</var>.</p></li>
 
-     <li><p>Set <var>closeWatcher</var>'s <span data-x="close-watcher-is-running-cancel">is running
-     cancel action</span> to false.</p></li>
+   <li><p>Set <var>closeWatcher</var>'s <span data-x="close-watcher-is-running-cancel">is running
+   cancel action</span> to false.</p></li>
 
-     <li>
-      <p>If <var>shouldContinue</var> is false, then:</p>
+   <li>
+    <p>If <var>shouldContinue</var> is false, then:</p>
 
-      <ol>
-       <li><p><span>Consume history-action user activation</span> given <var>window</var>.</p></li>
+    <ol>
+     <li><p><span>Assert</span>: <var>canPreventClose</var> is true.</p></li>
 
-       <li><p>Return false.</p></li>
-      </ol>
-     </li>
+     <li><p><span>Consume history-action user activation</span> given <var>window</var>.</p></li>
+
+     <li><p>Return false.</p></li>
     </ol>
 
     <p class="note">Note that since these substeps <span>consume history-action user
     activation</span>, <span data-x="close-watcher-request-close">requesting to close</span> a
-    <span>close watcher</span> twice without any intervening <span>user activation</span> will skip
-    these substeps.</p>
+    <span>close watcher</span> twice without any intervening <span>user activation</span> will
+    result in <var>canPreventClose</var> being false the second time.</p>
    </li>
 
    <li><p><span data-x="close-watcher-close">Close</span> <var>closeWatcher</var>.</p></li>
@@ -82401,10 +82405,12 @@ dictionary <dfn dictionary>CloseWatcherOptions</dfn> {
     object</span>, with:</p>
 
     <ul>
-     <li><p><i data-x="create-close-watcher-cancelAction">cancelAction</i> being to return the
-     result of <span data-x="concept-event-fire">firing an event</span> named <code
+     <li><p><i data-x="create-close-watcher-cancelAction">cancelAction</i> given
+     <var>canPreventClose</var> being to return the result of <span
+     data-x="concept-event-fire">firing an event</span> named <code
      data-x="event-cancel">cancel</code> at <span>this</span>, with the <code
-     data-x="dom-Event-cancelable">cancelable</code> attribute initialized to true.</p></li>
+     data-x="dom-Event-cancelable">cancelable</code> attribute initialized to
+     <var>canPreventClose</var>.</p></li>
 
      <li><p><i data-x="create-close-watcher-closeAction">closeAction</i> being to <span
      data-x="concept-event-fire">fire an event</span> named <code data-x="event-close">close</code>
@@ -82494,7 +82500,7 @@ picker.querySelector('.close-button').onclick = () => watcher.requestClose();</c
    <code>CloseWatcher</code> from being destroying. A typical use case is as follows:</p>
 
    <pre><code class="js">watcher.oncancel = async (e) => {
-  if (hasUnsavedData) {
+  if (hasUnsavedData && e.cancelable) {
     e.preventDefault();
 
     const userReallyWantsToClose = await askForConfirmation("Are you sure you want to close?");
@@ -82505,11 +82511,13 @@ picker.querySelector('.close-button').onclick = () => watcher.requestClose();</c
   }
 };</code></pre>
 
-   <p>For abuse prevention purposes, this event only fires if the page has <span>history-action
+   <p>For abuse prevention purposes, this event is only <code
+   data-x="dom-Event-cancelable">cancelable</code> if the page has <span>history-action
    activation</span>, which will be lost after any given <span>close request</span>. This ensures
    that if the user sends a close request twice in a row without any intervening user activation,
-   the request definitely succeeds; the second request ignores the <code
-   data-x="event-cancel">cancel</code> event handler and immediately closes the
+   the request definitely succeeds; the second request ignores any <code
+   data-x="event-cancel">cancel</code> event handler's attempt to call <code
+   data-x="dom-Event-preventDefault">preventDefault()</code> and proceeds to close the
    <code>CloseWatcher</code>.</p>
   </div>