Disentangle repaint-after-compositing from updateLayerPositionsAfterLayout

Source/core/rendering/RenderBox.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  *           (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2013 Adobe Systems Incorporated. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 1539 matching lines @@
 void RenderBox::invalidateTreeIfNeeded(const PaintInvalidationState& paintInvalidationState)
 {
     // FIXME: Currently only using this logic for RenderBox and its ilk. Ideally, RenderBlockFlows with
     // inline children should track a dirty rect in local coordinates for dirty lines instead of invalidating
     // the world.
     // FIXME: We should still be recursing through inline's children, as they can have boxes, but we don't
     // appear to have tests for this?
     // FIXME: SVG should probably also go through this unified paint invalidation system.
     ASSERT(!needsLayout());
 
-    if (!shouldCheckForPaintInvalidation())
+    if (!shouldCheckForPaintInvalidation(paintInvalidationState))
         return;
 
     bool establishesNewPaintInvalidationContainer = isPaintInvalidationContainer();
     const RenderLayerModelObject& newPaintInvalidationContainer = *adjustCompositedContainerForSpecialAncestors(establishesNewPaintInvalidationContainer ? this : &paintInvalidationState.paintInvalidationContainer());
     ASSERT(&newPaintInvalidationContainer == containerForPaintInvalidation());
 
-    invalidatePaintIfNeeded(paintInvalidationState, newPaintInvalidationContainer);
+    bool didFullyInvalidatePaint = invalidatePaintIfNeeded(paintInvalidationState, newPaintInvalidationContainer);
 
     // This is for the next invalidatePaintIfNeeded so must be after invalidatePaintIfNeeded.
     savePreviousBorderBoxSizeIfNeeded();
 
     PaintInvalidationState childTreeWalkState(paintInvalidationState, *this, newPaintInvalidationContainer);
+    if (didFullyInvalidatePaint)

[Julien - ping for review, 2014/08/01 21:20:01]

AFAICT this will make us force paint invalidation for other reasons than a
containing block shift. I would bet it's the reason why we have all these extra
invalidations. Those invalidations are not over-painting but they are a sign
that we are doing too much work.

I would be more comfortable if we limited this condition to only when we
actually need it.

[abarth-chromium, 2014/08/01 21:22:51]

Ok.  I'll tighten this down.

+        childTreeWalkState.setForceCheckForPaintInvalidation();
     RenderObject::invalidateTreeIfNeeded(childTreeWalkState);
 }
 
-void RenderBox::invalidatePaintIfNeeded(const PaintInvalidationState& paintInvalidationState, const RenderLayerModelObject& newPaintInvalidationContainer)
+bool RenderBox::invalidatePaintIfNeeded(const PaintInvalidationState& paintInvalidationState, const RenderLayerModelObject& newPaintInvalidationContainer)
 {
     const LayoutRect oldPaintInvalidationRect = previousPaintInvalidationRect();
     const LayoutPoint oldPositionFromPaintInvalidationContainer = previousPositionFromPaintInvalidationContainer();
     setPreviousPaintInvalidationRect(boundsRectForPaintInvalidation(&newPaintInvalidationContainer, &paintInvalidationState));
     setPreviousPositionFromPaintInvalidationContainer(RenderLayer::positionFromPaintInvalidationContainer(this, &newPaintInvalidationContainer, &paintInvalidationState));
 
     // If we are set to do a full paint invalidation that means the RenderView will be
     // issue paint invalidations. We can then skip issuing of paint invalidations for the child
     // renderers as they'll be covered by the RenderView.
     if (view()->doingFullPaintInvalidation())
-        return;
+        return false;
 
     if ((onlyNeededPositionedMovementLayout() && compositingState() != PaintsIntoOwnBacking)
         || (shouldDoFullPaintInvalidationIfSelfPaintingLayer()
             && hasLayer()
             && layer()->isSelfPaintingLayer())) {
         setShouldDoFullPaintInvalidation(true, MarkOnlyThis);
     }
 
-    if (!RenderObject::invalidatePaintIfNeeded(newPaintInvalidationContainer, oldPaintInvalidationRect, oldPositionFromPaintInvalidationContainer, paintInvalidationState))
+    bool didFullyInvalidatePaint = RenderObject::invalidatePaintIfNeeded(newPaintInvalidationContainer, oldPaintInvalidationRect, oldPositionFromPaintInvalidationContainer, paintInvalidationState);
+    if (!didFullyInvalidatePaint)
         invalidatePaintForOverflowIfNeeded();
 
     // Issue paint invalidations for any scrollbars if there is a scrollable area for this renderer.
-    if (enclosingLayer()) {
-        if (RenderLayerScrollableArea* area = enclosingLayer()->scrollableArea()) {
+    if (RenderLayer* layer = enclosingLayer()) {
+        if (RenderLayerScrollableArea* area = layer->scrollableArea()) {

[Julien - ping for review, 2014/08/01 21:20:01]

I wouldn't touch this, we spotted that this code doesn't seem right:
enclosingLayer() can repeatedly return the same RenderLayer in a subtree leading
to unneeded invalidations. We will change or refactor this code anyway.

[abarth-chromium, 2014/08/01 21:22:51]

ok

             if (area->hasVerticalBarDamage())
                 invalidatePaintRectangle(area->verticalBarDamage());
             if (area->hasHorizontalBarDamage())
                 invalidatePaintRectangle(area->horizontalBarDamage());
             area->resetScrollbarDamage();
         }
     }
+
+    return didFullyInvalidatePaint;
 }
 
 bool RenderBox::pushContentsClip(PaintInfo& paintInfo, const LayoutPoint& accumulatedOffset, ContentsClipBehavior contentsClipBehavior)
 {
     if (paintInfo.phase == PaintPhaseBlockBackground || paintInfo.phase == PaintPhaseSelfOutline || paintInfo.phase == PaintPhaseMask)
         return false;
 
     bool isControlClip = hasControlClip();
     bool isOverflowClip = hasOverflowClip() && !layer()->isSelfPaintingLayer();
 
@@ skipping 3087 matching lines @@
 RenderBox::BoxDecorationData::BoxDecorationData(const RenderStyle& style)
 {
     backgroundColor = style.visitedDependentColor(CSSPropertyBackgroundColor);
     hasBackground = backgroundColor.alpha() || style.hasBackgroundImage();
     ASSERT(hasBackground == style.hasBackground());
     hasBorder = style.hasBorder();
     hasAppearance = style.hasAppearance();
 }
 
 } // namespace blink