[CSS Grid Layout] Fix positioned grid children position and size

Source/core/rendering/RenderGrid.cpp

Index: Source/core/rendering/RenderGrid.cpp
diff --git a/Source/core/rendering/RenderGrid.cpp b/Source/core/rendering/RenderGrid.cpp
index b24a0b5504b229d8335598d4f41d51f58b3c8dc7..a6d0c103dc09b73b9113089a7314d5456b9b0e40 100644
--- a/Source/core/rendering/RenderGrid.cpp
+++ b/Source/core/rendering/RenderGrid.cpp
@@ -1054,28 +1054,47 @@ void RenderGrid::layoutGridItems()
     m_gridItemsOverflowingGridArea.resize(0);
 
     for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
-        if (child->isOutOfFlowPositioned()) {
-            // FIXME: Absolute positioned grid items should have a special
-            // behavior as described in the spec (crbug.com/273898):
-            // http://www.w3.org/TR/css-grid-1/#abspos-items
-            child->containingBlock()->insertPositionedObject(child);
-        }
-
         // Because the grid area cannot be styled, we don't need to adjust
         // the grid breadth to account for 'box-sizing'.
         LayoutUnit oldOverrideContainingBlockContentLogicalWidth = child->hasOverrideContainingBlockLogicalWidth() ? child->overrideContainingBlockContentLogicalWidth() : LayoutUnit();
         LayoutUnit oldOverrideContainingBlockContentLogicalHeight = child->hasOverrideContainingBlockLogicalHeight() ? child->overrideContainingBlockContentLogicalHeight() : LayoutUnit();
 
-        LayoutUnit overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(*child, ForColumns, sizingData.columnTracks);
-        LayoutUnit overrideContainingBlockContentLogicalHeight = gridAreaBreadthForChild(*child, ForRows, sizingData.rowTracks);
+        LayoutUnit columnExtraOffset = 0;
+        LayoutUnit rowExtraOffset = 0;
+        LayoutUnit overrideContainingBlockContentLogicalWidth;
+        LayoutUnit overrideContainingBlockContentLogicalHeight;
 
-        SubtreeLayoutScope layoutScope(*child);
-        if (oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && child->hasRelativeLogicalHeight()))
-            layoutScope.setNeedsLayout(child);
+        if (child->isOutOfFlowPositioned()) {
+            overrideContainingBlockContentLogicalWidth = gridAreaBreadthForPositionedChild(*child, ForColumns, sizingData.columnTracks, columnExtraOffset);
+            overrideContainingBlockContentLogicalHeight = gridAreaBreadthForPositionedChild(*child, ForRows, sizingData.rowTracks, rowExtraOffset);
+        } else {
+            overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(*child, ForColumns, sizingData.columnTracks);
+            overrideContainingBlockContentLogicalHeight = gridAreaBreadthForChild(*child, ForRows, sizingData.rowTracks);
+        }
 
         child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
         child->setOverrideContainingBlockContentLogicalHeight(overrideContainingBlockContentLogicalHeight);
 
+        if (child->isOutOfFlowPositioned()) {
+            child->containingBlock()->insertPositionedObject(child);
+
+            bool containerHasHorizontalWritingMode = isHorizontalWritingMode();
+            LayoutPoint position = LayoutPoint(startOfColumnForChild(*child), startOfRowForChild(*child));
+
+            RenderLayer* childLayer = child->layer();
+            // Set the layer position taking into account the extra size
+            // increased in RenderGrid::gridAreaBreadthForChild() due to the
+            // positioned items particularities.
+            childLayer->setStaticInlinePosition(containerHasHorizontalWritingMode ? position.x() - columnExtraOffset : position.y() - rowExtraOffset);
+            childLayer->setStaticBlockPosition(containerHasHorizontalWritingMode ? position.y() - rowExtraOffset : position.x() - columnExtraOffset);
+
+            continue;
+        }
+
+        SubtreeLayoutScope layoutScope(*child);
+        if (oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && child->hasRelativeLogicalHeight()))
+            layoutScope.setNeedsLayout(child);
+
         // FIXME: Grid items should stretch to fill their cells. Once we
         // implement grid-{column,row}-align, we can also shrink to fit. For
         // now, just size as if we were a regular child.
@@ -1109,13 +1128,68 @@ GridCoordinate RenderGrid::cachedGridCoordinate(const RenderBox& gridItem) const
     return m_gridItemCoordinate.get(&gridItem);
 }
 
+static LayoutUnit computeGridAreaBreadth(const GridSpan& span, const Vector<GridTrack>& tracks, LayoutUnit* extraInitialOffset = 0, bool startIsAuto = false, GridResolvedPosition* originalInitialPosition = 0)
+{
+    LayoutUnit gridAreaBreadth = 0;
+    for (GridSpan::iterator trackPosition = span.begin(); trackPosition != span.end(); ++trackPosition) {
+        LayoutUnit trackBreadth = tracks[trackPosition.toInt()].m_usedBreadth;
+        gridAreaBreadth += trackBreadth;
+        if (extraInitialOffset && startIsAuto && trackPosition < *originalInitialPosition)
+            *extraInitialOffset += trackBreadth;
+    }
+    return gridAreaBreadth;
+}
+
 LayoutUnit RenderGrid::gridAreaBreadthForChild(const RenderBox& child, GridTrackSizingDirection direction, const Vector<GridTrack>& tracks) const
 {
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
     const GridSpan& span = (direction == ForColumns) ? coordinate.columns : coordinate.rows;
+    return computeGridAreaBreadth(span, tracks);
+}
+
+LayoutUnit RenderGrid::gridAreaBreadthForPositionedChild(const RenderBox& child, GridTrackSizingDirection direction, const Vector<GridTrack>& tracks, LayoutUnit& extraInitialOffset) const

[Julien - ping for review, 2014/10/23 15:24:44]

'positioned' element covers any position != static in CSS 2.1. It's my mistake
on this one, I should have been precise in the first review.

[Manuel Rego, 2014/10/23 21:51:12]

Yes, I changed it to gridAreaBreadthForAbsolutelyPositionedChild().

+{
+    ASSERT(child.isOutOfFlowPositioned());

[Julien - ping for review, 2014/10/23 15:24:43]

out-of-flow positioned convers absolutely positioned *and fixed positioned*. You
want to check style()->position() == AbsolutePostion instead.

[Manuel Rego, 2014/10/23 21:51:12]

So, I've added a new method to check if it's actually an AbsolutePosition,
but also if the containing block is the grid.
Otherwise, we shouldn't do need any special code to manage those elements.

+
     LayoutUnit gridAreaBreadth = 0;
-    for (GridSpan::iterator trackPosition = span.begin(); trackPosition != span.end(); ++trackPosition)
-        gridAreaBreadth += tracks[trackPosition.toInt()].m_usedBreadth;
+    bool startIsAuto = false;
+    bool endIsAuto = false;
+    // For positioned items the containing block corresponds to the padding
+    // edges of the grid. So, we need to increase the breadth if the
+    // positions are auto.
+    if (direction == ForColumns) {
+        if (child.style()->gridColumnStart().isAuto()) {
+            startIsAuto = true;
+            gridAreaBreadth += paddingLeft();
+            extraInitialOffset += paddingLeft();
+        }
+        if (child.style()->gridColumnEnd().isAuto()) {
+            endIsAuto = true;
+            gridAreaBreadth += paddingRight();
+        }
+    } else {
+        if (child.style()->gridRowStart().isAuto()) {
+            startIsAuto = true;
+            gridAreaBreadth += paddingTop();
+            extraInitialOffset += paddingTop();
+        }
+        if (child.style()->gridRowEnd().isAuto()) {
+            endIsAuto = true;
+            gridAreaBreadth += paddingBottom();
+        }
+    }
+
+    const GridCoordinate& coordinate = cachedGridCoordinate(child);

[Julien - ping for review, 2014/10/23 15:24:44]

If the grid placement property is 'auto', we should position the positioned
element against the grid container's edges.

[Manuel Rego, 2014/10/23 21:51:12]

Actually according to the spec the containing block corresponds to the
padding edges of the grid.

That's why we're adding the padding to the breadth and also setting the
initial and final positions accordingly if start and end are auto (just a few
lines below).

I think this is already working as expected unless I'm missing something.

+    GridResolvedPosition initialPosition = (direction == ForColumns) ? coordinate.columns.resolvedInitialPosition : coordinate.rows.resolvedInitialPosition;
+    GridResolvedPosition originalInitialPosition = initialPosition;
+    GridResolvedPosition finalPosition = (direction == ForColumns) ? coordinate.columns.resolvedFinalPosition : coordinate.rows.resolvedFinalPosition;
+
+    if (startIsAuto)
+        initialPosition = GridResolvedPosition(0);
+    if (endIsAuto)
+        finalPosition = GridResolvedPosition((direction == ForColumns ? gridColumnCount() : gridRowCount()) - 1);
+
+    gridAreaBreadth += computeGridAreaBreadth(GridSpan(initialPosition, finalPosition), tracks, &extraInitialOffset, startIsAuto, &originalInitialPosition);
     return gridAreaBreadth;
 }