Track the currently scrolling ScrollNode instead of the scrolling layer

cc/trees/layer_tree_impl.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/trees/layer_tree_impl.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 45 matching lines @@
     scoped_refptr<SyncedProperty<ScaleGroup>> page_scale_factor,
     scoped_refptr<SyncedBrowserControls> top_controls_shown_ratio,
     scoped_refptr<SyncedElasticOverscroll> elastic_overscroll)
     : layer_tree_host_impl_(layer_tree_host_impl),
       source_frame_number_(-1),
       is_first_frame_after_commit_tracker_(-1),
       root_layer_for_testing_(nullptr),
       hud_layer_(nullptr),
       background_color_(0),
       has_transparent_background_(false),
-      last_scrolled_layer_id_(Layer::INVALID_ID),
+      last_scrolled_scroll_node_index_(ScrollTree::kInvalidNodeId),
       overscroll_elasticity_layer_id_(Layer::INVALID_ID),
       page_scale_layer_id_(Layer::INVALID_ID),
       inner_viewport_scroll_layer_id_(Layer::INVALID_ID),
       outer_viewport_scroll_layer_id_(Layer::INVALID_ID),
       page_scale_factor_(page_scale_factor),
       min_page_scale_factor_(0),
       max_page_scale_factor_(0),
       device_scale_factor_(1.f),
       painted_device_scale_factor_(1.f),
       elastic_overscroll_(elastic_overscroll),
@@ skipping 352 matching lines @@
         outer_container->bounds_delta());
   if (inner_scroll)
     property_trees_.SetInnerViewportScrollBoundsDelta(
         inner_scroll->bounds_delta());
 }
 
 void LayerTreeImpl::PushPropertiesTo(LayerTreeImpl* target_tree) {
   // The request queue should have been processed and does not require a push.
   DCHECK_EQ(ui_resource_request_queue_.size(), 0u);
 
-  LayerImpl* layer = target_tree->CurrentlyScrollingLayer();
+  auto* currently_scrolling_node = target_tree->CurrentlyScrollingNode();

[ajuma, 2017/02/28 22:59:37]

This gets a pointer to a node in the target tree's old screen. It looks like
that could get destroyed or overwritten on the next line when the new trees get
set (say if property tree topology changes). It seems safer to use a stable id
for this step (which, for now, would be owning_layer_id).

[pdr., 2017/03/01 07:21:03]

You're right, and fixing this makes the TreeSynch test much saner.

Longer-term, is there any existing mapping between property tree nodes across
active->pending? It shouldn't be too hard to make one for the current scroll
node, but it'll be a little awkward doing that using just stable layers.

[ajuma, 2017/03/01 15:29:20]

I think the longer term approach would be to use the mapping from element id to
index (element_id_to_scroll_node_index). In fact that might even work now, since
it looks like all scrollable layers and nodes are already assigned an ElementId.

   target_tree->SetPropertyTrees(&property_trees_);
-  target_tree->SetCurrentlyScrollingLayer(layer);
+  target_tree->SetCurrentlyScrollingNode(currently_scrolling_node);
   target_tree->property_trees()->scroll_tree.PushScrollUpdatesFromPendingTree(
       &property_trees_, target_tree);
 
   // This needs to be called early so that we don't clamp with incorrect max
   // offsets when UpdateViewportContainerSizes is called from e.g.
   // PushBrowserControls
   target_tree->UpdatePropertyTreesForBoundsDelta();
 
   if (next_activation_forces_redraw_) {
     target_tree->ForceRedrawNextActivation();
@@ skipping 170 matching lines @@
              ? InnerViewportScrollLayer()->scroll_clip_layer()
              : NULL;
 }
 
 LayerImpl* LayerTreeImpl::OuterViewportContainerLayer() const {
   return OuterViewportScrollLayer()
              ? OuterViewportScrollLayer()->scroll_clip_layer()
              : NULL;
 }
 
-LayerImpl* LayerTreeImpl::CurrentlyScrollingLayer() const {
+ScrollNode* LayerTreeImpl::CurrentlyScrollingNode() {
   DCHECK(IsActiveTree());
-  const ScrollNode* scroll_node =
-      property_trees_.scroll_tree.CurrentlyScrollingNode();
-  return LayerById(scroll_node ? scroll_node->owning_layer_id
-                               : Layer::INVALID_ID);
+  return property_trees_.scroll_tree.CurrentlyScrollingNode();
 }
 
-int LayerTreeImpl::LastScrolledLayerId() const {
-  return last_scrolled_layer_id_;
+const ScrollNode* LayerTreeImpl::CurrentlyScrollingNode() const {
+  return property_trees_.scroll_tree.CurrentlyScrollingNode();
 }
 
-void LayerTreeImpl::SetCurrentlyScrollingLayer(LayerImpl* layer) {
+int LayerTreeImpl::LastScrolledScrollNodeIndex() const {
+  return last_scrolled_scroll_node_index_;
+}
+
+void LayerTreeImpl::SetCurrentlyScrollingNode(ScrollNode* node) {
+  if (node)
+    last_scrolled_scroll_node_index_ = node->id;
+
   ScrollTree& scroll_tree = property_trees()->scroll_tree;
-  ScrollNode* scroll_node = scroll_tree.CurrentlyScrollingNode();
-  int old_id = scroll_node ? scroll_node->owning_layer_id : Layer::INVALID_ID;
-  int new_id = layer ? layer->id() : Layer::INVALID_ID;
-  int new_scroll_node_id =
-      layer ? layer->scroll_tree_index() : ScrollTree::kInvalidNodeId;
-  if (layer)
-    last_scrolled_layer_id_ = new_id;
+  ScrollNode* old_node = scroll_tree.CurrentlyScrollingNode();
+  // TODO(pdr): Refactor these to not use owning_layer_id.
+  int old_layer_id = old_node ? old_node->owning_layer_id : Layer::INVALID_ID;
+  int new_layer_id = node ? node->owning_layer_id : Layer::INVALID_ID;
 
-  if (old_id == new_id)
+  if (old_layer_id == new_layer_id)
     return;
 
   ScrollbarAnimationController* old_animation_controller =
-      layer_tree_host_impl_->ScrollbarAnimationControllerForId(old_id);
+      layer_tree_host_impl_->ScrollbarAnimationControllerForId(old_layer_id);
   ScrollbarAnimationController* new_animation_controller =
-      layer_tree_host_impl_->ScrollbarAnimationControllerForId(new_id);
+      layer_tree_host_impl_->ScrollbarAnimationControllerForId(new_layer_id);
 
   if (old_animation_controller)
     old_animation_controller->DidScrollEnd();
-  scroll_tree.set_currently_scrolling_node(new_scroll_node_id);
+  scroll_tree.set_currently_scrolling_node(node ? node->id
+                                                : ScrollTree::kInvalidNodeId);
   if (new_animation_controller)
     new_animation_controller->DidScrollBegin();
 }
 
-void LayerTreeImpl::ClearCurrentlyScrollingLayer() {
-  SetCurrentlyScrollingLayer(NULL);
+void LayerTreeImpl::ClearCurrentlyScrollingNode() {
+  SetCurrentlyScrollingNode(nullptr);
 }
 
 float LayerTreeImpl::ClampPageScaleFactorToLimits(
     float page_scale_factor) const {
   if (min_page_scale_factor_ && page_scale_factor < min_page_scale_factor_)
     page_scale_factor = min_page_scale_factor_;
   else if (max_page_scale_factor_ && page_scale_factor > max_page_scale_factor_)
     page_scale_factor = max_page_scale_factor_;
   return page_scale_factor;
 }
@@ skipping 1431 matching lines @@
 
 void LayerTreeImpl::ResetAllChangeTracking() {
   layers_that_should_push_properties_.clear();
   // Iterate over all layers, including masks.
   for (auto& layer : *layers_)
     layer->ResetChangeTracking();
   property_trees_.ResetAllChangeTracking();
 }
 
 }  // namespace cc