Move scroll paint property nodes to be owned by the transform tree

third_party/WebKit/Source/core/paint/PaintLayer.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights
  * reserved.
  *
  * Portions are Copyright (C) 1998 Netscape Communications Corporation.
  *
  * Other contributors:
  *   Robert O'Callahan <roc+@cs.cmu.edu>
  *   David Baron <dbaron@fas.harvard.edu>
  *   Christian Biesinger <cbiesinger@web.de>
@@ skipping 309 matching lines @@
     // a self-painting descendant in this case, there is no need to dirty our
     // ancestors further.
     if (layer->isSelfPaintingLayer()) {
       DCHECK(!parent() || parent()->m_hasSelfPaintingLayerDescendantDirty ||
              parent()->m_hasSelfPaintingLayerDescendant);
       break;
     }
   }
 }
 
+static const ScrollPaintPropertyNode* nearestScrollNode(
+    const TransformPaintPropertyNode* transform) {
+  if (transform->isRoot())
+    return nullptr;
+  if (const auto* scrollNode = transform->scrollNode())
+    return scrollNode;
+  return nearestScrollNode(transform->parent());

[chrishtr, 2017/01/26 21:22:58]

Please make this an iteration instead of recursion, to avoid stack overflows.

[pdr., 2017/01/27 20:09:33]

Done.

I found a bug fixing this: the root transform node referenced a null scroll node
which allowed for the root of the scroll tree to be null if no scrolling
occurred. This will be fine when we remove the non-null root concept, but is
wrong now. I fixed this by making the root transform node reference the root
scroll node. This is tested in ScrollsWithViewportFixedPosition.

+}
+
 bool PaintLayer::sticksToViewport() const {
   if (layoutObject()->style()->position() != FixedPosition &&
       layoutObject()->style()->position() != StickyPosition)
     return false;
 
+  // TODO(pdr): This approach of calculating the nearest scroll node is O(n).
+  // An option for improving this is to cache the nearest scroll node in
+  // the local border box properties.
   if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
     const ScrollPaintPropertyNode* ancestorTargetScrollNode;
     if (layoutObject()->style()->position() == FixedPosition) {
-      ancestorTargetScrollNode = layoutObject()
-                                     ->view()
-                                     ->paintProperties()
-                                     ->localBorderBoxProperties()
-                                     ->scroll();
+      ancestorTargetScrollNode =
+          nearestScrollNode(layoutObject()

[pdr., 2017/01/26 20:04:54]

This part of the change is not entirely trivial and is a downside to the new
approach.

[wkorman, 2017/01/26 21:01:10]

Do existing tests cover this logic sufficiently or is it worth adding new
specifically for this?

[pdr., 2017/01/27 20:09:33]

I think we have pretty good coverage here. I wanted to fix this in a followup to
keep this patch simple, but found there were too many test failures. Some
examples:
PaintLayerTest.ScrollsWithViewportFixedPosition
PaintLayerTest.ScrollsWithViewportFixedPositionInsideTransform
PaintLayerTest.ScrollsWithViewportFixedPositionInsideTransformNoScroll

+                                ->view()
+                                ->paintProperties()
+                                ->localBorderBoxProperties()
+                                ->transform());
     } else {
-      ancestorTargetScrollNode = layoutObject()
-                                     ->view()
-                                     ->paintProperties()
-                                     ->contentsProperties()
-                                     ->scroll();
+      ancestorTargetScrollNode = nearestScrollNode(layoutObject()
+                                                       ->view()
+                                                       ->paintProperties()
+                                                       ->contentsProperties()
+                                                       ->transform());
     }
 
-    return layoutObject()
-               ->paintProperties()
-               ->localBorderBoxProperties()
-               ->scroll() == ancestorTargetScrollNode;
+    return nearestScrollNode(layoutObject()
+                                 ->paintProperties()
+                                 ->localBorderBoxProperties()
+                                 ->transform()) == ancestorTargetScrollNode;
   }
 
   return (layoutObject()->style()->position() == FixedPosition &&
           layoutObject()->containerForFixedPosition() ==
               layoutObject()->view()) ||
          (layoutObject()->style()->position() == StickyPosition &&
           (!ancestorScrollingLayer() || ancestorScrollingLayer() == root()));
 }
 
 bool PaintLayer::scrollsWithRespectTo(const PaintLayer* other) const {
@@ skipping 2870 matching lines @@
 }
 
 void showLayerTree(const blink::LayoutObject* layoutObject) {
   if (!layoutObject) {
     LOG(INFO) << "Cannot showLayerTree. Root is (nil)";
     return;
   }
   showLayerTree(layoutObject->enclosingLayer());
 }
 #endif