[CSS Grid Layout] Implement auto-margins alignment of grid items

Source/core/layout/LayoutGrid.cpp

Index: Source/core/layout/LayoutGrid.cpp
diff --git a/Source/core/layout/LayoutGrid.cpp b/Source/core/layout/LayoutGrid.cpp
index 56826d06c9350e129bc8fc841290ceabc90320d6..273be8c2a60f358ded84fdbb91bfe168ec5d4ad6 100644
--- a/Source/core/layout/LayoutGrid.cpp
+++ b/Source/core/layout/LayoutGrid.cpp
@@ -1352,6 +1352,10 @@ void LayoutGrid::layoutGridItems()
 
         child->layoutIfNeeded();
 
+        // We need pending layouts to be done in order to compute auto-margins properly.

[Manuel Rego, 2015/09/24 11:38:26]

Probably we could add an ASSERT in the methods
to check that the layout has been done.

[jfernandez, 2015/09/25 13:47:02]

Acknowledged.

+        updateAutoMarginsInColumnAxisIfNeeded(*child);
+        updateAutoMarginsInRowAxisIfNeeded(*child);
+
 #if ENABLE(ASSERT)
         const GridCoordinate& coordinate = cachedGridCoordinate(*child);
         ASSERT(coordinate.columns.resolvedInitialPosition.toInt() < sizingData.columnTracks.size());
@@ -1608,15 +1612,10 @@ LayoutUnit LayoutGrid::computeMarginLogicalHeightForChild(const LayoutBox& child
 
 LayoutUnit LayoutGrid::availableAlignmentSpaceForChildBeforeStretching(LayoutUnit gridAreaBreadthForChild, const LayoutBox& child) const
 {
-    LayoutUnit childMarginLogicalHeight = marginLogicalHeightForChild(child);
-
     // Because we want to avoid multiple layouts, stretching logic might be performed before
     // children are laid out, so we can't use the child cached values. Hence, we need to
     // compute margins in order to determine the available height before stretching.
-    if (childMarginLogicalHeight == 0)
-        childMarginLogicalHeight = computeMarginLogicalHeightForChild(child);
-
-    return gridAreaBreadthForChild - childMarginLogicalHeight;
+    return gridAreaBreadthForChild - (child.needsLayout() ? computeMarginLogicalHeightForChild(child) : marginLogicalHeightForChild(child));
 }
 
 // FIXME: This logic is shared by LayoutFlexibleBox, so it should be moved to LayoutBox.
@@ -1663,6 +1662,88 @@ void LayoutGrid::applyStretchAlignmentToChildIfNeeded(LayoutBox& child)
     }
 }
 
+// TODO(lajava): This logic is shared by LayoutFlexibleBox, so it should be moved to LayoutBox.
+bool LayoutGrid::hasAutoMarginsInColumnAxis(const LayoutBox& child) const
+{
+    if (isHorizontalWritingMode())
+        return child.style()->marginTop().isAuto() || child.style()->marginBottom().isAuto();
+    return child.style()->marginLeft().isAuto() || child.style()->marginRight().isAuto();
+}
+
+// TODO(lajava): This logic is shared by LayoutFlexibleBox, so it should be moved to LayoutBox.
+bool LayoutGrid::hasAutoMarginsInRowAxis(const LayoutBox& child) const
+{
+    if (isHorizontalWritingMode())
+        return child.style()->marginLeft().isAuto() || child.style()->marginRight().isAuto();
+    return child.style()->marginTop().isAuto() || child.style()->marginBottom().isAuto();
+}
+
+// TODO(lajava): This logic is shared by LayoutFlexibleBox, so it should be moved to LayoutBox.
+void LayoutGrid::updateAutoMarginsInRowAxisIfNeeded(LayoutBox& child)
+{
+    ASSERT(!child.isOutOfFlowPositioned());
+
+    LayoutUnit availableAlignmentSpace = child.overrideContainingBlockContentLogicalWidth() - child.logicalWidth();
+    if (availableAlignmentSpace <= 0)
+        return;
+
+    bool isHorizontal = isHorizontalWritingMode();
+    Length topOrLeft = isHorizontal ? child.style()->marginLeft() : child.style()->marginTop();

[Manuel Rego, 2015/09/24 11:38:26]

To simplify the code and all the checks related to the writting mode
I guess you can use marginStart() here.

[jfernandez, 2015/09/25 13:47:02]

I think it wouldn't be an equivalent logic. Be aware that we should check out
here the value set in the CSS style, not the one finally resolved. 

[Manuel Rego, 2015/09/25 16:00:58]

BTW, I was thinking in style()->marginStart()
which is already checking the writing mode by itself.

+    Length bottomOrRight = isHorizontal ? child.style()->marginRight() : child.style()->marginBottom();

[Manuel Rego, 2015/09/24 11:38:27]

Ditto.

[jfernandez, 2015/09/25 13:47:02]

Replied.

+    if (topOrLeft.isAuto() && bottomOrRight.isAuto()) {
+        if (isHorizontal) {
+            child.setMarginLeft(availableAlignmentSpace / 2);
+            child.setMarginRight(availableAlignmentSpace / 2);
+        } else {
+            child.setMarginTop(availableAlignmentSpace / 2);
+            child.setMarginBottom(availableAlignmentSpace / 2);
+        }

[Manuel Rego, 2015/09/24 11:38:27]

Probably here you could use setMarginStart().

[jfernandez, 2015/09/25 13:47:02]

This could be true, but I'm not sure whether it'd be confusing, considering that
we cannot use start/end when retrieving the input data.

+    } else if (topOrLeft.isAuto()) {
+        if (isHorizontal)
+            child.setMarginLeft(availableAlignmentSpace);
+        else
+            child.setMarginTop(availableAlignmentSpace);

[Manuel Rego, 2015/09/24 11:38:27]

Ditto.

[jfernandez, 2015/09/25 13:47:02]

Replied.

+    } else if (bottomOrRight.isAuto()) {
+        if (isHorizontal)
+            child.setMarginRight(availableAlignmentSpace);
+        else
+            child.setMarginBottom(availableAlignmentSpace);

[Manuel Rego, 2015/09/24 11:38:27]

Ditto.

[jfernandez, 2015/09/25 13:47:02]

Replied.

+    }
+}
+
+// TODO(lajava): This logic is shared by LayoutFlexibleBox, so it should be moved to LayoutBox.
+void LayoutGrid::updateAutoMarginsInColumnAxisIfNeeded(LayoutBox& child)
+{
+    ASSERT(!child.isOutOfFlowPositioned());
+
+    LayoutUnit availableAlignmentSpace = child.overrideContainingBlockContentLogicalHeight() - child.logicalHeight();
+    if (availableAlignmentSpace <= 0)
+        return;
+
+    bool isHorizontal = isHorizontalWritingMode();
+    Length topOrLeft = isHorizontal ? child.style()->marginTop() : child.style()->marginLeft();

[Manuel Rego, 2015/09/24 11:38:26]

The same than in the row axis method.

[jfernandez, 2015/09/25 13:47:02]

Replied.

+    Length bottomOrRight = isHorizontal ? child.style()->marginBottom() : child.style()->marginRight();
+    if (topOrLeft.isAuto() && bottomOrRight.isAuto()) {
+        if (isHorizontal) {
+            child.setMarginTop(availableAlignmentSpace / 2);
+            child.setMarginBottom(availableAlignmentSpace / 2);
+        } else {
+            child.setMarginLeft(availableAlignmentSpace / 2);
+            child.setMarginRight(availableAlignmentSpace / 2);
+        }
+    } else if (topOrLeft.isAuto()) {
+        if (isHorizontal)
+            child.setMarginTop(availableAlignmentSpace);
+        else
+            child.setMarginLeft(availableAlignmentSpace);
+    } else if (bottomOrRight.isAuto()) {
+        if (isHorizontal)
+            child.setMarginBottom(availableAlignmentSpace);
+        else
+            child.setMarginRight(availableAlignmentSpace);
+    }
+}
+
 GridAxisPosition LayoutGrid::columnAxisPositionForChild(const LayoutBox& child) const
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
@@ -1760,6 +1841,8 @@ LayoutUnit LayoutGrid::columnAxisOffsetForChild(const LayoutBox& child) const
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
     LayoutUnit startOfRow = m_rowPositions[coordinate.rows.resolvedInitialPosition.toInt()];
     LayoutUnit startPosition = startOfRow + marginBeforeForChild(child);
+    if (hasAutoMarginsInColumnAxis(child))
+        return startPosition;
     GridAxisPosition axisPosition = columnAxisPositionForChild(child);
     switch (axisPosition) {
     case GridAxisStart:
@@ -1781,6 +1864,8 @@ LayoutUnit LayoutGrid::rowAxisOffsetForChild(const LayoutBox& child) const
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
     LayoutUnit startOfColumn = m_columnPositions[coordinate.columns.resolvedInitialPosition.toInt()];
     LayoutUnit startPosition = startOfColumn + marginStartForChild(child);
+    if (hasAutoMarginsInRowAxis(child))
+        return startPosition;
     GridAxisPosition axisPosition = rowAxisPositionForChild(child);
     switch (axisPosition) {
     case GridAxisStart: