Use unified invalidation path for repaint-only style changes

Source/core/rendering/RenderObject.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Dirk Mueller (mueller@kde.org)
  *           (C) 2004 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Google Inc. All rights reserved.
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 1580 matching lines @@
         return "invalidate paint rectangle";
     }
     ASSERT_NOT_REACHED();
     return "";
 }
 
 void RenderObject::invalidateTreeIfNeeded(const PaintInvalidationState& paintInvalidationState)
 {
     // If we didn't need paint invalidation then our children don't need as well.
     // Skip walking down the tree as everything should be fine below us.
-    if (!shouldCheckForPaintInvalidation())
+    if (!shouldCheckForSelfOrChildPaintInvalidation())
         return;
 
     clearPaintInvalidationState();
 
     for (RenderObject* child = slowFirstChild(); child; child = child->nextSibling()) {
         if (!child->isOutOfFlowPositioned())
             child->invalidateTreeIfNeeded(paintInvalidationState);
     }
 }
 
@@ skipping 436 matching lines @@
             setNeedsLayoutAndPrefWidthsRecalc();
         else if (updatedDiff.needsPositionedMovementLayout())
             setNeedsPositionedMovementLayout();
     }
 
     if (diff.transformChanged() && !needsLayout()) {
         if (RenderBlock* container = containingBlock())
             container->setNeedsOverflowRecalcAfterStyleChange();
     }
 
-    if (updatedDiff.needsRepaint()) {
-        // Invalidate paints with the new style, e.g., for example if we go from not having
-        // an outline to having an outline.
-
-        // The paintInvalidationForWholeRenderer() call is needed for non-layout changes to style. See the corresponding
-        // comment in RenderObject::styleWillChange for why.
-        if (needsLayout())
+    // FIXME: Should invalidate whole layer if updatedDiff.needsRepaintLayer(). crbug.com/394050.
+    if (diff.needsRepaintObject() || updatedDiff.needsRepaint()) {
+        // FIXME: For objects that don't save previousPaintInvalidationRect, we still need
+        // to invalidate now because their invalidateTreeIfNeeded won't invalidate themselves.
+        // crbug.com/394133.
+        // FIXME: Test of parentTransformDidChange in RenderSVGModelObject::invalidateTreeIfNeeded()
+        // prevents SVG objects from using unified invalidation for repaint-only style change.
+        // crbug.com/394619.
+        if (needsLayout() || (canSelfInvalidateDuringTreeInvalidation() && !isSVG()))
             setShouldDoFullPaintInvalidation(true);
-        else if (!selfNeedsLayout())
+        else
             paintInvalidationForWholeRenderer();
     }
 }
 
 static inline bool rendererHasBackground(const RenderObject* renderer)
 {
     return renderer && renderer->hasBackground();
 }
 
 void RenderObject::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
@@ skipping 15 matching lines @@
             // We might not have an enclosing layer yet because we might not be in the tree.
             if (RenderLayer* layer = enclosingLayer()) {
                 if (newStyle.visibility() == VISIBLE) {
                     layer->setHasVisibleContent();
                 } else if (layer->hasVisibleContent() && (this == layer->renderer() || layer->renderer()->style()->visibility() != VISIBLE)) {
                     layer->dirtyVisibleContentStatus();
                 }
             }
         }
 
-        // For style-only changes that need paint invalidation, we currently need to issue a paint invalidation before and after the style
-        // change. The paint invalidation before style change is accomplished here. The paint invalidation after style change is accomplished
-        // in RenderObject::setStyle.
         if (m_parent && diff.needsRepaintObject()) {
-            if (!diff.needsLayout() && !needsLayout())
-                paintInvalidationForWholeRenderer();
+            if (!diff.needsLayout() && !needsLayout()) {
+                // Invalidate before style change.
+                // FIXME: See FIXMEs in setStyle() before calling paintInvalidationForWholeRenderer
+                // for the reason of this paintInvalidationForWholeRenderer.
+                if (!canSelfInvalidateDuringTreeInvalidation() || isSVG())
+                    paintInvalidationForWholeRenderer();
+            }
         }
 
         if (isFloating() && (m_style->floating() != newStyle.floating()))
             // For changes in float styles, we need to conceivably remove ourselves
             // from the floating objects list.
             toRenderBox(this)->removeFloatingOrPositionedChildFromBlockLists();
         else if (isOutOfFlowPositioned() && (m_style->position() != newStyle.position()))
             // For changes in positioning styles, we need to conceivably remove ourselves
             // from the positioned objects list.
             toRenderBox(this)->removeFloatingOrPositionedChildFromBlockLists();
@@ skipping 1269 matching lines @@
 {
     ASSERT_NOT_REACHED();
     return false;
 }
 
 bool RenderObject::isRelayoutBoundaryForInspector() const
 {
     return objectIsRelayoutBoundary(this);
 }
 
+void RenderObject::setShouldDoFullPaintInvalidation(bool b)

[Julien - ping for review, 2014/07/18 18:49:44]

I really think we should split the setting from the clearing, which would remove
this boolean.

+{
+    m_bitfields.setShouldDoFullPaintInvalidation(b);
+    if (!b)
+        return;
+
+    // Make sure our parent is marked so that the next invalidateTreeIfNeeded can reach this object.
+    for (RenderObject* p = parent(); p && !p->shouldCheckForSelfOrChildPaintInvalidation(); p = p->parent())

[Julien - ping for review, 2014/07/18 18:49:44]

This is wrong for 2 reasons:
- We shouldn't walk the parent tree to invalidate, we should walk the containing
block chain :(
- We may end up calling that function during paintInvalidation (see
RenderBox::invalidateTreeIfNeeded), which will leave renderers marked for
invalidation for the next phase.

+        p->setChildNeedsPaintInvalidation(true);
+}
+
 void RenderObject::clearPaintInvalidationState()
 {
     setShouldDoFullPaintInvalidation(false);
     setShouldDoFullPaintInvalidationIfSelfPaintingLayer(false);
     setOnlyNeededPositionedMovementLayout(false);
     setNeededLayoutBecauseOfChildren(false);
     setShouldInvalidateOverflowForPaint(false);
     setLayoutDidGetCalled(false);
     setMayNeedPaintInvalidation(false);
+    setChildNeedsPaintInvalidation(false);
 }
 
 bool RenderObject::isAllowedToModifyRenderTreeStructure(Document& document)
 {
     return DeprecatedDisableModifyRenderTreeStructureAsserts::canModifyRenderTreeStateInAnyState()
         || document.lifecycle().stateAllowsRenderTreeMutations();
 }
 
 DeprecatedDisableModifyRenderTreeStructureAsserts::DeprecatedDisableModifyRenderTreeStructureAsserts()
     : m_disabler(gModifyRenderTreeStructureAnyState, true)
@@ skipping 30 matching lines @@
 {
     if (object1) {
         const WebCore::RenderObject* root = object1;
         while (root->parent())
             root = root->parent();
         root->showRenderTreeAndMark(object1, "*", object2, "-", 0);
     }
 }
 
 #endif