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

Source/core/page/scrolling/ScrollingCoordinator.cpp

Index: Source/core/page/scrolling/ScrollingCoordinator.cpp
diff --git a/Source/core/page/scrolling/ScrollingCoordinator.cpp b/Source/core/page/scrolling/ScrollingCoordinator.cpp
index 1905e88b6fa879564e33c7f06b9f5f89072514f2..dbb55ffd48d4084c68e20748e1742dd25f70153e 100644
--- a/Source/core/page/scrolling/ScrollingCoordinator.cpp
+++ b/Source/core/page/scrolling/ScrollingCoordinator.cpp
@@ -172,7 +172,7 @@ static void clearPositionConstraintExceptForLayer(GraphicsLayer* layer, Graphics
 
 static WebLayerPositionConstraint computePositionConstraint(const RenderLayer* layer)
 {
-    ASSERT(layer->compositedLayerMapping());
+    ASSERT(layer->hasCompositedLayerMapping());
     do {
         if (layer->renderer()->style()->position() == FixedPosition) {
             const RenderObject* fixedPositionObject = layer->renderer();
@@ -185,14 +185,42 @@ static WebLayerPositionConstraint computePositionConstraint(const RenderLayer* l
 
         // Composited layers that inherit a fixed position state will be positioned with respect to the nearest compositedLayerMapping's GraphicsLayer.
         // So, once we find a layer that has its own compositedLayerMapping, we can stop searching for a fixed position RenderObject.
-    } while (layer && layer->compositedLayerMapping());
+        //
+        // FIXME: I don't think that this is correct. First, the above comment is wrong: we stop searching
+        // precisely when we find a layer that does *not* have a compositedLayerMapping. Second, I think the
+        // logic is broken, even if we were to update this comment. Consider the following tree:
+        //
+        // + A - composited/fixed-pos
+        //   + B - composited/not-fixed-pos
+        //
+        // If we call computePositionConstraint for B, we skip the 'if' block above (because B is not
+        // fixed-pos), but we will continue marching up to A. Since A is fixed-pos we will return a
+        // position constraint. The result: the chain of composited layers below a composited, fixed-pos
+        // layer will all report that they have a position constraint. This seems wrong. Since composited
+        // layers inherit their positions, if their composited ancestor is fixed, they ought to inherit
+        // that fixed positioning and require no more intervention from the compositor for scroll
+        // compensation, etc.
+        //
+        // And there's another problem. Consider this tree.
+        //
+        // + A - composited
+        //   + B - not-composited/fixed-pos
+        //     + C - composited/not-fixed-pos
+        //
+        // In this case, we will not return a position constraint for C -- its parent is not composited
+        // and so we will not perform a second iteration of the for loop. This seems correct: C should
+        // get positioned correctly via CompositedLayerMapping::updateGraphicsLayerGeometry, and because
+        // we have a non-composited fixed-pos layer, we should not impl-scroll so we can rely on that
+        // main thread positioning logic to sort us out. The problem? This is inconsistent with the case
+        // above.
+    } while (layer && layer->hasCompositedLayerMapping());
     return WebLayerPositionConstraint();
 }
 
 void ScrollingCoordinator::updateLayerPositionConstraint(RenderLayer* layer)
 {
-    ASSERT(layer->compositedLayerMapping());
-    CompositedLayerMapping* compositedLayerMapping = layer->compositedLayerMapping();
+    ASSERT(layer->hasCompositedLayerMapping());
+    CompositedLayerMappingPtr compositedLayerMapping = layer->compositedLayerMapping();
     GraphicsLayer* mainLayer = compositedLayerMapping->childForSuperlayers();
 
     // Avoid unnecessary commits
@@ -510,7 +538,9 @@ void ScrollingCoordinator::setTouchEventTargetRects(const LayerHitTestRects& lay
         WebVector<WebRect> webRects(iter->value.size());
         for (size_t i = 0; i < iter->value.size(); ++i)
             webRects[i] = enclosingIntRect(iter->value[i]);
-        CompositedLayerMapping* compositedLayerMapping = layer->compositedLayerMapping();
+        // This should be ensured by convertLayerRectsToEnclosingCompositedLayer above.
+        ASSERT(layer->hasCompositedLayerMapping());
+        CompositedLayerMappingPtr compositedLayerMapping = layer->compositedLayerMapping();
         // If the layer is using composited scrolling, then it's the contents that these
         // rects apply to.
         GraphicsLayer* graphicsLayer = compositedLayerMapping->scrollingContentsLayer();
@@ -523,10 +553,14 @@ void ScrollingCoordinator::setTouchEventTargetRects(const LayerHitTestRects& lay
 
     // If there are any layers left that we haven't updated, clear them out.
     for (HashSet<const RenderLayer*>::iterator it = oldLayersWithTouchRects.begin(); it != oldLayersWithTouchRects.end(); ++it) {
-        if (CompositedLayerMapping* compositedLayerMapping = (*it)->compositedLayerMapping()) {
-            GraphicsLayer* graphicsLayer = compositedLayerMapping->scrollingContentsLayer();
+        // FIXME: This is a bug. What's happening here is that we're clearing touch regions for
+        // layers that we didn't visit above. That assumes a 1:1 mapping between RenderLayer and
+        // the graphics layer that owns the touch rects. This is false in the case of
+        // HasOwnBackingButPaintsIntoAncestor and will be extra-false in the world of squashing.
+        if ((*it)->hasCompositedLayerMapping()) {
+            GraphicsLayer* graphicsLayer = (*it)->compositedLayerMapping()->scrollingContentsLayer();
             if (!graphicsLayer)
-                graphicsLayer = compositedLayerMapping->mainGraphicsLayer();
+                graphicsLayer = (*it)->compositedLayerMapping()->mainGraphicsLayer();
             graphicsLayer->platformLayer()->setTouchEventHandlerRegion(WebVector<WebRect>());
         }
     }
@@ -553,7 +587,7 @@ void ScrollingCoordinator::touchEventTargetRectsDidChange(const Document*)
 void ScrollingCoordinator::updateScrollParentForGraphicsLayer(GraphicsLayer* child, RenderLayer* parent)
 {
     WebLayer* scrollParentWebLayer = 0;
-    if (parent && parent->compositedLayerMapping())
+    if (parent && parent->hasCompositedLayerMapping())
         scrollParentWebLayer = scrollingWebLayerForGraphicsLayer(parent->compositedLayerMapping()->parentForSublayers());
 
     child->setScrollParent(scrollParentWebLayer);
@@ -562,7 +596,7 @@ void ScrollingCoordinator::updateScrollParentForGraphicsLayer(GraphicsLayer* chi
 void ScrollingCoordinator::updateClipParentForGraphicsLayer(GraphicsLayer* child, RenderLayer* parent)
 {
     WebLayer* clipParentWebLayer = 0;
-    if (parent && parent->compositedLayerMapping())
+    if (parent && parent->hasCompositedLayerMapping())
         clipParentWebLayer = scrollingWebLayerForGraphicsLayer(parent->compositedLayerMapping()->parentForSublayers());
 
     child->setClipParent(clipParentWebLayer);