Fix scroll position adjustment for fixed-position layers

cc/trees/property_tree_builder.cc

Index: cc/trees/property_tree_builder.cc
diff --git a/cc/trees/property_tree_builder.cc b/cc/trees/property_tree_builder.cc
index 3da729e360d2c28134cae5cea83397d0df8af7b9..2e79178ce10ea023fb86c00cd4ac1f3ad22509b0 100644
--- a/cc/trees/property_tree_builder.cc
+++ b/cc/trees/property_tree_builder.cc
@@ -11,6 +11,7 @@
 #include "cc/layers/layer.h"
 #include "cc/trees/layer_tree_host.h"
 #include "ui/gfx/geometry/point_f.h"
+#include "ui/gfx/geometry/vector2d_conversions.h"
 
 namespace cc {
 
@@ -33,6 +34,7 @@ struct DataForRecursion {
   bool in_subtree_of_page_scale_application_layer;
   bool should_flatten;
   const gfx::Transform* device_transform;
+  gfx::Vector2d scroll_compensation_adjustment;
 };
 
 static Layer* GetTransformParent(const DataForRecursion& data, Layer* layer) {
@@ -141,18 +143,21 @@ void AddTransformNodeIfNeeded(const DataForRecursion& data_from_ancestor,
       parent_offset += to_parent.To2dTranslation();
     } else if (!is_fixed) {
       parent_offset = transform_parent->offset_to_transform_parent();
-    } else if (data_from_ancestor.transform_tree_parent !=
-               data_from_ancestor.transform_fixed_parent) {
-      gfx::Vector2dF fixed_offset = data_from_ancestor.transform_tree_parent
-                                        ->offset_to_transform_parent();
-      gfx::Transform parent_to_parent;
-      data_from_ancestor.transform_tree->ComputeTransform(
-          data_from_ancestor.transform_tree_parent->transform_tree_index(),
-          data_from_ancestor.transform_fixed_parent->transform_tree_index(),
-          &parent_to_parent);
-
-      fixed_offset += parent_to_parent.To2dTranslation();
-      parent_offset += fixed_offset;
+    } else {
+      if (data_from_ancestor.transform_tree_parent !=
+          data_from_ancestor.transform_fixed_parent) {
+        gfx::Vector2dF fixed_offset = data_from_ancestor.transform_tree_parent
+                                          ->offset_to_transform_parent();
+        gfx::Transform parent_to_parent;
+        data_from_ancestor.transform_tree->ComputeTransform(
+            data_from_ancestor.transform_tree_parent->transform_tree_index(),
+            data_from_ancestor.transform_fixed_parent->transform_tree_index(),
+            &parent_to_parent);
+
+        fixed_offset += parent_to_parent.To2dTranslation();
+        parent_offset += fixed_offset;
+      }
+      parent_offset += data_from_ancestor.scroll_compensation_adjustment;
     }
   }
 
@@ -160,6 +165,11 @@ void AddTransformNodeIfNeeded(const DataForRecursion& data_from_ancestor,
     data_for_children->transform_fixed_parent = layer;
   data_for_children->transform_tree_parent = layer;
 
+  if (layer->IsContainerForFixedPositionLayers() || is_fixed)
+    data_for_children->scroll_compensation_adjustment = gfx::Vector2d();

[ajuma, 2015/03/26 18:03:25]

Does clearing here when is_fixed is true work when a fixed position layer has a
fixed position descendant (which has the same container)?

[Ian Vollick, 2015/03/30 17:51:25]

It does, yeah. The current GraphicsLayer tree is constructed in such a way that
fixed pos layers will descend from one another (as it's a stacking context) and
would therefore "descendant" fixed pos layers would get compensated by virtue of
their ancestor's compensation. This matches that behavior. For the CDP analog to
this, see the computation of
current_layer_resets_scroll_compensation_for_descendants in LTHC's
ComputeScrollCompensationMatrixForChildren.

+  data_for_children->scroll_compensation_adjustment +=
+      gfx::ToRoundedVector2d(layer->ScrollCompensationAdjustment());

[ajuma, 2015/03/26 18:03:25]

It's not obvious that rounding is the right thing to do here. Does this work
when the compensation is smaller than 0.5? For example, suppose we have two
scrollers, each with scroll compensation adjustment 0.1. Rounding at each step
will mean data_for_children has an adjustment value of 2. Is this correct?

[Ian Vollick, 2015/03/30 17:51:25]

You're right -- this isn't the right place for it. What's happening here was an
attempt to mimic the tweaking of the inherited scroll componsation adjustment to
account for scroll snapping. It turns out that this is much easier to do by
simply saving the scroll snap and applying it directly which I've done in the
latest patch.

+
   if (!requires_node) {
     data_for_children->should_flatten |= layer->should_flatten_transform();
     gfx::Vector2dF local_offset = layer->position().OffsetFromOrigin() +