Handle nested position:sticky elements correctly (compositor)

third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp

Index: third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
diff --git a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
index 183c283f6643d581e622967a72eaaa90dd9ddf8c..abc2fc5f4f8f295da5f10ea0a0ad935d7a52c5e8 100644
--- a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
+++ b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
@@ -173,6 +173,61 @@ static ScrollingCoordinator* scrollingCoordinatorFromLayer(PaintLayer& layer) {
   return (!page) ? nullptr : page->scrollingCoordinator();
 }
 
+static const LayoutBoxModelObject* nearestCompositedStickyAncestor(
+    const StickyConstraintsMap& constraintsMap,
+    const StickyPositionScrollingConstraints& constraints) {
+  const LayoutBoxModelObject* obj = constraints.nearestStickyAncestor();
+  while (obj && !obj->layer()->compositedLayerMapping()) {
+    DCHECK(constraintsMap.contains(obj->layer()));
+    obj = constraintsMap.at(obj->layer()).nearestStickyAncestor();
+  }
+  return obj;
+}
+
+// Return the id of the nearest composited sticky layer that shifts
+// |constraints| sticky box, or -1 if no such layer exists.
+static int nearestCompositedStickyLayerShiftingStickyBox(
+    const StickyConstraintsMap& constraintsMap,
+    const StickyPositionScrollingConstraints& constraints) {
+  LayoutBoxModelObject* obj = constraints.nearestStickyBoxShiftingStickyBox();
+  while (obj && !obj->layer()->compositedLayerMapping()) {
+    DCHECK(constraintsMap.contains(obj->layer()));
+    obj = constraintsMap.at(obj->layer()).nearestStickyBoxShiftingStickyBox();
+  }
+
+  if (!obj)
+    return -1;
+
+  return obj->layer()
+      ->compositedLayerMapping()
+      ->mainGraphicsLayer()
+      ->platformLayer()
+      ->id();
+}
+
+// Return the id of the nearest composited sticky layer that shifts
+// |constraints| containing block, or -1 if no such layer exists.
+static int nearestCompositedStickyLayerShiftingContainingBlock(
+    const StickyConstraintsMap& constraintsMap,
+    const StickyPositionScrollingConstraints& constraints) {
+  LayoutBoxModelObject* obj =
+      constraints.nearestStickyBoxShiftingContainingBlock();
+  while (obj && !obj->layer()->compositedLayerMapping()) {
+    DCHECK(constraintsMap.contains(obj->layer()));
+    obj = constraintsMap.at(obj->layer())
+              .nearestStickyBoxShiftingContainingBlock();
+  }
+
+  if (!obj)
+    return -1;
+
+  return obj->layer()
+      ->compositedLayerMapping()
+      ->mainGraphicsLayer()
+      ->platformLayer()
+      ->id();
+}
+
 CompositedLayerMapping::CompositedLayerMapping(PaintLayer& layer)
     : m_owningLayer(layer),
       m_contentOffsetInCompositingLayerDirty(false),
@@ -308,14 +363,24 @@ void CompositedLayerMapping::updateStickyConstraints(
 
   WebLayerStickyPositionConstraint webConstraint;
   if (sticky) {
+    const StickyConstraintsMap& constraintsMap =
+        ancestorOverflowLayer->getScrollableArea()->stickyConstraintsMap();
     const StickyPositionScrollingConstraints& constraints =
-        ancestorOverflowLayer->getScrollableArea()->stickyConstraintsMap().at(
-            &m_owningLayer);
+        constraintsMap.at(&m_owningLayer);
 
-    // Find the layout offset of the unshifted sticky box within its
-    // compositingContainer. If the enclosing layer is not the scroller, then
-    // the offset must be adjusted to include the scroll offset to keep it
-    // relative to compositingContainer.
+    // Find the layout offset of the unshifted sticky box within its parent
+    // composited layer. This information is used by the compositor side to
+    // compute the additional offset required to keep the element stuck under
+    // compositor scrolling.
+    //
+    // Starting from the scroll container relative location, removing the
+    // enclosing layers offset and the content offset in the composited layer
+    // results in the parent-layer relative offset.
+    FloatPoint parentRelativeStickyBoxOffset =
+        constraints.scrollContainerRelativeStickyBoxRect().location();
+
+    // The enclosing layers offset returned from |convertToLayerCoords| must be
+    // adjusted for both scroll and ancestor sticky elements.
     LayoutPoint enclosingLayerOffset;
     compositingContainer->convertToLayerCoords(ancestorOverflowLayer,
                                                enclosingLayerOffset);
@@ -323,12 +388,17 @@ void CompositedLayerMapping::updateStickyConstraints(
       enclosingLayerOffset += LayoutSize(
           ancestorOverflowLayer->getScrollableArea()->getScrollOffset());
     }
+    if (const LayoutBoxModelObject* ancestor =
+            nearestCompositedStickyAncestor(constraintsMap, constraints)) {
+      enclosingLayerOffset -=
+          roundedIntSize(constraintsMap.at(ancestor->layer())
+                             .getTotalContainingBlockStickyOffset());

[flackr, 2017/03/17 15:01:56]

Do we not want to remove all of the sticky offset applied to the ancestor? i.e.
getTotalStickyBoxStickyOffset. Maybe add a test where the ancestor is also
shifted by non containers.

[smcgruer, 2017/03/17 17:36:52]

That fails compositing/overflow/composited-nested-sticky-deep.html , because
m_totalStickyBoxStickyOffset is only ancestorStickyBoxOffset + stickyOffset, so
doesn't include the ancestorContainingBlockOffset, so for all of those nested
elements its just stickyOffset.

[flackr, 2017/03/17 18:04:08]

Oh right, but wouldn't this would still be a bug not to include the
non-container additional offsite? Maybe we should add a method to the
constraints like, pardon the long name, 
getTotalScrollContainerRelativeStickyOffset?

[smcgruer, 2017/03/17 18:42:42]

As per discussion, we already dothis.

+    }
 
-    FloatPoint stickyBoxOffset =
-        constraints.scrollContainerRelativeStickyBoxRect().location();
     DCHECK(!m_contentOffsetInCompositingLayerDirty);
-    stickyBoxOffset.moveBy(FloatPoint(-enclosingLayerOffset) -
-                           FloatSize(contentOffsetInCompositingLayer()));
+    parentRelativeStickyBoxOffset.moveBy(
+        FloatPoint(-enclosingLayerOffset) -
+        FloatSize(contentOffsetInCompositingLayer()));
 
     webConstraint.isSticky = true;
     webConstraint.isAnchoredLeft =
@@ -347,13 +417,17 @@ void CompositedLayerMapping::updateStickyConstraints(
     webConstraint.rightOffset = constraints.rightOffset();
     webConstraint.topOffset = constraints.topOffset();
     webConstraint.bottomOffset = constraints.bottomOffset();
-    webConstraint.parentRelativeStickyBoxOffset =
-        roundedIntPoint(stickyBoxOffset);
+    webConstraint.parentRelativeStickyBoxOffset = parentRelativeStickyBoxOffset;
     webConstraint.scrollContainerRelativeStickyBoxRect =
-        enclosingIntRect(constraints.scrollContainerRelativeStickyBoxRect());
-    webConstraint.scrollContainerRelativeContainingBlockRect = enclosingIntRect(
-        constraints.scrollContainerRelativeContainingBlockRect());
-    // TODO(smcgruer): Copy fields for nested sticky in cc (crbug.com/672710)
+        constraints.scrollContainerRelativeStickyBoxRect();
+    webConstraint.scrollContainerRelativeContainingBlockRect =
+        constraints.scrollContainerRelativeContainingBlockRect();
+    webConstraint.nearestLayerShiftingStickyBox =
+        nearestCompositedStickyLayerShiftingStickyBox(constraintsMap,
+                                                      constraints);
+    webConstraint.nearestLayerShiftingContainingBlock =
+        nearestCompositedStickyLayerShiftingContainingBlock(constraintsMap,
+                                                            constraints);
   }
 
   m_graphicsLayer->setStickyPositionConstraint(webConstraint);