GestureNav: Adjust edge width for triggering navigation

This CL reduces the width of the area used for triggering
gesture navigation. The reported conflicts with other UI
can be alleviated.

Bug: 1128629
Change-Id: I726453adc3dfd1848332ee668d11662c44afdb55
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2448070
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814586}

chrome/android/java/src/org/chromium/chrome/browser/gesturenav/NavigationHandler.java

@@ -24,6 +24,7 @@ import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.browser.compositor.CompositorViewHolder.TouchEventObserver;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.gesturenav.NavigationBubble.CloseTarget;
 import org.chromium.ui.modelutil.PropertyModel;
 
@@ -34,10 +35,13 @@ import java.lang.annotation.RetentionPolicy;
  * Handles history overscroll navigation controlling the underlying UI widget.
  */
 class NavigationHandler implements TouchEventObserver {
-    // Width of a rectangluar area in dp on the left/right edge used for navigation.
+    // Default width of a rectangluar area in dp on the left/right edge used for navigation.
     // Swipe beginning from a point within these rects triggers the operation.
     @VisibleForTesting
-    static final float EDGE_WIDTH_DP = 48;
+    static final int DEFAULT_EDGE_WIDTH_DP = 24;
+
+    // Key name of the server-controlled parameter for the edge width.
+    private static final String EDGE_WIDTH_KEY = "gesture_navigation_triggering_area_width";
 
     // Weighted value to determine when to trigger an edge swipe. Initial scroll
     // vector should form 30 deg or below to initiate swipe action.
@@ -145,7 +149,12 @@ class NavigationHandler implements TouchEventObserver {
         mIsNativePage = isNativePage;
         mIsSheetExpanded = isSheetExpanded;
         mState = GestureState.NONE;
-        mEdgeWidthPx = EDGE_WIDTH_DP * parentView.getResources().getDisplayMetrics().density;
+
+        int edgeWidthDp = ChromeFeatureList.getFieldTrialParamByFeatureAsInt(
+                ChromeFeatureList.OVERSCROLL_HISTORY_NAVIGATION, EDGE_WIDTH_KEY,
+                DEFAULT_EDGE_WIDTH_DP);
+
+        mEdgeWidthPx = edgeWidthDp * parentView.getResources().getDisplayMetrics().density;
         mDetector = new GestureDetector(mContext, new SideNavGestureListener());
         mAttachStateListener = new View.OnAttachStateChangeListener() {
             @Override

chrome/android/javatests/src/org/chromium/chrome/browser/gesturenav/NavigationHandlerTest.java

@@ -47,8 +47,11 @@ import org.chromium.ui.test.util.UiRestriction;
  * Tests {@link NavigationHandler} navigating back/forward using overscroll history navigation.
  */
 @RunWith(ChromeJUnit4ClassRunner.class)
-@CommandLineFlags.
-Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "enable-features=OverscrollHistoryNavigation"})
+@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
+        "enable-features=OverscrollHistoryNavigation<GestureNavigation",
+        "force-fieldtrials=GestureNavigation/Enabled",
+        "force-fieldtrial-params=GestureNavigation.Enabled:"
+                + "gesture_navigation_triggering_area_width/36"})
 public class NavigationHandlerTest {
     private static final String RENDERED_PAGE = "/chrome/test/data/android/navigate/simple.html";
     private static final boolean LEFT_EDGE = true;
@@ -70,7 +73,7 @@ public class NavigationHandlerTest {
         DisplayMetrics displayMetrics = new DisplayMetrics();
         mActivityTestRule.getActivity().getWindowManager().getDefaultDisplay().getMetrics(
                 displayMetrics);
-        mEdgeWidthPx = displayMetrics.density * NavigationHandler.EDGE_WIDTH_DP;
+        mEdgeWidthPx = displayMetrics.density * NavigationHandler.DEFAULT_EDGE_WIDTH_DP;
         HistoryNavigationCoordinator coordinator = getNavigationCoordinator();
         mNavigationLayout = coordinator.getLayoutForTesting();
         mNavigationHandler = coordinator.getNavigationHandlerForTesting();

content/browser/android/overscroll_controller_android.cc

@@ -5,9 +5,11 @@
 #include "content/browser/android/overscroll_controller_android.h"
 
 #include "base/command_line.h"
+#include "base/metrics/field_trial_params.h"
 #include "cc/layers/layer.h"
 #include "components/viz/common/quads/compositor_frame_metadata.h"
 #include "content/public/browser/navigation_controller.h"
+#include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/use_zoom_for_dsf_policy.h"
 #include "third_party/blink/public/common/input/web_gesture_event.h"
@@ -58,8 +60,13 @@ std::unique_ptr<OverscrollRefresh> CreateRefreshEffect(
     return nullptr;
   }
 
+  float edge_width = base::GetFieldTrialParamByFeatureAsInt(
+                         features::kOverscrollHistoryNavigation,
+                         "gesture_navigation_triggering_area_width",
+                         OverscrollRefresh::kDefaultNavigationEdgeWidth) *
+                     dpi_scale;
   return std::make_unique<OverscrollRefresh>(overscroll_refresh_handler,
-                                             dpi_scale);
+                                             edge_width);
 }
 
 }  // namespace

ui/android/overscroll_refresh.cc

@@ -17,10 +17,6 @@ namespace {
 // release results in a small upward fling (quite common during a slow scroll).
 const float kMinFlingVelocityForActivation = -500.f;
 
-// The default distance in dp from a side of the device to start a navigation
-// from.
-const float kNavigationEdgeWidth = 48.f;
-
 // Weighted value used to determine whether a scroll should trigger vertical
 // scroll or horizontal navigation.
 const float kWeightAngle30 = 1.73f;
@@ -28,12 +24,12 @@ const float kWeightAngle30 = 1.73f;
 }  // namespace
 
 OverscrollRefresh::OverscrollRefresh(OverscrollRefreshHandler* handler,
-                                     float dpi_scale)
+                                     float edge_width)
     : scrolled_to_top_(true),
       top_at_scroll_start_(true),
       overflow_y_hidden_(false),
       scroll_consumption_state_(DISABLED),
-      edge_width_(kNavigationEdgeWidth * dpi_scale),
+      edge_width_(edge_width),
       handler_(handler) {
   DCHECK(handler);
 }
@@ -42,7 +38,7 @@ OverscrollRefresh::OverscrollRefresh()
     : scrolled_to_top_(true),
       overflow_y_hidden_(false),
       scroll_consumption_state_(DISABLED),
-      edge_width_(kNavigationEdgeWidth * 1.f),
+      edge_width_(kDefaultNavigationEdgeWidth * 1.f),
       handler_(nullptr) {}
 
 OverscrollRefresh::~OverscrollRefresh() {

ui/android/overscroll_refresh.h

@@ -36,12 +36,11 @@ class OverscrollRefreshHandler;
 // provided refresh handler.
 class UI_ANDROID_EXPORT OverscrollRefresh {
  public:
-  // Minmum number of overscrolling pull events required to activate the effect.
-  // Useful for avoiding accidental triggering when a scroll janks (is delayed),
-  // capping the impulse per event.
-  enum { kMinPullsToActivate = 3 };
+  // The default distance in dp from a side of the device to start a navigation
+  // from.
+  enum { kDefaultNavigationEdgeWidth = 24 };
 
-  OverscrollRefresh(OverscrollRefreshHandler* handler, float dpi_scale);
+  OverscrollRefresh(OverscrollRefreshHandler* handler, float edge_width);
 
   virtual ~OverscrollRefresh();
 
@@ -105,7 +104,7 @@ class UI_ANDROID_EXPORT OverscrollRefresh {
   float viewport_width_;
   float scroll_begin_x_;
   float scroll_begin_y_;
-  const float edge_width_;
+  const float edge_width_;  // in px
   gfx::Vector2dF cumulative_scroll_;
   OverscrollRefreshHandler* const handler_;
 

ui/android/overscroll_refresh_unittest.cc

@@ -13,6 +13,8 @@ namespace ui {
 
 const float kDipScale = 1.f;
 const gfx::PointF kStartPos(2.f, 2.f);
+const float kDefaultEdgeWidth =
+    OverscrollRefresh::kDefaultNavigationEdgeWidth * kDipScale;
 
 class OverscrollRefreshTest : public OverscrollRefreshHandler,
                               public testing::Test {
@@ -70,7 +72,7 @@ class OverscrollRefreshTest : public OverscrollRefreshHandler,
   void TestOverscrollBehavior(const cc::OverscrollBehavior& ob,
                               const gfx::Vector2dF& scroll_delta,
                               bool started) {
-    OverscrollRefresh effect(this, 1.f);
+    OverscrollRefresh effect(this, kDefaultEdgeWidth);
     effect.OnScrollBegin(kStartPos);
     EXPECT_FALSE(effect.WillHandleScrollUpdate(scroll_delta));
     EXPECT_FALSE(effect.IsActive());
@@ -89,7 +91,7 @@ class OverscrollRefreshTest : public OverscrollRefreshHandler,
 };
 
 TEST_F(OverscrollRefreshTest, Basic) {
-  OverscrollRefresh effect(this, kDipScale);
+  OverscrollRefresh effect(this, kDefaultEdgeWidth);
 
   EXPECT_FALSE(effect.IsActive());
   EXPECT_FALSE(effect.IsAwaitingScrollUpdateAck());
@@ -131,7 +133,7 @@ TEST_F(OverscrollRefreshTest, Basic) {
 }
 
 TEST_F(OverscrollRefreshTest, NotTriggeredIfInitialYOffsetIsNotZero) {
-  OverscrollRefresh effect(this, kDipScale);
+  OverscrollRefresh effect(this, kDefaultEdgeWidth);
 
   // A positive y scroll offset at the start of scroll will prevent activation,
   // even if the subsequent scroll overscrolls upward.
@@ -155,7 +157,7 @@ TEST_F(OverscrollRefreshTest, NotTriggeredIfInitialYOffsetIsNotZero) {
 }
 
 TEST_F(OverscrollRefreshTest, NotTriggeredIfOverflowYHidden) {
-  OverscrollRefresh effect(this, kDipScale);
+  OverscrollRefresh effect(this, kDefaultEdgeWidth);
 
   // overflow-y:hidden at the start of scroll will prevent activation.
   gfx::Vector2dF zero_offset;
@@ -177,7 +179,7 @@ TEST_F(OverscrollRefreshTest, NotTriggeredIfOverflowYHidden) {
 }
 
 TEST_F(OverscrollRefreshTest, NotTriggeredIfInitialScrollDownward) {
-  OverscrollRefresh effect(this, kDipScale);
+  OverscrollRefresh effect(this, kDefaultEdgeWidth);
   effect.OnScrollBegin(kStartPos);
 
   // A downward initial scroll will prevent activation, even if the subsequent
@@ -195,7 +197,7 @@ TEST_F(OverscrollRefreshTest, NotTriggeredIfInitialScrollDownward) {
 }
 
 TEST_F(OverscrollRefreshTest, NotTriggeredIfInitialScrollOrTouchConsumed) {
-  OverscrollRefresh effect(this, kDipScale);
+  OverscrollRefresh effect(this, kDefaultEdgeWidth);
   effect.OnScrollBegin(kStartPos);
   ASSERT_FALSE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 10)));
   ASSERT_TRUE(effect.IsAwaitingScrollUpdateAck());
@@ -217,7 +219,7 @@ TEST_F(OverscrollRefreshTest, NotTriggeredIfInitialScrollOrTouchConsumed) {
 }
 
 TEST_F(OverscrollRefreshTest, NotTriggeredIfFlungDownward) {
-  OverscrollRefresh effect(this, kDipScale);
+  OverscrollRefresh effect(this, kDefaultEdgeWidth);
   effect.OnScrollBegin(kStartPos);
   ASSERT_FALSE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 10)));
   ASSERT_TRUE(effect.IsAwaitingScrollUpdateAck());
@@ -232,7 +234,7 @@ TEST_F(OverscrollRefreshTest, NotTriggeredIfFlungDownward) {
 }
 
 TEST_F(OverscrollRefreshTest, NotTriggeredIfReleasedWithoutActivation) {
-  OverscrollRefresh effect(this, kDipScale);
+  OverscrollRefresh effect(this, kDefaultEdgeWidth);
   effect.OnScrollBegin(kStartPos);
   ASSERT_FALSE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 10)));
   ASSERT_TRUE(effect.IsAwaitingScrollUpdateAck());
@@ -248,7 +250,7 @@ TEST_F(OverscrollRefreshTest, NotTriggeredIfReleasedWithoutActivation) {
 }
 
 TEST_F(OverscrollRefreshTest, NotTriggeredIfReset) {
-  OverscrollRefresh effect(this, kDipScale);
+  OverscrollRefresh effect(this, kDefaultEdgeWidth);
   effect.OnScrollBegin(kStartPos);
   ASSERT_FALSE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 10)));
   ASSERT_TRUE(effect.IsAwaitingScrollUpdateAck());