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

Source/core/frame/FrameView.cpp

 /*
  * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
  *                     1999 Lars Knoll <knoll@kde.org>
  *                     1999 Antti Koivisto <koivisto@kde.org>
  *                     2000 Dirk Mueller <mueller@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  *           (C) 2006 Graham Dennis (graham.dennis@gmail.com)
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
@@ skipping 1401 matching lines @@
         }
 
         if (layer->hasAncestorWithFilterOutsets()) {
             // If the fixed layer has a blur/drop-shadow filter applied on at least one of its parents, we cannot
             // scroll using the fast path, otherwise the outsets of the filter will be moved around the page.
             return false;
         }
 
         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
             //        with the enclosing layer.
             updateRect.moveBy(scrollPosition());
             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);
             if (!isCompositedContentLayer && clipsRepaints())
                 updateRect.intersect(rectToScroll);
             if (!updateRect.isEmpty())
                 regionToUpdate.unite(updateRect);
         }
     }
 
@@ skipping 1830 matching lines @@
 void FrameView::willRemoveScrollbar(Scrollbar* scrollbar, ScrollbarOrientation orientation)
 {
     ScrollableArea::willRemoveScrollbar(scrollbar, orientation);
     if (AXObjectCache* cache = axObjectCache()) {
         cache->remove(scrollbar);
         cache->handleScrollbarUpdate(this);
     }
 }
 
 } // namespace WebCore