Fix bottom/right fixed-pos with non-overlay scrollbars.

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 "base/debug/trace_event.h"
 #include "cc/animation/keyframed_animation_curve.h"
 #include "cc/animation/scrollbar_animation_controller.h"
 #include "cc/debug/traced_value.h"
@@ skipping 125 matching lines @@
 }
 
 LayerImpl* LayerTreeImpl::RootScrollLayer() const {
   return root_scroll_layer_;
 }
 
 LayerImpl* LayerTreeImpl::RootContainerLayer() const {
   return root_scroll_layer_ ? root_scroll_layer_->parent() : NULL;
 }
 
+LayerImpl* LayerTreeImpl::ViewportLayer() const {
+  // Return the first ancestor of the root container layer which has
+  // nonzero bounds.
+  LayerImpl* root_container = RootContainerLayer();
+  if (!root_container)
+    return NULL;
+
+  LayerImpl* ancestor = root_container->parent();
+  while (ancestor) {
+    if (!ancestor->bounds().IsEmpty())
+      return ancestor;
+    ancestor = ancestor->parent();
+  }
+  return NULL;
+}
+
 LayerImpl* LayerTreeImpl::CurrentlyScrollingLayer() const {
   DCHECK(IsActiveTree());
   return currently_scrolling_layer_;
 }
 
+int LayerTreeImpl::NonOverlayScrollbarThickness(
+    ScrollbarOrientation orientation) const {
+  LayerImpl* viewport = ViewportLayer();
+  LayerImpl* root_container = RootContainerLayer();
+  if (!viewport || !root_container)
+    return 0;
+
+  DCHECK_GE(viewport->bounds().width(), root_container->bounds().width());
+  DCHECK_GE(viewport->bounds().height(), root_container->bounds().height());
+
+  if (orientation == VERTICAL)
+    return viewport->bounds().width() - root_container->bounds().width();
+  else
+    return viewport->bounds().height() - root_container->bounds().height();
+}
+
 void LayerTreeImpl::SetCurrentlyScrollingLayer(LayerImpl* layer) {
   if (currently_scrolling_layer_ == layer)
     return;
 
   if (currently_scrolling_layer_ &&
       currently_scrolling_layer_->scrollbar_animation_controller())
     currently_scrolling_layer_->scrollbar_animation_controller()->
         DidScrollGestureEnd(CurrentPhysicalTimeTicks());
   currently_scrolling_layer_ = layer;
   if (layer && layer->scrollbar_animation_controller())
@@ skipping 49 matching lines @@
 void LayerTreeImpl::UpdateMaxScrollOffset() {
   LayerImpl* root_scroll = RootScrollLayer();
   if (!root_scroll || !root_scroll->children().size())
     return;
 
   gfx::Vector2dF max_scroll = gfx::Rect(ScrollableSize()).bottom_right() -
       gfx::RectF(ScrollableViewportSize()).bottom_right();
 
   // The scrollable viewport size is based on device viewport instead of Blink's
   // container layer, so we need to adjust for non-overlay scrollbars.
-  ScrollbarLayerImpl* horiz = root_scroll->horizontal_scrollbar_layer();
-  ScrollbarLayerImpl* vertical = root_scroll->vertical_scrollbar_layer();
-  if (horiz && !horiz->is_overlay_scrollbar())
-    max_scroll.set_y(max_scroll.y() + horiz->thumb_thickness());
-  if (vertical && !vertical->is_overlay_scrollbar())
-    max_scroll.set_x(max_scroll.x() + vertical->thumb_thickness());
+  max_scroll.set_x(max_scroll.x() + NonOverlayScrollbarThickness(VERTICAL));

[enne (OOO), 2013/07/30 02:32:17]

This code seems like it'd be cleaner if it set
RenderLayerCompositor::m_scrollLayer's size to the right size rather than making
assumptions from other layers.

The advantage of that is for me is that we could stop finding the first child of
the scrollable layer to guess its bounds, and then we could avoid workarounds
like https://codereview.chromium.org/14221010/.

[aelias_OOO_until_Jul13, 2013/07/30 02:52:00]

OK, I'll try that.  It will be useful for root overflow:hidden support anyway. 
Note though that this will include the scrollbar size in ScrollableSize and you
objected to those semantics last time (though I think it's fine).

[enne (OOO), 2013/07/30 03:44:47]

Oh, I see what you're getting at.  What would make sense to me would be if
ScrollableSize were the size of the scrolling contents and
ScrollableViewportSize were the size of the viewport where content can appear
(and is shrunk for non-overlay scrollbars, as they logically clip the view).

[aelias_OOO_until_Jul13, 2013/07/30 04:06:13]

That actually is not correct.  In your page scale doc, non-overlay scrollbars
lie below the page scale layer.  They are in the same space as the contents but
not the inner viewport.  (Put another way, because the scrollbars scale up, they
take up a variable amount of the inner viewport that isn't known in advance.)

+  max_scroll.set_y(max_scroll.y() + NonOverlayScrollbarThickness(HORIZONTAL));
 
   // The viewport may be larger than the contents in some cases, such as
   // having a vertical scrollbar but no horizontal overflow.
   max_scroll.SetToMax(gfx::Vector2dF());
 
   root_scroll_layer_->SetMaxScrollOffset(gfx::ToFlooredVector2d(max_scroll));
 }
 
 void LayerTreeImpl::UpdateSolidColorScrollbars() {
   DCHECK(settings().solid_color_scrollbars);
@@ skipping 15 matching lines @@
         scrollable_viewport.width() / ScrollableSize().width());
   }
   if (ScrollbarLayerImpl* vertical = root_scroll->vertical_scrollbar_layer()) {
     vertical->set_vertical_adjust(vertical_adjust);
     vertical->set_visible_to_total_length_ratio(
         scrollable_viewport.height() / ScrollableSize().height());
   }
 }
 
 void LayerTreeImpl::UpdateDrawProperties() {
-  if (IsActiveTree() && RootScrollLayer() && RootContainerLayer())
+  if (IsActiveTree())
     UpdateRootScrollLayerSizeDelta();
 
   if (settings().solid_color_scrollbars &&
       IsActiveTree() &&
       RootScrollLayer()) {
     UpdateSolidColorScrollbars();
   }
 
   needs_update_draw_properties_ = false;
   render_surface_layer_list_.clear();
@@ skipping 247 matching lines @@
 void LayerTreeImpl::SetRootLayerScrollOffsetDelegate(
       LayerScrollOffsetDelegate* root_layer_scroll_offset_delegate) {
   root_layer_scroll_offset_delegate_ = root_layer_scroll_offset_delegate;
   if (root_scroll_layer_) {
     root_scroll_layer_->SetScrollOffsetDelegate(
         root_layer_scroll_offset_delegate_);
   }
 }
 
 void LayerTreeImpl::UpdateRootScrollLayerSizeDelta() {
-  LayerImpl* root_scroll = RootScrollLayer();
-  LayerImpl* root_container = RootContainerLayer();
-  DCHECK(root_scroll);
-  DCHECK(root_container);
+  LayerImpl* viewport_layer = ViewportLayer();
+  if (!viewport_layer)
+    return;
+
   DCHECK(IsActiveTree());
+  DCHECK(root_scroll_layer_);
 
-  gfx::Vector2dF scrollable_viewport_size =
-      gfx::RectF(ScrollableViewportSize()).bottom_right() - gfx::PointF();
+  // This corrects fixed-position elements for 1) dynamic viewport size changes
+  // caused by things like URL-bar hiding, and 2) impl-side page scale delta.
+  gfx::SizeF true_viewport_size =
+      gfx::ScaleSize(layer_tree_host_impl_->VisibleViewportSize(),
+                     1 / total_page_scale_factor());
+  gfx::SizeF blink_perceived_viewport_size =
+      gfx::ScaleSize(viewport_layer->bounds(), 1 / page_scale_factor());

[enne (OOO), 2013/07/30 02:32:17]

Just to make sure I'm reading this correctly, ViewportLayer() here is intending
to return the overflow controls host, which includes the non-overlay scrollbars
in its bounds?

It seems to me like you would want to have a bottom fixed pos element be flush
with the viewport *not* containing the non-overlay scrollbars, which is not what
I see this code doing.

Can you help me understand what's going on here?

[aelias_OOO_until_Jul13, 2013/07/30 02:52:00]

It doesn't matter because it's a delta.  This is comparing two viewports that
include the non-overlay scrollbar size.  Equivalently, I could instead compare
two viewports that exclude the non-overlay scrollbars (which would involve
explicitly subtracting the scrollbar size from true_viewport_size).  I did it
this way because it's less lines of code.

[enne (OOO), 2013/07/30 03:44:47]

I worked through an example, and I'll buy that explanation.  Thanks.

+  gfx::Vector2dF delta(
+      true_viewport_size.width() - blink_perceived_viewport_size.width(),
+      true_viewport_size.height() - blink_perceived_viewport_size.height());
 
-  gfx::Vector2dF original_viewport_size =
-      gfx::RectF(root_container->bounds()).bottom_right() -
-      gfx::PointF();
-  original_viewport_size.Scale(1 / page_scale_factor());
-
-  root_scroll->SetFixedContainerSizeDelta(
-      scrollable_viewport_size - original_viewport_size);
+  root_scroll_layer_->SetFixedContainerSizeDelta(delta);
 }
 
 void LayerTreeImpl::SetLatencyInfo(const ui::LatencyInfo& latency_info) {
   latency_info_.MergeWith(latency_info);
 }
 
 const ui::LatencyInfo& LayerTreeImpl::GetLatencyInfo() {
   return latency_info_;
 }
 
@@ skipping 34 matching lines @@
 const std::vector<LayerImpl*> LayerTreeImpl::LayersWithCopyOutputRequest()
     const {
   // Only the active tree needs to know about layers with copy requests, as
   // they are aborted if not serviced during draw.
   DCHECK(IsActiveTree());
 
   return layers_with_copy_output_request_;
 }
 
 }  // namespace cc