Issue invalidations when squashed layers move later or off of a grouped mapping.

Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp

Index: Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp
diff --git a/Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp b/Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp
index a8fccf28f6ffad24795b1d4eeac79bdb78c961fb..f5fac9fb2ecd8695afc399d59cebbef6a3a49949 100644
--- a/Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp
+++ b/Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp
@@ -2251,7 +2251,7 @@ IntRect CompositedDeprecatedPaintLayerMapping::pixelSnappedCompositedBounds() co
     return pixelSnappedIntRect(bounds);
 }
 
-void CompositedDeprecatedPaintLayerMapping::clearLayerGroupingIfNoPrecedingEntry(size_t indexToClear)
+bool CompositedDeprecatedPaintLayerMapping::invalidateLayerIfNoPrecedingEntry(size_t indexToClear)
 {
     DeprecatedPaintLayer* layerToRemove = m_squashedLayers[indexToClear].paintLayer;
     size_t previousIndex = 0;
@@ -2259,11 +2259,11 @@ void CompositedDeprecatedPaintLayerMapping::clearLayerGroupingIfNoPrecedingEntry
         if (m_squashedLayers[previousIndex].paintLayer == layerToRemove)
             break;
     }
-    if (previousIndex == indexToClear) {
-        // Assert on incorrect mappings between layers and groups
-        ASSERT(layerToRemove->groupedMapping() == this);

[chrishtr, 2015/08/12 19:29:55]

The new call site to this function may not satisfy this assert, since a layer
could have been removed from squashing and become individually composited and
also earlier in the paint order.

-        layerToRemove->setGroupedMapping(nullptr, DeprecatedPaintLayer::DoNotInvalidateLayerAndRemoveFromMapping);
+    if (previousIndex == indexToClear && layerToRemove->groupedMapping() == this) {
+        compositor()->paintInvalidationOnCompositingChange(layerToRemove);
+        return true;
     }
+    return false;
 }
 
 bool CompositedDeprecatedPaintLayerMapping::updateSquashingLayerAssignment(DeprecatedPaintLayer* squashedLayer, size_t nextSquashedLayerIndex)
@@ -2282,6 +2282,10 @@ bool CompositedDeprecatedPaintLayerMapping::updateSquashingLayerAssignment(Depre
         // Must invalidate before adding the squashed layer to the mapping.
         compositor()->paintInvalidationOnCompositingChange(squashedLayer);
 
+        // If the layer which was previously at |nextSquashedLayerIndex| is not earlier in the grouped mapping, invalidate its current
+        // backing now, since it will move later or be removed from the squashing layer.
+        invalidateLayerIfNoPrecedingEntry(nextSquashedLayerIndex);
+
         m_squashedLayers.insert(nextSquashedLayerIndex, paintInfo);

[Stephen Chennney, 2015/08/13 16:31:18]

Because we insert here now ...

     } else {
         // Must invalidate before adding the squashed layer to the mapping.
@@ -2326,8 +2330,11 @@ void CompositedDeprecatedPaintLayerMapping::finishAccumulatingSquashingLayers(si
         // Any additional squashed Layers in the array no longer belong here, but they might have been
         // added already at an earlier index. Clear pointers on those that do not appear in the valid set
         // before removing all the extra entries.
-        for (size_t i = nextSquashedLayerIndex; i < m_squashedLayers.size(); ++i)
-            clearLayerGroupingIfNoPrecedingEntry(i);
+        for (size_t i = nextSquashedLayerIndex; i < m_squashedLayers.size(); ++i) {
+            if (invalidateLayerIfNoPrecedingEntry(i))
+                m_squashedLayers[i].paintLayer->setGroupedMapping(nullptr, DeprecatedPaintLayer::DoNotInvalidateLayerAndRemoveFromMapping);

[Stephen Chennney, 2015/08/13 16:31:18]

... it's safe to only null here now. I think that's right but please take a
moment to consider it too before committing.

[chrishtr, 2015/08/13 17:05:59]

I think it's correct because the code here keeps the pointers up to date:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

I'm not sure what scenario was the original corner case that was missed and
necessitated https://codereview.chromium.org/1209033009, but I don't think CL
introduces any new one, so it's safe.

+        }
+
         m_squashedLayers.remove(nextSquashedLayerIndex, m_squashedLayers.size() - nextSquashedLayerIndex);
     }
 }