[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..7c4a6ab8c0398591b3f173a09bc8fa4565d310cd 100644
--- a/Source/core/rendering/RenderGrid.cpp
+++ b/Source/core/rendering/RenderGrid.cpp
@@ -1054,28 +1054,39 @@ 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

[Julien - ping for review, 2014/10/20 19:34:43]

Please do not link to the Working Draft as it's super old. We should look at the
Editor's draft for implementation
(http://dev.w3.org/csswg/css-grid/#abspos-items). This is true for the
description too.

-            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 columnExtraSize = 0;
+        LayoutUnit overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(*child, ForColumns, sizingData.columnTracks, &columnExtraSize);
+        LayoutUnit rowExtraSize = 0;
+        LayoutUnit overrideContainingBlockContentLogicalHeight = gridAreaBreadthForChild(*child, ForRows, sizingData.rowTracks, &rowExtraSize);
+
+        child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
+        child->setOverrideContainingBlockContentLogicalHeight(overrideContainingBlockContentLogicalHeight);
+
+        if (child->isOutOfFlowPositioned()) {
+            child->containingBlock()->insertPositionedObject(child);
+
+            bool isHorizontal = isHorizontalWritingMode();

[Julien - ping for review, 2014/10/20 19:34:44]

Not a great variable. How about containerHasHorizontalWritingMode?

[Manuel Rego, 2014/10/23 12:48:38]

Acknowledged.

+            LayoutPoint position = findChildLogicalPosition(*child);

[Julien - ping for review, 2014/10/20 19:34:43]

findChildLogicalPosition is going to apply 'justify-content' and 'align-content'
based on the grid cell's position / dimension which doesn't seem to be what we
should be doing.

[Manuel Rego, 2014/10/23 12:48:38]

Yes, I've changed it to avoid this issue.

+
+            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(isHorizontal ? position.x() - columnExtraSize : position.y() - rowExtraSize);
+            childLayer->setStaticBlockPosition(isHorizontal ? position.y() - rowExtraSize : position.x() - columnExtraSize);
+
+            continue;
+        }
 
         SubtreeLayoutScope layoutScope(*child);
         if (oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && child->hasRelativeLogicalHeight()))
             layoutScope.setNeedsLayout(child);
 
-        child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
-        child->setOverrideContainingBlockContentLogicalHeight(overrideContainingBlockContentLogicalHeight);
-
         // 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 +1120,51 @@ GridCoordinate RenderGrid::cachedGridCoordinate(const RenderBox& gridItem) const
     return m_gridItemCoordinate.get(&gridItem);
 }
 
-LayoutUnit RenderGrid::gridAreaBreadthForChild(const RenderBox& child, GridTrackSizingDirection direction, const Vector<GridTrack>& tracks) const
+LayoutUnit RenderGrid::gridAreaBreadthForChild(const RenderBox& child, GridTrackSizingDirection direction, const Vector<GridTrack>& tracks, LayoutUnit* extraSize) const

[Julien - ping for review, 2014/10/20 19:34:44]

This really looks like it should be split into 2 functions:
* gridAreaBreadthForChild is unchanged.
* gridAreaBreadthForPositionedChild should call gridAreaBreadthForChild (or some
part of gridAreaBreadthForChild that is suitable for it).

The code in the patch spends most of its time dealing with positioned oddities.

[Manuel Rego, 2014/10/23 12:48:38]

I agree, most of my changes were only dealing with positioned stuff, so I've
separated it in 2 different methods as suggested.

 {
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
     const GridSpan& span = (direction == ForColumns) ? coordinate.columns : coordinate.rows;
+
     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;
+    if (extraSize && child.isOutOfFlowPositioned()) {
+        // 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();
+                *extraSize += paddingLeft();
+            }
+            if (child.style()->gridColumnEnd().isAuto()) {
+                endIsAuto = true;
+                gridAreaBreadth += paddingRight();
+            }
+        } else {
+            if (child.style()->gridRowStart().isAuto()) {
+                startIsAuto = true;
+                gridAreaBreadth += paddingTop();
+                *extraSize += paddingTop();
+            }
+            if (child.style()->gridRowEnd().isAuto()) {
+                endIsAuto = true;
+                gridAreaBreadth += paddingBottom();
+            }
+        }
+    }
+
+    GridResolvedPosition initialPosition = startIsAuto ? GridResolvedPosition(0) : span.resolvedInitialPosition;
+    GridResolvedPosition finalPosition = endIsAuto ? GridResolvedPosition((direction == ForColumns ? gridColumnCount() : gridRowCount()) - 1) : span.resolvedFinalPosition;
+    GridSpan iteratorSpan(initialPosition, finalPosition);
+
+    for (GridSpan::iterator trackPosition = iteratorSpan.begin(); trackPosition != iteratorSpan.end(); ++trackPosition) {
+        LayoutUnit trackBreadth = tracks[trackPosition.toInt()].m_usedBreadth;
+        gridAreaBreadth += trackBreadth;
+        if (extraSize && startIsAuto && trackPosition < span.resolvedInitialPosition)
+            *extraSize += trackBreadth;
+    }
     return gridAreaBreadth;
 }