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
TBR: [email protected]
(cherry picked from commit 245022d0f27873e9a51de5be1ba304f83a50acaf)
Change-Id: I726453adc3dfd1848332ee668d11662c44afdb55
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2448070
Commit-Queue: Jinsuk Kim <[email protected]>
Reviewed-by: Theresa <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#814586}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2469416
Reviewed-by: Jinsuk Kim <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#353}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/gesturenav/NavigationHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/gesturenav/NavigationHandler.java
index 5888f0e..23ee6510 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/gesturenav/NavigationHandler.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/gesturenav/NavigationHandler.java
@@ -24,6 +24,7 @@
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 @@
* 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 @@
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
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/gesturenav/NavigationHandlerTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/gesturenav/NavigationHandlerTest.java
index f8ecafe..ed8c78e 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/gesturenav/NavigationHandlerTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/gesturenav/NavigationHandlerTest.java
@@ -47,8 +47,11 @@
* Tests {@link NavigationHandler} navigating back/forward using overscroll history navigation.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
-@CommandLineFlags.
-Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "enable-features=OverscrollHistoryNavigation"})
[email protected]({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 @@
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();
diff --git a/content/browser/android/overscroll_controller_android.cc b/content/browser/android/overscroll_controller_android.cc
index 6decf98..24703c8 100644
--- a/content/browser/android/overscroll_controller_android.cc
+++ b/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 @@
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
diff --git a/ui/android/overscroll_refresh.cc b/ui/android/overscroll_refresh.cc
index 9d7a443..311774e 100644
--- a/ui/android/overscroll_refresh.cc
+++ b/ui/android/overscroll_refresh.cc
@@ -17,10 +17,6 @@
// 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 @@
} // 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 @@
: 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() {
diff --git a/ui/android/overscroll_refresh.h b/ui/android/overscroll_refresh.h
index b650d46..57c9c7a 100644
--- a/ui/android/overscroll_refresh.h
+++ b/ui/android/overscroll_refresh.h
@@ -36,12 +36,11 @@
// 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 @@
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_;
diff --git a/ui/android/overscroll_refresh_unittest.cc b/ui/android/overscroll_refresh_unittest.cc
index df9184f..2253156 100644
--- a/ui/android/overscroll_refresh_unittest.cc
+++ b/ui/android/overscroll_refresh_unittest.cc
@@ -13,6 +13,8 @@
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 @@
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 @@
};
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, 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, 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, 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, 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, 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, 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, NotTriggeredIfReset) {
- OverscrollRefresh effect(this, kDipScale);
+ OverscrollRefresh effect(this, kDefaultEdgeWidth);
effect.OnScrollBegin(kStartPos);
ASSERT_FALSE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 10)));
ASSERT_TRUE(effect.IsAwaitingScrollUpdateAck());
