When performing a fast-path scroll, issue repaints for viewport-constrained layers

Source/core/frame/FrameView.cpp

Index: Source/core/frame/FrameView.cpp
diff --git a/Source/core/frame/FrameView.cpp b/Source/core/frame/FrameView.cpp
index 4b5e2eda8e9f66024f1aecb789beed729c4e07be..77965e1ab35e9d7ebf3f0fe8ce2e025082105183 100644
--- a/Source/core/frame/FrameView.cpp
+++ b/Source/core/frame/FrameView.cpp
@@ -1419,8 +1419,8 @@ bool FrameView::scrollContentsFastPath(const IntSize& scrollDelta, const IntRect
 
         IntRect updateRect = pixelSnappedIntRect(layer->repainter().repaintRectIncludingNonCompositingDescendants());
 
-        RenderLayer* enclosingCompositingLayer = layer->enclosingCompositingLayer(ExcludeSelf);
-        if (enclosingCompositingLayer && !enclosingCompositingLayer->renderer()->isRenderView()) {
+        const RenderLayerModelObject* repaintContainer = layer->renderer()->containerForRepaint();
+        if (repaintContainer && !repaintContainer->isRenderView()) {

[ojan, 2014/06/04 20:07:29]

Can we really get a null repaintContainer here? It looks to me that
containerForRepaint will only return null if the renderer isn't rooted. But if
it's not rooted I would expect we can't get to this code.

I'm not 100% sure. But, I'd be happier with an ASSERT(!repaintContainer) if it
passes the tests.

[chrishtr, 2014/06/04 20:11:41]

Yes it can happen. crbug.com/380147

[leviw_travelin_and_unemployed, 2014/06/04 20:37:25]

Do you have a test case? We shouldn't be getting a null repaint value here...

[chrishtr, 2014/06/04 21:18:57]

compositing/reflections/nested-reflection-transformed.html

It happens while building the DOM tree:

<null>
 0x7fafae82f77e WebCore::RenderObject::boundsRectForRepaint()
#4 0x7fafae7f99e3 WebCore::RenderLayerRepainter::computeRepaintRects()
#5 0x7fafae7f97a2 WebCore::RenderLayerRepainter::repaintAfterLayout()
#6 0x7fafae7d6f0b WebCore::RenderLayer::updateLayerPositions()
#7 0x7fafae7f8258 WebCore::RenderLayerModelObject::styleDidChange()
#8 0x7fafae73fddc WebCore::RenderBox::styleDidChange()
#9 0x7fafae832f99 WebCore::RenderObject::setStyle()
#10 0x7fafae7f9445 WebCore::RenderLayerReflectionInfo::updateAfterStyleChange()
#11 0x7fafae7dc712 WebCore::RenderLayer::updateReflectionInfo()
#12 0x7fafae7e75be WebCore::RenderLayer::styleChanged()
#13 0x7fafae7f8378 WebCore::RenderLayerModelObject::styleDidChange()
#14 0x7fafae73fddc WebCore::RenderBox::styleDidChange()
#15 0x7fafae6da956 WebCore::RenderBlock::styleDidChange()
#16 0x7fafae71ca16 WebCore::RenderBlockFlow::styleDidChange()
#17 0x7fafae832f99 WebCore::RenderObject::setStyle()
#18 0x7fafadcb04c1
WebCore::RenderTreeBuilder::createRendererForElementIfNeeded()
#19 0x7fafadc36a4f WebCore::Element::attach()
#20 0x7fafadbb90d3 WebCore::ContainerNode::attachChildren()
#21 0x7fafadbb531f WebCore::ContainerNode::attach()
#22 0x7fafadc36ab7 WebCore::Element::attach()

[ojan, 2014/06/07 17:35:23]

I don't see scrollContentsFastPath on that stack. I know that we can get null
repaintContainers in general (i.e. for renderers that are not rooted), but what
I'm asking is if we can get them in scrollContentsFastPath.

[chrishtr, 2014/06/09 16:40:17]

Ok I see, I misinterpreted your question. It does look impossible. I'll add the
assert.

             // If the fixed-position layer is contained by a composited layer that is not its containing block,
             // then we have to invalidate that enclosing layer, not the RenderView.
             // FIXME: Why do we need to issue this invalidation? Won't the fixed position element just scroll
@@ -1429,7 +1429,7 @@ bool FrameView::scrollContentsFastPath(const IntSize& scrollDelta, const IntRect
             IntRect previousRect = updateRect;
             previousRect.move(scrollDelta);
             updateRect.unite(previousRect);
-            enclosingCompositingLayer->repainter().setBackingNeedsRepaintInRect(updateRect);
+            layer->renderer()->repaintUsingContainer(repaintContainer, updateRect, InvalidationScroll);
         } else {
             // Coalesce the repaints that will be issued to the renderView.
             updateRect = contentsToRootView(updateRect);