hasSelfPaintingLayerDescendant should clean m_hasSelfPaintingLayerDescendantDirty

Previously, it was possible to read stale data from
hasSelfPaintingLayerDescendant. After this CL, we clean out the dirty bit
automatically when you read this state. In order to make this work, this CL
separates hasSelfPaintingLayerDescendant from updateDescendantDependentFlags.

R=esprehn@chromium.org

Review URL: https://codereview.chromium.org/468633002

git-svn-id: svn://svn.chromium.org/blink/trunk@180124 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/rendering/RenderLayer.cpp

@@ -275,15 +275,20 @@ void RenderLayer::updateLayerPositionRecursive()
         child->updateLayerPositionRecursive();
 }
 
-void RenderLayer::setAncestorChainHasSelfPaintingLayerDescendant()
+void RenderLayer::updateHasSelfPaintingLayerDescendant() const
 {
-    for (RenderLayer* layer = this; layer; layer = layer->parent()) {
-        if (!layer->m_hasSelfPaintingLayerDescendantDirty && layer->hasSelfPaintingLayerDescendant())
-            break;
+    ASSERT(m_hasSelfPaintingLayerDescendantDirty);
+
+    m_hasSelfPaintingLayerDescendant = false;
 
-        layer->m_hasSelfPaintingLayerDescendantDirty = false;
-        layer->m_hasSelfPaintingLayerDescendant = true;
+    for (RenderLayer* child = firstChild(); child; child = child->nextSibling()) {
+        if (child->isSelfPaintingLayer() || child->hasSelfPaintingLayerDescendant()) {
+            m_hasSelfPaintingLayerDescendant = true;
+            break;
         }
+    }
+
+    m_hasSelfPaintingLayerDescendantDirty = false;
 }
 
 void RenderLayer::dirtyAncestorChainHasSelfPaintingLayerDescendantStatus()
@@ -293,7 +298,7 @@ void RenderLayer::dirtyAncestorChainHasSelfPaintingLayerDescendantStatus()
         // If we have reached a self-painting layer, we know our parent should have a self-painting descendant
         // in this case, there is no need to dirty our ancestors further.
         if (layer->isSelfPaintingLayer()) {
-            ASSERT(!parent() || parent()->m_hasSelfPaintingLayerDescendantDirty || parent()->hasSelfPaintingLayerDescendant());
+            ASSERT(!parent() || parent()->m_hasSelfPaintingLayerDescendantDirty || parent()->m_hasSelfPaintingLayerDescendant);
             break;
         }
     }
@@ -650,25 +655,19 @@ void RenderLayer::updateDescendantDependentFlagsForEntireSubtree()
 
 void RenderLayer::updateDescendantDependentFlags()
 {
-    if (m_visibleDescendantStatusDirty || m_hasSelfPaintingLayerDescendantDirty) {
+    if (m_visibleDescendantStatusDirty) {
         m_hasVisibleDescendant = false;
-        m_hasSelfPaintingLayerDescendant = false;
 
         for (RenderLayer* child = firstChild(); child; child = child->nextSibling()) {
             child->updateDescendantDependentFlags();
 
-            bool hasVisibleDescendant = child->m_hasVisibleContent || child->m_hasVisibleDescendant;
-            bool hasSelfPaintingLayerDescendant = child->isSelfPaintingLayer() || child->hasSelfPaintingLayerDescendant();
-
-            m_hasVisibleDescendant |= hasVisibleDescendant;
-            m_hasSelfPaintingLayerDescendant |= hasSelfPaintingLayerDescendant;
-
-            if (m_hasVisibleDescendant && m_hasSelfPaintingLayerDescendant)
+            if (child->m_hasVisibleContent || child->m_hasVisibleDescendant) {
+                m_hasVisibleDescendant = true;
                 break;
             }
+        }
 
         m_visibleDescendantStatusDirty = false;
-        m_hasSelfPaintingLayerDescendantDirty = false;
     }
 
     if (m_blendInfo.childLayerHasBlendModeStatusDirty()) {
@@ -1263,12 +1262,10 @@ void RenderLayer::addChild(RenderLayer* child, RenderLayer* beforeChild)
     }
 
     dirtyAncestorChainVisibleDescendantStatus();
+    dirtyAncestorChainHasSelfPaintingLayerDescendantStatus();
 
     child->updateDescendantDependentFlags();
 
-    if (child->isSelfPaintingLayer() || child->hasSelfPaintingLayerDescendant())
-        setAncestorChainHasSelfPaintingLayerDescendant();
-
     if (child->blendInfo().hasBlendMode() || child->blendInfo().childLayerHasBlendMode())
         m_blendInfo.setAncestorChainBlendedDescendant();
 }
@@ -1302,6 +1299,8 @@ RenderLayer* RenderLayer::removeChild(RenderLayer* oldChild)
     oldChild->setNextSibling(0);
     oldChild->m_parent = 0;
 
+    dirtyAncestorChainHasSelfPaintingLayerDescendantStatus();
+
     oldChild->updateDescendantDependentFlags();
 
     if (oldChild->m_hasVisibleContent || oldChild->m_hasVisibleDescendant)
@@ -1310,9 +1309,6 @@ RenderLayer* RenderLayer::removeChild(RenderLayer* oldChild)
     if (oldChild->m_blendInfo.hasBlendMode() || oldChild->blendInfo().childLayerHasBlendMode())
         m_blendInfo.dirtyAncestorChainBlendedDescendantStatus();
 
-    if (oldChild->isSelfPaintingLayer() || oldChild->hasSelfPaintingLayerDescendant())
-        dirtyAncestorChainHasSelfPaintingLayerDescendantStatus();
-
     return oldChild;
 }
 
@@ -3405,16 +3401,13 @@ bool RenderLayer::shouldBeSelfPaintingLayer() const
 
 void RenderLayer::updateSelfPaintingLayer()
 {
-    bool isSelfPaintingLayer = this->shouldBeSelfPaintingLayer();
+    bool isSelfPaintingLayer = shouldBeSelfPaintingLayer();
     if (this->isSelfPaintingLayer() == isSelfPaintingLayer)
         return;
 
     m_isSelfPaintingLayer = isSelfPaintingLayer;
-    if (!parent())
-        return;
-    if (isSelfPaintingLayer)
-        parent()->setAncestorChainHasSelfPaintingLayerDescendant();
-    else
+
+    if (parent())
         parent()->dirtyAncestorChainHasSelfPaintingLayerDescendantStatus();
 }
 

third_party/WebKit/Source/core/rendering/RenderLayer.h

@@ -114,6 +114,7 @@ public:
 
     void styleChanged(StyleDifference, const RenderStyle* oldStyle);
 
+    // FIXME: Many people call this function while it has out-of-date information.
     bool isSelfPaintingLayer() const { return m_isSelfPaintingLayer; }
 
     void setLayerType(LayerType layerType) { m_layerType = layerType; }
@@ -186,10 +187,6 @@ public:
     // True if this layer container renderers that paint.
     bool hasNonEmptyChildRenderers() const;
 
-    // FIXME: We should ASSERT(!m_hasSelfPaintingLayerDescendantDirty); here but we hit the same bugs as visible content above.
-    // Part of the issue is with subtree relayout: we don't check if our ancestors have some descendant flags dirty, missing some updates.
-    bool hasSelfPaintingLayerDescendant() const { return m_hasSelfPaintingLayerDescendant; }
-
     // Will ensure that hasNonCompositiedChild are up to date.
     void updateScrollingStateAfterCompositingChange();
     bool hasVisibleNonLayerContent() const { return m_hasVisibleNonLayerContent; }
@@ -544,6 +541,16 @@ private:
     void setFirstChild(RenderLayer* first) { m_first = first; }
     void setLastChild(RenderLayer* last) { m_last = last; }
 
+    void updateHasSelfPaintingLayerDescendant() const;
+
+    bool hasSelfPaintingLayerDescendant() const
+    {
+        if (m_hasSelfPaintingLayerDescendantDirty)
+            updateHasSelfPaintingLayerDescendant();
+        ASSERT(!m_hasSelfPaintingLayerDescendantDirty);
+        return m_hasSelfPaintingLayerDescendant;
+    }
+
     LayoutPoint renderBoxLocation() const { return renderer()->isBox() ? toRenderBox(renderer())->location() : LayoutPoint(); }
 
     void paintLayerContentsAndReflection(GraphicsContext*, const LayerPaintingInfo&, PaintLayerFlags);
@@ -645,8 +652,8 @@ private:
 
     // If have no self-painting descendants, we don't have to walk our children during painting. This can lead to
     // significant savings, especially if the tree has lots of non-self-painting layers grouped together (e.g. table cells).
-    unsigned m_hasSelfPaintingLayerDescendant : 1;
-    unsigned m_hasSelfPaintingLayerDescendantDirty : 1;
+    mutable unsigned m_hasSelfPaintingLayerDescendant : 1;
+    mutable unsigned m_hasSelfPaintingLayerDescendantDirty : 1;
 
     const unsigned m_isRootLayer : 1;