diff options
author | hush <hush@chromium.org> | 2015-04-06 20:49:50 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-04-07 03:50:22 +0000 |
commit | 33370e19d3c6a43d0cb7c87f90aadb79d44dd741 (patch) | |
tree | fcdedeca49228d7232dc82cf34b7616ecf4d2c62 /cc/layers | |
parent | 4882adfdf046e2a17b4ec3e2457609ef591cdc71 (diff) | |
download | chromium_src-33370e19d3c6a43d0cb7c87f90aadb79d44dd741.zip chromium_src-33370e19d3c6a43d0cb7c87f90aadb79d44dd741.tar.gz chromium_src-33370e19d3c6a43d0cb7c87f90aadb79d44dd741.tar.bz2 |
Refactor delegated scrolling in LayerImpl
When the root layer scroll offset delegate's scroll offset is bigger
than that of the outer viewport scroll layer, the root scroll offset is
distributed between inner and outer viewport scroll layer. However, the
layer that gets assigned a new scroll offset first will immediately
update the root layer delegate scroll offset without waiting for the
rest of the undistributed scrolls to be assigned to other layer. This
causes the associated bug 470360.
This CL simplified the logic around delegated scrolling by removing
LayerImpl::ScrollOffsetDelegate. LayerImpl will always have the most
recent and correct scroll offset.
LTHI and LTI still delegate the root layer scroll to an external source
(Android WebView).
There are 2 cases for a LayerImpl to change its scroll offset:
1. When the root layer scroll delegate scrolls, the new scroll offset will be
synchronously distributed to the inner and outer viewport scroll layer.
2. The LayerImpl scrolls by itself, like ScrollBy() or PushScrollOffset(). In
this case, root layer scroll delegate will be updated with the new scroll
offset.
BUG=470360
Review URL: https://codereview.chromium.org/1038173002
Cr-Commit-Position: refs/heads/master@{#324012}
Diffstat (limited to 'cc/layers')
-rw-r--r-- | cc/layers/layer_impl.cc | 50 | ||||
-rw-r--r-- | cc/layers/layer_impl.h | 20 | ||||
-rw-r--r-- | cc/layers/layer_impl_unittest.cc | 158 |
3 files changed, 20 insertions, 208 deletions
diff --git a/cc/layers/layer_impl.cc b/cc/layers/layer_impl.cc index 3ffeb73..8b37d12 100644 --- a/cc/layers/layer_impl.cc +++ b/cc/layers/layer_impl.cc @@ -16,7 +16,6 @@ #include "cc/debug/layer_tree_debug_state.h" #include "cc/debug/micro_benchmark_impl.h" #include "cc/debug/traced_value.h" -#include "cc/input/layer_scroll_offset_delegate.h" #include "cc/layers/layer_utils.h" #include "cc/layers/painted_scrollbar_layer_impl.h" #include "cc/output/copy_output_request.h" @@ -49,7 +48,6 @@ LayerImpl::LayerImpl(LayerTreeImpl* tree_impl, layer_id_(id), layer_tree_impl_(tree_impl), scroll_offset_(scroll_offset), - scroll_offset_delegate_(nullptr), scroll_clip_layer_(nullptr), should_scroll_on_main_thread_(false), have_wheel_event_handlers_(false), @@ -164,9 +162,6 @@ void LayerImpl::SetScrollParent(LayerImpl* parent) { if (scroll_parent_ == parent) return; - // Having both a scroll parent and a scroll offset delegate is unsupported. - DCHECK(!scroll_offset_delegate_); - if (parent) DCHECK_EQ(layer_tree_impl()->LayerById(parent->id()), parent); @@ -385,8 +380,6 @@ void LayerImpl::GetContentsResourceId(ResourceProvider::ResourceId* resource_id, } gfx::Vector2dF LayerImpl::ScrollBy(const gfx::Vector2dF& scroll) { - RefreshFromScrollDelegate(); - gfx::ScrollOffset adjusted_scroll(scroll); if (layer_tree_impl()->settings().use_pinch_virtual_viewport) { if (!user_scrollable_horizontal_) @@ -1108,25 +1101,21 @@ void LayerImpl::SetContentsScale(float contents_scale_x, NoteLayerPropertyChanged(); } -void LayerImpl::SetScrollOffsetDelegate( - ScrollOffsetDelegate* scroll_offset_delegate) { - // Having both a scroll parent and a scroll offset delegate is unsupported. - DCHECK(!scroll_parent_); - RefreshFromScrollDelegate(); - scroll_offset_delegate_ = scroll_offset_delegate; - if (scroll_offset_delegate_) - scroll_offset_delegate_->SetCurrentScrollOffset(CurrentScrollOffset()); -} - bool LayerImpl::IsExternalFlingActive() const { - return scroll_offset_delegate_ && - scroll_offset_delegate_->IsExternalFlingActive(); + return layer_tree_impl_->IsExternalFlingActive(); } void LayerImpl::SetCurrentScrollOffset(const gfx::ScrollOffset& scroll_offset) { DCHECK(IsActive()); if (scroll_offset_->SetCurrent(scroll_offset)) - DidUpdateScrollOffset(); + DidUpdateScrollOffset(false); +} + +void LayerImpl::SetCurrentScrollOffsetFromDelegate( + const gfx::ScrollOffset& scroll_offset) { + DCHECK(IsActive()); + if (scroll_offset_->SetCurrent(scroll_offset)) + DidUpdateScrollOffset(true); } void LayerImpl::PushScrollOffsetFromMainThread( @@ -1141,8 +1130,6 @@ void LayerImpl::PushScrollOffsetFromMainThreadAndClobberActiveValue( } gfx::ScrollOffset LayerImpl::PullDeltaForMainThread() { - RefreshFromScrollDelegate(); - // TODO(miletus): Remove all this temporary flooring machinery when // Blink fully supports fractional scrolls. gfx::ScrollOffset current_offset = CurrentScrollOffset(); @@ -1158,13 +1145,6 @@ gfx::ScrollOffset LayerImpl::PullDeltaForMainThread() { return delta; } -void LayerImpl::RefreshFromScrollDelegate() { - if (scroll_offset_delegate_) { - SetCurrentScrollOffset( - gfx::ScrollOffset(scroll_offset_delegate_->GetCurrentScrollOffset())); - } -} - gfx::ScrollOffset LayerImpl::CurrentScrollOffset() const { return scroll_offset_->Current(IsActive()); } @@ -1203,16 +1183,12 @@ void LayerImpl::PushScrollOffset(const gfx::ScrollOffset* scroll_offset) { } if (changed) - DidUpdateScrollOffset(); + DidUpdateScrollOffset(false); } -void LayerImpl::DidUpdateScrollOffset() { - if (scroll_offset_delegate_) { - scroll_offset_delegate_->SetCurrentScrollOffset(CurrentScrollOffset()); - scroll_offset_delegate_->Update(); - RefreshFromScrollDelegate(); - } - +void LayerImpl::DidUpdateScrollOffset(bool is_from_root_delegate) { + if (!is_from_root_delegate) + layer_tree_impl()->DidUpdateScrollOffset(id()); NoteLayerPropertyChangedForSubtree(); ScrollbarParametersDidChange(false); } diff --git a/cc/layers/layer_impl.h b/cc/layers/layer_impl.h index b13dce9..2cfa040 100644 --- a/cc/layers/layer_impl.h +++ b/cc/layers/layer_impl.h @@ -80,16 +80,6 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver, public LayerAnimationValueProvider, public AnimationDelegate { public: - // Allows for the ownership of the total scroll offset to be delegated outside - // of the layer. - class ScrollOffsetDelegate { - public: - virtual void SetCurrentScrollOffset(const gfx::ScrollOffset& new_value) = 0; - virtual gfx::ScrollOffset GetCurrentScrollOffset() = 0; - virtual bool IsExternalFlingActive() const = 0; - virtual void Update() const = 0; - }; - typedef SyncedProperty<AdditionGroup<gfx::ScrollOffset>> SyncedScrollOffset; typedef LayerImplList RenderSurfaceListType; typedef LayerImplList LayerListType; @@ -390,11 +380,11 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver, float contents_scale_y() const { return draw_properties_.contents_scale_y; } void SetContentsScale(float contents_scale_x, float contents_scale_y); - void SetScrollOffsetDelegate(ScrollOffsetDelegate* scroll_offset_delegate); - void RefreshFromScrollDelegate(); bool IsExternalFlingActive() const; void SetCurrentScrollOffset(const gfx::ScrollOffset& scroll_offset); + void SetCurrentScrollOffsetFromDelegate( + const gfx::ScrollOffset& scroll_offset); void PushScrollOffsetFromMainThread(const gfx::ScrollOffset& scroll_offset); // This method is similar to PushScrollOffsetFromMainThread but will cause the // scroll offset given to clobber any scroll changes on the active tree in the @@ -636,7 +626,10 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver, private: void PushScrollOffset(const gfx::ScrollOffset* scroll_offset); - void DidUpdateScrollOffset(); + // If the new scroll offset is assigned from the root scroll offset delegate, + // LayerImpl won't inform the root scroll offset delegate about the scroll + // change to avoid feedback. + void DidUpdateScrollOffset(bool is_from_root_delegate); void NoteLayerPropertyChangedForDescendantsInternal(); virtual const char* LayerTypeAsString() const; @@ -672,7 +665,6 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver, // Properties synchronized from the associated Layer. gfx::Point3F transform_origin_; gfx::Size bounds_; - ScrollOffsetDelegate* scroll_offset_delegate_; LayerImpl* scroll_clip_layer_; bool scrollable_ : 1; bool should_scroll_on_main_thread_ : 1; diff --git a/cc/layers/layer_impl_unittest.cc b/cc/layers/layer_impl_unittest.cc index 1cdbd59..d9811bc 100644 --- a/cc/layers/layer_impl_unittest.cc +++ b/cc/layers/layer_impl_unittest.cc @@ -516,112 +516,7 @@ TEST_F(LayerImplScrollTest, ScrollByWithNonZeroOffset) { EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); } -class ScrollDelegateIgnore : public LayerImpl::ScrollOffsetDelegate { - public: - void SetCurrentScrollOffset(const gfx::ScrollOffset& new_value) override { - last_attempted_set_offset_ = new_value; - } - gfx::ScrollOffset last_attempted_set_offset() const { - return last_attempted_set_offset_; - } - - gfx::ScrollOffset GetCurrentScrollOffset() override { - return gfx::ScrollOffset(fixed_offset_); - } - bool IsExternalFlingActive() const override { return false; } - void Update() const override { } - - void set_fixed_offset(const gfx::Vector2dF& fixed_offset) { - fixed_offset_ = fixed_offset; - } - - private: - gfx::ScrollOffset last_attempted_set_offset_; - gfx::Vector2dF fixed_offset_; -}; - -TEST_F(LayerImplScrollTest, ScrollByWithIgnoringDelegate) { - gfx::ScrollOffset scroll_offset(10, 5); - layer()->PushScrollOffsetFromMainThread(scroll_offset); - - EXPECT_VECTOR_EQ(scroll_offset, layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); - EXPECT_VECTOR_EQ(gfx::Vector2dF(), layer()->ScrollDelta()); - - ScrollDelegateIgnore delegate; - gfx::Vector2dF fixed_offset(32, 12); - delegate.set_fixed_offset(fixed_offset); - layer()->SetScrollOffsetDelegate(&delegate); - layer()->RefreshFromScrollDelegate(); - - EXPECT_VECTOR_EQ(fixed_offset, layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); - - layer()->ScrollBy(gfx::Vector2dF(-100, 100)); - - EXPECT_VECTOR_EQ(fixed_offset, layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); - - layer()->SetScrollOffsetDelegate(nullptr); - - EXPECT_VECTOR_EQ(fixed_offset, layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); - - gfx::Vector2dF scroll_delta(1, 1); - layer()->ScrollBy(scroll_delta); - - EXPECT_VECTOR_EQ(fixed_offset + scroll_delta, layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); -} - -class ScrollDelegateAccept : public LayerImpl::ScrollOffsetDelegate { - public: - void SetCurrentScrollOffset(const gfx::ScrollOffset& new_value) override { - current_offset_ = new_value; - } - gfx::ScrollOffset GetCurrentScrollOffset() override { - return current_offset_; - } - bool IsExternalFlingActive() const override { return false; } - void Update() const override { } - - private: - gfx::ScrollOffset current_offset_; -}; - -TEST_F(LayerImplScrollTest, ScrollByWithAcceptingDelegate) { - gfx::ScrollOffset scroll_offset(10, 5); - layer()->PushScrollOffsetFromMainThread(scroll_offset); - - EXPECT_VECTOR_EQ(scroll_offset, layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); - EXPECT_VECTOR_EQ(gfx::Vector2dF(), layer()->ScrollDelta()); - - ScrollDelegateAccept delegate; - layer()->SetScrollOffsetDelegate(&delegate); - - EXPECT_VECTOR_EQ(scroll_offset, layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); - EXPECT_VECTOR_EQ(gfx::Vector2dF(), layer()->ScrollDelta()); - - layer()->ScrollBy(gfx::Vector2dF(-100, 100)); - - EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 80), layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); - - layer()->SetScrollOffsetDelegate(nullptr); - - EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 80), layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); - - gfx::Vector2dF scroll_delta(1, 1); - layer()->ScrollBy(scroll_delta); - - EXPECT_VECTOR_EQ(gfx::Vector2dF(1, 80), layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); -} - -TEST_F(LayerImplScrollTest, ApplySentScrollsNoDelegate) { +TEST_F(LayerImplScrollTest, ApplySentScrollsNoListener) { gfx::ScrollOffset scroll_offset(10, 5); gfx::Vector2dF scroll_delta(20.5f, 8.5f); gfx::Vector2d sent_scroll_delta(12, -3); @@ -646,57 +541,6 @@ TEST_F(LayerImplScrollTest, ApplySentScrollsNoDelegate) { layer()->BaseScrollOffset()); } -TEST_F(LayerImplScrollTest, ApplySentScrollsWithIgnoringDelegate) { - gfx::ScrollOffset scroll_offset(10, 5); - gfx::Vector2d sent_scroll_delta(12, -3); - gfx::Vector2dF fixed_offset(32, 12); - - layer()->PushScrollOffsetFromMainThread(scroll_offset); - layer()->ScrollBy(sent_scroll_delta); - layer()->PullDeltaForMainThread(); - layer()->SetCurrentScrollOffset(scroll_offset); - ScrollDelegateIgnore delegate; - delegate.set_fixed_offset(fixed_offset); - layer()->SetScrollOffsetDelegate(&delegate); - layer()->RefreshFromScrollDelegate(); - - EXPECT_VECTOR_EQ(fixed_offset, layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); - - layer()->ApplySentScrollDeltasFromAbortedCommit(); - - EXPECT_VECTOR_EQ(fixed_offset, delegate.last_attempted_set_offset()); - - EXPECT_VECTOR_EQ(fixed_offset, layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(gfx::ScrollOffsetWithDelta(scroll_offset, sent_scroll_delta), - layer()->BaseScrollOffset()); -} - -TEST_F(LayerImplScrollTest, ApplySentScrollsWithAcceptingDelegate) { - gfx::ScrollOffset scroll_offset(10, 5); - gfx::Vector2d sent_scroll_delta(12, -3); - gfx::Vector2dF scroll_delta(20.5f, 8.5f); - - layer()->PushScrollOffsetFromMainThread(scroll_offset); - layer()->ScrollBy(sent_scroll_delta); - layer()->PullDeltaForMainThread(); - ScrollDelegateAccept delegate; - layer()->SetScrollOffsetDelegate(&delegate); - layer()->SetCurrentScrollOffset(scroll_offset + - gfx::ScrollOffset(scroll_delta)); - - EXPECT_VECTOR_EQ(gfx::ScrollOffsetWithDelta(scroll_offset, scroll_delta), - layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(scroll_offset, layer()->BaseScrollOffset()); - - layer()->ApplySentScrollDeltasFromAbortedCommit(); - - EXPECT_VECTOR_EQ(gfx::ScrollOffsetWithDelta(scroll_offset, scroll_delta), - layer()->CurrentScrollOffset()); - EXPECT_VECTOR_EQ(gfx::ScrollOffsetWithDelta(scroll_offset, sent_scroll_delta), - layer()->BaseScrollOffset()); -} - TEST_F(LayerImplScrollTest, ScrollUserUnscrollableLayer) { gfx::ScrollOffset scroll_offset(10, 5); gfx::Vector2dF scroll_delta(20.5f, 8.5f); |