Initial scroll-blocks-on compositor integration

Source/core/rendering/compositing/CompositingReasonFinder.cpp

Index: Source/core/rendering/compositing/CompositingReasonFinder.cpp
diff --git a/Source/core/rendering/compositing/CompositingReasonFinder.cpp b/Source/core/rendering/compositing/CompositingReasonFinder.cpp
index 4a1613a74dc17a88fd34aa59b9a906896da76d94..87ff3b36fb0a90a035a76d141961ea056f92f23e 100644
--- a/Source/core/rendering/compositing/CompositingReasonFinder.cpp
+++ b/Source/core/rendering/compositing/CompositingReasonFinder.cpp
@@ -49,6 +49,11 @@ CompositingReasons CompositingReasonFinder::directReasons(const RenderLayer* lay
 {
     ASSERT(potentialCompositingReasonsFromStyle(layer->renderer()) == layer->potentialCompositingReasonsFromStyle());
     CompositingReasons styleDeterminedDirectCompositingReasons = layer->potentialCompositingReasonsFromStyle() & CompositingReasonComboAllDirectStyleDeterminedReasons;
+
+    // Apply optimizations for scroll-blocks-on which require comparing style between objects.
+    if ((styleDeterminedDirectCompositingReasons & CompositingReasonScrollBlocksOn) && !requiresCompositingForScrollBlocksOn(layer->renderer()))
+        styleDeterminedDirectCompositingReasons &= ~CompositingReasonScrollBlocksOn;
+
     return styleDeterminedDirectCompositingReasons | nonStyleDeterminedDirectReasons(layer);
 }
 
@@ -93,6 +98,9 @@ CompositingReasons CompositingReasonFinder::potentialCompositingReasonsFromStyle
     if (style->hasPerspective())
         reasons |= CompositingReasonPerspectiveWith3DDescendants;
 
+    if (style->hasScrollBlocksOn() && !renderer->isDocumentElement())
+        reasons |= CompositingReasonScrollBlocksOn;
+
     // If the implementation of createsGroup changes, we need to be aware of that in this part of code.
     ASSERT((renderer->isTransparent() || renderer->hasMask() || renderer->hasFilter() || style->hasBlendMode()) == renderer->createsGroup());
 
@@ -122,7 +130,7 @@ CompositingReasons CompositingReasonFinder::potentialCompositingReasonsFromStyle
 bool CompositingReasonFinder::requiresCompositingForTransform(RenderObject* renderer) const
 {
     // Note that we ask the renderer if it has a transform, because the style may have transforms,
-    // but the renderer may be an inline that doesn't suppport them.
+    // but the renderer may be an inline that doesn't support them.
     return renderer->hasTransformRelatedProperty() && renderer->style()->transform().has3DOperation();
 }
 
@@ -170,4 +178,41 @@ bool CompositingReasonFinder::requiresCompositingForPositionFixed(const RenderLa
     return layer->scrollsWithViewport() && m_renderView.frameView()->isScrollable();
 }
 
+bool CompositingReasonFinder::requiresCompositingForScrollBlocksOn(const RenderObject* renderer) const
+{
+    // Note that the other requires* functions run at RenderObject::styleDidChange time and so can rely
+    // only on the style of their object.  This function runs at CompositingRequirementsUpdater::update
+    // time, and so can consider the style of other objects.
+    RenderStyle* style = renderer->style();
+
+    // We should only get here by CompositingReasonScrollBlocksOn being a potential compositing reason.
+    ASSERT(style->hasScrollBlocksOn() && !renderer->isDocumentElement());
+
+    // scroll-blocks-on style is propagated from the document element to the document.
+    ASSERT(!renderer->isRenderView()
+        || !renderer->document().documentElement()
+        || !renderer->document().documentElement()->renderer()
+        || renderer->document().documentElement()->renderer()->style()->scrollBlocksOn() == style->scrollBlocksOn());
+
+    // Ensure iframes don't get composited unnecessarily.
+    if (renderer->isRenderView()) {
+        if (const FrameView* parentFrame = toRenderView(renderer)->frameView()->parentFrameView()) {
+            if (const RenderView* parentRenderer = parentFrame->renderView()) {

[Ian Vollick, 2015/01/06 04:13:51]

I'm a bit confused by this, but I'm sure it's just me misunderstanding the spec.
Isn't it possible to have a scrolling div within the parent frame that sets
scroll blocks on? Why do we check the parent frame only and not the containing
scroller?

[Rick Byers, 2015/01/08 21:08:11]

It's the union of all bits set in the containing block ancestor list that
matters.  At a minimum I need to avoid promoting all iframes to composited
layers when scroll-blocks-on hasn't been explicitly used by the developer at
all.  So this is just the simplest possible check that the bits in this frame
_might_ be relevant (there redundant if any ancestor contains the same bits, but
it's only the root that has any bits set by default).

[Ian Vollick, 2015/01/09 15:17:33]

Got it. Thank you for the detailed comment; it really helped.

+                // Does this frame contain only blocks-on bits already present in the parent frame?

[skobes, 2015/01/07 18:49:24]

What happens if we have fewer blocks-on bits than the parent?  Wouldn't we also
want to composite in that case?

[Rick Byers, 2015/01/08 21:08:11]

No, see the doc and CC side of the patch.  Since this property isn't inherited
(due to feedback from esprehn about potential style cascade cost), at scroll
time we compute the effective value from the union of all bits in the ancestor
chain.  So if we have fewer bits than the parent than it's redundant.

I've added comments to try to make this clearer here.

+                if (!(style->scrollBlocksOn() & ~parentRenderer->style()->scrollBlocksOn()))
+                    return false;
+            }
+        }
+        return true;
+    }
+
+    // NOTE: Really I only need a new layer if this object contains additional bits from those
+    // set by all objects in it's containing block chain.  Such an optimization would avoid layer
+    // explosion in pathological cases like '*' rules.  We could consider tracking the current
+    // state in CompositingRequirementsUpdater::update or perhaps just apply some simpler
+    // optimizations here (like testing only against our direct containing block).
+
+    return true;
+}
+
 }