Close watchers: always fire cancel events

Sometimes they will have cancelable = false, but they will now always
fire. See whatwg/html#10047.

Bug: 41484805, 40054591
Change-Id: Ica682043fb56c729f4c331e9f3bd0590d3b1d088
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5465306
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1291009}

third_party/blink/renderer/core/html/closewatcher/close_watcher.cc

@@ -119,7 +119,7 @@ void CloseWatcher::WatcherStack::SetHadUserInteraction(
   }
 }
 
-bool CloseWatcher::WatcherStack::CanFireCancelEvent() const {
+bool CloseWatcher::WatcherStack::CancelEventCanBeCancelable() const {
   return watcher_groups_.size() < allowed_groups_ &&
          window_->GetFrame()->IsHistoryUserActivationActive();
 }
@@ -209,18 +209,21 @@ bool CloseWatcher::requestClose() {
   }
 
   WatcherStack& stack = *DomWindow()->closewatcher_stack();
-  if (stack.CanFireCancelEvent()) {
-    Event& cancel_event = *Event::CreateCancelable(event_type_names::kCancel);
-    {
-      base::AutoReset<bool> scoped_committing(&dispatching_cancel_, true);
-      DispatchEvent(cancel_event);
-    }
-    if (cancel_event.defaultPrevented()) {
-      if (DomWindow()) {
-        DomWindow()->GetFrame()->ConsumeHistoryUserActivation();
-      }
-      return false;
+  Event& cancel_event =
+      stack.CancelEventCanBeCancelable()
+          ? *Event::CreateCancelable(event_type_names::kCancel)
+          : *Event::Create(event_type_names::kCancel);
+
+  {
+    base::AutoReset<bool> scoped_committing(&dispatching_cancel_, true);
+    DispatchEvent(cancel_event);
+  }
+
+  if (cancel_event.defaultPrevented()) {
+    if (DomWindow()) {
+      DomWindow()->GetFrame()->ConsumeHistoryUserActivation();
     }
+    return false;
   }
 
   close();

third_party/blink/renderer/core/html/closewatcher/close_watcher.h

@@ -64,7 +64,7 @@ class CloseWatcher final : public EventTarget, public ExecutionContextClient {
     void Remove(CloseWatcher*);
 
     void SetHadUserInteraction(bool);
-    bool CanFireCancelEvent() const;
+    bool CancelEventCanBeCancelable() const;
 
     void Trace(Visitor*) const;
 

third_party/blink/renderer/core/html/html_dialog_element.cc

@@ -415,7 +415,9 @@ void HTMLDialogElement::CloseWatcherFiredCancel(Event* close_watcher_event) {
     return;
   // https://wicg.github.io/close-watcher/#patch-dialog cancelAction
 
-  Event* dialog_event = Event::CreateCancelable(event_type_names::kCancel);
+  Event* dialog_event = close_watcher_event->cancelable()
+                            ? Event::CreateCancelable(event_type_names::kCancel)
+                            : Event::Create(event_type_names::kCancel);
   DispatchEvent(*dialog_event);
   if (dialog_event->defaultPrevented())
     close_watcher_event->preventDefault();

third_party/blink/web_tests/external/wpt/close-watcher/abortsignal.html

@@ -49,7 +49,7 @@
   watcher.requestClose();
   controller.abort();
 
-  assert_equals(oncancel_call_count_, 0);
+  assert_equals(oncancel_call_count_, 1);
   assert_equals(onclose_call_count_, 1);
 }, "requestClose() then abortController.abort() fires only one close event");
 
@@ -92,7 +92,7 @@
   await sendCloseRequest();
   controller.abort();
 
-  assert_equals(oncancel_call_count_, 0);
+  assert_equals(oncancel_call_count_, 1);
   assert_equals(onclose_call_count_, 1);
 }, "Esc key then abortController.abort() fires only one close event");
 

third_party/blink/web_tests/external/wpt/close-watcher/basic.html

@@ -14,8 +14,8 @@
 
   watcher.requestClose();
 
-  assert_array_equals(events, ["close"]);
-}, "requestClose() with no user activation only fires close");
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
+}, "requestClose() with no user activation");
 
 test(t => {
   let events = [];
@@ -25,7 +25,7 @@
   watcher.requestClose();
 
   assert_array_equals(events, []);
-}, "destroy() then requestClose() fires no events");
+}, "destroy() then requestClose()");
 
 test(t => {
   let events = [];
@@ -36,18 +36,18 @@
 
   watcher.requestClose();
   assert_array_equals(events, ["close"]);
-}, "close() then requestClose() fires only one close event");
+}, "close() then requestClose()");
 
 test(t => {
   let events = [];
   let watcher = createRecordingCloseWatcher(t, events);
 
   watcher.requestClose();
-  assert_array_equals(events, ["close"]);
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
 
   watcher.destroy();
-  assert_array_equals(events, ["close"]);
-}, "requestClose() then destroy() fires only one close event");
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
+}, "requestClose() then destroy()");
 
 test(t => {
   let events = [];
@@ -58,7 +58,7 @@
 
   watcher.destroy();
   assert_array_equals(events, ["close"]);
-}, "close() then destroy() fires only one close event");
+}, "close() then destroy()");
 
 promise_test(async t => {
   let events = [];
@@ -68,7 +68,7 @@
   await sendCloseRequest();
 
   assert_array_equals(events, []);
-}, "destroy() then close request fires no events");
+}, "destroy() then close request");
 
 promise_test(async t => {
   let events = [];
@@ -77,6 +77,6 @@
   await sendCloseRequest();
   watcher.destroy();
 
-  assert_array_equals(events, ["close"]);
-}, "Close request then destroy() fires only one close event");
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
+}, "Close request then destroy()");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/esc-key/keypress.html

@@ -16,6 +16,6 @@
 
   await sendEscKey();
 
-  assert_array_equals(events, ["close"]);
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
 }, "A keypress listener can NOT prevent the Esc keypress from being interpreted as a close request");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/esc-key/keyup.html

@@ -16,6 +16,6 @@
 
   await sendEscKey();
 
-  assert_array_equals(events, ["close"]);
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
 }, "A keyup listener can NOT prevent the Esc keypress from being interpreted as a close request");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/esc-key/not-user-activation.html

@@ -14,6 +14,6 @@
 
   await sendEscKey();
 
-  assert_array_equals(events, ["close"]);
-}, "Esc key does not count as user activation, so if it is the sole user interaction, that fires close but not cancel");
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
+}, "Esc key does not count as user activation, so if it is the sole user interaction, cancel is cancelable=false");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/inside-event-listeners.html

@@ -17,10 +17,10 @@
   await test_driver.bless("give user activation so that cancel will fire", () => {
     watcher.requestClose();
   });
-  assert_array_equals(events, ["cancel"]);
+  assert_array_equals(events, ["cancel[cancelable=true]"]);
 
   watcher.requestClose();
-  assert_array_equals(events, ["cancel"], "since it was inactive, no more events fired");
+  assert_array_equals(events, ["cancel[cancelable=true]"], "since it was inactive, no more events fired");
 }, "destroy() inside oncancel");
 
 test(t => {
@@ -30,10 +30,10 @@
   watcher.onclose = () => { watcher.destroy(); }
 
   watcher.requestClose();
-  assert_array_equals(events, ["close"]);
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
 
   watcher.requestClose();
-  assert_array_equals(events, ["close"], "since it was inactive, no more events fired");
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"], "since it was inactive, no more events fired");
 }, "destroy() inside onclose");
 
 promise_test(async t => {
@@ -45,10 +45,10 @@
   await test_driver.bless("give user activation so that cancel will fire", () => {
     watcher.requestClose();
   });
-  assert_array_equals(events, ["cancel", "close"]);
+  assert_array_equals(events, ["cancel[cancelable=true]", "close"]);
 
   watcher.requestClose();
-  assert_array_equals(events, ["cancel", "close"], "since it was inactive, no more events fired");
+  assert_array_equals(events, ["cancel[cancelable=true]", "close"], "since it was inactive, no more events fired");
 }, "close() inside oncancel");
 
 test(t => {
@@ -58,10 +58,10 @@
   watcher.onclose = () => { watcher.close(); }
 
   watcher.requestClose();
-  assert_array_equals(events, ["close"]);
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
 
   watcher.requestClose();
-  assert_array_equals(events, ["close"], "since it was inactive, no more events fired");
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"], "since it was inactive, no more events fired");
 }, "close() inside onclose");
 
 promise_test(async t => {
@@ -73,10 +73,10 @@
   await test_driver.bless("give user activation so that cancel will fire", () => {
     watcher.requestClose();
   });
-  assert_array_equals(events, ["cancel", "close"]);
+  assert_array_equals(events, ["cancel[cancelable=true]", "close"]);
 
   watcher.requestClose();
-  assert_array_equals(events, ["cancel", "close"], "since it was inactive, no more events fired");
+  assert_array_equals(events, ["cancel[cancelable=true]", "close"], "since it was inactive, no more events fired");
 }, "requestClose() inside oncancel");
 
 test(t => {
@@ -86,9 +86,9 @@
   watcher.onclose = () => { watcher.requestClose(); }
 
   watcher.requestClose();
-  assert_array_equals(events, ["close"]);
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
 
   watcher.requestClose();
-  assert_array_equals(events, ["close"], "since it was inactive, no more events fired");
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"], "since it was inactive, no more events fired");
 }, "requestClose() inside onclose");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/resources/helpers.js

@@ -26,9 +26,14 @@ window.createRecordingCloseWatcher = (t, events, name, type, parentWatcher) => {
     t.add_cleanup(() => watcher.destroy());
   }
 
-  const prefix = name === undefined ? "" : name + " ";
-  watcher.addEventListener('cancel', () => events.push(prefix + "cancel"));
-  watcher.addEventListener('close', () => events.push(prefix + "close"));
+  const prefix = name === undefined ? '' : name + ' ';
+  watcher.addEventListener('cancel', e => {
+    const cancelable = e.cancelable ? '[cancelable=true]' : '[cancelable=false]';
+    events.push(prefix + 'cancel' + cancelable);
+  });
+  watcher.addEventListener('close', () => {
+    events.push(prefix + 'close');
+  });
 
   return watcher;
 };

third_party/blink/web_tests/external/wpt/close-watcher/user-activation/README.md

@@ -0,0 +1,25 @@
+# Close watcher user activation tests
+
+These tests are all in separate files (or test variants) because we need to be
+sure we're starting from zero user activation.
+
+## Note on variants vs. `-dialog` and `-CloseWatcher` files
+
+We endeavor to have all the tests in these files cover both `<dialog>` elements
+and the `CloseWatcher` API. (And sometimes the `popover=""` attribute.)
+
+When the test expectations are the same for both `<dialog>` and `CloseWatcher`,
+we use WPT's variants feature.
+
+However, in some cases different expectations are necessary. This is because
+`<dialog>`s queue a task to fire their `close` event, and do not queue a task
+to fire their `cancel` event. Thus, when you have two `<dialog>`s grouped
+together, you get the somewhat-strange behavior of both `cancel`s firing first,
+then both `close`s. Whereas `CloseWatcher`s do not have this issue; both events
+fire synchronously.
+
+(Note that scheduling the `cancel` event for `<dialog>`s is not really possible,
+since it would then fire after the dialog has been closed in the DOM and
+visually. So the only reasonable fix for this would be to stop scheduling the
+`close` event for dialogs. That's risky from a compat standpoint, so for now,
+we test the strange behavior.)

third_party/blink/web_tests/external/wpt/close-watcher/user-activation/n-activate-preventDefault.html

@@ -22,10 +22,10 @@
   await sendCloseRequest();
   await waitForPotentialCloseEvent();
 
-  assert_array_equals(events, ["cancel"]);
+  assert_array_equals(events, ["cancel[cancelable=true]"]);
 
   await sendCloseRequest();
   await waitForPotentialCloseEvent();
-  assert_array_equals(events, ["cancel", "close"]);
+  assert_array_equals(events, ["cancel[cancelable=true]", "cancel[cancelable=false]", "close"]);
 }, "Create a close watcher without user activation that preventDefault()s cancel; send user activation");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/user-activation/n-activate.html

@@ -22,6 +22,6 @@
 
   await sendCloseRequest();
   await waitForPotentialCloseEvent();
-  assert_array_equals(events, ["cancel", "close"]);
+  assert_array_equals(events, ["cancel[cancelable=true]", "close"]);
 }, "Create a close watcher without user activation; send user activation");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/user-activation/n-closerequest-n.html

@@ -19,12 +19,12 @@
 
   await sendCloseRequest();
   await waitForPotentialCloseEvent();
-  assert_array_equals(events, ["watcher1 close"]);
+  assert_array_equals(events, ["watcher1 cancel[cancelable=false]", "watcher1 close"]);
 
   createRecordingCloseWatcher(t, events, "watcher2", type);
 
   await sendCloseRequest();
   await waitForPotentialCloseEvent();
-  assert_array_equals(events, ["watcher1 close", "watcher2 close"]);
+  assert_array_equals(events, ["watcher1 cancel[cancelable=false]", "watcher1 close", "watcher2 cancel[cancelable=false]", "watcher2 close"]);
 }, "Create a close watcher without user activation; send a close request; create a close watcher without user activation");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/user-activation/n-destroy-n.html

@@ -26,6 +26,6 @@
 
   await sendCloseRequest();
   await waitForPotentialCloseEvent();
-  assert_array_equals(events, ["watcher2 close"]);
+  assert_array_equals(events, ["watcher2 cancel[cancelable=false]", "watcher2 close"]);
 }, "Create a close watcher without user activation; destroy the close watcher; create a close watcher without user activation");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/user-activation/n.html

@@ -20,6 +20,6 @@
 
   await sendCloseRequest();
   await waitForPotentialCloseEvent();
-  assert_array_equals(events, ["close"]);
+  assert_array_equals(events, ["cancel[cancelable=false]", "close"]);
 }, "Create a close watcher without user activation");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/user-activation/nn.html → third_party/blink/web_tests/external/wpt/close-watcher/user-activation/nn-CloseWatcher.html

@@ -1,6 +1,4 @@
 <!doctype html>
-<meta name=variant content="?dialog">
-<meta name=variant content="?CloseWatcher">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <script src="/resources/testdriver.js"></script>
@@ -11,7 +9,7 @@
 
 <body>
 <script>
-const type = location.search.substring(1);
+const type = "CloseWatcher";
 
 promise_test(async t => {
   const events = [];
@@ -21,6 +19,6 @@
 
   await sendCloseRequest();
   await waitForPotentialCloseEvent();
-  assert_array_equals(events, ["watcher2 close", "watcher1 close"]);
+  assert_array_equals(events, ["watcher2 cancel[cancelable=false]", "watcher2 close", "watcher1 cancel[cancelable=false]", "watcher1 close"]);
 }, "Create two close watchers without user activation");
 </script>

third_party/blink/web_tests/external/wpt/close-watcher/user-activation/nn-activate-CloseWatcher.html

@@ -7,10 +7,6 @@
 <script src="/common/top-layer.js"></script>
 <script src="../resources/helpers.js"></script>
 
-<!--
-  See note in sibling -dialog.html file.
--->
-
 <body>
 <script>
 const type = "CloseWatcher";
@@ -25,6 +21,6 @@
 
   await sendCloseRequest();
   await waitForPotentialCloseEvent();
-  assert_array_equals(events, ["watcher2 cancel", "watcher2 close", "watcher1 cancel", "watcher1 close"]);
+  assert_array_equals(events, ["watcher2 cancel[cancelable=true]", "watcher2 close", "watcher1 cancel[cancelable=true]", "watcher1 close"]);
 }, "Create two CloseWatchers without user activation; send user activation");
 </script>