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 2b11d48670b148fad2330a31a67ba95c030d952d..49642c115df62dfa6cc209f3b10720cf048b84b2 100644
--- a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
+++ b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
@@ -172,6 +172,27 @@ static ScrollingCoordinator* scrollingCoordinatorFromLayer(PaintLayer& layer) {
   return (!page) ? nullptr : page->scrollingCoordinator();
 }
 
+static WebLayer* nearestCompositedStickyLayer(
+    const StickyConstraintsMap& constraintsMap,
+    LayoutBoxModelObject* obj) {
+  while (obj && obj->compositingState() == NotComposited) {

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

I think technically you want to find compositingState() == PaintsIntoOwnBacking,
but it's probably better / more consistent to just check if the object has a
compositedLayerMapping since that's what we intend to access.

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

Done.

+    DCHECK(constraintsMap.contains(obj->layer()));
+    const StickyPositionScrollingConstraints constraints =
+        constraintsMap.at(obj->layer());
+    obj = constraints.nearestStickyBoxShiftingStickyBox()

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

This seems wrong, don't we need to follow the same chain as the one we started
on? i.e. if we're interested in nearestStickyBoxShiftingContainingBlock then
only follow those pointers?

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

I believe you're correct, done. Adding a test is waiting on resolving copying
back pointers to main from cc.

+              ? constraints.nearestStickyBoxShiftingStickyBox()
+              : constraints.nearestStickyBoxShiftingContainingBlock();
+  }
+
+  if (!obj)
+    return nullptr;
+
+  return obj->layer()
+      ->compositedLayerMapping()
+      ->mainGraphicsLayer()
+      ->platformLayer();
+}
+
 CompositedLayerMapping::CompositedLayerMapping(PaintLayer& layer)
     : m_owningLayer(layer),
       m_contentOffsetInCompositingLayerDirty(false),
@@ -307,9 +328,10 @@ 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
@@ -352,7 +374,13 @@ void CompositedLayerMapping::updateStickyConstraints(
         enclosingIntRect(constraints.scrollContainerRelativeStickyBoxRect());
     webConstraint.scrollContainerRelativeContainingBlockRect = enclosingIntRect(
         constraints.scrollContainerRelativeContainingBlockRect());
-    // TODO(smcgruer): Copy fields for nested sticky in cc (crbug.com/672710)
+    webConstraint.nearestLayerShiftingStickyBox = nearestCompositedStickyLayer(
+        constraintsMap, constraints.nearestStickyBoxShiftingStickyBox());
+    webConstraint.nearestLayerShiftingContainingBlock =
+        nearestCompositedStickyLayer(
+            constraintsMap,
+            constraints.nearestStickyBoxShiftingContainingBlock());
+    webConstraint.localStickyOffset = constraints.getLocalStickyOffset();

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

This seems to be passed in, but unused as far as I can tell. I'm inclined to
think that we shouldn't pass this in since it changes with every scroll but we
don't update the constraints every scroll.

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

Its used in draw_property_utils::CalculateTotalStickyOffsetToScroller. We need
it to adjust the main_thread_offset.

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

As discussed, we should try to work out the local offset on the compositor side
since it will change with calls to Layer::SetPosition without the sticky
constraints being updated.

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

Done.

   }
 
   m_graphicsLayer->setStickyPositionConstraint(webConstraint);