[css-grid] Handle min-content/max-content with orthogonal flows

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 5221e3ece7743c67a854c39e39114332caa1b9f9..7272bef6e4f91b9a6d5c3c60a28b34c833365ff5 100644
--- a/Source/core/rendering/RenderGrid.cpp
+++ b/Source/core/rendering/RenderGrid.cpp
@@ -650,9 +650,8 @@ LayoutUnit RenderGrid::logicalHeightForChild(RenderBox& child, Vector<GridTrack>
 LayoutUnit RenderGrid::minContentForChild(RenderBox& child, GridTrackSizingDirection direction, Vector<GridTrack>& columnTracks)
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    // FIXME: Properly support orthogonal writing mode.
     if (hasOrthogonalWritingMode)
-        return 0;
+        direction = direction == ForColumns ? ForRows : ForColumns;

[svillar, 2015/01/12 08:27:51]

As a side note, we have a lot of direction==ForColumns checks inside RenderGrid.
I remember having an inline isForColumns() at some point. I think it makes sense
to re-add it.

 
     if (direction == ForColumns) {
         // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
@@ -666,9 +665,8 @@ LayoutUnit RenderGrid::minContentForChild(RenderBox& child, GridTrackSizingDirec
 LayoutUnit RenderGrid::maxContentForChild(RenderBox& child, GridTrackSizingDirection direction, Vector<GridTrack>& columnTracks)
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    // FIXME: Properly support orthogonal writing mode.
     if (hasOrthogonalWritingMode)
-        return LayoutUnit();
+        direction = direction == ForColumns ? ForRows : ForColumns;
 
     if (direction == ForColumns) {
         // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
@@ -1130,8 +1128,10 @@ void RenderGrid::layoutGridItems()
         LayoutUnit overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(*child, ForColumns, sizingData.columnTracks);
         LayoutUnit overrideContainingBlockContentLogicalHeight = gridAreaBreadthForChild(*child, ForRows, sizingData.rowTracks);
 
+        bool hasOrthogonalWritingMode = child->isHorizontalWritingMode() != isHorizontalWritingMode();

[Julien - ping for review, 2015/01/21 09:32:10]

Why do we need to _always_ force a layout in the orthogonal writing mode case?

+
         SubtreeLayoutScope layoutScope(*child);
-        if (oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && child->hasRelativeLogicalHeight()))
+        if (hasOrthogonalWritingMode || oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && child->hasRelativeLogicalHeight()))
             layoutScope.setNeedsLayout(child);
 
         child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
@@ -1150,8 +1150,9 @@ void RenderGrid::layoutGridItems()
 
         // Keep track of children overflowing their grid area as we might need to paint them even if the grid-area is
         // not visible
-        if (child->logicalHeight() > overrideContainingBlockContentLogicalHeight
-            || child->logicalWidth() > overrideContainingBlockContentLogicalWidth)
+        bool overflowingContainingBlockHeight = hasOrthogonalWritingMode ? child->logicalWidth() > overrideContainingBlockContentLogicalHeight : child->logicalHeight() > overrideContainingBlockContentLogicalHeight;
+        bool overflowingContainingBlockWidth = hasOrthogonalWritingMode ? child->logicalHeight() > overrideContainingBlockContentLogicalWidth : child->logicalHeight() > overrideContainingBlockContentLogicalWidth;
+        if (overflowingContainingBlockHeight || overflowingContainingBlockWidth)
             m_gridItemsOverflowingGridArea.append(child);
     }
 
@@ -1653,14 +1654,19 @@ LayoutUnit RenderGrid::contentPositionAndDistributionColumnOffset(LayoutUnit ava
 
 LayoutPoint RenderGrid::findChildLogicalPosition(const RenderBox& child, LayoutSize contentAlignmentOffset) const
 {
+    bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
+    LayoutUnit rowPosition = rowPositionForChild(child);
     LayoutUnit columnPosition = columnPositionForChild(child);
     // We stored m_columnPositions's data ignoring the direction, hence we might need now
     // to translate positions from RTL to LTR, as it's more convenient for painting.
     if (!style()->isLeftToRightDirection())
         columnPosition = (m_columnPositions[m_columnPositions.size() - 1] + borderAndPaddingLogicalLeft()) - columnPosition - child.logicalWidth();
 
+    // We stored column's and row's positions without considering orthogonal flows, so now we
+    // need to flip the final LayoutPoint coordinates if that's the case.
+    LayoutPoint childLocation = hasOrthogonalWritingMode ? LayoutPoint(rowPosition, columnPosition) : LayoutPoint(columnPosition, rowPosition);

[Julien - ping for review, 2015/01/21 09:32:10]

I really don't understand this line. All the offset computation should be done
in the grid container's coordinate system as per CSS writing-mode:

"In the positioning phase [...] calculations are performed according to the
writing mode of the containing block of the box establishing the orthogonal
flow."

+
     // The Content Alignment offset accounts for the RTL to LTR flip.
-    LayoutPoint childLocation(columnPosition, rowPositionForChild(child));
     childLocation.move(contentAlignmentOffset);

[Julien - ping for review, 2015/01/21 09:32:10]

This is also incoherent with flipping coordinates above, which means we probably
don't support alignment with writing mode correctly.

 
     return childLocation;