Make ShouldHaveFocusAppearance() consistent with :focus-visible.

Also simplify the model, and fix an issue where :focus-visible didn't match if `preventDefault()` was called on the keyboard event.

Now the model is that any trusted keyboard event will set had_keyboard_event to true, and any focus from mouse will re-set it to false.

Bug: 1010549
Change-Id: Ifcb9bfbef746b91d99fc8ef42a8ef0e223fca2c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863061Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Alice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707709}

third_party/blink/renderer/core/css/selector_checker.cc

@@ -1439,6 +1439,9 @@ bool SelectorChecker::MatchesFocusVisiblePseudoClass(const Element& element) {
   if (force_pseudo_state)
     return true;
 
+  if (!element.IsFocused() || !IsFrameFocused(element))
+    return false;
+
   const Document& document = element.GetDocument();
   bool always_show_focus_ring = element.MayTriggerVirtualKeyboard();
   bool last_focus_from_mouse =
@@ -1447,8 +1450,8 @@ bool SelectorChecker::MatchesFocusVisiblePseudoClass(const Element& element) {
       document.LastFocusType() == kWebFocusTypeMouse;
   bool had_keyboard_event = document.HadKeyboardEvent();
 
-  return element.IsFocused() && (!last_focus_from_mouse || had_keyboard_event ||
-                                 always_show_focus_ring);
+  return (!last_focus_from_mouse || had_keyboard_event ||
+          always_show_focus_ring);
 }
 
 // static

third_party/blink/renderer/core/dom/events/event_dispatcher.cc

@@ -227,6 +227,12 @@ inline EventDispatchContinuation EventDispatcher::DispatchEventPreProcess(
     pre_dispatch_event_handler_result =
         activation_target->PreDispatchEventHandler(*event_);
   }
+
+  // If this is a trusted keyboard event, update the keyboard event state and
+  // trigger :focus-visible matching if necessary.
+  if (event_->isTrusted() && event_->IsKeyboardEvent())
+    node_->UpdateHadKeyboardEvent(*event_);
+
   return (event_->GetEventPath().IsEmpty() || event_->PropagationStopped())
              ? kDoneDispatching
              : kContinueDispatching;
@@ -351,7 +357,6 @@ inline void EventDispatcher::DispatchEventPostProcess(
   } else if (!event_->DefaultHandled() && is_trusted_or_click) {
     // Non-bubbling events call only one default event handler, the one for the
     // target.
-    node_->WillCallDefaultEventHandler(*event_);
     node_->DefaultEventHandler(*event_);
     DCHECK(!event_->defaultPrevented());
     // For bubbling events, call default event handlers on the same targets in
@@ -359,8 +364,6 @@ inline void EventDispatcher::DispatchEventPostProcess(
     if (!event_->DefaultHandled() && event_->bubbles()) {
       wtf_size_t size = event_->GetEventPath().size();
       for (wtf_size_t i = 1; i < size; ++i) {
-        event_->GetEventPath()[i].GetNode().WillCallDefaultEventHandler(
-            *event_);
         event_->GetEventPath()[i].GetNode().DefaultEventHandler(*event_);
         DCHECK(!event_->defaultPrevented());
         if (event_->DefaultHandled())

third_party/blink/renderer/core/dom/node.cc

@@ -2883,15 +2883,8 @@ void Node::DefaultEventHandler(Event& event) {
   }
 }
 
-void Node::WillCallDefaultEventHandler(const Event& event) {
-  if (!event.IsKeyboardEvent())
-    return;
-
-  if (!IsFocused() || GetDocument().LastFocusType() != kWebFocusTypeMouse)
-    return;
-
-  if (event.type() != event_type_names::kKeydown ||
-      GetDocument().HadKeyboardEvent())
+void Node::UpdateHadKeyboardEvent(const Event& event) {
+  if (GetDocument().HadKeyboardEvent())
     return;
 
   GetDocument().SetHadKeyboardEvent(true);
@@ -3005,7 +2998,7 @@ HTMLSlotElement* Node::assignedSlotForBinding() {
 }
 
 void Node::SetFocused(bool flag, WebFocusType focus_type) {
-  if (!flag || focus_type == kWebFocusTypeMouse)
+  if (focus_type == kWebFocusTypeMouse)
     GetDocument().SetHadKeyboardEvent(false);
   GetDocument().UserActionElements().SetFocused(this, flag);
 }

third_party/blink/renderer/core/dom/node.h

@@ -839,7 +839,7 @@ class CORE_EXPORT Node : public EventTarget {
 
   // Perform the default action for an event.
   virtual void DefaultEventHandler(Event&);
-  void WillCallDefaultEventHandler(const Event&);
+  void UpdateHadKeyboardEvent(const Event&);
   // Should return true if this Node has activation behavior.
   // https://dom.spec.whatwg.org/#eventtarget-activation-behavior
   virtual bool HasActivationBehavior() const;

third_party/blink/renderer/core/html/forms/html_form_control_element.cc

@@ -25,6 +25,7 @@
 #include "third_party/blink/renderer/core/html/forms/html_form_control_element.h"
 
 #include "third_party/blink/renderer/core/accessibility/ax_object_cache.h"
+#include "third_party/blink/renderer/core/css/selector_checker.h"
 #include "third_party/blink/renderer/core/dom/element_traversal.h"
 #include "third_party/blink/renderer/core/dom/events/event.h"
 #include "third_party/blink/renderer/core/dom/events/event_dispatch_forbidden_scope.h"
@@ -296,8 +297,7 @@ bool HTMLFormControlElement::MayTriggerVirtualKeyboard() const {
 }
 
 bool HTMLFormControlElement::ShouldHaveFocusAppearance() const {
-  return (GetDocument().LastFocusType() != kWebFocusTypeMouse) ||
-         GetDocument().HadKeyboardEvent() || MayTriggerVirtualKeyboard();
+  return SelectorChecker::MatchesFocusVisiblePseudoClass(*this);
 }
 
 int HTMLFormControlElement::tabIndex() const {

third_party/blink/web_tests/external/wpt/css/selectors/focus-visible-011.html

+<!DOCTYPE html>
+<html>
+<head>
+  <meta charset="utf-8" />
+  <title>CSS Test (Selectors): :focus-visible matches even if preventDefault() is called</title>
+  <link rel="author" title="Alice Boxhall" href="aboxhall@chromium.org" />
+  <link rel="help" href="https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo" />
+  <script src="/resources/testharness.js"></script>
+  <script src="/resources/testharnessreport.js"></script>
+  <script src="/resources/testdriver.js"></script>
+  <script src="/resources/testdriver-vendor.js"></script>
+  <style>
+    button {
+      border: 0;
+    }
+
+    #next:focus-visible {
+      outline: darkgreen auto 5px;
+    }
+
+    #next:focus:not(:focus-visible) {
+      background-color: tomato;
+      outline: 0;
+    }
+  </style>
+</head>
+<body>
+  This test checks that <code>:focus-visible</code> matches after a keyboard event,
+  even if the event handler calls preventDefault() on the event.
+  <ul id="instructions">
+    <li>Click "Click here and press right arrow.".</li>
+    <li>Press the right arrow key.</li>
+    <li>If "Focus moves here." has a red background, then the test result is FAILURE.
+        If it has a green outline, then the test result is SUCCESS.</li>
+  </ul>
+  <br />
+  <button id="start" tabindex="0">Click here and press right arrow.</button>
+  <button id="next" tabindex="-1">Focus moves here.</button>
+  <script>
+    start.addEventListener('keydown', (e) => {
+      next.focus();
+    });
+
+    async_test(async function(t) {
+      next.addEventListener("focus", t.step_func(() => {
+        assert_equals(getComputedStyle(next).outlineColor, "rgb(0, 100, 0)");
+        t.done()
+      }));
+
+      // \ue014 -> ARROW_RIGHT
+      test_driver.send_keys(start, "\ue014").catch(t.step_func(() => {
+        assert_true(false, "send_keys not implemented yet");
+        t.done();
+      }));
+    }, ":focus-visible matches even if preventDefault() is called");
+  </script>
+</body>
+</html>

third_party/blink/web_tests/resources/testdriver-vendor.js

@@ -95,6 +95,14 @@
         eventSenderKeys = "Tab";
       } else if (charCode == 0xE050) {
         eventSenderKeys = "ShiftRight";
+      } else if (charCode == 0xE012) {
+        eventSenderKeys = "ArrowLeft";
+      } else if (charCode == 0xE013) {
+        eventSenderKeys = "ArrowUp";
+      } else if (charCode == 0xE014) {
+        eventSenderKeys = "ArrowRight";
+      } else if (charCode == 0xE015) {
+        eventSenderKeys = "ArrowDown";
       } else if (charCode >= 0xE000 && charCode <= 0xF8FF) {
         reject(new Error("No support for this code: U+" + charCode.toString(16)));
       }