Turn on the reverse scrolling gesture feature.

This CL enable the feature of reverse scrolling gestures.

 - Enable reverse scrolling gesture feature.
 - Get rid of the notification for overview gestures.
 - Add toast for the gestures entering overview when the natural
 scrolling is OFF.

Bug: 1107183
Change-Id: Ibbabca1b5deb5b37c8b697ff732f5312519e3adc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2510350Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Xiaodan Zhu <zxdan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824219}

ash/ash_prefs.cc

@@ -37,7 +37,6 @@
 #include "ash/touch/touch_devices_controller.h"
 #include "ash/wallpaper/wallpaper_controller_impl.h"
 #include "ash/wm/desks/desks_restore_util.h"
-#include "ash/wm/gestures/wm_gesture_handler.h"
 #include "chromeos/components/quick_answers/public/cpp/quick_answers_prefs.h"
 #include "chromeos/constants/chromeos_pref_names.h"
 #include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
@@ -75,7 +74,6 @@ void RegisterProfilePrefs(PrefRegistrySimple* registry, bool for_test) {
   ShelfController::RegisterProfilePrefs(registry);
   TouchDevicesController::RegisterProfilePrefs(registry, for_test);
   tray::VPNListView::RegisterProfilePrefs(registry);
-  WmGestureHandler::RegisterProfilePrefs(registry);
   MediaTray::RegisterProfilePrefs(registry);
 
   // Provide prefs registered in the browser for ash_unittests.

ash/ash_strings.grd

@@ -520,11 +520,11 @@ This file contains the strings for ash.
       <message name="IDS_GESTURE_NOTIFICATION_MESSAGE_LEARN_MORE" desc="The body of the notification to show off new gestures. This notification body links to the help page for the gestures.">
         Use gestures to quickly switch between apps and interact with your Chromebook in tablet mode.
       </message>
-      <message name="IDS_OVERVIEW_REVERSE_GESTURE_NOTIFICATION_TITLE" desc="The title of the notification to change reverse gesture when entering/exiting overview mode">
-        The Overview touchpad gesture has changed
+      <message name="IDS_CHANGE_ENTER_OVERVIEW_REVERSE_GESTURE" desc="The toast of changing enter overview gesture">
+        The touchpad gesture to enter Overview has changed. Swipe up with three fingers to enter Overview.
       </message>
-      <message name="IDS_OVERVIEW_REVERSE_GESTURE_NOTIFICATION_MESSAGE" desc="The body of the notification to change reverse gesture when entering/exiting overview mode">
-        Swipe up with three fingers to open and down to close the Overview.
+      <message name="IDS_CHANGE_EXIT_OVERVIEW_REVERSE_GESTURE" desc="The toast of changing exit overview gesture">
+        The touchpad gesture to exit Overview has changed. Swipe down with three fingers to exit Overview.
       </message>
       <message name="IDS_CHANGE_NEXT_DESK_REVERSE_GESTURE" desc="The toast of changing reverse gesture to switch to the next desk.">
         The touchpad gesture to switch to the next desk has changed. Swipe right with four fingers to switch to the next desk.

ash/ash_strings_grd/IDS_CHANGE_ENTER_OVERVIEW_REVERSE_GESTURE.png.sha1

+33e60d7fae900329ccf2d68cfd7a84138a10b515
\ No newline at end of file

ash/ash_strings_grd/IDS_CHANGE_EXIT_OVERVIEW_REVERSE_GESTURE.png.sha1

+89bcdf911a81ba210c539d5ad983c6b8208ff9a9
\ No newline at end of file

ash/ash_strings_grd/IDS_OVERVIEW_REVERSE_GESTURE_NOTIFICATION_MESSAGE.png.sha1

-20bc6189a45f4cd148cfa3bae5f41ed4b9223588
\ No newline at end of file

ash/ash_strings_grd/IDS_OVERVIEW_REVERSE_GESTURE_NOTIFICATION_TITLE.png.sha1

-20bc6189a45f4cd148cfa3bae5f41ed4b9223588
\ No newline at end of file

ash/public/cpp/ash_features.cc

@@ -118,7 +118,7 @@ const base::Feature kHideShelfControlsInTabletMode{
     "HideShelfControlsInTabletMode", base::FEATURE_ENABLED_BY_DEFAULT};
 
 const base::Feature kReverseScrollGestures{"EnableReverseScrollGestures",
-                                           base::FEATURE_DISABLED_BY_DEFAULT};
+                                           base::FEATURE_ENABLED_BY_DEFAULT};
 
 const base::Feature kSystemTrayMicGainSetting{"SystemTrayMicGainSetting",
                                               base::FEATURE_ENABLED_BY_DEFAULT};

ash/public/cpp/ash_pref_names.cc

@@ -579,11 +579,6 @@ const char kColorModeThemed[] = "cros.system.color_mode_themed";
 const char kAppNotificationBadgingEnabled[] =
     "ash.app_notification_badging_enabled";
 
-// An integer pref that counts how many times the reverse gesture notification
-// shows.
-const char kReverseGestureNotificationCount[] =
-    "ash.wm.reverse_gesture_notification_count";
-
 // An integer pref that indicates whether global media controls is pinned to
 // shelf or it's unset and need to be determined by screen size during runtime.
 const char kGlobalMediaControlsPinned[] =

ash/public/cpp/ash_pref_names.h

@@ -209,8 +209,6 @@ ASH_PUBLIC_EXPORT extern const char kColorModeThemed[];
 
 ASH_PUBLIC_EXPORT extern const char kAppNotificationBadgingEnabled[];
 
-ASH_PUBLIC_EXPORT extern const char kReverseGestureNotificationCount[];
-
 ASH_PUBLIC_EXPORT extern const char kGlobalMediaControlsPinned[];
 
 }  // namespace prefs

ash/wm/gestures/wm_gesture_handler.cc

@@ -6,7 +6,6 @@
 
 #include "ash/public/cpp/ash_features.h"
 #include "ash/public/cpp/ash_pref_names.h"
-#include "ash/public/cpp/notification_utils.h"
 #include "ash/public/cpp/toast_data.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
@@ -32,10 +31,8 @@ namespace ash {
 
 namespace {
 
-constexpr char kOverviewGestureNotificationId[] =
-    "ash.wm.reverse_overview_gesture";
-constexpr int kReverseGestureNotificationShowLimit = 3;
-
+constexpr char kEnterOverviewToastId[] = "ash.wm.reverse_enter_overview_toast";
+constexpr char kExitOverviewToastId[] = "ash.wm.reverse_exit_overview_toast";
 constexpr char kSwitchNextDeskToastId[] = "ash.wm.reverse_next_desk_toast";
 constexpr char kSwitchLastDeskToastId[] = "ash.wm.reverse_last_desk_toast";
 
@@ -43,8 +40,10 @@ constexpr base::TimeDelta kToastDurationMs =
     base::TimeDelta::FromMilliseconds(2500);
 
 // Check if the user used the wrong gestures.
-bool gDidWrongNextDeskGesture = false;
-bool gDidWrongLastDeskGesture = false;
+bool g_did_wrong_enter_overview_gesture = false;
+bool g_did_wrong_exit_overview_gesture = false;
+bool g_did_wrong_next_desk_gesture = false;
+bool g_did_wrong_last_desk_gesture = false;
 
 // Is the reverse scrolling for touchpad on.
 bool IsNaturalScrollOn() {
@@ -71,50 +70,42 @@ float GetOffset(float offset) {
   return IsNaturalScrollOn() ? -offset : offset;
 }
 
-void ShowOverviewGestureNotification() {
-  int reverse_gesture_notification_count =
-      Shell::Get()->session_controller()->GetActivePrefService()->GetInteger(
-          prefs::kReverseGestureNotificationCount);
+void ShowReverseGestureToast(const char* toast_id, int message_id) {
+  Shell::Get()->toast_manager()->Show(
+      ToastData(toast_id, l10n_util::GetStringUTF16(message_id),
+                kToastDurationMs.InMilliseconds(), base::nullopt));
+}
 
-  if (reverse_gesture_notification_count >=
-      kReverseGestureNotificationShowLimit) {
-    return;
+// When reverse scrolling for touchpad is Off, if the user performs wrong
+// vertical gestures (i.e., swiping down/up with three fingers to enter/exit
+// overview), a toast will show up to tell user the correct gesture. The toast
+// will be removed after M89.
+bool MaybeHandleWrongVerticalGesture(float offset_y, bool in_overview) {
+  const bool correct_gesture = in_overview ? (offset_y < 0) : (offset_y > 0);
+
+  if (!features::IsReverseScrollGesturesEnabled() || IsNaturalScrollOn())
+    return correct_gesture;
+
+  bool* const did_wrong_ptr = in_overview ? &g_did_wrong_exit_overview_gesture
+                                          : &g_did_wrong_enter_overview_gesture;
+  const char* toast_id =
+      in_overview ? kExitOverviewToastId : kEnterOverviewToastId;
+
+  if (correct_gesture) {
+    *did_wrong_ptr = false;
+    Shell::Get()->toast_manager()->Cancel(toast_id);
+    return true;
   }
 
-  base::string16 title = l10n_util::GetStringUTF16(
-      IDS_OVERVIEW_REVERSE_GESTURE_NOTIFICATION_TITLE);
-  base::string16 message = l10n_util::GetStringUTF16(
-      IDS_OVERVIEW_REVERSE_GESTURE_NOTIFICATION_MESSAGE);
-
-  std::unique_ptr<message_center::Notification> notification =
-      CreateSystemNotification(
-          message_center::NOTIFICATION_TYPE_SIMPLE,
-          kOverviewGestureNotificationId, title, message, base::string16(),
-          GURL(),
-          message_center::NotifierId(
-              message_center::NotifierType::SYSTEM_COMPONENT,
-              kOverviewGestureNotificationId),
-          message_center::RichNotificationData(), nullptr, gfx::VectorIcon(),
-          message_center::SystemNotificationWarningLevel::NORMAL);
-
-  // Make the notification popup again if it has been in message center.
-  if (message_center::MessageCenter::Get()->FindVisibleNotificationById(
-          kOverviewGestureNotificationId)) {
-    message_center::MessageCenter::Get()->RemoveNotification(
-        kOverviewGestureNotificationId, false);
+  if (*did_wrong_ptr) {
+    ShowReverseGestureToast(
+        toast_id, in_overview ? IDS_CHANGE_EXIT_OVERVIEW_REVERSE_GESTURE
+                              : IDS_CHANGE_ENTER_OVERVIEW_REVERSE_GESTURE);
+  } else {
+    *did_wrong_ptr = true;
   }
-  message_center::MessageCenter::Get()->AddNotification(
-      std::move(notification));
-
-  Shell::Get()->session_controller()->GetActivePrefService()->SetInteger(
-      prefs::kReverseGestureNotificationCount,
-      reverse_gesture_notification_count + 1);
-}
 
-void ShowReverseGestureToast(const char* toast_id, int message_id) {
-  Shell::Get()->toast_manager()->Show(
-      ToastData(toast_id, l10n_util::GetStringUTF16(message_id),
-                kToastDurationMs.InMilliseconds(), base::nullopt));
+  return false;
 }
 
 // The amount the fingers must move in a direction before a continuous gesture
@@ -131,34 +122,16 @@ 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 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 (!features::IsReverseScrollGesturesEnabled() || IsNaturalScrollOn())
-        return false;
 
-      ShowOverviewGestureNotification();
-    }
+  if (!MaybeHandleWrongVerticalGesture(GetOffset(scroll_y), in_overview))
+    return false;
 
+  if (in_overview) {
     base::RecordAction(base::UserMetricsAction("Touchpad_Gesture_Overview"));
     if (overview_controller->AcceptSelection())
       return true;
     overview_controller->EndOverview();
   } else {
-    // 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 (!features::IsReverseScrollGesturesEnabled() || IsNaturalScrollOn())
-        return false;
-
-      ShowOverviewGestureNotification();
-    }
-
     auto* window_cycle_controller = Shell::Get()->window_cycle_controller();
     if (window_cycle_controller->IsCycling())
       window_cycle_controller->CancelCycling();
@@ -170,42 +143,62 @@ bool Handle3FingerVerticalScroll(float scroll_y) {
   return true;
 }
 
+// When reverse scrolling for touchpad is Off, if the user performs wrong
+// horizontal gestures (i.e., swiping left/right with four fingers to switch to
+// the next/previous desk), a toast will show up to tell user the correct
+// gesture. The toast will be removed after M89.
+void MaybeHandleWrongHorizontalGesture(bool move_left,
+                                       const Desk* previous_desk,
+                                       const Desk* next_desk) {
+  if (!features::IsReverseScrollGesturesEnabled() || IsNaturalScrollOn())
+    return;
+
+  // Perform wrong gesture on the first desk.
+  if (move_left && next_desk && !previous_desk) {
+    if (!g_did_wrong_next_desk_gesture) {
+      g_did_wrong_next_desk_gesture = true;
+    } else {
+      ShowReverseGestureToast(kSwitchNextDeskToastId,
+                              IDS_CHANGE_NEXT_DESK_REVERSE_GESTURE);
+    }
+    return;
+  }
+
+  // Perform wrong gesture on the last desk.
+  if (!move_left && !next_desk && previous_desk) {
+    if (!g_did_wrong_last_desk_gesture) {
+      g_did_wrong_last_desk_gesture = true;
+    } else {
+      ShowReverseGestureToast(kSwitchLastDeskToastId,
+                              IDS_CHANGE_LAST_DESK_REVERSE_GESTURE);
+    }
+    return;
+  }
+
+  g_did_wrong_next_desk_gesture = false;
+  g_did_wrong_last_desk_gesture = false;
+
+  auto* toast_manager = Shell::Get()->toast_manager();
+  toast_manager->Cancel(kSwitchNextDeskToastId);
+  toast_manager->Cancel(kSwitchLastDeskToastId);
+}
+
 // Handles horizontal 4-finger scroll by switching desks if possible.
 // Returns true if the gesture was handled.
 bool HandleDesksSwitchHorizontalScroll(float scroll_x) {
   if (std::fabs(scroll_x) < WmGestureHandler::kHorizontalThresholdDp)
     return false;
 
-  if (features::IsReverseScrollGesturesEnabled() && !IsNaturalScrollOn()) {
-    if (GetOffset(scroll_x) > 0 && !DesksController::Get()->GetNextDesk() &&
-        DesksController::Get()->GetPreviousDesk()) {
-      if (!gDidWrongLastDeskGesture) {
-        gDidWrongLastDeskGesture = true;
-      } else {
-        ShowReverseGestureToast(kSwitchLastDeskToastId,
-                                IDS_CHANGE_LAST_DESK_REVERSE_GESTURE);
-      }
-    } else if (GetOffset(scroll_x) < 0 &&
-               !DesksController::Get()->GetPreviousDesk() &&
-               DesksController::Get()->GetNextDesk()) {
-      if (!gDidWrongNextDeskGesture) {
-        gDidWrongNextDeskGesture = true;
-      } else {
-        ShowReverseGestureToast(kSwitchNextDeskToastId,
-                                IDS_CHANGE_NEXT_DESK_REVERSE_GESTURE);
-      }
-    } else {
-      gDidWrongNextDeskGesture = false;
-      gDidWrongLastDeskGesture = false;
-      Shell::Get()->toast_manager()->Cancel(kSwitchNextDeskToastId);
-      Shell::Get()->toast_manager()->Cancel(kSwitchLastDeskToastId);
-    }
-  }
+  auto* desks_controller = DesksController::Get();
+  const bool move_left = GetOffset(scroll_x) < 0;
+
+  MaybeHandleWrongHorizontalGesture(move_left,
+                                    desks_controller->GetPreviousDesk(),
+                                    desks_controller->GetNextDesk());
 
   // If touchpad reverse scroll is on, the swipe direction will invert.
-  return DesksController::Get()->ActivateAdjacentDesk(
-      /*going_left=*/GetOffset(scroll_x) < 0,
-      DesksSwitchSource::kDeskSwitchTouchpad);
+  return desks_controller->ActivateAdjacentDesk(
+      move_left, DesksSwitchSource::kDeskSwitchTouchpad);
 }
 
 }  // namespace
@@ -316,11 +309,6 @@ bool WmGestureHandler::ProcessEventImpl(int finger_count,
   return moved;
 }
 
-// static
-void WmGestureHandler::RegisterProfilePrefs(PrefRegistrySimple* registry) {
-  registry->RegisterIntegerPref(prefs::kReverseGestureNotificationCount, 0);
-}
-
 bool WmGestureHandler::EndScroll() {
   if (!scroll_data_)
     return false;

ash/wm/gestures/wm_gesture_handler.h

@@ -55,8 +55,6 @@ class ASH_EXPORT WmGestureHandler {
   // otherwise.
   bool ProcessScrollEvent(const ui::ScrollEvent& event);
 
-  static void RegisterProfilePrefs(PrefRegistrySimple* registry);
-
  private:
   // A struct containing the relevant data during a scroll session.
   struct ScrollData {

ash/wm/gestures/wm_gesture_handler_unittest.cc

@@ -33,9 +33,6 @@ constexpr int kNumFingersForWindowCycle = 2;
 constexpr int kNumFingersForHighlight = 3;
 constexpr int kNumFingersForDesksSwitch = 4;
 
-constexpr char kOverviewGestureNotificationId[] =
-    "ash.wm.reverse_overview_gesture";
-
 bool InOverviewSession() {
   return Shell::Get()->overview_controller()->InOverviewSession();
 }
@@ -51,18 +48,6 @@ bool IsNaturalScrollOn() {
          pref->GetBoolean(prefs::kNaturalScroll);
 }
 
-bool IsOverviewReverseGestureNotificationShown() {
-  return message_center::MessageCenter::Get()->FindVisibleNotificationById(
-      kOverviewGestureNotificationId);
-}
-
-void CloseOverviewReverseGestureNotification() {
-  if (IsOverviewReverseGestureNotificationShown()) {
-    message_center::MessageCenter::Get()->RemoveNotification(
-        kOverviewGestureNotificationId, true);
-  }
-}
-
 int GetOffsetX(int offset) {
   // The handler code uses the new directions which is the reverse of the old
   // handler code. Reverse the offset if the ReverseScrollGestures feature is
@@ -141,32 +126,17 @@ TEST_F(WmGestureHandlerTest, WrongVerticalScrolls) {
 
   const float long_scroll = 2 * WmGestureHandler::kVerticalThresholdDp;
 
-  // Swiping down can enter overview but a notification will be shown.
+  // Swiping down cannot enter overview.
   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 triggers the notification for the last time.
-  Scroll(0, -long_scroll, 3);
+  // Enter overview.
+  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.
+  // Swiping up cannot exit overview.
   Scroll(0, long_scroll, 3);
-  EXPECT_FALSE(InOverviewSession());
-  // No notification will be shown anymore.
-  EXPECT_FALSE(IsOverviewReverseGestureNotificationShown());
+  EXPECT_TRUE(InOverviewSession());
 }
 
 // Tests three or four finger horizontal scroll gesture (depending on flags) to