[Android] Suppress pull-to-refresh when overscroll glow visible
Avoid accidental refreshes by suppressing the effect when the overscroll
glow animation is still visible. This naturally prevents the case where
the user repeatedly flings/scrolls to the top, releasing before they
realize that the scroll has turned into a pull-to-refresh.
Note that suppression is only provided on L; the glow effect on previous
platforms is both too long and too visually faint to provide a good feedback
signal for when the user can start pulling to refresh.
BUG=456300
Review URL: https://codereview.chromium.org/879273007
Cr-Commit-Position: refs/heads/master@{#315832}
(cherry picked from commit 2dbb7e87a8cd907e4d782cff5212dc6f61626e7d)
Review URL: https://codereview.chromium.org/935833003
Cr-Commit-Position: refs/branch-heads/2272@{#319}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}
diff --git a/content/browser/android/overscroll_controller_android.cc b/content/browser/android/overscroll_controller_android.cc
index 76a6c473..68c2006 100644
--- a/content/browser/android/overscroll_controller_android.cc
+++ b/content/browser/android/overscroll_controller_android.cc
@@ -30,13 +30,27 @@
// action will be activated.
const int kDefaultRefreshDragTargetDips = 64;
+bool IsAndroidLOrNewer() {
+ static bool android_l_or_newer =
+ base::android::BuildInfo::GetInstance()->sdk_int() >= kAndroidLSDKVersion;
+ return android_l_or_newer;
+}
+
+bool ShouldDisableRefreshWhenGlowActive() {
+ // Suppressing refresh detection when the glow is still animating prevents
+ // visual confusion and accidental activation after repeated scrolls. However,
+ // only the L effect is guaranteed to be both sufficiently brief and prominent
+ // to provide a meaningful "wait" signal. The refresh effect on previous
+ // Android releases can be quite faint, depending on the OEM-supplied
+ // overscroll resources, and lasts nearly twice as long.
+ return IsAndroidLOrNewer();
+}
+
scoped_ptr<EdgeEffectBase> CreateGlowEdgeEffect(
ui::ResourceManager* resource_manager,
float dpi_scale) {
DCHECK(resource_manager);
- static bool use_l_flavoured_effect =
- base::android::BuildInfo::GetInstance()->sdk_int() >= kAndroidLSDKVersion;
- if (use_l_flavoured_effect)
+ if (IsAndroidLOrNewer())
return scoped_ptr<EdgeEffectBase>(new EdgeEffectL(resource_manager));
return scoped_ptr<EdgeEffectBase>(
@@ -95,10 +109,18 @@
if (!enabled_)
return false;
- // Suppress refresh detection for fullscreen web apps.
- if (!refresh_effect_ || is_fullscreen_)
+ if (!refresh_effect_)
return false;
+ // Suppress refresh detection for fullscreen HTML5 scenarios, e.g., video.
+ if (is_fullscreen_)
+ return false;
+
+ if (glow_effect_) {
+ if (glow_effect_->IsActive() && ShouldDisableRefreshWhenGlowActive())
+ return false;
+ }
+
bool handled = false;
bool maybe_needs_animate = false;
switch (event.type) {
diff --git a/content/browser/android/overscroll_glow.cc b/content/browser/android/overscroll_glow.cc
index 8985b6a..4928d9f 100644
--- a/content/browser/android/overscroll_glow.cc
+++ b/content/browser/android/overscroll_glow.cc
@@ -92,6 +92,16 @@
edge_effects_[i]->Finish();
}
+bool OverscrollGlow::IsActive() const {
+ if (!initialized_)
+ return false;
+ for (size_t i = 0; i < EDGE_COUNT; ++i) {
+ if (!edge_effects_[i]->IsFinished())
+ return true;
+ }
+ return false;
+}
+
bool OverscrollGlow::OnOverscrolled(base::TimeTicks current_time,
gfx::Vector2dF accumulated_overscroll,
gfx::Vector2dF overscroll_delta,
@@ -168,10 +178,8 @@
return false;
}
- for (size_t i = 0; i < EDGE_COUNT; ++i) {
- if (!edge_effects_[i]->IsFinished())
- return true;
- }
+ if (IsActive())
+ return true;
Detach();
return false;
diff --git a/content/browser/android/overscroll_glow.h b/content/browser/android/overscroll_glow.h
index 88e016e1..3c763e8 100644
--- a/content/browser/android/overscroll_glow.h
+++ b/content/browser/android/overscroll_glow.h
@@ -66,6 +66,9 @@
// Reset the effect to its inactive state, clearing any active effects.
void Reset();
+ // Whether the effect is active, either being pulled or receding.
+ bool IsActive() const;
+
private:
enum Axis { AXIS_X, AXIS_Y };
diff --git a/content/browser/android/overscroll_refresh.cc b/content/browser/android/overscroll_refresh.cc
index 7631d6cd..13b2329 100644
--- a/content/browser/android/overscroll_refresh.cc
+++ b/content/browser/android/overscroll_refresh.cc
@@ -44,10 +44,6 @@
// Input threshold required to start glowing.
const float kGlowActivationThreshold = 0.85f;
-// Useful for avoiding accidental triggering when a scroll janks (is delayed),
-// capping the impulse per event.
-const int kMinPullsToActivate = 4;
-
// Minimum alpha for the effect layer.
const float kMinAlpha = 0.3f;
@@ -148,7 +144,7 @@
state_ = STATE_PULL;
delta *= kDragRate;
- float max_delta = target_drag_ / kMinPullsToActivate;
+ float max_delta = target_drag_ / OverscrollRefresh::kMinPullsToActivate;
delta = Clamp(delta, -max_delta, max_delta);
drag_ += delta;
diff --git a/content/browser/android/overscroll_refresh.h b/content/browser/android/overscroll_refresh.h
index 074b3ade..fb325f8 100644
--- a/content/browser/android/overscroll_refresh.h
+++ b/content/browser/android/overscroll_refresh.h
@@ -44,6 +44,11 @@
// and beyond a particular threshold when released.
class CONTENT_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 };
+
// Both |resource_manager| and |client| must not be null.
// |target_drag_offset_pixels| is the threshold beyond which the effect
// will trigger a refresh action when released. When |mirror| is true,
diff --git a/content/browser/android/overscroll_refresh_unittest.cc b/content/browser/android/overscroll_refresh_unittest.cc
index 66ad4c7..449c2ad8 100644
--- a/content/browser/android/overscroll_refresh_unittest.cc
+++ b/content/browser/android/overscroll_refresh_unittest.cc
@@ -48,6 +48,11 @@
return triggered;
}
+ void PullBeyondActivationThreshold(OverscrollRefresh* effect) {
+ for (int i = 0; i < OverscrollRefresh::kMinPullsToActivate; ++i)
+ EXPECT_TRUE(effect->WillHandleScrollUpdate(gfx::Vector2dF(0, 100)));
+ }
+
protected:
void SignalRefreshCompleted() { still_refreshing_ = false; }
@@ -88,12 +93,8 @@
EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, -50)));
EXPECT_TRUE(effect.IsActive());
- // Feed enough scrolls to the effect to exceeds tht threshold.
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 100)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 100)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 100)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 100)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 100)));
+ // Feed enough scrolls to the effect to exceeds the threshold.
+ PullBeyondActivationThreshold(&effect);
EXPECT_TRUE(effect.IsActive());
// Ending the scroll while beyond the threshold should trigger a refresh.
@@ -127,12 +128,8 @@
ASSERT_TRUE(effect.IsAwaitingScrollUpdateAck());
effect.OnScrollUpdateAck(false);
ASSERT_TRUE(effect.IsActive());
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
+ PullBeyondActivationThreshold(&effect);
+ ASSERT_TRUE(effect.IsActive());
effect.OnScrollEnd(gfx::Vector2dF(0, 0));
ASSERT_TRUE(GetAndResetRefreshTriggered());
@@ -254,12 +251,8 @@
ASSERT_TRUE(effect.IsActive());
// Ensure the pull exceeds the necessary threshold.
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
+ PullBeyondActivationThreshold(&effect);
+ ASSERT_TRUE(effect.IsActive());
// Terminating the pull with a down-directed fling should prevent triggering.
effect.OnScrollEnd(gfx::Vector2dF(0, -1000));
@@ -276,12 +269,8 @@
ASSERT_TRUE(effect.IsActive());
// Ensure the pull exceeds the necessary threshold.
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
- EXPECT_TRUE(effect.WillHandleScrollUpdate(gfx::Vector2dF(0, 50)));
+ PullBeyondActivationThreshold(&effect);
+ ASSERT_TRUE(effect.IsActive());
// An early release should prevent the refresh action from firing.
effect.ReleaseWithoutActivation();