Allow perfect scroll bubbling between outer/inner viewports

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 <algorithm>
 #include <limits>
 
 #include "base/basictypes.h"
@@ skipping 2604 matching lines @@
       if (settings_.report_overscroll_only_for_scrollable_axes) {
         if (std::abs(active_tree_->TotalMaxScrollOffset().x()) <= kEpsilon ||
             !layer_impl->user_scrollable_horizontal())
           unused_root_delta.set_x(0.0f);
         if (std::abs(active_tree_->TotalMaxScrollOffset().y()) <= kEpsilon ||
             !layer_impl->user_scrollable_vertical())
           unused_root_delta.set_y(0.0f);
       }
     }
 
+    // Scrolls should bubble perfectly between the outer and inner viewports.
+    bool allow_unrestricted_bubbling_for_current_layer =
+        layer_impl == OuterViewportScrollLayer();
+    bool allow_bubbling_for_current_layer =
+        allow_unrestricted_bubbling_for_current_layer || should_bubble_scrolls_;
+
     // If the layer wasn't able to move, try the next one in the hierarchy.
     bool did_move_layer_x = std::abs(applied_delta.x()) > kEpsilon;
     bool did_move_layer_y = std::abs(applied_delta.y()) > kEpsilon;
     did_scroll_x |= did_move_layer_x;
     did_scroll_y |= did_move_layer_y;
     if (!did_move_layer_x && !did_move_layer_y) {
-      // Scrolls should always bubble between the outer and inner viewports
-      if (should_bubble_scrolls_ || !did_lock_scrolling_layer_ ||
-          layer_impl == OuterViewportScrollLayer())
+      if (allow_bubbling_for_current_layer || !did_lock_scrolling_layer_)
         continue;
       else
         break;
     }
 
     did_lock_scrolling_layer_ = true;
-    if (!should_bubble_scrolls_) {
+    if (!allow_bubbling_for_current_layer) {
       active_tree_->SetCurrentlyScrollingLayer(layer_impl);
       break;
     }
 
-    // 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.
-    float angle_threshold = 45;
-    if (MathUtil::SmallestAngleBetweenVectors(
-            applied_delta, pending_delta) < angle_threshold) {
-      pending_delta = gfx::Vector2dF();
-      break;
+    if (allow_unrestricted_bubbling_for_current_layer) {
+      pending_delta -= applied_delta;
+    } else {
+      // 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.
+      float angle_threshold = 45;
+      if (MathUtil::SmallestAngleBetweenVectors(applied_delta, pending_delta) <
+          angle_threshold) {
+        pending_delta = gfx::Vector2dF();

[bokan, 2014/11/19 00:58:33]

I know you didn't write this, but do we need to clear the pending_delta if we're
breaking out of the loop? Looks like it isn't referenced outside  the loop.

[jdduke (slow), 2014/11/19 16:20:14]

I'm guessing there was some trailing code that used it that has since been
deleted? I'd be inclined to remove the zero-ing code, but since the variable is
visible outside the for loop I think it's fine as-is. A shame for loop
statements don't allow different variable type declarations.

+        break;
+      }
+
+      // Allow further movement only on an axis perpendicular to the direction
+      // in which the layer moved.
+      gfx::Vector2dF perpendicular_axis(-applied_delta.y(), applied_delta.x());
+      pending_delta =
+          MathUtil::ProjectVector(pending_delta, perpendicular_axis);
     }
 
-    // Allow further movement only on an axis perpendicular to the direction in
-    // which the layer moved.
-    gfx::Vector2dF perpendicular_axis(-applied_delta.y(), applied_delta.x());
-    pending_delta = MathUtil::ProjectVector(pending_delta, perpendicular_axis);
-
     if (gfx::ToRoundedVector2d(pending_delta).IsZero())
       break;
   }
 
   bool did_scroll_content = did_scroll_x || did_scroll_y;
   if (did_scroll_content) {
     // If we are scrolling with an active scroll handler, forward latency
     // tracking information to the main thread so the delay introduced by the
     // handler is accounted for.
     if (scroll_affects_scroll_handler())
@@ skipping 761 matching lines @@
 }
 
 void LayerTreeHostImpl::UnregisterPictureLayerImpl(PictureLayerImpl* layer) {
   std::vector<PictureLayerImpl*>::iterator it =
       std::find(picture_layers_.begin(), picture_layers_.end(), layer);
   DCHECK(it != picture_layers_.end());
   picture_layers_.erase(it);
 }
 
 }  // namespace cc