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

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 3f1f14514de61762a9f3f9e0390c314f4d736c59..91ac7dcdb23423a148756724ae578f21aef90e4f 100644
--- a/Source/core/rendering/RenderLayer.cpp
+++ b/Source/core/rendering/RenderLayer.cpp
@@ -289,7 +289,7 @@ void RenderLayer::updateLayerPositions(RenderGeometryMap* geometryMap, UpdateLay
 
     // Clear the IsCompositingUpdateRoot flag once we've found the first compositing layer in this update.
     bool isUpdateRoot = (flags & IsCompositingUpdateRoot);
-    if (compositedLayerMapping())
+    if (hasCompositedLayerMapping())
         flags &= ~IsCompositingUpdateRoot;
 
     if (useRegionBasedColumns() && renderer()->isInFlowRenderFlowThread()) {
@@ -303,7 +303,7 @@ void RenderLayer::updateLayerPositions(RenderGeometryMap* geometryMap, UpdateLay
     for (RenderLayer* child = firstChild(); child; child = child->nextSibling())
         child->updateLayerPositions(geometryMap, flags);
 
-    if ((flags & UpdateCompositingLayers) && compositedLayerMapping()) {
+    if ((flags & UpdateCompositingLayers) && hasCompositedLayerMapping()) {
         CompositedLayerMapping::UpdateAfterLayoutFlags updateFlags = CompositedLayerMapping::CompositingChildrenOnly;
         if (flags & NeedsFullRepaintInBacking)
             updateFlags |= CompositedLayerMapping::NeedsFullRepaint;
@@ -535,7 +535,8 @@ void RenderLayer::updateBlendMode()
         if (!hadBlendMode || !hasBlendMode())
             dirtyAncestorChainBlendedDescendantStatus();
 
-        if (compositedLayerMapping())
+        // The compositor can only apply one blend mode per composited layer.
+        if (compositingState() == PaintsIntoOwnBacking)

[shawnsingh, 2013/11/12 09:55:50]

Wouldn't blend-mode still work for a layer that paints into an ancestor, because
it would then use the software implementation of blend-modes within the other
layer?  So I think it is valid to accept both compositing states
PaintsIntoOwnBacking and HasOwnBackingButPaintsIntoAncestor.  Code elsewhere
with paintsWithBlendMode() will recognize in that scenario that it needs to use
the software blend mode implementation.

If you agree, I think this is a scenario where we could go ahead and use
compositingState() to define the condition rather than
hasCompositedLayerMapping().

[Ian Vollick, 2013/11/14 04:12:47]

It's my impression that the paints-into-ancestor mode was for creating GL's for
clipping, preserves3d'ing and perspective'ifying composited descendants -- not
for painting. The blend mode is still available via the style. Isn't that what
will be consulted when we paint this layer into its composited ancestor?

             compositedLayerMapping()->setBlendMode(newBlendMode);
     }
 }
@@ -642,7 +643,11 @@ void RenderLayer::updatePagination()
     m_isPaginated = false;
     m_enclosingPaginationLayer = 0;
 
-    if (compositedLayerMapping() || !parent())
+    // If this layer is composited and paints into its own backing, it will be placed in its
+    // correct position via CompositedLayerMapping::updateGraphicsLayerGeometry. If this layer
+    // is not composited or paints into another backing, we'll have to update pagination so
+    // that it gets positioned correctly within the layer in which it is painted.
+    if (compositingState() == PaintsIntoOwnBacking || !parent())
         return; // FIXME: We will have to deal with paginated compositing layers someday.
                 // FIXME: For now the RenderView can't be paginated.  Eventually printing will move to a model where it is though.
 
@@ -993,7 +998,10 @@ bool RenderLayer::updateLayerPosition()
             localPoint += offset;
         }
     } else if (parent()) {
-        if (compositedLayerMapping()) {
+        // Rationale: composited layers need to be positioned in their correct column. If we've been squashed into another layer,
+        // or paint into another composited layer, we will already be positioned into the column-correct location. If we also
+        // adjust our position, we'll be double compensating.
+        if (compositingState() == PaintsIntoOwnBacking) {

[shawnsingh, 2013/11/12 09:55:50]

I think this will break for a scenario where a layer has compositingState
HasOwnBackingButPaintsIntoAncestor and is located on the second column.  Either
it will get positioned incorrectly because the CompositedLayerMapping assumes
that the "composited" layer is positioned correctly including it's multi-column
offset, or it may get hit tested incorrectly for a similar reason.

[Ian Vollick, 2013/11/14 04:12:47]

I think a layer that paints-into-ancestor would itself draw fine (I think the
painting code would get it positioned correctly), but I'm now nervous about the
composited descendants -- I bet they won't be positioned correctly if we skip
this. I think we need a layout test here.

             // FIXME: Composited layers ignore pagination, so about the best we can do is make sure they're offset into the appropriate column.
             // They won't split across columns properly.
             LayoutSize columnOffset;
@@ -1113,11 +1121,11 @@ static inline const RenderLayer* compositingContainer(const RenderLayer* layer)
 // enclosingCompositingLayerForRepaint().
 RenderLayer* RenderLayer::enclosingCompositingLayer(bool includeSelf) const
 {
-    if (includeSelf && compositedLayerMapping())
+    if (includeSelf && hasCompositedLayerMapping())
         return const_cast<RenderLayer*>(this);
 
     for (const RenderLayer* curr = compositingContainer(this); curr; curr = compositingContainer(curr)) {
-        if (curr->compositedLayerMapping())
+        if (curr->hasCompositedLayerMapping())
             return const_cast<RenderLayer*>(curr);
     }
 
@@ -1191,7 +1199,12 @@ bool RenderLayer::hasAncestorWithFilterOutsets() const
 
 RenderLayer* RenderLayer::clippingRootForPainting() const
 {
-    if (compositedLayerMapping())
+    // The compositor will handle clipping (via GraphicsLayers), but only for
+    // those layers that own a backing. All other layers (those that are, say,
+    // squashed or paint into another backing) must handle their clipping in
+    // software as they paint into their associated compositing ancestor for
+    // repaint.
+    if (compositingState() == PaintsIntoOwnBacking)

[shawnsingh, 2013/11/12 09:55:50]

red flag in comment - "only for layers that own a backing" --> includes layers
that HasOwnBackingButPaintsIntoAncestor.  So either the comment is wrong or the
condition is wrong?

[Ian Vollick, 2013/11/14 04:12:47]

VWhen I say "own a backing", I mean owns a mapping that is painted into. Could
we distinguish between mapping and backing? Is there better verbage we could be
using?

         return const_cast<RenderLayer*>(this);
 
     const RenderLayer* current = this;
@@ -1230,11 +1243,16 @@ bool RenderLayer::isTransparent() const
 
 RenderLayer* RenderLayer::transparentPaintingAncestor()
 {
-    if (compositedLayerMapping())
+    // The compositor will handle applying opacity for true composited layers.
+    // If we squish into a layer that is not transparent, or we paint into another
+    // layer's backing we won't be able to rely on the compositor to do this; we
+    // must handle our transparency in software as we paint into our ancestor
+    // compositing layer for repaint.
+    if (compositingState() == PaintsIntoOwnBacking)

[shawnsingh, 2013/11/12 09:55:50]

This seems like something we should make a layout test for the
paintsIntoAncestor case...

         return 0;
 
     for (RenderLayer* curr = parent(); curr; curr = curr->parent()) {
-        if (curr->compositedLayerMapping())
+        if (curr->compositingState() == PaintsIntoOwnBacking)
             return 0;
         if (curr->isTransparent())
             return curr;
@@ -1764,8 +1782,8 @@ void RenderLayer::updateScrollableArea()
 
 PassOwnPtr<Vector<FloatRect> > RenderLayer::collectTrackedRepaintRects() const
 {
-    if (CompositedLayerMapping* mapping = compositedLayerMapping())
-        return mapping->collectTrackedRepaintRects();
+    if (hasCompositedLayerMapping())
+        return compositedLayerMapping()->collectTrackedRepaintRects();
     return nullptr;
 }
 
@@ -3583,7 +3601,7 @@ IntRect RenderLayer::calculateLayerBounds(const RenderLayer* ancestorLayer, cons
 
     if (m_reflectionInfo) {
         RenderLayer* reflectionLayer = m_reflectionInfo->reflectionLayer();
-        if (!reflectionLayer->compositedLayerMapping()) {
+        if (!reflectionLayer->hasCompositedLayerMapping()) {

[Ian Vollick, 2013/11/14 04:12:47]

I should mention that my assumption (which I should have enforced with an ASSERT
here) was the reflection would either paint into the same backing as |this| or
it would paint into its own backing.

             IntRect childUnionBounds = reflectionLayer->calculateLayerBounds(this, 0, descendantFlags);
             unionBounds.unite(childUnionBounds);
         }
@@ -3600,7 +3618,7 @@ IntRect RenderLayer::calculateLayerBounds(const RenderLayer* ancestorLayer, cons
     // This applies to all z-order lists below.
     RenderLayerStackingNodeIterator iterator(*m_stackingNode.get(), AllChildren);
     while (RenderLayerStackingNode* node = iterator.next()) {
-        if (flags & IncludeCompositedDescendants || !node->layer()->compositedLayerMapping()) {
+        if (flags & IncludeCompositedDescendants || node->layer()->compositingState() != PaintsIntoOwnBacking) {

[shawnsingh, 2013/11/12 09:55:50]

Can we get a layout test for this?  I had made this a FIXME (which you can
remove 2 lines above this) because we ought to regard this a bug and get a
layout test that reproduces the incorrect composited layer bounds. 

[Ian Vollick, 2013/11/14 04:12:47]

Actually, this is hit by a layout test:
transforms/3d/point-mapping/3d-point-mapping-deep.html

             IntRect childUnionBounds = node->layer()->calculateLayerBounds(this, 0, descendantFlags);
             unionBounds.unite(childUnionBounds);
         }
@@ -3643,7 +3661,7 @@ CompositingState RenderLayer::compositingState() const
     return PaintsIntoOwnBacking;
 }
 
-CompositedLayerMapping* RenderLayer::ensureCompositedLayerMapping()
+CompositedLayerMappingPtr RenderLayer::ensureCompositedLayerMapping()
 {
     if (!m_compositedLayerMapping) {
         m_compositedLayerMapping = adoptPtr(new CompositedLayerMapping(this));
@@ -3730,7 +3748,9 @@ bool RenderLayer::listBackgroundIsKnownToBeOpaqueInRect(const Vector<RenderLayer
 
     for (Vector<RenderLayerStackingNode*>::const_reverse_iterator iter = list->rbegin(); iter != list->rend(); ++iter) {
         const RenderLayer* childLayer = (*iter)->layer();
-        if (childLayer->compositedLayerMapping())
+        // We skip layers that paint into their own backing (they do not affect the
+        // opaqueness of us).
+        if (childLayer->compositingState() == PaintsIntoOwnBacking)
             continue;
 
         if (!childLayer->canUseConvertToLayerCoords())
@@ -3903,7 +3923,7 @@ inline bool RenderLayer::needsCompositingLayersRebuiltForClip(const RenderStyle*
 inline bool RenderLayer::needsCompositingLayersRebuiltForOverflow(const RenderStyle* oldStyle, const RenderStyle* newStyle) const
 {
     ASSERT(newStyle);
-    return !compositedLayerMapping() && oldStyle && (oldStyle->overflowX() != newStyle->overflowX()) && m_stackingNode->ancestorStackingContainerNode()->layer()->hasCompositingDescendant();
+    return !hasCompositedLayerMapping() && oldStyle && (oldStyle->overflowX() != newStyle->overflowX()) && m_stackingNode->ancestorStackingContainerNode()->layer()->hasCompositingDescendant();
 }
 
 inline bool RenderLayer::needsCompositingLayersRebuiltForFilters(const RenderStyle* oldStyle, const RenderStyle* newStyle, bool didPaintWithFilters) const
@@ -3947,7 +3967,7 @@ void RenderLayer::updateFilters(const RenderStyle* oldStyle, const RenderStyle*
     updateOrRemoveFilterClients();
     // During an accelerated animation, both WebKit and the compositor animate properties.
     // However, WebKit shouldn't ask the compositor to update its filters if the compositor is performing the animation.
-    bool shouldUpdateFilters = compositedLayerMapping() && !renderer()->animation().isRunningAcceleratedAnimationOnRenderer(renderer(), CSSPropertyWebkitFilter);
+    bool shouldUpdateFilters = compositingState() == PaintsIntoOwnBacking && !renderer()->animation().isRunningAcceleratedAnimationOnRenderer(renderer(), CSSPropertyWebkitFilter);
     if (shouldUpdateFilters)
         compositedLayerMapping()->updateFilters(renderer()->style());
     updateOrRemoveFilterEffectRenderer();
@@ -3986,7 +4006,7 @@ void RenderLayer::styleChanged(StyleDifference, const RenderStyle* oldStyle)
         || needsCompositingLayersRebuiltForOverflow(oldStyle, newStyle)
         || needsCompositingLayersRebuiltForFilters(oldStyle, newStyle, didPaintWithFilters))
         compositor()->setCompositingLayersNeedRebuild();
-    else if (compositedLayerMapping())
+    else if (hasCompositedLayerMapping())
         compositedLayerMapping()->updateGraphicsLayerGeometry();
 }