Don't coerce pointers to compositor layer mappings to booleans.

Source/core/rendering/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 451 matching lines @@
         updateInternalHierarchy();
 
     if (updateMaskLayer(renderer->hasMask()))
         m_graphicsLayer->setMaskLayer(m_maskLayer.get());
 
     bool needsChildClippingMask = (renderer->style()->hasBorderRadius() || renderer->style()->clipPath()) && isAcceleratedContents(renderer);
     if (updateClippingMaskLayers(needsChildClippingMask))
         m_graphicsLayer->setContentsClippingMaskLayer(m_childClippingMaskLayer.get());
 
     if (m_owningLayer->reflectionInfo()) {
-        if (m_owningLayer->reflectionInfo()->reflectionLayer()->compositedLayerMapping()) {
+        if (m_owningLayer->reflectionInfo()->reflectionLayer()->compositingState() == PaintsIntoOwnBacking) {
             GraphicsLayer* reflectionLayer = m_owningLayer->reflectionInfo()->reflectionLayer()->compositedLayerMapping()->mainGraphicsLayer();
             m_graphicsLayer->setReplicatedByLayer(reflectionLayer);
         }
     } else {
         m_graphicsLayer->setReplicatedByLayer(0);
     }
 
     updateBackgroundColor(isSimpleContainerCompositingLayer());
 
     if (isDirectlyCompositedImage())
@@ skipping 215 matching lines @@
             backgroundSize = frameView->visibleContentRect().size();
         }
         m_backgroundLayer->setPosition(backgroundPosition);
         if (backgroundSize != m_backgroundLayer->size()) {
             m_backgroundLayer->setSize(backgroundSize);
             m_backgroundLayer->setNeedsDisplay();
         }
         m_backgroundLayer->setOffsetFromRenderer(m_graphicsLayer->offsetFromRenderer());
     }
 
-    if (m_owningLayer->reflectionInfo() && m_owningLayer->reflectionInfo()->reflectionLayer()->compositedLayerMapping()) {
-        CompositedLayerMapping* reflectionCompositedLayerMapping = m_owningLayer->reflectionInfo()->reflectionLayer()->compositedLayerMapping();
+    if (m_owningLayer->reflectionInfo() && m_owningLayer->reflectionInfo()->reflectionLayer()->compositingState() == PaintsIntoOwnBacking) {
+        CompositedLayerMappingPtr reflectionCompositedLayerMapping = m_owningLayer->reflectionInfo()->reflectionLayer()->compositedLayerMapping();
         reflectionCompositedLayerMapping->updateGraphicsLayerGeometry();
 
         // The reflection layer has the bounds of m_owningLayer->reflectionLayer(),
         // but the reflected layer is the bounds of this layer, so we need to position it appropriately.
         FloatRect layerBounds = compositedBounds();
         FloatRect reflectionLayerBounds = reflectionCompositedLayerMapping->compositedBounds();
         reflectionCompositedLayerMapping->mainGraphicsLayer()->setReplicatedLayerPosition(FloatPoint(layerBounds.location() - reflectionLayerBounds.location()));
     }
 
     if (m_scrollingLayer) {
@@ skipping 630 matching lines @@
     parent->stackingNode()->updateLayerListsIfNeeded();
 
 #if !ASSERT_DISABLED
     LayerListMutationDetector mutationChecker(parent->stackingNode());
 #endif
 
     if (Vector<RenderLayerStackingNode*>* normalFlowList = parent->stackingNode()->normalFlowList()) {
         size_t listSize = normalFlowList->size();
         for (size_t i = 0; i < listSize; ++i) {
             RenderLayer* curLayer = normalFlowList->at(i)->layer();
-            if (!curLayer->compositedLayerMapping()
+            if (curLayer->compositingState() == NotComposited

[shawnsingh, 2013/11/05 10:20:27]

I think this should be written as:
  if (curLayer->compositingState() == NotComposited ||
curLayer->compositingState() == HasOwnBackingButPaintsIntoAncestor)

And same for the next two usages below.


Also, I think we should put a FIXME on this function to (1) rename curLayer to
childLayer and (2) change this code to use the Iterator.

[Ian Vollick, 2013/11/08 03:35:12]

I agree with you on this, but it is a behavior change. I'd like to retain the
current behavior and do the bugfix in a subsequent patch. I've updated things to
use the iterator, though.

                 && (curLayer->hasVisibleContent() || hasVisibleNonCompositingDescendant(curLayer)))
                 return true;
         }
     }
 
     if (parent->stackingNode()->isStackingContainer()) {
         if (!parent->hasVisibleDescendant())
             return false;
 
         // Use the m_hasCompositingDescendant bit to optimize?
         if (Vector<RenderLayerStackingNode*>* negZOrderList = parent->stackingNode()->negZOrderList()) {
             size_t listSize = negZOrderList->size();
             for (size_t i = 0; i < listSize; ++i) {
                 RenderLayer* curLayer = negZOrderList->at(i)->layer();
-                if (!curLayer->compositedLayerMapping()
+                if (curLayer->compositingState() == NotComposited
                     && (curLayer->hasVisibleContent() || hasVisibleNonCompositingDescendant(curLayer)))
                     return true;
             }
         }
 
         if (Vector<RenderLayerStackingNode*>* posZOrderList = parent->stackingNode()->posZOrderList()) {
             size_t listSize = posZOrderList->size();
             for (size_t i = 0; i < listSize; ++i) {
                 RenderLayer* curLayer = posZOrderList->at(i)->layer();
-                if (!curLayer->compositedLayerMapping()
+                if (curLayer->compositingState() == NotComposited
                     && (curLayer->hasVisibleContent() || hasVisibleNonCompositingDescendant(curLayer)))
                     return true;
             }
         }
     }
 
     return false;
 }
 
 // FIXME: By name the implementation is correct. But the code that uses this function means something

[shawnsingh, 2013/11/05 10:20:27]

By the way, the 3 lines you are changing in
hasVisibleNonCompositingDescendants() actually addresses this FIXME, so you can
probably remove this FIXME.

However, I'm pretty sure we can construct a layout test that actually reproduces
a bug due to the incorrect usage of compostedLayerMapping() before this patch. 
So we probably should do that (which is why I didn't change these in my first
compositingState patch).

[Ian Vollick, 2013/11/08 03:35:12]

I think I've spoken to this with my previous comment.

 // very slightly different - the implementation needs to also include composited descendants that
 // don't paint into their own backing, and instead paint into this backing.
 bool CompositedLayerMapping::hasVisibleNonCompositingDescendantLayers() const
 {
     return hasVisibleNonCompositingDescendant(m_owningLayer);
 }
 
 bool CompositedLayerMapping::containsPaintedContent(bool isSimpleContainer) const
 {
     if (isSimpleContainer || paintsIntoCompositedAncestor() || m_artificiallyInflatedBounds || m_owningLayer->isReflection())
@@ skipping 619 matching lines @@
     } else if (graphicsLayer == m_scrollingContentsLayer.get()) {
         name = "Scrolling Contents Layer";
     } else {
         ASSERT_NOT_REACHED();
     }
 
     return name;
 }
 
 } // namespace WebCore