Move MaxScrollOffset 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 282 matching lines @@
   LayerImpl* inner_container = InnerViewportContainerLayer();
   LayerImpl* outer_container = OuterViewportContainerLayer();
 
   UpdateClipTreeForBoundsDeltaOnLayer(inner_container,
                                       &property_trees_.clip_tree);
   UpdateClipTreeForBoundsDeltaOnLayer(InnerViewportScrollLayer(),
                                       &property_trees_.clip_tree);
   UpdateClipTreeForBoundsDeltaOnLayer(outer_container,
                                       &property_trees_.clip_tree);
 
-  TransformTree& transform_tree = property_trees_.transform_tree;
   if (inner_container)
-    transform_tree.SetInnerViewportBoundsDelta(inner_container->bounds_delta());
+    property_trees_.SetInnerViewportContainerBoundsDelta(
+        inner_container->bounds_delta());
   if (outer_container)
-    transform_tree.SetOuterViewportBoundsDelta(outer_container->bounds_delta());
+    property_trees_.SetOuterViewportContainerBoundsDelta(
+        outer_container->bounds_delta());

[ajuma, 2016/02/09 22:35:05]

Do we need to do the same for the inner viewport scroll layer? It's a good idea
to add a test for this too (something like a layer tree host test that sets a
bounds delta on the inner viewport scroll layer just before a commit/activation
happens, and then verifies property trees are correctly taking the bounds delta
into account after activation).

 }
 
 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);
 
   target_tree->SetPropertyTrees(property_trees_);
 
   if (next_activation_forces_redraw_) {
     target_tree->ForceRedrawNextActivation();
@@ skipping 162 matching lines @@
     return;
   LayerTreeHostCommon::CallFunctionForSubtree(
       root_layer(), [](LayerImpl* layer) {
         layer->UpdatePropertyTreeForScrollingAndAnimationIfNeeded();
       });
 }
 
 void LayerTreeImpl::SetPageScaleOnActiveTree(float active_page_scale) {
   DCHECK(IsActiveTree());
   if (page_scale_factor()->SetCurrent(
-          ClampPageScaleFactorToLimits(active_page_scale)))
+          ClampPageScaleFactorToLimits(active_page_scale))) {
+    gfx::Transform identity_transform;
+    if (PageScaleLayer())
+      UpdatePageScaleFactorInPropertyTrees(
+          property_trees(), PageScaleLayer(),
+          ClampPageScaleFactorToLimits(active_page_scale),

[ajuma, 2016/02/09 22:35:05]

I think you can use current_page_scale_factor() here without having to clamp
again.

+          device_scale_factor(), identity_transform);

[ajuma, 2016/02/09 22:35:05]

Instead of identity_transform, this should use the actual device transform
(layer_tree_host_impl_->DrawTransform()).

     DidUpdatePageScale();
+  }
 }
 
 void LayerTreeImpl::PushPageScaleFromMainThread(float page_scale_factor,
                                                 float min_page_scale_factor,
                                                 float max_page_scale_factor) {
   PushPageScaleFactorAndLimits(&page_scale_factor, min_page_scale_factor,
                                max_page_scale_factor);
 }
 
 void LayerTreeImpl::PushPageScaleFactorAndLimits(const float* page_scale_factor,
                                                  float min_page_scale_factor,
                                                  float max_page_scale_factor) {
   DCHECK(page_scale_factor || IsActiveTree());
   bool changed_page_scale = false;
+
+  changed_page_scale |=
+      SetPageScaleFactorLimits(min_page_scale_factor, max_page_scale_factor);
+
   if (page_scale_factor) {
     DCHECK(!IsActiveTree() || !layer_tree_host_impl_->pending_tree());
     changed_page_scale |= page_scale_factor_->Delta() != 1.f;
     // TODO(enne): Once CDP goes away, ignore this call below.  The only time
     // the property trees will differ is if there's been a page scale on the
     // compositor thread after the begin frame, which is the delta check above.
     changed_page_scale |=
         page_scale_factor_->PushFromMainThread(*page_scale_factor);
+    if (PageScaleLayer()) {
+      gfx::Transform identity_transform;
+      UpdatePageScaleFactorInPropertyTrees(
+          property_trees(), PageScaleLayer(),
+          ClampPageScaleFactorToLimits(*page_scale_factor),

[ajuma, 2016/02/09 22:35:05]

If you move this to the end of this method then you can use
current_page_scale_factor() here.

+          device_scale_factor(), identity_transform);

[ajuma, 2016/02/09 22:35:05]

Instead of identity_transform, this should be
layer_tree_host_impl_->DrawTransform().

+    } else {
+      property_trees()->transform_tree.set_page_scale_factor(
+          ClampPageScaleFactorToLimits(*page_scale_factor));

[ajuma, 2016/02/09 22:35:05]

Is this code needed? It seems wrong to have a non-1 page scale factor without a
page scale layer, and if we don't have a page scale layer then the property
trees we just pushed should already have page scale 1 in the transform tree.
Perhaps just DCHECK(*page_scale_factor == 1) here? 

[sunxd, 2016/02/09 23:03:56]

This is also for testing, LayerTreeHostCommonTestBase::
    ExecuteCalculateDrawPropertiesWithPropertyTrees(Layer* root_layer)
(https://code.google.com/p/chromium/codesearch#chromium/src/cc/test/layer_tree...)
sets page_scale_layer to be nullptr even though the layer tree contains a valid
page_scale_layer. Should I change that code? I was afraid of breaking other
stuff if I use layer_tree_host()->page_scale_layer() here.

[ajuma, 2016/02/10 00:15:06]

Checking for a non-null PageScaleLayer() is fine, but in the case where it's
null, page_scale_factor should be 1, so property trees should already have the
right value. That is, doing
if (PageScaleLayer()) {
  ...
} else {
  DCHECK(*page_scale_factor == 1)
}
should work.

+    }
   }
+
   if (IsActiveTree()) {
     // TODO(enne): Pushing from pending to active should never require
     // DidUpdatePageScale.  The values should already be set by the fully
     // computed property trees being synced from one tree to another.  Remove
     // this once CDP goes away.
     changed_page_scale |= page_scale_factor_->PushPendingToActive();
   }
 
-  changed_page_scale |=
-      SetPageScaleFactorLimits(min_page_scale_factor, max_page_scale_factor);
-
   if (changed_page_scale)
     DidUpdatePageScale();
 }
 
 void LayerTreeImpl::set_top_controls_shrink_blink_size(bool shrink) {
   if (top_controls_shrink_blink_size_ == shrink)
     return;
 
   top_controls_shrink_blink_size_ = shrink;
   if (IsActiveTree())
@@ skipping 1514 matching lines @@
                                                    const gfx::BoxF& box,
                                                    gfx::BoxF* bounds) const {
   *bounds = gfx::BoxF();
   return layer_tree_host_impl_->animation_host()
              ? layer_tree_host_impl_->animation_host()
                    ->TransformAnimationBoundsForBox(layer->id(), box, bounds)
              : true;
 }
 
 }  // namespace cc