Commit 431583b9 authored by aelias's avatar aelias Committed by Commit bot

Make aborted commits inform the scroll delegate.

This way of writing it was considered in
https://codereview.chromium.org/19106007#msg30 but the direct
scroll_delta_ was felt to be slightly cleaner and with no behavior
difference unless "you had some weird stateful scroll offset delegate."
Pinch viewport mode indeed introduced state inside
LayerScrollOffsetDelegateProxy to buffer the values provided by the
inner and outer viewports before calling the delegate, so this is now
more correct.  Specifically, it's now necessary to echo back to the
delegate the values forced by its getter, or they may be clobbered with
older values later.

NOTRY=true
BUG=426891

Review URL: https://codereview.chromium.org/665233004

Cr-Commit-Position: refs/heads/master@{#302711}
parent 96d45c70
......@@ -397,23 +397,22 @@ bool LayerImpl::user_scrollable(ScrollbarOrientation orientation) const {
}
void LayerImpl::ApplySentScrollDeltasFromAbortedCommit() {
if (sent_scroll_delta_.IsZero())
return;
// Pending tree never has sent scroll deltas
DCHECK(layer_tree_impl()->IsActiveTree());
// The combination of pending tree and aborted commits with impl scrolls
// shouldn't happen; we don't know how to update its deltas correctly.
DCHECK(!layer_tree_impl()->FindPendingTreeLayerById(id()));
// Apply sent scroll deltas to scroll position / scroll delta as if the
// main thread had applied them and then committed those values.
//
// This function should not change the total scroll offset; it just shifts
// some of the scroll delta to the scroll offset. Therefore, adjust these
// variables directly rather than calling the scroll offset delegate to
// avoid sending it multiple spurious calls.
//
// Because of the way scroll delta is calculated with a delegate, this will
// leave the total scroll offset unchanged on this layer regardless of
// whether a delegate is being used.
scroll_offset_ += gfx::ScrollOffset(sent_scroll_delta_);
scroll_delta_ -= sent_scroll_delta_;
sent_scroll_delta_ = gfx::Vector2dF();
SetScrollOffsetAndDelta(
scroll_offset_ + gfx::ScrollOffset(sent_scroll_delta_),
ScrollDelta() - sent_scroll_delta_);
SetSentScrollDelta(gfx::Vector2dF());
}
void LayerImpl::ApplyScrollDeltasSinceBeginMainFrame() {
......
......@@ -499,7 +499,13 @@ TEST_F(LayerImplScrollTest, ScrollByWithNonZeroOffset) {
class ScrollDelegateIgnore : public LayerImpl::ScrollOffsetDelegate {
public:
void SetTotalScrollOffset(const gfx::ScrollOffset& new_value) override {}
void SetTotalScrollOffset(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 GetTotalScrollOffset() override {
return gfx::ScrollOffset(fixed_offset_);
}
......@@ -511,6 +517,7 @@ class ScrollDelegateIgnore : public LayerImpl::ScrollOffsetDelegate {
}
private:
gfx::ScrollOffset last_attempted_set_offset_;
gfx::Vector2dF fixed_offset_;
};
......@@ -634,6 +641,8 @@ TEST_F(LayerImplScrollTest, ApplySentScrollsWithIgnoringDelegate) {
layer()->ApplySentScrollDeltasFromAbortedCommit();
EXPECT_VECTOR_EQ(fixed_offset, delegate.last_attempted_set_offset());
EXPECT_VECTOR_EQ(fixed_offset, layer()->TotalScrollOffset());
EXPECT_VECTOR_EQ(gfx::ScrollOffsetWithDelta(scroll_offset, sent_scroll_delta),
layer()->scroll_offset());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment