Move MaxScrollOffset to property_trees

cc/trees/layer_tree_impl.cc

Index: cc/trees/layer_tree_impl.cc
diff --git a/cc/trees/layer_tree_impl.cc b/cc/trees/layer_tree_impl.cc
index 46561da920d67ada5df4ccbb8314cf19304699e9..b7c72026382cb75fdb0a64cac2a4ed6be51de099 100644
--- a/cc/trees/layer_tree_impl.cc
+++ b/cc/trees/layer_tree_impl.cc
@@ -300,11 +300,12 @@ void LayerTreeImpl::UpdatePropertyTreesForBoundsDelta() {
   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) {
@@ -487,8 +488,15 @@ void LayerTreeImpl::UpdatePropertyTreeScrollingAndAnimationFromMainThread() {
 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,
@@ -503,6 +511,10 @@ void LayerTreeImpl::PushPageScaleFactorAndLimits(const float* 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;
@@ -511,7 +523,18 @@ void LayerTreeImpl::PushPageScaleFactorAndLimits(const float* page_scale_factor,
     // 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
@@ -520,9 +543,6 @@ void LayerTreeImpl::PushPageScaleFactorAndLimits(const float* page_scale_factor,
     changed_page_scale |= page_scale_factor_->PushPendingToActive();
   }
 
-  changed_page_scale |=
-      SetPageScaleFactorLimits(min_page_scale_factor, max_page_scale_factor);
-
   if (changed_page_scale)
     DidUpdatePageScale();
 }