cros: Filter mouse events for window cycle lists until mouse used.

Currently the initial position of the mouse can trigger the various
mouse event handlers for window cycle items. This causes the focus ring
to be set on unexpected items, despite the user just using their
keyboard.

This CL makes the window cycle event filter track the user's initial
mouse position, filtering mouse events until the user actually moves
their mouse or they click, drag, etc.

Test: manual + added
Bug: 1143275
Change-Id: I6a3b160aa2beb10e32c3b15b2d25e6f70cc5ca2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2529569
Commit-Queue: Jeremy Chinsen <chinsenj@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826967}

ash/wm/window_cycle_controller_unittest.cc

@@ -1176,6 +1176,55 @@ TEST_F(InteractiveWindowCycleControllerTest, TapSelect) {
   EXPECT_TRUE(wm::IsActiveWindow(w2.get()));
 }
 
+// Tests that mouse events are filtered until the mouse is actually used,
+// preventing the mouse from unexpectedly triggering events.
+// See crbug.com/1143275.
+TEST_F(InteractiveWindowCycleControllerTest, FilterMouseEventsUntilUsed) {
+  std::unique_ptr<Window> w0 = CreateTestWindow();
+  std::unique_ptr<Window> w1 = CreateTestWindow();
+  std::unique_ptr<Window> w2 = CreateTestWindow();
+  EventCounter event_count;
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  WindowCycleController* controller = Shell::Get()->window_cycle_controller();
+
+  // Start cycling.
+  // Current window order is [2,1,0].
+  controller->StartCycling();
+  auto item_views = GetWindowCycleItemViews();
+  item_views[2]->AddPreTargetHandler(&event_count);
+
+  // Move the mouse over to the third item and complete cycling. These mouse
+  // events shouldn't be filtered since the user has moved their mouse.
+  generator->MoveMouseTo(gfx::Point(0, 0));
+  const gfx::Point third_item_center =
+      GetWindowCycleItemViews()[2]->GetBoundsInScreen().CenterPoint();
+  generator->MoveMouseTo(third_item_center);
+  controller->CompleteCycling();
+  EXPECT_TRUE(wm::IsActiveWindow(w0.get()));
+  EXPECT_LT(0, event_count.GetMouseEventCountAndReset());
+
+  // Start cycling again while the mouse is over where the third item will be
+  // when cycling starts.
+  // Current window order is [0,2,1].
+  controller->StartCycling();
+  item_views = GetWindowCycleItemViews();
+  item_views[2]->AddPreTargetHandler(&event_count);
+
+  // Generate mouse events at the cursor's initial position. These mouse events
+  // should be filtered because the user hasn't moved their mouse yet.
+  generator->MoveMouseTo(third_item_center);
+  controller->CompleteCycling();
+  EXPECT_TRUE(wm::IsActiveWindow(w0.get()));
+  EXPECT_EQ(0, event_count.GetMouseEventCountAndReset());
+
+  // Start cycling again and click. This should not be filtered out.
+  // Current window order is [0,2,1].
+  controller->StartCycling();
+  generator->PressLeftButton();
+  EXPECT_FALSE(controller->IsCycling());
+  EXPECT_TRUE(wm::IsActiveWindow(w1.get()));
+}
+
 // When a user has the window cycle list open and clicks outside of it, it
 // should cancel cycling.
 TEST_F(InteractiveWindowCycleControllerTest,

ash/wm/window_cycle_event_filter.cc

@@ -5,16 +5,24 @@
 #include "ash/wm/window_cycle_event_filter.h"
 
 #include "ash/accelerators/debug_commands.h"
+#include "ash/display/screen_ash.h"
 #include "ash/public/cpp/ash_features.h"
 #include "ash/shell.h"
 #include "ash/wm/window_cycle_controller.h"
 #include "ash/wm/window_cycle_list.h"
 #include "base/bind.h"
 #include "ui/events/event.h"
+#include "ui/wm/core/coordinate_conversion.h"
 
 namespace ash {
 
-WindowCycleEventFilter::WindowCycleEventFilter() {
+// The distance a user has to move their mouse from |initial_mouse_location_|
+// before this stops filtering mouse events.
+constexpr int kMouseMovementThreshold = 5;
+
+WindowCycleEventFilter::WindowCycleEventFilter()
+    : initial_mouse_location_(
+          display::Screen::GetScreen()->GetCursorScreenPoint()) {
   Shell::Get()->AddPreTargetHandler(this);
   // Handling release of "Alt" must come before other pretarget handlers
   // (specifically, the partial screenshot handler). See crbug.com/651939
@@ -89,6 +97,26 @@ bool WindowCycleEventFilter::ShouldRepeatKey(ui::KeyEvent* event) const {
          !repeat_timer_.IsRunning();
 }
 
+void WindowCycleEventFilter::SetHasUserUsedMouse(ui::MouseEvent* event) {
+  if (event->type() != ui::ET_MOUSE_MOVED &&
+      event->type() != ui::ET_MOUSE_ENTERED &&
+      event->type() != ui::ET_MOUSE_EXITED) {
+    // If a user clicks/drags/scrolls mouse wheel, then they have used the
+    // mouse.
+    has_user_used_mouse_ = true;
+    return;
+  }
+
+  aura::Window* target = static_cast<aura::Window*>(event->target());
+  aura::Window* event_root = target->GetRootWindow();
+  gfx::Point event_screen_point = event->root_location();
+  wm::ConvertPointToScreen(event_root, &event_screen_point);
+  if ((initial_mouse_location_ - event_screen_point).Length() >
+      kMouseMovementThreshold) {
+    has_user_used_mouse_ = true;
+  }
+}
+
 WindowCycleController::Direction WindowCycleEventFilter::GetDirection(
     ui::KeyEvent* event) const {
   DCHECK(IsTriggerKey(event));
@@ -103,13 +131,18 @@ WindowCycleController::Direction WindowCycleEventFilter::GetDirection(
 }
 
 void WindowCycleEventFilter::OnMouseEvent(ui::MouseEvent* event) {
+  if (!has_user_used_mouse_)
+    SetHasUserUsedMouse(event);
+
   if (features::IsInteractiveWindowCycleListEnabled()) {
     WindowCycleController* window_cycle_controller =
         Shell::Get()->window_cycle_controller();
     const bool cycle_list_is_visible =
         window_cycle_controller->IsWindowListVisible();
-    if (window_cycle_controller->IsEventInCycleView(event) ||
-        !cycle_list_is_visible) {
+    const bool event_should_not_be_filtered =
+        has_user_used_mouse_ &&
+        window_cycle_controller->IsEventInCycleView(event);
+    if (event_should_not_be_filtered || !cycle_list_is_visible) {
       return;
     } else if (event->type() == ui::ET_MOUSE_PRESSED && cycle_list_is_visible) {
       // Close the window cycle list if a user clicks outside of it.

ash/wm/window_cycle_event_filter.h

@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "base/timer/timer.h"
 #include "ui/events/event_handler.h"
+#include "ui/gfx/geometry/point.h"
 
 namespace ash {
 
@@ -54,6 +55,10 @@ class ASH_EXPORT WindowCycleEventFilter : public ui::EventHandler {
   // direction given by |event|.
   bool ShouldRepeatKey(ui::KeyEvent* event) const;
 
+  // Given |event|, determine if the user has used their mouse, i.e. moved or
+  // clicked.
+  void SetHasUserUsedMouse(ui::MouseEvent* event);
+
   // Returns the direction the window cycle should cycle depending on the
   // combination of keys being pressed.
   WindowCycleController::Direction GetDirection(ui::KeyEvent* event) const;
@@ -66,6 +71,18 @@ class ASH_EXPORT WindowCycleEventFilter : public ui::EventHandler {
 
   AltReleaseHandler alt_release_handler_;
 
+  // Stores the initial mouse coordinates. Used to determine whether
+  // |has_user_used_mouse_| when this handles mouse events.
+  gfx::Point initial_mouse_location_;
+
+  // Bool for tracking whether the user has used their mouse. If this is false,
+  // mouse events should be filtered. This is to prevent the initial mouse
+  // position from triggering window cycle items' mouse event handlers despite a
+  // user not moving their mouse. Should be set to true when a user moves their
+  // mouse enough or clicks/drags/mousewheel scrolls.
+  // See crbug.com/114375.
+  bool has_user_used_mouse_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(WindowCycleEventFilter);
 };