Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2022)

Unified Diff: cc/trees/layer_tree_host_impl_unittest.cc

Issue 2842553003: Call SAC::DidScrollUpdate only for compositor-triggered scrolls. (Closed)
Patch Set: rebase Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: cc/trees/layer_tree_host_impl_unittest.cc
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index 15d79a976344f89f323a0adb985343030b042cbf..7b3cba39e14741dc3abc2191506b1eba44ba6555 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -2919,8 +2919,10 @@ class LayerTreeHostImplTestScrollbarAnimation : public LayerTreeHostImplTest {
LayerImpl* root = host_impl_->active_tree()->InnerViewportContainerLayer();
scrollbar->SetScrollElementId(scroll->element_id());
root->test_properties()->AddChild(std::move(scrollbar));
+ scroll->set_needs_show_scrollbars(true);
host_impl_->active_tree()->BuildPropertyTreesForTesting();
host_impl_->active_tree()->DidBecomeActive();
+ host_impl_->active_tree()->ShowScrollbars();
DrawFrame();
// SetScrollElementId will initialize the scrollbar which will cause it to
@@ -3056,8 +3058,8 @@ class LayerTreeHostImplTestScrollbarAnimation : public LayerTreeHostImplTest {
host_impl_->DidFinishImplFrame();
}
- // Setting the scroll offset outside a scroll should also cause the
- // scrollbar to appear and to schedule a scrollbar animation.
+ // Setting the scroll offset outside a scroll should not cause the
+ // scrollbar to appear or schedule a scrollbar animation.
if (host_impl_->active_tree()
->property_trees()
->scroll_tree.UpdateScrollOffsetBaseForTesting(
@@ -3067,57 +3069,9 @@ class LayerTreeHostImplTestScrollbarAnimation : public LayerTreeHostImplTest {
host_impl_->InnerViewportScrollLayer()->id());
EXPECT_FALSE(did_request_next_frame_);
EXPECT_FALSE(did_request_redraw_);
- if (expecting_animations) {
- EXPECT_EQ(base::TimeDelta::FromMilliseconds(20),
- requested_animation_delay_);
- EXPECT_FALSE(animation_task_.Equals(base::Closure()));
- requested_animation_delay_ = base::TimeDelta();
- animation_task_ = base::Closure();
- } else {
- EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
- EXPECT_TRUE(animation_task_.Equals(base::Closure()));
- }
-
- if (expecting_animations) {
- // Scrolling should have stopped the animation, so we should not be
- // getting redraws.
- begin_frame_args =
- CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 5, fake_now);
- host_impl_->WillBeginImplFrame(begin_frame_args);
- host_impl_->Animate();
- EXPECT_FALSE(did_request_next_frame_);
- did_request_next_frame_ = false;
- EXPECT_FALSE(did_request_redraw_);
- did_request_redraw_ = false;
- host_impl_->DidFinishImplFrame();
- }
-
- // For Andrdoid, scrollbar animation is not triggered unnecessarily.
- // For Aura Overlay Scrollbar, scrollbar appears even if scroll offset did
- // not change.
- host_impl_->ScrollBegin(BeginState(gfx::Point()).get(),
- InputHandler::WHEEL);
- host_impl_->ScrollBy(UpdateState(gfx::Point(), gfx::Vector2dF(5, 0)).get());
- EXPECT_FALSE(did_request_next_frame_);
- EXPECT_TRUE(did_request_redraw_);
- did_request_redraw_ = false;
EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
EXPECT_TRUE(animation_task_.Equals(base::Closure()));
- host_impl_->ScrollEnd(EndState().get());
- EXPECT_FALSE(did_request_next_frame_);
- EXPECT_FALSE(did_request_redraw_);
- if (animator == LayerTreeSettings::AURA_OVERLAY) {
- EXPECT_EQ(base::TimeDelta::FromMilliseconds(20),
- requested_animation_delay_);
- EXPECT_FALSE(animation_task_.Equals(base::Closure()));
- requested_animation_delay_ = base::TimeDelta();
- animation_task_ = base::Closure();
- } else {
- EXPECT_EQ(base::TimeDelta(), requested_animation_delay_);
- EXPECT_TRUE(animation_task_.Equals(base::Closure()));
- }
-
// Changing page scale triggers scrollbar animation.
host_impl_->active_tree()->PushPageScaleFromMainThread(1.f, 1.f, 4.f);
host_impl_->active_tree()->SetPageScaleOnActiveTree(1.1f);
@@ -3267,6 +3221,7 @@ TEST_F(LayerTreeHostImplTest, ScrollbarVisibilityChangeCausesRedrawAndCommit) {
scrollbar->SetScrollElementId(scroll->element_id());
container->test_properties()->AddChild(std::move(scrollbar));
host_impl_->pending_tree()->PushPageScaleFromMainThread(1.f, 1.f, 1.f);
+ scroll->set_needs_show_scrollbars(true);
host_impl_->pending_tree()->BuildPropertyTreesForTesting();
host_impl_->ActivateSyncTree();
@@ -3407,19 +3362,11 @@ TEST_F(LayerTreeHostImplTest, ScrollbarRegistration) {
EXPECT_TRUE(host_impl_->ScrollbarAnimationControllerForElementId(
root_scroll->element_id()));
- // Changing one of the viewport layers should result in a scrollbar animation
- // update.
- animation_task_ = base::Closure();
- host_impl_->active_tree()->InnerViewportContainerLayer()->SetBoundsDelta(
- gfx::Vector2dF(10, 10));
- EXPECT_FALSE(animation_task_.Equals(base::Closure()));
- animation_task_ = base::Closure();
- host_impl_->active_tree()->OuterViewportScrollLayer()->SetCurrentScrollOffset(
- gfx::ScrollOffset(10, 10));
- EXPECT_FALSE(animation_task_.Equals(base::Closure()));
+ // Scrolling the viewport should result in a scrollbar animation update.
animation_task_ = base::Closure();
- host_impl_->active_tree()->InnerViewportScrollLayer()->SetCurrentScrollOffset(
- gfx::ScrollOffset(10, 10));
+ host_impl_->ScrollBegin(BeginState(gfx::Point()).get(), InputHandler::WHEEL);
+ host_impl_->ScrollBy(UpdateState(gfx::Point(), gfx::Vector2d(10, 10)).get());
+ host_impl_->ScrollEnd(EndState().get());
EXPECT_FALSE(animation_task_.Equals(base::Closure()));
animation_task_ = base::Closure();
@@ -3445,9 +3392,8 @@ TEST_F(LayerTreeHostImplTest, ScrollbarRegistration) {
// update.
animation_task_ = base::Closure();
child_clip_ptr->SetBounds(gfx::Size(200, 200));
- EXPECT_FALSE(animation_task_.Equals(base::Closure()));
- animation_task_ = base::Closure();
- child_ptr->SetCurrentScrollOffset(gfx::ScrollOffset(10, 10));
+ child_ptr->set_needs_show_scrollbars(true);
+ host_impl_->active_tree()->ShowScrollbars();
EXPECT_FALSE(animation_task_.Equals(base::Closure()));
animation_task_ = base::Closure();
@@ -12269,6 +12215,7 @@ void LayerTreeHostImplTest::SetupMouseMoveAtTestScrollbarStates(
// Capture scrollbar_1, then move mouse to scrollbar_2's layer, should post an
// event to fade out scrollbar_1.
+ scrollbar_1_animation_controller->DidScrollUpdate();
bokan 2017/04/25 22:56:48 Do you know why this wasn't previously needed?
skobes 2017/04/27 00:46:31 The test was counting on the scrollbar to be made
bokan 2017/04/27 19:38:58 Got it, thanks for the explanation.
animation_task_ = base::Closure();
host_impl_->MouseDown();

Powered by Google App Engine
This is Rietveld 408576698