Use scroll-boundary-behavior to control overscroll-refresh/glow on android.

content/browser/android/overscroll_controller_android_unittest.cc

Index: content/browser/android/overscroll_controller_android_unittest.cc
diff --git a/content/browser/android/overscroll_controller_android_unittest.cc b/content/browser/android/overscroll_controller_android_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d4d2459ace3814397cfab10b09a5f8ccfc8bae7c
--- /dev/null
+++ b/content/browser/android/overscroll_controller_android_unittest.cc
@@ -0,0 +1,165 @@
+// Copyright (c) 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/android/overscroll_controller_android.h"
+#include <memory>
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "cc/layers/layer.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/android/overscroll_glow.h"
+#include "ui/android/overscroll_refresh.h"
+#include "ui/android/resources/resource_manager_impl.h"
+#include "ui/android/window_android_compositor.h"
+#include "ui/events/blink/did_overscroll_params.h"
+
+using ui::OverscrollGlow;
+using ui::OverscrollRefresh;
+using ui::WindowAndroidCompositor;
+using cc::CopyOutputRequest;
+using ::testing::_;
+using ::testing::Return;
+
+namespace content {
+

[majidvp, 2017/07/12 21:04:21]

all test logic should be inside an unnamed namespace e.g.,
namespace  {
 /// Test
}

See other tests in this directory for example.

[sunyunjia, 2017/07/19 20:44:11]

Done.

+class MockCompositor : public WindowAndroidCompositor {
+ public:
+  MOCK_METHOD1(AttachLayerForReadback, void(scoped_refptr<cc::Layer>));
+  void RequestCopyOfOutputOnRootLayer(
+      std::unique_ptr<cc::CopyOutputRequest> request) override {}
+  MOCK_METHOD0(SetNeedsAnimate, void());
+  MOCK_METHOD0(GetResourceManager, ui::ResourceManagerImpl&());
+  MOCK_METHOD0(GetFrameSinkId, cc::FrameSinkId());
+  MOCK_METHOD1(AddChildFrameSink, void(const cc::FrameSinkId& frame_sink_id));
+  MOCK_METHOD1(RemoveChildFrameSink,
+               void(const cc::FrameSinkId& frame_sink_id));
+};
+
+class MockGlow : public OverscrollGlow {
+ public:
+  MockGlow() : OverscrollGlow() {}
+  MOCK_METHOD5(OnOverscrolled,
+               bool(base::TimeTicks,
+                    const gfx::Vector2dF&,
+                    gfx::Vector2dF,
+                    gfx::Vector2dF,
+                    const gfx::Vector2dF&));
+};
+
+class MockRefresh : public OverscrollRefresh {
+ public:
+  MockRefresh() : OverscrollRefresh() {}
+  MOCK_METHOD1(OnOverscrolled, void(bool));
+  MOCK_CONST_METHOD0(IsActive, bool());
+  MOCK_CONST_METHOD0(IsAwaitingScrollUpdateAck, bool());
+};
+
+class OverscrollControllerAndroidUnitTest : public testing::Test {
+ public:
+  OverscrollControllerAndroidUnitTest() {
+    compositor = base::MakeUnique<MockCompositor>();
+    glow = base::MakeUnique<MockGlow>();
+    refresh = base::MakeUnique<MockRefresh>();
+    controller = base::MakeUnique<OverscrollControllerAndroid>(
+        glow.get(), refresh.get(), compositor.get(), 560);
+  }
+
+  std::unique_ptr<MockCompositor> compositor;
+  std::unique_ptr<MockGlow> glow;
+  std::unique_ptr<MockRefresh> refresh;
+  std::unique_ptr<OverscrollControllerAndroid> controller;
+};
+
+TEST_F(OverscrollControllerAndroidUnitTest, BasicNavigation) {

[majidvp, 2017/07/12 21:04:20]

The test name needs to be a bit more descriptive in particular
it should mention you are verifying ScrollBoundaryBehaviorTypeAuto

[sunyunjia, 2017/07/19 20:44:11]

Done.

[majidvp, 2017/07/20 13:26:55]

not: the new name still does not convey that it is related to
ScrollBoundaryBehavior.

+  ui::DidOverscrollParams params;
+  params.accumulated_overscroll = gfx::Vector2dF(0, 20);
+  params.latest_overscroll_delta = gfx::Vector2dF(0, 20);
+  params.current_fling_velocity = gfx::Vector2dF(0, 20);
+  params.causal_event_viewport_point = gfx::PointF(100, 100);
+  params.scroll_boundary_behavior.y = cc::ScrollBoundaryBehavior::
+      ScrollBoundaryBehaviorType::kScrollBoundaryBehaviorTypeAuto;
+
+  EXPECT_CALL(*refresh, OnOverscrolled(true));
+  EXPECT_CALL(*refresh, IsActive()).WillOnce(Return(true));
+  EXPECT_CALL(*refresh, IsAwaitingScrollUpdateAck()).Times(0);
+  EXPECT_CALL(*glow, OnOverscrolled(_, _, _, _, _)).Times(0);
+
+  controller->OnOverscrolled(params);
+  testing::Mock::VerifyAndClearExpectations(refresh.get());
+}
+
+TEST_F(OverscrollControllerAndroidUnitTest, ContainPreventsNavigation) {
+  ui::DidOverscrollParams params;
+  params.accumulated_overscroll = gfx::Vector2dF(0, 1);

[majidvp, 2017/07/12 21:04:21]

nit: first test uses |20| but this one uses |1|. I believe any non-zero
value is fine so there is no difference here. So you might as well
use the same value so future reader know it is the same.

The only difference is scroll_boundary_behavior.

Event better to have a helper function that creates a 
simple ui::DidOverscrollParams with default vertical delta and ...
and then you only override the scroll_boundary_behavior. This helps
readability as well.

[sunyunjia, 2017/07/19 20:44:12]

Done.

+  params.latest_overscroll_delta = gfx::Vector2dF(0, 1);
+  params.current_fling_velocity = gfx::Vector2dF(0, 1);
+  params.causal_event_viewport_point = gfx::PointF(100, 100);
+  params.scroll_boundary_behavior.y = cc::ScrollBoundaryBehavior::
+      ScrollBoundaryBehaviorType::kScrollBoundaryBehaviorTypeContain;
+
+  EXPECT_CALL(*refresh, OnOverscrolled(false));
+  EXPECT_CALL(*refresh, IsActive()).WillOnce(Return(false));
+  EXPECT_CALL(*refresh, IsAwaitingScrollUpdateAck()).WillOnce(Return(false));
+  EXPECT_CALL(*glow,
+              OnOverscrolled(_, gfx::Vector2dF(0, 560), gfx::Vector2dF(0, 560),
+                             gfx::Vector2dF(0, 560), _));
+
+  controller->OnOverscrolled(params);
+  testing::Mock::VerifyAndClearExpectations(refresh.get());
+  testing::Mock::VerifyAndClearExpectations(glow.get());
+
+  // Test that the "contain" set on x-axis would not affect navigation.
+  params.scroll_boundary_behavior.y = cc::ScrollBoundaryBehavior::
+      ScrollBoundaryBehaviorType::kScrollBoundaryBehaviorTypeAuto;
+  params.scroll_boundary_behavior.x = cc::ScrollBoundaryBehavior::
+      ScrollBoundaryBehaviorType::kScrollBoundaryBehaviorTypeContain;
+
+  EXPECT_CALL(*refresh, OnOverscrolled(true));
+  EXPECT_CALL(*refresh, IsActive()).WillOnce(Return(true));
+  EXPECT_CALL(*refresh, IsAwaitingScrollUpdateAck()).Times(0);
+  EXPECT_CALL(*glow, OnOverscrolled(_, _, _, _, _)).Times(0);
+
+  controller->OnOverscrolled(params);
+  testing::Mock::VerifyAndClearExpectations(refresh.get());
+  testing::Mock::VerifyAndClearExpectations(glow.get());
+}
+
+TEST_F(OverscrollControllerAndroidUnitTest, NonePreventsNavigationAndGlow) {
+  ui::DidOverscrollParams params;
+  params.accumulated_overscroll = gfx::Vector2dF(0, 1);
+  params.latest_overscroll_delta = gfx::Vector2dF(0, 1);
+  params.current_fling_velocity = gfx::Vector2dF(0, 1);
+  params.causal_event_viewport_point = gfx::PointF(100, 100);
+  params.scroll_boundary_behavior.y = cc::ScrollBoundaryBehavior::
+      ScrollBoundaryBehaviorType::kScrollBoundaryBehaviorTypeNone;
+
+  EXPECT_CALL(*refresh, OnOverscrolled(false));
+  EXPECT_CALL(*refresh, IsActive()).WillOnce(Return(false));
+  EXPECT_CALL(*refresh, IsAwaitingScrollUpdateAck()).WillOnce(Return(false));
+  EXPECT_CALL(*glow, OnOverscrolled(_, gfx::Vector2dF(), gfx::Vector2dF(),
+                                    gfx::Vector2dF(), _));
+
+  controller->OnOverscrolled(params);
+  testing::Mock::VerifyAndClearExpectations(refresh.get());
+  testing::Mock::VerifyAndClearExpectations(glow.get());
+
+  // Test that the "none" set on y-axis would not affect glow on x-axis.
+  params.accumulated_overscroll = gfx::Vector2dF(1, 1);
+  params.latest_overscroll_delta = gfx::Vector2dF(1, 1);
+  params.current_fling_velocity = gfx::Vector2dF(1, 1);
+
+  EXPECT_CALL(*refresh, OnOverscrolled(false));
+  EXPECT_CALL(*refresh, IsActive()).WillOnce(Return(false));
+  EXPECT_CALL(*refresh, IsAwaitingScrollUpdateAck()).WillOnce(Return(false));
+  EXPECT_CALL(*glow,
+              OnOverscrolled(_, gfx::Vector2dF(560, 0), gfx::Vector2dF(560, 0),
+                             gfx::Vector2dF(560, 0), _));
+
+  controller->OnOverscrolled(params);
+  testing::Mock::VerifyAndClearExpectations(refresh.get());
+  testing::Mock::VerifyAndClearExpectations(glow.get());
+}
+
+}  // namespace content