Fix accumulating offsets for fixed positioned layers

Source/core/paint/DeprecatedPaintLayer.cpp

Index: Source/core/paint/DeprecatedPaintLayer.cpp
diff --git a/Source/core/paint/DeprecatedPaintLayer.cpp b/Source/core/paint/DeprecatedPaintLayer.cpp
index 8e4c167af218d24dba9db17b5e09ded6ba62f9f2..c3b1b96f97efbce72cc247409655bafa931989df 100644
--- a/Source/core/paint/DeprecatedPaintLayer.cpp
+++ b/Source/core/paint/DeprecatedPaintLayer.cpp
@@ -1379,7 +1379,8 @@ static inline const DeprecatedPaintLayer* accumulateOffsetTowardsAncestor(const
     if (position == FixedPosition) {
         // For a fixed layers, we need to walk up to the root to see if there's a fixed position container
         // (e.g. a transformed layer). It's an error to call convertToLayerCoords() across a layer with a transform,
-        // so we should always find the ancestor at or before we find the fixed position container.
+        // so we should always find the ancestor at or before we find the fixed position container, if
+        // the container is transformed.
         DeprecatedPaintLayer* fixedPositionContainerLayer = 0;
         bool foundAncestor = false;
         for (DeprecatedPaintLayer* currLayer = layer->parent(); currLayer; currLayer = currLayer->parent()) {
@@ -1388,7 +1389,11 @@ static inline const DeprecatedPaintLayer* accumulateOffsetTowardsAncestor(const
 
             if (isFixedPositionedContainer(currLayer)) {
                 fixedPositionContainerLayer = currLayer;
-                ASSERT_UNUSED(foundAncestor, foundAncestor);
+                // A layer that has a transform-related property but not a
+                // transform still acts as a fixed-position container.
+                // Accumulating offsets across such layers is allowed.
+                if (currLayer->transform())
+                    ASSERT_UNUSED(foundAncestor, foundAncestor);
                 break;
             }
         }
@@ -1398,11 +1403,13 @@ static inline const DeprecatedPaintLayer* accumulateOffsetTowardsAncestor(const
         if (fixedPositionContainerLayer != ancestorLayer) {
             LayoutPoint fixedContainerCoords;
             layer->convertToLayerCoords(fixedPositionContainerLayer, fixedContainerCoords);
+            location += fixedContainerCoords;
 
-            LayoutPoint ancestorCoords;
-            ancestorLayer->convertToLayerCoords(fixedPositionContainerLayer, ancestorCoords);
-
-            location += (fixedContainerCoords - ancestorCoords);
+            if (foundAncestor) {

[chrishtr, 2015/04/30 17:46:01]

Why would it not find the ancestor? What does this have to do with the change
here?

[ajuma, 2015/04/30 17:57:15]

The code previously assumed that ancestorLayer was either the
fixedPositionContainerLayer or its descendant. We were hitting the assert (and
rendering incorrectly) when that wasn't the case. With the assert fixed, we
still need to compute the offset correctly for this case (where the
ancestorLayer is also an ancestor of the fixedPositionContainerLayer). We can't
call convertToLayerCoords below in that case, since that expects an ancestor of
ancestorLayer. So instead we only compute the offset up to the
fixedPositionContainerLayer, and allow the loop in the caller to then compute
the offset from the fixedPositionContainerLayer to the ancestorLayer.

+                LayoutPoint ancestorCoords;
+                ancestorLayer->convertToLayerCoords(fixedPositionContainerLayer, ancestorCoords);
+                location += -ancestorCoords;
+            }
         } else {
             // LayoutView has been handled in the first top-level 'if' block above.
             ASSERT(ancestorLayer != layoutObject->view()->layer());
@@ -1410,7 +1417,7 @@ static inline const DeprecatedPaintLayer* accumulateOffsetTowardsAncestor(const
 
             location += layer->location();
         }
-        return ancestorLayer;
+        return foundAncestor ? ancestorLayer : fixedPositionContainerLayer;
     }
 
     DeprecatedPaintLayer* parentLayer;