Fix scroll chaining for non-descendants of root scroller.

cc/trees/layer_tree_host_impl.cc

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 tests. Taking a closer look now,
it does seem like we should return outer and the tests were just overfitting.
Changed to Outer and fixed tests.

 
-  // 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, and why it was removed.

Do you know what this was doing?

[bokan, 2016/09/23 19:57:01]

I believe it's needed because the ScrollbarAnimationController for the viewport
is registered on the outer viewport scroll layer below. It's removed since we
now return the outer scroll layer from FindScrollLayerForDeviceViewportPoint
(and there's no way we can get back the inner).

   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();
 }