Handle nested position:sticky elements correctly (compositor)

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 ef843d5d9890e67baba1396156fbca65bf06cb85..de37d5661d60bbcd5e5f435715c533032beaa2ef 100644
--- a/cc/trees/property_tree_builder.cc
+++ b/cc/trees/property_tree_builder.cc
@@ -513,6 +513,19 @@ static inline bool ShouldFlattenTransform(LayerImpl* layer) {
   return layer->test_properties()->should_flatten_transform;
 }
 
+static inline gfx::Vector2dF CalculateTotalStickyOffsetToScroller(
+    LayerImpl* layer,
+    int scroll_ancestor_layer_id) {
+  return gfx::Vector2dF();

[flackr, 2017/03/09 20:22:08]

Add NOTREACHED() or equivalent in this function.

[smcgruer, 2017/03/10 16:37:10]

Done.

+}
+
+static inline gfx::Vector2dF CalculateTotalStickyOffsetToScroller(
+    Layer* layer,
+    int scroll_ancestor_layer_id) {
+  return draw_property_utils::CalculateTotalStickyOffsetToScroller(
+      layer, scroll_ancestor_layer_id);
+}
+
 template <typename LayerType>
 bool AddTransformNodeIfNeeded(
     const DataForRecursion<LayerType>& data_from_ancestor,
@@ -759,10 +772,29 @@ bool AddTransformNodeIfNeeded(
             .AddNodeAffectedByOuterViewportBoundsDelta(node->id);
       }
     }
-    sticky_data->main_thread_offset =
-        layer->position().OffsetFromOrigin() -
-        sticky_data->constraints.parent_relative_sticky_box_offset
-            .OffsetFromOrigin();
+    // The sticky box offset calculated in CompositedLayerMapping must be
+    // adjusted for the enclosing layer's sticky offset.
+    // TODO(smcgruer): Maybe do this blink side instead.

[flackr, 2017/03/09 20:22:08]

Given that we may have to do this on the cc side for layer position updates
there's probably not much to be saved avoiding it here, although we could sync
the accumulated offset over so that it doesn't need to be recalculated here.

[smcgruer, 2017/03/10 16:37:10]

Acknowledged.

+    gfx::Point sticky_box_offset(
+        sticky_data->constraints.parent_relative_sticky_box_offset);
+    if (Parent(layer)->id() != scroll_ancestor->owning_layer_id) {
+      sticky_box_offset +=
+          gfx::ToFlooredVector2d(CalculateTotalStickyOffsetToScroller(
+              Parent(layer), scroll_ancestor->owning_layer_id));
+    }
+    sticky_data->main_thread_offset = layer->position().OffsetFromOrigin() -
+                                      sticky_box_offset.OffsetFromOrigin();

[flackr, 2017/03/09 20:22:08]

It looks like you pass in the main thread offset in the constraint's
localStickyOffset.

[smcgruer, 2017/03/10 16:37:10]

I don't quite follow this comment; can you clarify?

[flackr, 2017/03/13 19:49:45]

I think we chatted offline about this. There is a main thread offset being
passed in as part of the constraints called localStickyOffset but this will be
stale after some scrolling happens because we shouldn't regularly update the
sticky constraints.

[smcgruer, 2017/03/14 22:04:06]

Done.

+    // Copy the ancestor nodes for later use.

[flackr, 2017/03/09 20:22:08]

It's probably worth pointing out in the comment that because these point to
ancestors they should already be set.

[smcgruer, 2017/03/10 16:37:10]

Done.

+    if (Layer* layer =
+            sticky_data->constraints.nearest_layer_shifting_sticky_box) {
+      sticky_data->nearest_node_shifting_sticky_box =
+          layer->transform_tree_index();
+    }
+    if (Layer* layer =
+            sticky_data->constraints.nearest_layer_shifting_containing_block) {
+      sticky_data->nearest_node_shifting_containing_block =
+          layer->transform_tree_index();
+    }
   }
 
   node->needs_local_transform_update = true;