[CSS Grid Layout] Stretch value for align and justify properties.

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 40f2d1e2ac51956f535803cef84a2da8f2d36154..9d6ac85d4667436b34cb5ccd3337ccd87e8a9c84 100644
--- a/Source/core/rendering/RenderGrid.cpp
+++ b/Source/core/rendering/RenderGrid.cpp
@@ -616,6 +616,8 @@ GridTrackSize RenderGrid::gridTrackSize(GridTrackSizingDirection direction, size
 
 LayoutUnit RenderGrid::logicalHeightForChild(RenderBox& child, Vector<GridTrack>& columnTracks)
 {
+    child.clearOverrideLogicalContentHeight();

[Julien - ping for review, 2014/10/08 15:21:47]

I don't understand the need for this as we do the following below (and there is
no early return):

child.setOverrideContainingBlockContentLogicalHeight(-1);

[jfernandez, 2014/10/12 22:40:27]

Well, if I understood it correctly, OverrideContainingBlockContentLogicalHeight
is different from OverrideLogicalContentHeight. Actually, the former seems Grid
specific which is something confused me. 

The OverrideLogicalContentHeight api handles some fields in the
RenderBoxRareData struct and it seems to be used to override the intrinsic
height/width of the box. The RenderFlexibleBox implementation uses such api to
implement the 'stretch' behavior, so that's why I used it.

Also, the RenderBox::computeLogicalHeight uses the OverrideLogicalContentHeight
api to compute the actual/computed height of the box. 

On the other hand, the  OverrideContainingBlockLogicalHeigh used in the
RenderGrid implementation is independent of the OverrideLogicalContentHeight and
is not used at all in the RenderBox::computeLogicalHeight. I don't really
understand why grid needs its own stuff for this; there is actually a FIXME to
move the logic to the RenderBoxRareData structure.

So, the reason why the clearOverrideLogicalContentHeight call is needed is that
etOverrideContainingBlockContentLogicalHeight(-1) does not do the job, so the
OverrideLogicalContentHeight flag is still there and it causes some repaint
tests to fail.

[Julien - ping for review, 2014/10/21 00:43:12]

You're right, I misread. RenderGrid needs to pretend that its children's sizes
are resolved against a different containing block's size (the grid cell's, not
the grid container's). We introduced OverrideContainingBlockContentLogicalHeight
to do this without having to introduce a synthetic grid cell's renderer.

Btw OverrideLogicalContentHeight is also used by table.
Yes, that's the right API to stretch / constrain a logical height.

[jfernandez, 2014/10/22 09:31:37]

Done.

+
     SubtreeLayoutScope layoutScope(child);
     LayoutUnit oldOverrideContainingBlockContentLogicalWidth = child.hasOverrideContainingBlockLogicalWidth() ? child.overrideContainingBlockContentLogicalWidth() : LayoutUnit();
     LayoutUnit overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(child, ForColumns, columnTracks);
@@ -1073,6 +1075,8 @@ void RenderGrid::layoutGridItems()
         // 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.
+        applyStretchAlignmentToChildIfNeeded(*child, overrideContainingBlockContentLogicalHeight);
+
         child->layoutIfNeeded();
 
 #if ENABLE(ASSERT)
@@ -1103,6 +1107,22 @@ GridCoordinate RenderGrid::cachedGridCoordinate(const RenderBox& gridItem) const
     return m_gridItemCoordinate.get(&gridItem);
 }
 
+// FIXME: We should move this logic to the StyleAdjuster or the StyleBuilder.
+static ItemPosition resolveAlignment(const RenderStyle* parentStyle, const RenderStyle* childStyle)
+{
+    ItemPosition align = childStyle->alignSelf();
+    // The auto keyword computes to the parent's align-items computed value, or to "stretch", if not set or "auto".
+    if (align == ItemPositionAuto)
+        align = (parentStyle->alignItems() == ItemPositionAuto) ? ItemPositionStretch : parentStyle->alignItems();
+    return align;
+}
+
+void RenderGrid::applyStretchAlignmentToChildIfNeeded(RenderBox& child, LayoutUnit overrideLogicalContentHeight)
+{
+    if (child.style()->logicalHeight().isAuto() && resolveAlignment(style(), child.style()) == ItemPositionStretch)
+        child.setOverrideLogicalContentHeight(overrideLogicalContentHeight);

[Julien - ping for review, 2014/10/08 15:21:47]

This is good for a first pass but it's not totally correct. You need to
constrain the overrideLogicalHeight by min-max.

We should match flexbox's implementation as much as possible in this case as
there is a lot of similarities. See
constrainedChildIntrinsicContentLogicalHeight.

[jfernandez, 2014/10/12 22:40:27]

Acknowledged.

+}
+
 LayoutUnit RenderGrid::gridAreaBreadthForChild(const RenderBox& child, GridTrackSizingDirection direction, const Vector<GridTrack>& tracks) const
 {
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
@@ -1246,6 +1266,7 @@ LayoutUnit RenderGrid::columnPositionForChild(const RenderBox& child) const
     case ItemPositionAuto:
         break;
     case ItemPositionStretch:
+        return startOfColumnForChild(child);
     case ItemPositionBaseline:
     case ItemPositionLastBaseline:
         // FIXME: Implement the previous values. For now, we always start align the child.
@@ -1293,16 +1314,6 @@ LayoutUnit RenderGrid::centeredRowPositionForChild(const RenderBox& child) const
     return startOfRow + std::max<LayoutUnit>(0, endOfRow - startOfRow - child.logicalHeight()) / 2;
 }
 
-// FIXME: We should move this logic to the StyleAdjuster or the StyleBuilder.
-static ItemPosition resolveAlignment(const RenderStyle* parentStyle, const RenderStyle* childStyle)
-{
-    ItemPosition align = childStyle->alignSelf();
-    // The auto keyword computes to the parent's align-items computed value, or to "stretch", if not set or "auto".
-    if (align == ItemPositionAuto)
-        align = (parentStyle->alignItems() == ItemPositionAuto) ? ItemPositionStretch : parentStyle->alignItems();
-    return align;
-}
-

[Julien - ping for review, 2014/10/08 15:21:47]

Instead of moving this code, can we just move
applyStretchAlignmentToChildIfNeeded below? (less history churn)

 LayoutUnit RenderGrid::rowPositionForChild(const RenderBox& child) const
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
@@ -1361,7 +1372,6 @@ LayoutUnit RenderGrid::rowPositionForChild(const RenderBox& child) const
     case ItemPositionEnd:
         return endOfRowForChild(child);
     case ItemPositionStretch:
-        // FIXME: Implement the Stretch value. For now, we always start align the child.
         return startOfRowForChild(child);
     case ItemPositionBaseline:
     case ItemPositionLastBaseline: