Show Overview gesture changing when reverse scrolling is off

The overview gesture is changed when the reverse scrolling option
is turned off in M87. The notification should be shown when the
user turns off the reverse scrolling.

Bug: 1107183
Change-Id: I4f1113a815d1594a4428834406471a2fb577d1e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2436949Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Xiaodan Zhu <zxdan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811737}

ash/wm/gestures/wm_gesture_handler.cc

@@ -120,15 +120,15 @@ bool Handle3FingerVerticalScroll(float scroll_y) {
   auto* overview_controller = Shell::Get()->overview_controller();
   const bool in_overview = overview_controller->InOverviewSession();
   if (in_overview) {
-    // If touchpad reverse scroll is off, only swip down can exit overview. If
-    // touchpad reverse scroll is on, in M87 swip up can also exit overview but
+    // If touchpad reverse scroll is on, only swip down can exit overview. If
+    // touchpad reverse scroll is off, in M87 swip up can also exit overview but
     // show notification; in M88, swip up will only show notification; in M89
     // the notification is removed.
     if (GetOffset(scroll_y) > 0) {
-      if (!IsNaturalScrollOn())
+      if (IsNaturalScrollOn())
         return false;
-      else
-        ShowOverviewGestureNotification();
+
+      ShowOverviewGestureNotification();
     }
 
     base::RecordAction(base::UserMetricsAction("Touchpad_Gesture_Overview"));
@@ -136,15 +136,15 @@ bool Handle3FingerVerticalScroll(float scroll_y) {
       return true;
     overview_controller->EndOverview();
   } else {
-    // If touchpad reverse scroll is off, only swip up can enter overview. If
-    // touchpad reverse scroll is on, in M87 swip down can also enter overview
+    // If touchpad reverse scroll is on, only swip up can enter overview. If
+    // touchpad reverse scroll is off, in M87 swip down can also enter overview
     // but show notification; in M88, swip down will only show notification; in
     // M89 the notification is removed.
     if (GetOffset(scroll_y) < 0) {
-      if (!IsNaturalScrollOn())
+      if (IsNaturalScrollOn())
         return false;
-      else
-        ShowOverviewGestureNotification();
+
+      ShowOverviewGestureNotification();
     }
 
     auto* window_cycle_controller = Shell::Get()->window_cycle_controller();

ash/wm/gestures/wm_gesture_handler_unittest.cc

@@ -106,17 +106,41 @@ TEST_F(WmGestureHandlerTest, VerticalScrolls) {
   Scroll(0, long_scroll, 3);
   EXPECT_TRUE(InOverviewSession());
 
-  // Swiping up again does nothing.
-  Scroll(0, long_scroll, 3);
-  EXPECT_TRUE(InOverviewSession());
-
   // Swiping down exits.
   Scroll(0, -long_scroll, 3);
   EXPECT_FALSE(InOverviewSession());
+}
+
+// Tests wrong gestures that swiping down to enter and up to exit overview.
+TEST_F(WmGestureHandlerTest, WrongVerticalScrolls) {
+  const float long_scroll = 2 * WmGestureHandler::kVerticalThresholdDp;
+
+  // Swiping down can enter overview but a notification will be shown.
+  Scroll(0, -long_scroll, 3);
+  EXPECT_TRUE(InOverviewSession());
+  // Notification is shown for the first time.
+  EXPECT_TRUE(IsOverviewReverseGestureNotificationShown());
+  CloseOverviewReverseGestureNotification();
+
+  // Swiping up can exit overview, but a notification will be shown.
+  Scroll(0, long_scroll, 3);
+  EXPECT_FALSE(InOverviewSession());
+  // Notification is shown for the second time.
+  EXPECT_TRUE(IsOverviewReverseGestureNotificationShown());
+  CloseOverviewReverseGestureNotification();
 
-  // Swiping down again does nothing.
+  // Swiping down triggers the notification for the last time.
   Scroll(0, -long_scroll, 3);
+  EXPECT_TRUE(InOverviewSession());
+  // Notification is shown for the third (last) time.
+  EXPECT_TRUE(IsOverviewReverseGestureNotificationShown());
+  CloseOverviewReverseGestureNotification();
+
+  // Doing wrong gesture again won't trigger notification anymore.
+  Scroll(0, long_scroll, 3);
   EXPECT_FALSE(InOverviewSession());
+  // No notification will be shown anymore.
+  EXPECT_FALSE(IsOverviewReverseGestureNotificationShown());
 }
 
 // Tests three or four finger horizontal scroll gesture (depending on flags) to
@@ -421,41 +445,20 @@ class ReverseGestureHandlerTest : public WmGestureHandlerTest {
 TEST_F(ReverseGestureHandlerTest, Overview) {
   const float long_scroll = 2 * WmGestureHandler::kVerticalThresholdDp;
 
-  // Swipe down with three fingers.
-  Scroll(0, -long_scroll, 3);
-  // When keep old overview gesture is on, the old gesture also works.
+  // Use the new gestures.
+  // Swiping up with three fingers enters overview.
+  Scroll(0, long_scroll, 3);
   EXPECT_TRUE(InOverviewSession());
-  // Show notification for the first time.
-  EXPECT_TRUE(IsOverviewReverseGestureNotificationShown());
-  // Close the Notification.
-  CloseOverviewReverseGestureNotification();
 
-  // Swipe up with three fingers.
+  // Swiping up again with three fingers does nothing.
   Scroll(0, long_scroll, 3);
-  EXPECT_FALSE(InOverviewSession());
-  // Show notification for the second time.
-  EXPECT_TRUE(IsOverviewReverseGestureNotificationShown());
-  // Close the Notification.
-  CloseOverviewReverseGestureNotification();
-
-  // Swipe down with three fingers.
-  Scroll(0, -long_scroll, 3);
   EXPECT_TRUE(InOverviewSession());
-  // Show notification for the third time.
-  EXPECT_TRUE(IsOverviewReverseGestureNotificationShown());
-  // Close the Notification.
-  CloseOverviewReverseGestureNotification();
 
-  // Swipe up with three fingers.
-  Scroll(0, long_scroll, 3);
+  // Swiping down with three fingers exits overview.
+  Scroll(0, -long_scroll, 3);
   EXPECT_FALSE(InOverviewSession());
-  // The notification won't show anymore.
-  EXPECT_FALSE(IsOverviewReverseGestureNotificationShown());
 
-  // Use the new gestures.
-  // Swipe up with three fingers.
-  Scroll(0, long_scroll, 3);
-  EXPECT_TRUE(InOverviewSession());
+  // Swiping down again with three fingers does nothing.
   Scroll(0, -long_scroll, 3);
   EXPECT_FALSE(InOverviewSession());
 }