Don't access compositing state for blending in RenderLayer add/remove child

Source/core/rendering/RenderLayer.cpp

Index: Source/core/rendering/RenderLayer.cpp
diff --git a/Source/core/rendering/RenderLayer.cpp b/Source/core/rendering/RenderLayer.cpp
index 20f618d211c61b254a05595db9ba40aaa1bb68a3..e979c9b9cace2e8ea85d89846c4f494481ee6b32 100644
--- a/Source/core/rendering/RenderLayer.cpp
+++ b/Source/core/rendering/RenderLayer.cpp
@@ -857,9 +857,8 @@ void RenderLayer::updateDescendantDependentFlags()
             if (!child->stackingNode()->isStackingContext())
                 child->updateDescendantDependentFlags();
 
-            bool childLayerHasBlendMode = child->blendInfo().childLayerHasBlendModeWhileDirty();
-            childLayerHasBlendMode |= child->paintsWithBlendMode()
-                || (childLayerHasBlendMode && !child->stackingNode()->isStackingContext());

[Stephen Chennney, 2014/03/26 14:37:52]

Note that this logic is broken, and I believe I have modified it to have the
same effect without the confusion.

In particular, the && clause involving stacking context can only be true if
childLayerHasBlendMode is true, but we will already set the result true because
we || with that sooner. In other words, there's no way the && clause can be true
without the entire expression being true anyway.

It makes me wonder, however, whether this original code is a bug.

[rosca, 2014/03/27 14:52:32]

I agree that your change will not change the old logic, which doesn't look to be
right.

I think the correct condition would be:
bool childLayerHasBlendMode = child->blendInfo().hasBlendMode() ||
(!child->stackingNode()->isStackingContext() &&
child->blendInfo().childLayerHasBlendMode());

Why: we don't care if a stacking context child has blending children, we need
this info only for non-stacking context children. For non-stacking context
children we know that the flag is not dirty, as we call
updateDescendantDependentFlags() right above.

I've run the layout tests with all these changes and they pass. If you are ok
with this line, you can insert it to your patch. Tell me if you want me to add
this change with another review along with a test that makes sure this code is
touched.

+            bool childLayerHadBlendMode = child->blendInfo().childLayerHasBlendModeWhileDirty();
+            bool childLayerHasBlendMode = childLayerHadBlendMode || child->blendInfo().hasBlendMode();
 
             m_blendInfo.setChildLayerHasBlendMode(childLayerHasBlendMode);
 
@@ -1447,7 +1446,7 @@ void RenderLayer::addChild(RenderLayer* child, RenderLayer* beforeChild)
     if (child->isSelfPaintingLayer() || child->hasSelfPaintingLayerDescendant())
         setAncestorChainHasSelfPaintingLayerDescendant();
 
-    if (child->paintsWithBlendMode() || child->blendInfo().childLayerHasBlendMode())
+    if (child->blendInfo().hasBlendMode() || child->blendInfo().childLayerHasBlendMode())
         m_blendInfo.setAncestorChainBlendedDescendant();
 
     if (subtreeContainsOutOfFlowPositionedLayer(child)) {
@@ -1511,7 +1510,7 @@ RenderLayer* RenderLayer::removeChild(RenderLayer* oldChild)
     if (oldChild->m_hasVisibleContent || oldChild->m_hasVisibleDescendant)
         dirtyAncestorChainVisibleDescendantStatus();
 
-    if (oldChild->paintsWithBlendMode() || oldChild->blendInfo().childLayerHasBlendMode())
+    if (oldChild->m_blendInfo.hasBlendMode() || oldChild->blendInfo().childLayerHasBlendMode())
         m_blendInfo.dirtyAncestorChainBlendedDescendantStatus();
 
     if (oldChild->isSelfPaintingLayer() || oldChild->hasSelfPaintingLayerDescendant())
@@ -3663,8 +3662,6 @@ bool RenderLayer::paintsWithTransform(PaintBehavior paintBehavior) const
 
 bool RenderLayer::paintsWithBlendMode() const
 {
-    // https://code.google.com/p/chromium/issues/detail?id=343759
-    DisableCompositingQueryAsserts disabler;
     return m_blendInfo.hasBlendMode() && compositingState() != PaintsIntoOwnBacking;
 }