Handle nested position:sticky elements

cc/trees/property_tree.cc

Index: cc/trees/property_tree.cc
diff --git a/cc/trees/property_tree.cc b/cc/trees/property_tree.cc
index 9850577ff4ed778e9f6fd3ce8eb87e7cd57ef092..5d66017d8fd895e7e1b09e6f2f365faab98f965b 100644
--- a/cc/trees/property_tree.cc
+++ b/cc/trees/property_tree.cc
@@ -346,9 +346,8 @@ bool TransformTree::CombineInversesBetween(int source_id,
 gfx::Vector2dF StickyPositionOffset(TransformTree* tree, TransformNode* node) {
   if (node->sticky_position_constraint_id == -1)
     return gfx::Vector2dF();
-  const StickyPositionNodeData* sticky_data =
-      tree->StickyPositionData(node->id);
-  const LayerStickyPositionConstraint& constraint = sticky_data->constraints;
+  StickyPositionNodeData* sticky_data = tree->StickyPositionData(node->id);
+  LayerStickyPositionConstraint& constraint = sticky_data->constraints;
   ScrollNode* scroll_node =
       tree->property_trees()->scroll_tree.Node(sticky_data->scroll_ancestor);
   gfx::ScrollOffset scroll_offset =
@@ -368,9 +367,15 @@ gfx::Vector2dF StickyPositionOffset(TransformTree* tree, TransformNode* node) {
       scroll_position,
       gfx::SizeF(tree->property_trees()->scroll_tree.scroll_clip_layer_bounds(
           scroll_node->id)));
-  gfx::Vector2dF sticky_offset(
-      constraint.scroll_container_relative_sticky_box_rect.OffsetFromOrigin());
-  gfx::Vector2dF layer_offset(sticky_data->main_thread_offset);
+  // TODO(smcgruer): This is very likely wrong, but the MTO seems wrong for
+  // nested elements. For example, in https://output.jsbin.com/bovegil it
+  // is [0,30] for the outer (correct) but then [0,-50] for the middle and
+  // [0,-25] for the inner (that seems wrong).

[flackr, 2017/02/02 18:18:11]

Sounds like we need to update the way this is computed in
CompositedLayerMapping.cpp to be the additional main thread sticky offset from
the nearest ancestor sticky element? In your demo we want the inner ones to have
a main thread offset of 0 (as in 0 additional main thread sticky offset from the
already offset top sticky element).

[smcgruer, 2017/02/02 20:21:26]

Is there any guarantee in CompositedLayerMapping::updateStickyConstraints that
main thread stick calculation will have happened? (Or that it is being called
top-down on the sticky tree.)

[flackr, 2017/02/02 23:31:15]

Yes, compositing inputs are clean at at that point and during the compositing
inputs update we update the positions of sticky position layers so we will have
already computed the sticky position offset top-down.

+  LOG(INFO) << "Node " << node->id << ": main_thread_offset is "
+            << sticky_data->main_thread_offset.ToString();
+  gfx::Vector2dF layer_offset;
+  if (!constraint.nearest_sticky_element_shifting_containing_block)
+    layer_offset = sticky_data->main_thread_offset;
 
   // In each of the following cases, we measure the limit which is the point
   // that the element should stick to, clamping on one side to 0 (because sticky
@@ -381,57 +386,102 @@ gfx::Vector2dF StickyPositionOffset(TransformTree* tree, TransformNode* node) {
   // Note: The order of applying the sticky constraints is applied such that
   // left offset takes precedence over right offset, and top takes precedence
   // over bottom offset.
+
+  // TODO(smcgruer): I'm not sure why, but at points when scrolling this method
+  // is called with valid ancestor pointers *BUT* they have a
+  // transform_tree_index of -1.

[flackr, 2017/02/02 18:18:11]

We can't access the Layers at this point, I believe they will be invalid at the
point when a new tree is committed. I think we should convert the layer pointers
to transform tree indices when generating the sticky position data for the
transform tree.

[smcgruer, 2017/02/02 20:21:26]

Done.

+
+  gfx::Vector2dF ancestor_sticky_box_offset;
+  if (constraint.nearest_sticky_element_shifting_sticky_box &&
+      constraint.nearest_sticky_element_shifting_sticky_box
+              ->transform_tree_index() >= 0) {
+    const StickyPositionNodeData* sticky_data = tree->StickyPositionData(
+        constraint.nearest_sticky_element_shifting_sticky_box
+            ->transform_tree_index());
+    ancestor_sticky_box_offset =
+        sticky_data->constraint.total_sticky_box_sticky_offset;
+  }
+
+  gfx::Vector2dF ancestor_containing_block_offset;
+  if (constraint.nearest_sticky_element_shifting_containing_block &&
+      constraint.nearest_sticky_element_shifting_containing_block
+              ->transform_tree_index() >= 0) {
+    const StickyPositionNodeData* sticky_data = tree->StickyPositionData(
+        constraint.nearest_sticky_element_shifting_containing_block
+            ->transform_tree_index());
+    ancestor_containing_block_offset =
+        sticky_data->constraint.total_containing_block_sticky_offset;
+  }
+
+  gfx::Rect sticky_box_rect =
+      constraint.scroll_container_relative_sticky_box_rect;
+  gfx::Rect containing_block_rect =
+      constraint.scroll_container_relative_containing_block_rect;
+  sticky_box_rect.Offset(ancestor_sticky_box_offset.x(),
+                         ancestor_sticky_box_offset.y());
+  sticky_box_rect.Offset(ancestor_containing_block_offset.x(),
+                         ancestor_containing_block_offset.y());
+  containing_block_rect.Offset(ancestor_containing_block_offset.x(),
+                               ancestor_containing_block_offset.y());
+
+  gfx::Vector2dF sticky_offset(sticky_box_rect.OffsetFromOrigin());
+
   if (constraint.is_anchored_right) {
     float right_limit = clip.right() - constraint.right_offset;
-    float right_delta = std::min<float>(
-        0, right_limit -
-               constraint.scroll_container_relative_sticky_box_rect.right());
-    float available_space = std::min<float>(
-        0, constraint.scroll_container_relative_containing_block_rect.x() -
-               constraint.scroll_container_relative_sticky_box_rect.x());
+    float right_delta =
+        std::min<float>(0, right_limit - sticky_box_rect.right());
+    float available_space =
+        std::min<float>(0, containing_block_rect.x() - sticky_box_rect.x());
     if (right_delta < available_space)
       right_delta = available_space;
     sticky_offset.set_x(sticky_offset.x() + right_delta);
   }
   if (constraint.is_anchored_left) {
     float left_limit = clip.x() + constraint.left_offset;
-    float left_delta = std::max<float>(
-        0,
-        left_limit - constraint.scroll_container_relative_sticky_box_rect.x());
+    float left_delta = std::max<float>(0, left_limit - sticky_box_rect.x());
     float available_space = std::max<float>(
-        0, constraint.scroll_container_relative_containing_block_rect.right() -
-               constraint.scroll_container_relative_sticky_box_rect.right());
+        0, containing_block_rect.right() - sticky_box_rect.right());
     if (left_delta > available_space)
       left_delta = available_space;
     sticky_offset.set_x(sticky_offset.x() + left_delta);
   }
   if (constraint.is_anchored_bottom) {
     float bottom_limit = clip.bottom() - constraint.bottom_offset;
-    float bottom_delta = std::min<float>(
-        0, bottom_limit -
-               constraint.scroll_container_relative_sticky_box_rect.bottom());
-    float available_space = std::min<float>(
-        0, constraint.scroll_container_relative_containing_block_rect.y() -
-               constraint.scroll_container_relative_sticky_box_rect.y());
+    float bottom_delta =
+        std::min<float>(0, bottom_limit - sticky_box_rect.bottom());
+    float available_space =
+        std::min<float>(0, containing_block_rect.y() - sticky_box_rect.y());
     if (bottom_delta < available_space)
       bottom_delta = available_space;
     sticky_offset.set_y(sticky_offset.y() + bottom_delta);
   }
   if (constraint.is_anchored_top) {
     float top_limit = clip.y() + constraint.top_offset;
-    float top_delta = std::max<float>(
-        0,
-        top_limit - constraint.scroll_container_relative_sticky_box_rect.y());
+    float top_delta = std::max<float>(0, top_limit - sticky_box_rect.y());
     float available_space = std::max<float>(
-        0, constraint.scroll_container_relative_containing_block_rect.bottom() -
-               constraint.scroll_container_relative_sticky_box_rect.bottom());
+        0, containing_block_rect.bottom() - sticky_box_rect.bottom());
     if (top_delta > available_space)
       top_delta = available_space;
     sticky_offset.set_y(sticky_offset.y() + top_delta);
   }
-  return sticky_offset - layer_offset - node->source_to_parent -
-         constraint.scroll_container_relative_sticky_box_rect
-             .OffsetFromOrigin();
+
+  constraint.total_sticky_box_sticky_offset =
+      ancestor_sticky_box_offset + sticky_offset -
+      sticky_box_rect.OffsetFromOrigin();
+  constraint.total_containing_block_sticky_offset =
+      ancestor_containing_block_offset + sticky_offset -
+      sticky_box_rect.OffsetFromOrigin();
+
+  // TODO(smcgruer): Confusing bit here. With the current state of the code, the
+  // following values are returned for https://output.jsbin.com/bovegil : [0,0]
+  // for the outer, [0,0] for the middle, and [0,25] for the inner. That *seems*
+  // like the wrong value for the outer and the right value for the inner, but
+  // visually it is correct for outer and wrong for inner! (outer is offset
+  // correctly, inner is offset too far...).

[flackr, 2017/02/02 18:18:11]

Keep in mind what we are returning is the additional offset which has not
already been applied to the layer from the main thread, [0, 0] is the correct
sticky offset until we do some additional cc scrolling.

[smcgruer, 2017/02/02 20:21:26]

Ah, makes sense. 

+  gfx::Vector2dF result(sticky_offset - layer_offset - node->source_to_parent -
+                        sticky_box_rect.OffsetFromOrigin());
+  LOG(INFO) << "Node " << node->id << ": result is " << result.ToString();
+  return result;
 }
 
 void TransformTree::UpdateLocalTransform(TransformNode* node) {