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

Unified Diff: cc/trees/layer_tree_host_impl.cc

Issue 2365793002: Fix scroll chaining for non-descendants of root scroller. (Closed)
Patch Set: Fix tests Created 4 years, 3 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.cc
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index ab28b3454c5a1a1f5c7e6f45047e61167d1ade96..203fa027c755f18343e84ec9d1d5f81bcbbf3df2 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -2567,16 +2567,15 @@ LayerImpl* LayerTreeHostImpl::FindScrollLayerForDeviceViewportPoint(
}
}
+ // The outer viewport layer represents the viewport.
+ if (potentially_scrolling_layer_impl == InnerViewportScrollLayer())
+ potentially_scrolling_layer_impl = OuterViewportScrollLayer();
+
// Falling back to the root scroll layer ensures generation of root overscroll
- // notifications. The inner viewport layer represents the viewport during
- // scrolling.
+ // notifications.
if (!potentially_scrolling_layer_impl)
potentially_scrolling_layer_impl = InnerViewportScrollLayer();
tdresser 2016/09/23 15:59:58 Shouldn't this be OuterViewportScrollLayer?
bokan 2016/09/23 19:57:01 Yah, I wasn't sure since changing it broke some te
- // The inner viewport layer represents the viewport.
- if (potentially_scrolling_layer_impl == OuterViewportScrollLayer())
- potentially_scrolling_layer_impl = InnerViewportScrollLayer();
-
if (potentially_scrolling_layer_impl) {
// Ensure that final layer scrolls on impl thread (crbug.com/625100)
ScrollNode* scroll_node =
@@ -2848,19 +2847,18 @@ InputHandler::ScrollStatus LayerTreeHostImpl::ScrollAnimated(
if (scroll_node) {
for (; scroll_tree.parent(scroll_node);
scroll_node = scroll_tree.parent(scroll_node)) {
- if (!scroll_node->scrollable ||
- scroll_node->is_outer_viewport_scroll_layer)
+ if (!scroll_node->scrollable)
continue;
- if (scroll_node->is_inner_viewport_scroll_layer) {
+ if (scroll_node->is_outer_viewport_scroll_layer ||
+ scroll_node->is_inner_viewport_scroll_layer) {
gfx::Vector2dF scrolled =
viewport()->ScrollAnimated(pending_delta, delayed_by);
// Viewport::ScrollAnimated returns pending_delta as long as it
// starts an animation.
if (scrolled == pending_delta)
return scroll_status;
- pending_delta -= scrolled;
- continue;
+ break;
}
gfx::Vector2dF scroll_delta =
@@ -3003,7 +3001,7 @@ void LayerTreeHostImpl::ApplyScroll(ScrollNode* scroll_node,
// details.
const float kEpsilon = 0.1f;
- if (scroll_node->is_inner_viewport_scroll_layer) {
+ if (scroll_node->is_outer_viewport_scroll_layer) {
bool affect_top_controls = !wheel_scrolling_;
Viewport::ScrollResult result = viewport()->ScrollBy(
delta, viewport_point, scroll_state->is_direct_manipulation(),
@@ -3024,7 +3022,7 @@ void LayerTreeHostImpl::ApplyScroll(ScrollNode* scroll_node,
bool scrolled = std::abs(applied_delta.x()) > kEpsilon;
scrolled = scrolled || std::abs(applied_delta.y()) > kEpsilon;
- if (scrolled && !scroll_node->is_inner_viewport_scroll_layer) {
+ if (scrolled && !scroll_node->is_outer_viewport_scroll_layer) {
// If the applied delta is within 45 degrees of the input
// delta, bail out to make it easier to scroll just one layer
// in one direction without affecting any of its parents.
@@ -3061,11 +3059,9 @@ void LayerTreeHostImpl::DistributeScrollDelta(ScrollState* scroll_state) {
scroll_node = scroll_tree.parent(scroll_node)) {
if (scroll_node->is_outer_viewport_scroll_layer) {
// Don't chain scrolls past the outer viewport scroll layer. Once we
- // reach that, we should scroll the viewport, which is represented by
- // the inner viewport scroll layer.
- ScrollNode* inner_viewport_scroll_node =
- scroll_tree.Node(InnerViewportScrollLayer()->scroll_tree_index());
- current_scroll_chain.push_front(inner_viewport_scroll_node);
+ // reach that, we're viewport scrolling which is special and handled by
+ // cc's Viewport class.
+ current_scroll_chain.push_front(scroll_node);
break;
}
@@ -3239,8 +3235,6 @@ void LayerTreeHostImpl::MouseMoveAt(const gfx::Point& viewport_point) {
LayerImpl* scroll_layer_impl = FindScrollLayerForDeviceViewportPoint(
device_viewport_point, InputHandler::TOUCHSCREEN, layer_impl,
&scroll_on_main_thread, &main_thread_scrolling_reasons);
- if (scroll_layer_impl == InnerViewportScrollLayer())
- scroll_layer_impl = OuterViewportScrollLayer();
tdresser 2016/09/23 15:59:58 I'm a bit confused by why this was here before, an
bokan 2016/09/23 19:57:01 I believe it's needed because the ScrollbarAnimati
if (scroll_on_main_thread || !scroll_layer_impl)
return;
@@ -3288,7 +3282,7 @@ void LayerTreeHostImpl::PinchGestureBegin() {
client_->RenewTreePriority();
pinch_gesture_end_should_clear_scrolling_layer_ = !CurrentlyScrollingLayer();
active_tree_->SetCurrentlyScrollingLayer(
- active_tree_->InnerViewportScrollLayer());
+ active_tree_->OuterViewportScrollLayer());
top_controls_manager_->PinchBegin();
}

Powered by Google App Engine
This is Rietveld 408576698