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

Source/core/rendering/compositing/CompositedLayerMapping.cpp

 /*
  * Copyright (C) 2009, 2010, 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 1872 matching lines @@
 }
 
 bool CompositedLayerMapping::updateRequiresOwnBackingStoreForAncestorReasons(const RenderLayer* compositingAncestorLayer)
 {
     unsigned previousRequiresOwnBackingStoreForAncestorReasons = m_requiresOwnBackingStoreForAncestorReasons;
     bool previousPaintsIntoCompositedAncestor = paintsIntoCompositedAncestor();
     bool canPaintIntoAncestor = compositingAncestorLayer
         && (compositingAncestorLayer->compositedLayerMapping()->mainGraphicsLayer()->drawsContent()
             || compositingAncestorLayer->compositedLayerMapping()->paintsIntoCompositedAncestor());
 
-    if (paintsIntoCompositedAncestor() != previousPaintsIntoCompositedAncestor)
+    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);
-
-    // FIXME: this is bogus. We need to make this assignment before the check above.
-    m_requiresOwnBackingStoreForAncestorReasons = !canPaintIntoAncestor;
+        m_requiresOwnBackingStoreForAncestorReasons = !m_requiresOwnBackingStoreForAncestorReasons;
+    }
 
     return m_requiresOwnBackingStoreForAncestorReasons != previousRequiresOwnBackingStoreForAncestorReasons;
 }
 
 bool CompositedLayerMapping::updateRequiresOwnBackingStoreForIntrinsicReasons()
 {
     unsigned previousRequiresOwnBackingStoreForIntrinsicReasons = m_requiresOwnBackingStoreForIntrinsicReasons;
     bool previousPaintsIntoCompositedAncestor = paintsIntoCompositedAncestor();
     RenderObject* renderer = m_owningLayer.renderer();
     m_requiresOwnBackingStoreForIntrinsicReasons = m_owningLayer.isRootLayer()
         || (m_owningLayer.compositingReasons() & CompositingReasonComboReasonsThatRequireOwnBacking)
         || m_owningLayer.transform()
         || m_owningLayer.clipsCompositingDescendantsWithBorderRadius() // FIXME: Revisit this if the paintsIntoCompositedAncestor state is removed.
         || renderer->isTransparent()
         || renderer->hasMask()
         || 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;
 }
 
 void CompositedLayerMapping::setBlendMode(WebBlendMode blendMode)
 {
     if (m_ancestorClippingLayer) {
         m_ancestorClippingLayer->setBlendMode(blendMode);
         m_graphicsLayer->setBlendMode(WebBlendModeNormal);
     } else {
@@ skipping 355 matching lines @@
     } else if (graphicsLayer == m_scrollingBlockSelectionLayer.get()) {
         name = "Scrolling Block Selection Layer";
     } else {
         ASSERT_NOT_REACHED();
     }
 
     return name;
 }
 
 } // namespace blink