Check subtree paint invalidation when overflow clip status changes

Before https://crrev.com/392652 because we included descendant overflow into container overflow and the paint invalidation rect covered the changed part of descendants. Call setNeedsPaintInvalidationSubtree() to force checking of paint invalidation of descendants when overflow clip status changes. The other changes are needed because we need to move clearPaintInvalidationFlags() after newPaintInvalidationState.updateForChildren(). Several overriding invalidateTreeIfNeeded() methods in subclasses are removed because they have been the same as inherited ones after we changed the paint invalidation tree walk order. BUG=613475 TEST=paint/invalidation/overflow-hidden-to-visible.html TEST=paint/invalidation/overflow-visible-to-hidden.html Review-Url: https://codereview.chromium.org/2000763003 Cr-Commit-Position: refs/heads/master@{#395491} Review URL: https://codereview.chromium.org/2007003003 . Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#44} Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
huningxin · May 25, 2016 · 3104da0 · 3104da0
1 parent 9aae238
commit 3104da0
Show file tree

Hide file tree

Showing 13 changed files with 71 additions and 44 deletions.
diff --git a/third_party/WebKit/LayoutTests/fast/repaint/erase-overflow-expected.txt b/third_party/WebKit/LayoutTests/fast/repaint/erase-overflow-expected.txt
@@ -15,6 +15,10 @@
           "object": "LayoutBlockFlow DIV id='target' class='square'",
           "rect": [8, 8, 100, 100],
           "reason": "style change"
+        },
+        {
+          "object": "LayoutBlockFlow DIV class='square'",
+          "reason": "none"
         }
       ]
     }

diff --git a/third_party/WebKit/LayoutTests/paint/invalidation/overflow-hidden-to-visible-expected.html b/third_party/WebKit/LayoutTests/paint/invalidation/overflow-hidden-to-visible-expected.html
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+Tests paint invalidation of overflowing contents when the container's overflow changes from hidden to visible.
+Passes if there is no red.
+<div id="green" style="width: 200px; height: 400px; background-color: green"></div>
diff --git a/third_party/WebKit/LayoutTests/paint/invalidation/overflow-hidden-to-visible.html b/third_party/WebKit/LayoutTests/paint/invalidation/overflow-hidden-to-visible.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+Tests paint invalidation of overflowing contents when the container's overflow changes from hidden to visible.
+Passes if there is no red.
+<div id="indicator" style="position: absolute; z-index: -1; width: 200px; height: 400px; background-color: red"></div>
+<div id="container" style="width: 200px; height: 200px; overflow: hidden">
+  <div style="width: 0; height: 0">
+    <div id="green" style="width: 200px; height: 400px; background-color: green"></div>
+  </div>
+</div>
+<script src="../../resources/run-after-layout-and-paint.js"></script>
+<script>
+runAfterLayoutAndPaint(function() {
+   container.style.overflow = 'visible';
+}, true);
+</script>
diff --git a/third_party/WebKit/LayoutTests/paint/invalidation/overflow-visible-to-hidden-expected.html b/third_party/WebKit/LayoutTests/paint/invalidation/overflow-visible-to-hidden-expected.html
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+Tests paint invalidation of overflowing contents when the container's overflow changes from visible to hidden.
+Passes if there is no red.
+<div id="green" style="width: 200px; height: 200px; background-color: green"></div>
diff --git a/third_party/WebKit/LayoutTests/paint/invalidation/overflow-visible-to-hidden.html b/third_party/WebKit/LayoutTests/paint/invalidation/overflow-visible-to-hidden.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+Tests paint invalidation of overflowing contents when the container's overflow changes from visible to hidden.
+Passes if there is no red.
+<div id="container" style="width: 200px; height: 200px; overflow: visible">
+  <div style="width: 0; height: 0">
+    <div id="green" style="width: 200px; height: 200px; background-color: green"></div>
+    <div id="red" style="width: 200px; height: 200px; background-color: red"></div>
+  </div>
+</div>
+<script src="../../resources/run-after-layout-and-paint.js"></script>
+<script>
+runAfterLayoutAndPaint(function() {
+   container.style.overflow = 'hidden';
+}, true);
+</script>
diff --git a/third_party/WebKit/Source/core/layout/LayoutBlock.cpp b/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
@@ -245,8 +245,7 @@ void LayoutBlock::updateFromStyle()
     if (shouldClipOverflow != hasOverflowClip()) {
         if (!shouldClipOverflow)
             getScrollableArea()->invalidateAllStickyConstraints();
-        for (LayoutObject* child = firstChild(); child; child = child->nextSibling())
-            child->setMayNeedPaintInvalidation();
+        setMayNeedPaintInvalidationSubtree();
     }
     setHasOverflowClip(shouldClipOverflow);
 }

diff --git a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
@@ -393,6 +393,9 @@ void LayoutBoxModelObject::invalidateTreeIfNeeded(const PaintInvalidationState&
     if (!shouldCheckForPaintInvalidation(newPaintInvalidationState))
         return;
 
+    if (mayNeedPaintInvalidationSubtree())
+        newPaintInvalidationState.setForceSubtreeInvalidationCheckingWithinContainer();
+
     LayoutRect previousPaintInvalidationRect = this->previousPaintInvalidationRect();
     LayoutPoint previousPosition = previousPositionFromPaintInvalidationBacking();
     PaintInvalidationReason reason = invalidatePaintIfNeeded(newPaintInvalidationState);

diff --git a/third_party/WebKit/Source/core/layout/LayoutObject.cpp b/third_party/WebKit/Source/core/layout/LayoutObject.cpp
@@ -1313,6 +1313,10 @@ void LayoutObject::invalidateTreeIfNeeded(const PaintInvalidationState& paintInv
         return;
 
     PaintInvalidationState newPaintInvalidationState(paintInvalidationState, *this);
+
+    if (mayNeedPaintInvalidationSubtree())
+        newPaintInvalidationState.setForceSubtreeInvalidationCheckingWithinContainer();
+
     PaintInvalidationReason reason = invalidatePaintIfNeeded(newPaintInvalidationState);
     clearPaintInvalidationFlags(newPaintInvalidationState);
 
@@ -3454,6 +3458,14 @@ void LayoutObject::setMayNeedPaintInvalidation()
     frameView()->scheduleVisualUpdateForPaintInvalidationIfNeeded();
 }
 
+void LayoutObject::setMayNeedPaintInvalidationSubtree()
+{
+    if (mayNeedPaintInvalidationSubtree())
+        return;
+    m_bitfields.setMayNeedPaintInvalidationSubtree(true);
+    setMayNeedPaintInvalidation();
+}
+
 void LayoutObject::clearPaintInvalidationFlags(const PaintInvalidationState& paintInvalidationState)
 {
     // paintInvalidationStateIsDirty should be kept in sync with the
@@ -3464,6 +3476,7 @@ void LayoutObject::clearPaintInvalidationFlags(const PaintInvalidationState& pai
     m_bitfields.setNeededLayoutBecauseOfChildren(false);
     m_bitfields.setShouldInvalidateOverflowForPaint(false);
     m_bitfields.setMayNeedPaintInvalidation(false);
+    m_bitfields.setMayNeedPaintInvalidationSubtree(false);
     m_bitfields.setShouldInvalidateSelection(false);
 }
 

diff --git a/third_party/WebKit/Source/core/layout/LayoutObject.h b/third_party/WebKit/Source/core/layout/LayoutObject.h
@@ -1361,6 +1361,9 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver, public DisplayIte
     bool mayNeedPaintInvalidation() const { return m_bitfields.mayNeedPaintInvalidation(); }
     void setMayNeedPaintInvalidation();
 
+    bool mayNeedPaintInvalidationSubtree() const { return m_bitfields.mayNeedPaintInvalidationSubtree(); }
+    void setMayNeedPaintInvalidationSubtree();
+
     bool shouldInvalidateSelection() const { return m_bitfields.shouldInvalidateSelection(); }
     void setShouldInvalidateSelection();
 
@@ -1722,6 +1725,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver, public DisplayIte
             , m_shouldInvalidateOverflowForPaint(false)
             , m_childShouldCheckForPaintInvalidation(false)
             , m_mayNeedPaintInvalidation(false)
+            , m_mayNeedPaintInvalidationSubtree(false)
             , m_shouldInvalidateSelection(false)
             , m_neededLayoutBecauseOfChildren(false)
             , m_floating(false)
@@ -1757,7 +1761,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver, public DisplayIte
         {
         }
 
-        // 32 bits have been used in the first word, and 17 in the second.
+        // 32 bits have been used in the first word, and 18 in the second.
 
         // Self needs layout means that this layout object is marked for a full layout.
         // This is the default layout but it is expensive as it recomputes everything.
@@ -1808,6 +1812,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver, public DisplayIte
         ADD_BOOLEAN_BITFIELD(shouldInvalidateOverflowForPaint, ShouldInvalidateOverflowForPaint); // TODO(wangxianzhu): Remove for slimming paint v2.
         ADD_BOOLEAN_BITFIELD(childShouldCheckForPaintInvalidation, ChildShouldCheckForPaintInvalidation);
         ADD_BOOLEAN_BITFIELD(mayNeedPaintInvalidation, MayNeedPaintInvalidation);
+        ADD_BOOLEAN_BITFIELD(mayNeedPaintInvalidationSubtree, MayNeedPaintInvalidationSubtree);
         ADD_BOOLEAN_BITFIELD(shouldInvalidateSelection, ShouldInvalidateSelection); // TODO(wangxianzhu): Remove for slimming paint v2.
         ADD_BOOLEAN_BITFIELD(neededLayoutBecauseOfChildren, NeededLayoutBecauseOfChildren); // TODO(wangxianzhu): Remove for slimming paint v2.
 

diff --git a/third_party/WebKit/Source/core/layout/svg/LayoutSVGInline.cpp b/third_party/WebKit/Source/core/layout/svg/LayoutSVGInline.cpp
@@ -145,17 +145,9 @@ void LayoutSVGInline::removeChild(LayoutObject* child)
 
 void LayoutSVGInline::invalidateTreeIfNeeded(const PaintInvalidationState& paintInvalidationState)
 {
-    ASSERT(!needsLayout());
-
-    if (!shouldCheckForPaintInvalidation(paintInvalidationState))
-        return;
-
-    PaintInvalidationState newPaintInvalidationState(paintInvalidationState, *this);
-    PaintInvalidationReason reason = invalidatePaintIfNeeded(newPaintInvalidationState);
-    clearPaintInvalidationFlags(newPaintInvalidationState);
-
-    newPaintInvalidationState.updateForChildren(reason);
-    invalidatePaintOfSubtreesIfNeeded(newPaintInvalidationState);
+    // TODO(wangxianzhu): Verify if the inherited LayoutBoxModelObject::invalidateTreeIfNeeded()
+    // is applicable here. If yes, remove this overriding method.
+    LayoutObject::invalidateTreeIfNeeded(paintInvalidationState);
 }
 
 } // namespace blink
diff --git a/third_party/WebKit/Source/core/layout/svg/LayoutSVGModelObject.cpp b/third_party/WebKit/Source/core/layout/svg/LayoutSVGModelObject.cpp
@@ -130,21 +130,4 @@ IntRect LayoutSVGModelObject::absoluteElementBoundingBoxRect() const
     return localToAbsoluteQuad(FloatQuad(paintInvalidationRectInLocalSVGCoordinates())).enclosingBoundingBox();
 }
 
-void LayoutSVGModelObject::invalidateTreeIfNeeded(const PaintInvalidationState& paintInvalidationState)
-{
-    ASSERT(!needsLayout());
-
-    // 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(paintInvalidationState))
-        return;
-
-    PaintInvalidationState newPaintInvalidationState(paintInvalidationState, *this);
-    PaintInvalidationReason reason = invalidatePaintIfNeeded(newPaintInvalidationState);
-    clearPaintInvalidationFlags(newPaintInvalidationState);
-
-    newPaintInvalidationState.updateForChildren(reason);
-    invalidatePaintOfSubtreesIfNeeded(newPaintInvalidationState);
-}
-
 } // namespace blink
diff --git a/third_party/WebKit/Source/core/layout/svg/LayoutSVGModelObject.h b/third_party/WebKit/Source/core/layout/svg/LayoutSVGModelObject.h
@@ -76,8 +76,6 @@ class LayoutSVGModelObject : public LayoutObject {
     bool nodeAtPoint(HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) final;
     IntRect absoluteElementBoundingBoxRect() const final;
 
-    void invalidateTreeIfNeeded(const PaintInvalidationState&) final;
-
 protected:
     FloatRect m_paintInvalidationBoundingBox;
 };

diff --git a/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp b/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp
@@ -396,17 +396,9 @@ void LayoutSVGText::removeChild(LayoutObject* child)
 
 void LayoutSVGText::invalidateTreeIfNeeded(const PaintInvalidationState& paintInvalidationState)
 {
-    ASSERT(!needsLayout());
-
-    if (!shouldCheckForPaintInvalidation(paintInvalidationState))
-        return;
-
-    PaintInvalidationState newPaintInvalidationState(paintInvalidationState, *this);
-    PaintInvalidationReason reason = invalidatePaintIfNeeded(newPaintInvalidationState);
-    clearPaintInvalidationFlags(newPaintInvalidationState);
-
-    newPaintInvalidationState.updateForChildren(reason);
-    invalidatePaintOfSubtreesIfNeeded(newPaintInvalidationState);
+    // TODO(wangxianzhu): Verify if the inherited LayoutBoxModelObject::invalidateTreeIfNeeded()
+    // is applicable here. If yes, remove this overriding method.
+    LayoutObject::invalidateTreeIfNeeded(paintInvalidationState);
 }
 
 } // namespace blink