Move LayerImpl's bounds_delta to property trees

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 244 matching lines @@
           scrollbar->SetScrollLayerLength(scroll_size.width());
     } else {
       scrollbar_needs_animation |= y_offset_did_change |=
           scrollbar->SetCurrentPos(current_offset.y());
       clip_layer_size_did_change |=
           scrollbar->SetClipLayerLength(clip_size.height());
       scroll_layer_size_did_change |=
           scrollbar->SetScrollLayerLength(scroll_size.height());
     }
     scrollbar_needs_animation |=
-        scrollbar->SetVerticalAdjust(clip_layer->bounds_delta().y());
+        scrollbar->SetVerticalAdjust(clip_layer->ViewportBoundsDelta().y());
   }
 
   scrollbar_needs_animation |=
       (clip_layer_size_did_change || scroll_layer_size_did_change);
 
   if (y_offset_did_change && IsViewportLayerId(scroll_layer_id))
     TRACE_COUNTER_ID1("cc", "scroll_offset_y", scroll_layer->id(),
                       current_offset.y());
 
   if (scrollbar_needs_animation) {
@@ skipping 103 matching lines @@
 std::unique_ptr<OwnedLayerImplList> LayerTreeImpl::DetachLayers() {
   root_layer_for_testing_ = nullptr;
   layer_list_.clear();
   render_surface_layer_list_.clear();
   set_needs_update_draw_properties();
   std::unique_ptr<OwnedLayerImplList> ret = std::move(layers_);
   layers_.reset(new OwnedLayerImplList);
   return ret;
 }
 
-static void UpdateClipTreeForBoundsDeltaOnLayer(LayerImpl* layer,
-                                                ClipTree* clip_tree) {
-  if (layer && layer->masks_to_bounds()) {
-    ClipNode* clip_node = clip_tree->Node(layer->clip_tree_index());
-    if (clip_node) {
-      DCHECK_EQ(layer->id(), clip_node->owning_layer_id);
-      gfx::SizeF bounds = gfx::SizeF(layer->bounds());
-      if (clip_node->clip.size() != bounds) {
-        clip_node->clip.set_size(bounds);
-        clip_tree->set_needs_update(true);
-      }
-    }
-  }
-}
-
 void LayerTreeImpl::SetPropertyTrees(PropertyTrees* property_trees) {
   std::vector<std::unique_ptr<RenderSurfaceImpl>> old_render_surfaces;
   property_trees_.effect_tree.TakeRenderSurfaces(&old_render_surfaces);
   property_trees_ = *property_trees;
   bool render_surfaces_changed =
       property_trees_.effect_tree.CreateOrReuseRenderSurfaces(
           &old_render_surfaces, this);
   if (render_surfaces_changed)
     set_needs_update_draw_properties();
   property_trees->effect_tree.PushCopyRequestsTo(&property_trees_.effect_tree);
   property_trees_.is_main_thread = false;
   property_trees_.is_active = IsActiveTree();
   property_trees_.transform_tree.set_source_to_parent_updates_allowed(false);
   // The value of some effect node properties (like is_drawn) depends on
   // whether we are on the active tree or not. So, we need to update the
   // effect tree.
   if (IsActiveTree())
     property_trees_.effect_tree.set_needs_update(true);
 }
 
-void LayerTreeImpl::UpdatePropertyTreesForBoundsDelta() {
-  DCHECK(IsActiveTree());
-  LayerImpl* inner_container = InnerViewportContainerLayer();
-  LayerImpl* outer_container = OuterViewportContainerLayer();
-  LayerImpl* inner_scroll = InnerViewportScrollLayer();
-
-  UpdateClipTreeForBoundsDeltaOnLayer(inner_container,
-                                      &property_trees_.clip_tree);
-  UpdateClipTreeForBoundsDeltaOnLayer(InnerViewportScrollLayer(),
-                                      &property_trees_.clip_tree);
-  UpdateClipTreeForBoundsDeltaOnLayer(outer_container,
-                                      &property_trees_.clip_tree);
-
-  if (inner_container)
-    property_trees_.SetInnerViewportContainerBoundsDelta(
-        inner_container->bounds_delta());
-  if (outer_container)
-    property_trees_.SetOuterViewportContainerBoundsDelta(
-        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);
 
   // To maintain the current scrolling node we need to use element ids which
   // are stable across the property tree update in SetPropertyTrees.
   ElementId scrolling_element_id;
   if (ScrollNode* scrolling_node = target_tree->CurrentlyScrollingNode())
     scrolling_element_id = scrolling_node->element_id;
 
   target_tree->SetPropertyTrees(&property_trees_);
 
   ScrollNode* scrolling_node = nullptr;
   if (scrolling_element_id) {
     auto& scroll_tree = target_tree->property_trees()->scroll_tree;
     scrolling_node = scroll_tree.FindNodeFromElementId(scrolling_element_id);
   }
   target_tree->SetCurrentlyScrollingNode(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();

[ajuma, 2017/04/24 17:37:09]

Just to make sure, the newly-pushed property trees now get their bounds delta
values set whenever UpdateViewportContainerSizes is called next?

[pdr., 2017/04/24 20:06:49]

I don't think so. Instead, the newly-pushed property trees are pushed with the
correct viewport bounds deltas to begin with in
target_tree->SetPropertyTrees(&property_trees_); We no longer need to handle the
case when the layer impl's viewport bounds delta is different from the property
trees's bounds delta.

[ajuma, 2017/04/24 21:06:20]

My understanding was that the pending tree never found about about bounds delta
values (e.g., the only non-test caller of LayerImpl::SetBoundsDelta is
UpdateViewportContainerSizes, and this only touches the active tree), so the
newly pushed trees wouldn't have the right bounds delta values to start with.

[pdr., 2017/04/25 04:01:55]

You are right, here's my (new) understanding:
The viewport bounds deltas are only ever set on the active tree.
ActivateSyncTree does two things:
1) pushes pending->active which can set an incorrect (uninitialized pending)
value for the bounds delta
2) calls UpdateViewportContainerSizes to calculate the correct bounds delta on
the active tree
It's safe to remove this call because UpdateViewportContainerSizes is called in
ActivateSyncTree.

+reviewer Bokan, you wrote https://codereview.chromium.org/2149033002 and may
have context here. Could you please review this too?

[bokan, 2017/04/25 19:01:46]

Yes, I think your understanding is correct but I'm not understanding my original
fix now that I look back on it. The same reasoning should have applied back then
- the SetBoundsDelta calls in UpdateViewportContainerSizes should have been
updating the deltas in the new property tree. I may have gotten the analysis
wrong in my commit message - perhaps it wasn't the UpdateViewportContainerSizes
call after all.

One suspicious thing that jumps out at me is ScrollTree::MaxScrollOffset only
accounts for the delta on the inner viewport layer and not the outer. PDR, does
the bug in https://crbug.com/627911 reproduce with your patch?

-
   if (next_activation_forces_redraw_) {
     target_tree->ForceRedrawNextActivation();
     next_activation_forces_redraw_ = false;
   }
 
   target_tree->PassSwapPromises(std::move(swap_promise_list_));
   swap_promise_list_.clear();
 
   target_tree->set_browser_controls_shrink_blink_size(
       browser_controls_shrink_blink_size_);
@@ skipping 1696 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