Fix paint invalidation when composited layers change from paints-into-ancestor to self-painting.

Source/core/rendering/compositing/CompositedLayerMapping.cpp

Index: Source/core/rendering/compositing/CompositedLayerMapping.cpp
diff --git a/Source/core/rendering/compositing/CompositedLayerMapping.cpp b/Source/core/rendering/compositing/CompositedLayerMapping.cpp
index 2bee39b3bbe77d163122873f20ebb097d53a526e..ff353377186130438ae2f4fbca7e372222f28611 100644
--- a/Source/core/rendering/compositing/CompositedLayerMapping.cpp
+++ b/Source/core/rendering/compositing/CompositedLayerMapping.cpp
@@ -1890,12 +1890,15 @@ bool CompositedLayerMapping::updateRequiresOwnBackingStoreForAncestorReasons(con
         && (compositingAncestorLayer->compositedLayerMapping()->mainGraphicsLayer()->drawsContent()
             || compositingAncestorLayer->compositedLayerMapping()->paintsIntoCompositedAncestor());
 
-    if (paintsIntoCompositedAncestor() != previousPaintsIntoCompositedAncestor)
-        compositor()->paintInvalidationOnCompositingChange(&m_owningLayer);
-
-    // FIXME: this is bogus. We need to make this assignment before the check above.
     m_requiresOwnBackingStoreForAncestorReasons = !canPaintIntoAncestor;

[pdr., 2014/08/28 20:21:13]

This code is really only tracking changes to
m_requiresOwnBackingStoreForAncestorReasons changes but it's deciding that using
paintsIntoCompositedAncestor() as a proxy. What about refactoring this to just
track changes to m_requiresOwnBackingStoreForAncestorReasons directly:

bool newRequiresOwnBackingStoreForAncestorReasons = !compositingAncestorLayer
    ||
!(compositingAncestorLayer->compositedLayerMapping()->mainGraphicsLayer()->drawsContent()
        ||
compositingAncestorLayer->compositedLayerMapping()->paintsIntoCompositedAncestor());

bool backingStoreRequirementChanging =
m_requiresOwnBackingStoreForAncestorReasons !=
newRequiresOwnBackingStoreForAncestorReasons;
if (backingStoreRequirementChanging)
    compositor()->paintInvalidationOnCompositingChange(&m_owningLayer);

m_requiresOwnBackingStoreForAncestorReasons =
newRequiresOwnBackingStoreForAncestorReasons;
return backingStoreRequirementChanging;

[chrishtr, 2014/08/28 20:27:56]

That would invalidate more than necessary. Only if we transition between the
states where paintsIntoCompositedAncestor() returns a different value will any
compositing state change that requires invalidation. And that is more than just
m_requiresOwnBackingStoreForAncestorReasons changing, in the case when
m_requiresOwnBackingStoreForIntrinsicReasons was already true.

 
+    if (paintsIntoCompositedAncestor() != previousPaintsIntoCompositedAncestor) {
+        // Back out the change temporarily while invalidating with respect to the old container.
+        m_requiresOwnBackingStoreForAncestorReasons = !m_requiresOwnBackingStoreForAncestorReasons;
+        compositor()->paintInvalidationOnCompositingChange(&m_owningLayer);
+        m_requiresOwnBackingStoreForAncestorReasons = !m_requiresOwnBackingStoreForAncestorReasons;
+    }
+
     return m_requiresOwnBackingStoreForAncestorReasons != previousRequiresOwnBackingStoreForAncestorReasons;
 }
 
@@ -1913,8 +1916,12 @@ bool CompositedLayerMapping::updateRequiresOwnBackingStoreForIntrinsicReasons()
         || renderer->hasReflection()
         || renderer->hasFilter();
 
-    if (paintsIntoCompositedAncestor() != previousPaintsIntoCompositedAncestor)
+    if (paintsIntoCompositedAncestor() != previousPaintsIntoCompositedAncestor) {
+        // Back out the change temporarily while invalidating with respect to the old container.
+        m_requiresOwnBackingStoreForIntrinsicReasons = !m_requiresOwnBackingStoreForIntrinsicReasons;
         compositor()->paintInvalidationOnCompositingChange(&m_owningLayer);
+        m_requiresOwnBackingStoreForIntrinsicReasons = !m_requiresOwnBackingStoreForIntrinsicReasons;
+    }
 
     return m_requiresOwnBackingStoreForIntrinsicReasons != previousRequiresOwnBackingStoreForIntrinsicReasons;
 }