cros: Refactor window cycle list logic out of WmGestureHandler.

Currently the WmGestureHandler handles events for the wm and also the
window cycle list. There is also the WindowCycleEventFilter which
handles some events for the window cycle list. This was implemented
like this to share logic for scrolling, but after further investigation
the common logic has been determined to not warrant having the handling
for the window cycle list to live in two separate event handlers.

This CL refactors the WmGestureHandler, moving all window cycle list
logic to the WindowCycleEventFilter.

A followup CL will refactor the WindowCycleEventFilter to reorder fcns
according to order in its header file.

Test: manual + existing tests.
Bug: 1136554
Change-Id: I055d0533169992ad1022bdd14da9ce89d91f7e5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2606494Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Jeremy Chinsen <chinsenj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842584}

ash/wm/gestures/wm_gesture_handler.cc

@@ -53,13 +53,6 @@ bool IsNaturalScrollOn() {
          pref->GetBoolean(prefs::kNaturalScroll);
 }
 
-// Is reverse scrolling for mouse wheel on.
-bool IsReverseScrollOn() {
-  PrefService* pref =
-      Shell::Get()->session_controller()->GetActivePrefService();
-  return pref->GetBoolean(prefs::kMouseReverseScroll);
-}
-
 // Reverse an offset when the reverse scrolling is on.
 float GetOffset(float offset) {
   // The handler code uses the new directions which is the reverse of the old
@@ -204,24 +197,6 @@ WmGestureHandler::WmGestureHandler()
 
 WmGestureHandler::~WmGestureHandler() = default;
 
-bool WmGestureHandler::ProcessWheelEvent(const ui::MouseEvent& event) {
-  if (event.IsMouseWheelEvent() &&
-      Shell::Get()->window_cycle_controller()->IsCycling()) {
-    if (!scroll_data_)
-      scroll_data_ = ScrollData();
-
-    // Convert mouse wheel events into three-finger scrolls for window cycle
-    // list and also swap y offset with x offset.
-    return ProcessEventImpl(
-        /*finger_count=*/3,
-        IsReverseScrollOn() ? event.AsMouseWheelEvent()->y_offset()
-                            : -event.AsMouseWheelEvent()->y_offset(),
-        event.AsMouseWheelEvent()->x_offset());
-  }
-
-  return false;
-}
-
 bool WmGestureHandler::ProcessScrollEvent(const ui::ScrollEvent& event) {
   // ET_SCROLL_FLING_CANCEL means a touchpad swipe has started.
   if (event.type() == ui::ET_SCROLL_FLING_CANCEL) {
@@ -248,8 +223,8 @@ bool WmGestureHandler::ProcessEventImpl(int finger_count,
   if (!scroll_data_)
     return false;
 
-  // Only two, three or four finger scrolls are supported.
-  if (finger_count != 2 && finger_count != 3 && finger_count != 4) {
+  // Only three or four finger scrolls are supported.
+  if (finger_count != 3 && finger_count != 4) {
     scroll_data_.reset();
     return false;
   }
@@ -261,21 +236,12 @@ bool WmGestureHandler::ProcessEventImpl(int finger_count,
     return false;
   }
 
-  if (finger_count == 2 && !IsNaturalScrollOn()) {
-    // Two finger swipe from left to right should move the list right regardless
-    // of natural scroll settings.
-    delta_x = -delta_x;
-  }
-
   scroll_data_->scroll_x += delta_x;
   scroll_data_->scroll_y += delta_y;
 
-  // If the requirements to cycle the window cycle list or  move the overview
-  // selector are met, reset |scroll_data_|. If both are open, cycle the window
-  // cycle list.
-  const bool moved = CycleWindowCycleList(finger_count, scroll_data_->scroll_x,
-                                          scroll_data_->scroll_y) ||
-                     MoveOverviewSelection(finger_count, scroll_data_->scroll_x,
+  // If the requirements to move the overview selector are met, reset
+  // |scroll_data_|.
+  const bool moved = MoveOverviewSelection(finger_count, scroll_data_->scroll_x,
                                            scroll_data_->scroll_y);
 
   if (is_enhanced_desk_animations_ && finger_count == 4) {
@@ -358,25 +324,6 @@ bool WmGestureHandler::MoveOverviewSelection(int finger_count,
   return true;
 }
 
-bool WmGestureHandler::CycleWindowCycleList(int finger_count,
-                                            float scroll_x,
-                                            float scroll_y) {
-  if (!features::IsInteractiveWindowCycleListEnabled() ||
-      (finger_count != 2 && finger_count != 3)) {
-    return false;
-  }
-
-  auto* window_cycle_controller = Shell::Get()->window_cycle_controller();
-  const bool is_cycling = window_cycle_controller->IsCycling();
-  if (!ShouldHorizontallyScroll(is_cycling, scroll_x, scroll_y))
-    return false;
-
-  window_cycle_controller->HandleCycleWindow(
-      scroll_x > 0 ? WindowCycleController::FORWARD
-                   : WindowCycleController::BACKWARD);
-  return true;
-}
-
 bool WmGestureHandler::ShouldHorizontallyScroll(bool in_session,
                                                 float scroll_x,
                                                 float scroll_y) {

ash/wm/gestures/wm_gesture_handler.h

@@ -10,28 +10,15 @@
 #include "components/prefs/pref_registry_simple.h"
 
 namespace ui {
-class MouseEvent;
 class ScrollEvent;
 }
 
 namespace ash {
 
-// TODO(chinsenj): Consider renaming this to WmEventHandler and moving to parent
-// directory since this now handles mouse wheel events.
-
 // This handles the following interactions:
 //   - 3-finger touchpad scroll events to enter/exit overview mode and move the
 //   overview highlight if it is visible.
 //   - 4-finger horizontal scrolls to switch desks.
-//
-// This handles the following interactions if the InteractiveWindowCycleList
-// flag is enabled. TODO(chinsenj): Merge these comments when the flag is
-// removed.
-//   - 3-finger horizontal touchpad scroll events to cycle the window cycle
-//   list.
-//   - 2-finger horizontal touchpad scroll events to cycle the window cycle
-//   list.
-//   - Mouse wheel events to cycle the window cycle list.
 class ASH_EXPORT WmGestureHandler {
  public:
   // The thresholds of performing a wm action with a touchpad three or four
@@ -49,15 +36,9 @@ class ASH_EXPORT WmGestureHandler {
   WmGestureHandler& operator=(const WmGestureHandler&) = delete;
   virtual ~WmGestureHandler();
 
-  // Processes a mouse wheel event and may cycle the window cycle list. Returns
-  // true if the event has been handled and should not be processed further,
-  // false otherwise.
-  bool ProcessWheelEvent(const ui::MouseEvent& event);
-
-  // Processes a scroll event and may switch desks, start overview, move the
-  // overview highlight or cycle the window cycle list. Returns true if
-  // the event has been handled and should not be processed further, false
-  // otherwise.
+  // Processes a scroll event and may switch desks, start overview or move the
+  // overview highlight. Returns true if the event has been handled and should
+  // not be processed further, false otherwise.
   bool ProcessScrollEvent(const ui::ScrollEvent& event);
 
  private:
@@ -75,9 +56,8 @@ class ASH_EXPORT WmGestureHandler {
     bool continuous_gesture_started = false;
   };
 
-  // Called by ProcessWheelEvent() and ProcessScrollEvent(). Depending on
-  // |finger_count|, may switch desks, start overview, move the overview
-  // highlight or cycle the window cycle list. Returns true if the
+  // Called by ProcessScrollEvent(). Depending on |finger_count|, may switch
+  // desks, start overview or move the overview highlight. Returns true if the
   // event has been handled and should not be processed further, false
   // otherwise. Forwards events to DesksController if
   // |is_enhanced_desk_animations_| is true.
@@ -90,12 +70,8 @@ class ASH_EXPORT WmGestureHandler {
   // the middle of scrolls and when scrolls have ended.
   bool MoveOverviewSelection(int finger_count, float scroll_x, float scroll_y);
 
-  // Tries to cycle the window cycle list. Returns true if successful.
-  // Called in the middle of scrolls and when scrolls have ended.
-  bool CycleWindowCycleList(int finger_count, float scroll_x, float scroll_y);
-
-  // Returns whether or not a given session of overview/window cycle list should
-  // horizontally scroll.
+  // Returns whether or not a given session of overview should horizontally
+  // scroll.
   bool ShouldHorizontallyScroll(bool in_session,
                                 float scroll_x,
                                 float scroll_y);

ash/wm/system_gesture_event_filter.cc

@@ -24,11 +24,6 @@ void SystemGestureEventFilter::OnMouseEvent(ui::MouseEvent* event) {
           ui::TouchScreensAvailability::ENABLED) {
     base::RecordAction(base::UserMetricsAction("Mouse_Down"));
   }
-
-  if (event->IsMouseWheelEvent() && wm_gesture_handler_ &&
-      wm_gesture_handler_->ProcessWheelEvent(*event)) {
-    event->StopPropagation();
-  }
 }
 
 void SystemGestureEventFilter::OnScrollEvent(ui::ScrollEvent* event) {

ash/wm/window_cycle/window_cycle_event_filter.cc

@@ -7,11 +7,15 @@
 #include "ash/accelerators/debug_commands.h"
 #include "ash/display/screen_ash.h"
 #include "ash/public/cpp/ash_features.h"
+#include "ash/public/cpp/ash_pref_names.h"
+#include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
 #include "ash/wm/window_cycle/window_cycle_controller.h"
 #include "ash/wm/window_cycle/window_cycle_list.h"
 #include "base/bind.h"
+#include "components/prefs/pref_service.h"
 #include "ui/events/event.h"
+#include "ui/events/types/event_type.h"
 #include "ui/wm/core/coordinate_conversion.h"
 
 namespace ash {
@@ -20,6 +24,21 @@ namespace ash {
 // before this stops filtering mouse events.
 constexpr int kMouseMovementThreshold = 5;
 
+// Is the reverse scrolling for touchpad on.
+bool IsNaturalScrollOn() {
+  PrefService* pref =
+      Shell::Get()->session_controller()->GetActivePrefService();
+  return pref->GetBoolean(prefs::kTouchpadEnabled) &&
+         pref->GetBoolean(prefs::kNaturalScroll);
+}
+
+// Is reverse scrolling for mouse wheel on.
+bool IsReverseScrollOn() {
+  PrefService* pref =
+      Shell::Get()->session_controller()->GetActivePrefService();
+  return pref->GetBoolean(prefs::kMouseReverseScroll);
+}
+
 WindowCycleEventFilter::WindowCycleEventFilter()
     : initial_mouse_location_(
           display::Screen::GetScreen()->GetCursorScreenPoint()) {
@@ -117,6 +136,87 @@ void WindowCycleEventFilter::SetHasUserUsedMouse(ui::MouseEvent* event) {
   }
 }
 
+void WindowCycleEventFilter::ProcessMouseEvent(ui::MouseEvent* event) {
+  auto* window_cycle_controller = Shell::Get()->window_cycle_controller();
+  if (event->type() == ui::ET_MOUSE_PRESSED &&
+      !window_cycle_controller->IsEventInCycleView(event)) {
+    // Close the window cycle list if a user clicks outside of it.
+    window_cycle_controller->CancelCycling();
+    return;
+  }
+
+  if (event->IsMouseWheelEvent()) {
+    if (!scroll_data_)
+      scroll_data_ = ScrollData();
+    const ui::MouseWheelEvent* wheel_event = event->AsMouseWheelEvent();
+    const float y_offset = wheel_event->y_offset();
+    // Convert mouse wheel events into three-finger scrolls for window cycle
+    // list and also swap y offset with x offset.
+    if (ProcessEventImpl(/*finger_count=*/3,
+                         IsReverseScrollOn() ? y_offset : -y_offset,
+                         wheel_event->x_offset())) {
+      event->SetHandled();
+      event->StopPropagation();
+    }
+  }
+}
+
+bool WindowCycleEventFilter::ProcessEventImpl(int finger_count,
+                                              float delta_x,
+                                              float delta_y) {
+  if (!scroll_data_ || !features::IsInteractiveWindowCycleListEnabled())
+    return false;
+
+  if (finger_count != 2 && finger_count != 3) {
+    scroll_data_.reset();
+    return false;
+  }
+
+  if (scroll_data_->finger_count != 0 &&
+      scroll_data_->finger_count != finger_count) {
+    scroll_data_.reset();
+    return false;
+  }
+
+  if (finger_count == 2 && !IsNaturalScrollOn()) {
+    // Two finger swipe from left to right should move the list right regardless
+    // of natural scroll settings.
+    delta_x = -delta_x;
+  }
+
+  scroll_data_->scroll_x += delta_x;
+  scroll_data_->scroll_y += delta_y;
+
+  const bool moved = CycleWindowCycleList(finger_count, scroll_data_->scroll_x,
+                                          scroll_data_->scroll_y);
+
+  if (moved)
+    scroll_data_ = ScrollData();
+  scroll_data_->finger_count = finger_count;
+  return moved;
+}
+
+bool WindowCycleEventFilter::CycleWindowCycleList(int finger_count,
+                                                  float scroll_x,
+                                                  float scroll_y) {
+  if (!features::IsInteractiveWindowCycleListEnabled() ||
+      (finger_count != 2 && finger_count != 3)) {
+    return false;
+  }
+
+  auto* window_cycle_controller = Shell::Get()->window_cycle_controller();
+  if (!window_cycle_controller->IsCycling() ||
+      std::fabs(scroll_x) < std::fabs(scroll_y) ||
+      std::fabs(scroll_x) < kHorizontalThresholdDp) {
+    return false;
+  }
+
+  window_cycle_controller->HandleCycleWindow(
+      scroll_x > 0 ? WindowCycleController::FORWARD
+                   : WindowCycleController::BACKWARD);
+  return true;
+}
+
 WindowCycleController::Direction WindowCycleEventFilter::GetDirection(
     ui::KeyEvent* event) const {
   DCHECK(IsTriggerKey(event));
@@ -134,19 +234,17 @@ void WindowCycleEventFilter::OnMouseEvent(ui::MouseEvent* event) {
   if (!has_user_used_mouse_)
     SetHasUserUsedMouse(event);
 
-  if (features::IsInteractiveWindowCycleListEnabled()) {
+  if (features::IsInteractiveWindowCycleListEnabled() && has_user_used_mouse_) {
     WindowCycleController* window_cycle_controller =
         Shell::Get()->window_cycle_controller();
     const bool cycle_list_is_visible =
         window_cycle_controller->IsWindowListVisible();
-    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) {
+    if (cycle_list_is_visible)
+      ProcessMouseEvent(event);
+
+    if (window_cycle_controller->IsEventInCycleView(event) ||
+        !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.
-      window_cycle_controller->CancelCycling();
     }
   }
 
@@ -159,6 +257,28 @@ void WindowCycleEventFilter::OnMouseEvent(ui::MouseEvent* event) {
   }
 }
 
+void WindowCycleEventFilter::OnScrollEvent(ui::ScrollEvent* event) {
+  // ET_SCROLL_FLING_CANCEL means a touchpad swipe has started.
+  if (event->type() == ui::ET_SCROLL_FLING_CANCEL) {
+    scroll_data_ = ScrollData();
+    return;
+  }
+
+  // ET_SCROLL_FLING_START means a touchpad swipe has ended.
+  if (event->type() == ui::ET_SCROLL_FLING_START) {
+    scroll_data_.reset();
+    return;
+  }
+
+  DCHECK_EQ(ui::ET_SCROLL, event->type());
+
+  if (ProcessEventImpl(event->finger_count(), event->x_offset(),
+                       event->y_offset())) {
+    event->SetHandled();
+    event->StopPropagation();
+  }
+}
+
 void WindowCycleEventFilter::OnGestureEvent(ui::GestureEvent* event) {
   if (features::IsInteractiveWindowCycleListEnabled() &&
       Shell::Get()->window_cycle_controller()->IsEventInCycleView(event)) {

ash/wm/window_cycle/window_cycle_event_filter.h

@@ -8,10 +8,18 @@
 #include "ash/ash_export.h"
 #include "ash/wm/window_cycle/window_cycle_controller.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/timer/timer.h"
 #include "ui/events/event_handler.h"
 #include "ui/gfx/geometry/point.h"
 
+namespace ui {
+class GestureEvent;
+class KeyEvent;
+class MouseEvent;
+class ScrollEvent;
+}  // namespace ui
+
 namespace ash {
 
 // Created by WindowCycleController when cycling through windows. Eats all key
@@ -19,15 +27,30 @@ namespace ash {
 // Also allows users to cycle using right/left keys.
 class ASH_EXPORT WindowCycleEventFilter : public ui::EventHandler {
  public:
+  // The threshold of performing an action with a touchpad or mouse wheel
+  // scroll.
+  static constexpr float kHorizontalThresholdDp = 330.f;
+
   WindowCycleEventFilter();
   ~WindowCycleEventFilter() override;
 
   // Overridden from ui::EventHandler:
   void OnKeyEvent(ui::KeyEvent* event) override;
   void OnMouseEvent(ui::MouseEvent* event) override;
+  void OnScrollEvent(ui::ScrollEvent* event) override;
   void OnGestureEvent(ui::GestureEvent* event) override;
 
  private:
+  // A struct containing the relevant data during a scroll session.
+  struct ScrollData {
+    int finger_count = 0;
+
+    // Values are cumulative (ex. |scroll_x| is the total x distance moved
+    // since the scroll began.
+    float scroll_x = 0.f;
+    float scroll_y = 0.f;
+  };
+
   class AltReleaseHandler : public ui::EventHandler {
    public:
     AltReleaseHandler();
@@ -59,6 +82,20 @@ class ASH_EXPORT WindowCycleEventFilter : public ui::EventHandler {
   // clicked.
   void SetHasUserUsedMouse(ui::MouseEvent* event);
 
+  // Depending on the properties of |event|, may cycle the window cycle list or
+  // complete cycling.
+  void ProcessMouseEvent(ui::MouseEvent* event);
+
+  // Called by ProcessMouseEvent() and OnScrollEvent(). May cycle the window
+  // cycle list. Returns true if the event has been handled and should not be
+  // processed further, false otherwise.
+  bool ProcessEventImpl(int finger_count, float delta_x, float delta_y);
+
+  // Based on the given scroll data, determine whether we should cycle the
+  // window cycle list. Return true if we do cycle the window cycle list,
+  // otherwise return false.
+  bool CycleWindowCycleList(int finger_count, float scroll_x, float scroll_y);
+
   // Returns the direction the window cycle should cycle depending on the
   // combination of keys being pressed.
   WindowCycleController::Direction GetDirection(ui::KeyEvent* event) const;
@@ -83,6 +120,10 @@ class ASH_EXPORT WindowCycleEventFilter : public ui::EventHandler {
   // See crbug.com/114375.
   bool has_user_used_mouse_ = false;
 
+  // Stores the current scroll session data. If it does not exist, there is no
+  // active scroll session.
+  base::Optional<ScrollData> scroll_data_;
+
   DISALLOW_COPY_AND_ASSIGN(WindowCycleEventFilter);
 };