Refactor IsScrolledBy and IsClosestScrollAncestor to use ScrollNode ids

cc/trees/layer_tree_host_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_host_impl.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 136 matching lines @@
   bool scroll_on_main_thread = (scroll_thread == MAIN_THREAD);
   if (IsWheelBasedScroll(type)) {
     UMA_HISTOGRAM_BOOLEAN("Renderer4.CompositorWheelScrollUpdateThread",
                           scroll_on_main_thread);
   } else {
     UMA_HISTOGRAM_BOOLEAN("Renderer4.CompositorTouchScrollUpdateThread",
                           scroll_on_main_thread);
   }
 }
 
-// Return true if scrollable 'ancestor' is the same layer as 'child' or its
-// ancestor along the scroll tree.
+// Return true if scrollable 'ancestor' has the same scroll node as 'child' or
+// an ancestor along the scroll tree.
 bool IsScrolledBy(LayerImpl* child, LayerImpl* ancestor) {
   DCHECK(ancestor && ancestor->scrollable());
   if (!child)
     return false;
 
   ScrollTree& scroll_tree =
       child->layer_tree_impl()->property_trees()->scroll_tree;
   for (ScrollNode* scroll_node = scroll_tree.Node(child->scroll_tree_index());
        scroll_node; scroll_node = scroll_tree.parent(scroll_node)) {
-    if (scroll_node->owning_layer_id == ancestor->id())
+    if (scroll_node->id == ancestor->scroll_tree_index())

[ajuma, 2017/02/21 16:05:25]

Just wanted to point out that this could change behavior if IsScrolledBy was
called with |ancestor| not owning a scroll node (since previously, that would
always return false, but now it could return true). But it looks like this only
gets called when |ancestor| owns a scroll node.

[bokan, 2017/02/21 19:45:25]

Ali, could you help me understand how that could happen? If ancestor doesn't own
a ScrollNode, won't scroll_tree_index be invalid and this check always be false?

[ajuma, 2017/02/21 20:07:56]

Every layer has a scroll_tree_index. If a layer doesn't own a ScrollNode, it's
scroll_tree_index will be that of the closest ancestor that owns one (where
'ancestor' is either real ancestor in the main-thread Layer tree or
Layer::scroll_parent ancestor if there is one). Non-test LayerImpls should never
have invalid indices for any of the property trees.

[bokan, 2017/02/21 20:25:22]

Ah, got it - I was misunderstanding how that worked. Thanks.

[pdr., 2017/02/23 05:44:26]

+1, I had the same misunderstanding.

       return true;
   }
   return false;
 }
 
 }  // namespace
 
 DEFINE_SCOPED_UMA_HISTOGRAM_TIMER(PendingTreeDurationHistogramTimer,
                                   "Scheduling.%s.PendingTreeDuration");
 
@@ skipping 2440 matching lines @@
                                     LayerImpl* scroll_ancestor) {
   DCHECK(scroll_ancestor);
   if (!child)
     return false;
   ScrollTree& scroll_tree =
       child->layer_tree_impl()->property_trees()->scroll_tree;
   ScrollNode* scroll_node = scroll_tree.Node(child->scroll_tree_index());
   for (; scroll_tree.parent(scroll_node);
        scroll_node = scroll_tree.parent(scroll_node)) {
     if (scroll_node->scrollable)
-      return scroll_node->owning_layer_id == scroll_ancestor->id();
+      return scroll_node->id == scroll_ancestor->scroll_tree_index();

[ajuma, 2017/02/21 16:05:25]

Same comment as above, but this time it looks like this can indeed get called
(from ScrollBegin) with |scroll_ancestor| not owning a scroll node (it can be a
scrollbar layer that isn't itself scrollable). For background on why scrollbar
layers are a special case, see https://codereview.chromium.org/1878323002 and
https://codereview.chromium.org/238803005. I wonder if we just need to make that
special-casing more explicit somehow? sunxd@, any ideas?

[sunxd, 2017/02/22 22:25:19]

In some cases we have MainThreadScrollingReason::kScrollbarScrolling for
scrollbars, but at least for SolidColorScrollbarLayer, we do not have such a
reason, so probably we do not have a scroll node for this layer.

   }
   return false;
 }
 
 InputHandler::ScrollStatus LayerTreeHostImpl::ScrollBeginImpl(
     ScrollState* scroll_state,
     LayerImpl* scrolling_layer_impl,
     InputHandler::ScrollInputType type) {
   DCHECK(scroll_state);
   DCHECK(scroll_state->delta_x() == 0 && scroll_state->delta_y() == 0);
@@ skipping 1476 matching lines @@
       worker_context_visibility_ =
           worker_context->CacheController()->ClientBecameVisible();
     } else {
       worker_context->CacheController()->ClientBecameNotVisible(
           std::move(worker_context_visibility_));
     }
   }
 }
 
 }  // namespace cc