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

Unified Diff: cc/trees/layer_tree_host_impl.cc

Issue 2365793002: Fix scroll chaining for non-descendants of root scroller. (Closed)
Patch Set: Rebase + Hack for telemetry Created 4 years, 2 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
« no previous file with comments | « cc/trees/layer_tree_host_impl.h ('k') | cc/trees/layer_tree_host_impl_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 8a67a401c76dfd96b76e62c8263145926814df02..0480f863ce7bd0a68a64ef7a67d2052005d44b1a 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -1964,6 +1964,12 @@ void LayerTreeHostImpl::ActivateSyncTree() {
// before the swap.
pending_tree_->ProcessUIResourceRequestQueue();
+ // TODO(bokan): The tree sync might destroy the currently scrolling layer.
+ // Normally, this would be fine and we'd just drop incoming scrolls but
+ // some telemetry tests rely on the viewport not having this behavior.
+ // Those should be fixed and this removed. crbug.com/652905.
+ bool restore_currently_scrolling_viewport = IsCurrentlyScrollingViewport();
bokan 2016/10/05 00:24:08 I needed to add this hack to preserver behavior te
aelias_OOO_until_Jul13 2016/10/05 21:55:42 This patch is nonurgent, so I'm not sure why we sh
+
if (pending_tree_->needs_full_tree_sync()) {
TreeSynchronizer::SynchronizeTrees(pending_tree_.get(),
active_tree_.get());
@@ -1994,6 +2000,11 @@ void LayerTreeHostImpl::ActivateSyncTree() {
DCHECK(!recycle_tree_);
pending_tree_.swap(recycle_tree_);
+ if (restore_currently_scrolling_viewport &&
+ !IsCurrentlyScrollingViewport()) {
+ active_tree_->SetCurrentlyScrollingLayer(viewport()->MainScrollLayer());
+ }
+
// If we commit to the active tree directly, this is already done during
// commit.
ActivateAnimations();
@@ -2223,6 +2234,10 @@ void LayerTreeHostImpl::SetLayerTreeMutator(
mutator_->SetClient(this);
}
+LayerImpl* LayerTreeHostImpl::ViewportMainScrollLayer() {
+ return viewport()->MainScrollLayer();
+}
+
void LayerTreeHostImpl::CleanUpTileManagerAndUIResources() {
ClearUIResources();
tile_manager_.FinishTasksAndCleanUp();
@@ -2831,19 +2846,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 (viewport()->MainScrollLayer() &&
+ scroll_node->owner_id == viewport()->MainScrollLayer()->id()) {
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 =
@@ -3043,18 +3057,19 @@ void LayerTreeHostImpl::DistributeScrollDelta(ScrollState* scroll_state) {
std::list<const ScrollNode*> current_scroll_chain;
ScrollTree& scroll_tree = active_tree_->property_trees()->scroll_tree;
ScrollNode* scroll_node = scroll_tree.CurrentlyScrollingNode();
+ ScrollNode* viewport_scroll_node =
+ viewport()->MainScrollLayer()
+ ? scroll_tree.Node(viewport()->MainScrollLayer()->scroll_tree_index())
+ : nullptr;
if (scroll_node) {
+ // TODO(bokan): The loop checks for a null parent but don't we still want to
+ // distribute to the root scroll node?
for (; scroll_tree.parent(scroll_node);
scroll_node = scroll_tree.parent(scroll_node)) {
- if (scroll_node->is_outer_viewport_scroll_layer) {
- // TODO(bokan): This should use Viewport::MainScrollLayer once that
- // returns the outer viewport scroll layer.
+ if (scroll_node == viewport_scroll_node) {
// Don't chain scrolls past the outer viewport scroll layer. Once we
// reach that, we should scroll the viewport which is represented by the
// main viewport scroll layer.
- DCHECK(viewport()->MainScrollLayer());
- ScrollNode* viewport_scroll_node = scroll_tree.Node(
- viewport()->MainScrollLayer()->scroll_tree_index());
DCHECK(viewport_scroll_node);
current_scroll_chain.push_front(viewport_scroll_node);
break;
@@ -3252,9 +3267,11 @@ 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);
+
// Scrollbars for the viewport are registered with the outer viewport layer.
if (scroll_layer_impl == InnerViewportScrollLayer())
scroll_layer_impl = OuterViewportScrollLayer();
+
if (scroll_on_main_thread || !scroll_layer_impl)
return;
« no previous file with comments | « cc/trees/layer_tree_host_impl.h ('k') | cc/trees/layer_tree_host_impl_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698