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

third_party/WebKit/Source/core/layout/LayoutGrid.cpp

 /*
  * Copyright (C) 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 214 matching lines @@
     size_t m_columnIndex;
     size_t m_childIndex;
 };
 
 struct LayoutGrid::GridSizingData {
     WTF_MAKE_NONCOPYABLE(GridSizingData);
     STACK_ALLOCATED();
 public:
     GridSizingData(size_t gridColumnCount, size_t gridRowCount)
         : columnTracks(gridColumnCount)
-        , rowTracks(gridRowCount)
+        , rowTracks(0)
     {
     }
 
     Vector<GridTrack> columnTracks;
     Vector<GridTrack> rowTracks;
     Vector<size_t> contentSizedTracksIndex;
 
     // Performance optimization: hold onto these Vectors until the end of Layout to avoid repeated malloc / free.
     Vector<GridTrack*> filteredTracks;
     Vector<GridItemWithSpan> itemsSortedByIncreasingSpan;
@@ skipping 106 matching lines @@
 
         updateLogicalWidth();
         bool logicalHeightWasIndefinite = computeContentLogicalHeight(MainOrPreferredSize, style()->logicalHeight(), -1) == -1;
 
         TextAutosizer::LayoutScope textAutosizerLayoutScope(this);
 
         placeItemsOnGrid();
 
         GridSizingData sizingData(gridColumnCount(), gridRowCount());
 
+        // 1- First, the track sizing algorithm is used to resolve the sizes of the grid columns.
         // At this point the logical width is always definite as the above call to updateLogicalWidth()
         // properly resolves intrinsic sizes. We cannot do the same for heights though because many code
         // paths inside updateLogicalHeight() require a previous call to setLogicalHeight() to resolve
         // heights properly (like for positioned items for example).
-        computeTrackSizesForDirection(ForColumns, sizingData, availableLogicalWidth());
+        LayoutUnit availableSpaceForColumns = availableLogicalWidth();
+        computeTrackSizesForDirection(ForColumns, sizingData, availableSpaceForColumns);
 
+        // 2- Next, the track sizing algorithm resolves the sizes of the grid rows, using the
+        // grid column sizes calculated in the previous step.
         if (logicalHeightWasIndefinite)
             computeIntrinsicLogicalHeight(sizingData);
         else
             computeTrackSizesForDirection(ForRows, sizingData, availableLogicalHeight(ExcludeMarginBorderPadding));
         setLogicalHeight(computeTrackBasedLogicalHeight(sizingData) + borderAndPaddingLogicalHeight());
 
         LayoutUnit oldClientAfterEdge = clientLogicalBottom();
         updateLogicalHeight();
 
         // The above call might have changed the grid's logical height depending on min|max height restrictions.
         // Update the sizes of the rows whose size depends on the logical height (also on definite|indefinite sizes).
         if (logicalHeightWasIndefinite)
             computeTrackSizesForDirection(ForRows, sizingData, logicalHeight());
 
         // Grid container should have the minimum height of a line if it's editable. That doesn't affect track sizing though.
         if (hasLineIfEmpty())
             setLogicalHeight(std::max(logicalHeight(), minimumLogicalHeightForEmptyLine()));
 
+        // 3- If the min-content contribution of any grid items have changed based on the row
+        // sizes calculated in step 2, steps 1 and 2 are repeated with the new min-content
+        // contribution (once only).
+        // TODO (lajava): is enough just verifying whether there are any orthogonal item ?
+        // TODO (lajava): Could we find better ways to fulfill such spec statement ?
+        // TODO (lajava): Could we avoid iterating over all the grid items ?"
+        // TODO (lajava): Why we dont need to repeat row tracks sizing as spec suggests (I was unable to find any test case needing so) ?"

[svillar, 2015/12/01 12:43:14]

We need to do that for example if we have two items in the same cell, one with
horizontal writing mode and the other with vertical writing mode. In such a
configuration, changing the column/row size will force one of the items to
re-layout forcing us to recompute the row/column.

+        if (hasAnyOrthogonalChild()) {
+            // In orthogonal flow cases column tracks size is determined based on the computed
+            // row track sizes, hence we need to repeat computeUsedBreadthOfGridTracks for columns.
+            computeTrackSizesForDirection(ForColumns, sizingData, availableSpaceForColumns);
+        }
+

[svillar, 2015/12/01 12:43:14]

This should be placed before the if (hasLineIfEmpty()) block. Otherwise step 3
could compute a grid height too small to be editable.

[jfernandez, 2015/12/01 15:13:38]

Acknowledged.

         applyStretchAlignmentToTracksIfNeeded(ForColumns, sizingData);
         applyStretchAlignmentToTracksIfNeeded(ForRows, sizingData);
 
         layoutGridItems(sizingData);
 
         if (size() != previousSize)
             relayoutChildren = true;
 
         layoutPositionedObjects(relayoutChildren || isDocumentElement());
 
@@ skipping 74 matching lines @@
 }
 
 void LayoutGrid::computeUsedBreadthOfGridTracks(GridTrackSizingDirection direction, GridSizingData& sizingData, LayoutUnit& baseSizesWithoutMaximization, LayoutUnit& growthLimitsWithoutMaximization, AvailableSpaceType availableSpaceType)
 {
     LayoutUnit& freeSpace = sizingData.freeSpaceForDirection(direction);
     const LayoutUnit initialFreeSpace = freeSpace;
     Vector<GridTrack>& tracks = (direction == ForColumns) ? sizingData.columnTracks : sizingData.rowTracks;
     Vector<size_t> flexibleSizedTracksIndex;
     sizingData.contentSizedTracksIndex.shrink(0);
 
+    if (direction == ForRows && tracks.size() == 0)
+        tracks.grow(gridRowCount());

[cbiesinger, 2015/11/26 02:26:16]

why is this now needed, when it wasn't before?

[jfernandez, 2015/11/26 08:58:27]

Since I needed a way to detect we are using rowTracks vector before populating
it with data, I decided to not initializing it so I can check out whether it was
null or not. So, we have initialize the vector the first time we run the track
sizing algorithm in the ForRow direction.

+
     const LayoutUnit maxSize = std::max(LayoutUnit(), initialFreeSpace);
     // 1. Initialize per Grid track variables.
     for (size_t i = 0; i < tracks.size(); ++i) {
         GridTrack& track = tracks[i];
         GridTrackSize trackSize = gridTrackSize(direction, i);
         const GridLength& minTrackBreadth = trackSize.minTrackBreadth();
         const GridLength& maxTrackBreadth = trackSize.maxTrackBreadth();
 
         track.setBaseSize(computeUsedBreadthOfMinLength(minTrackBreadth, maxSize));
         track.setGrowthLimit(computeUsedBreadthOfMaxLength(maxTrackBreadth, track.baseSize(), maxSize));
@@ skipping 53 matching lines @@
 
         for (size_t i = 0; i < flexibleSizedTracksIndex.size(); ++i) {
             GridIterator iterator(m_grid, direction, flexibleSizedTracksIndex[i]);
             while (LayoutBox* gridItem = iterator.nextGridItem()) {
                 const GridSpan span = cachedGridSpan(*gridItem, direction);
 
                 // Do not include already processed items.
                 if (i > 0 && span.resolvedInitialPosition.toInt() <= flexibleSizedTracksIndex[i - 1])
                     continue;
 
-                flexFraction = std::max(flexFraction, findFlexFactorUnitSize(tracks, span, direction, maxContentForChild(*gridItem, direction, sizingData.columnTracks)));
+                flexFraction = std::max(flexFraction, findFlexFactorUnitSize(tracks, span, direction, maxContentForChild(*gridItem, direction, sizingData)));
             }
         }
     }
 
     for (const auto& trackIndex : flexibleSizedTracksIndex) {
         GridTrackSize trackSize = gridTrackSize(direction, trackIndex);
 
         LayoutUnit oldBaseSize = tracks[trackIndex].baseSize();
         LayoutUnit baseSize = std::max<LayoutUnit>(oldBaseSize, flexFraction * trackSize.maxTrackBreadth().flex());
         if (LayoutUnit increment = baseSize - oldBaseSize) {
@@ skipping 77 matching lines @@
             flexFactorSum += trackSize.maxTrackBreadth().flex();
         }
     }
 
     // The function is not called if we don't have <flex> grid tracks
     ASSERT(!flexibleTracksIndexes.isEmpty());
 
     return computeFlexFactorUnitSize(tracks, direction, flexFactorSum, leftOverSpace, flexibleTracksIndexes);
 }
 
+bool LayoutGrid::hasAnyOrthogonalChild() const
+{
+    for (LayoutBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
+        if (child->isHorizontalWritingMode() != isHorizontalWritingMode())
+            return true;
+    }
+    return false;
+}
+
 bool LayoutGrid::hasDefiniteLogicalSize(GridTrackSizingDirection direction) const
 {
     return (direction == ForRows) ? hasDefiniteLogicalHeight() : hasDefiniteLogicalWidth();
 }
 
+LayoutUnit LayoutGrid::overrideContainingBlockBreadthForChild(const LayoutBox& child, GridTrackSizingDirection direction)
+{

[svillar, 2015/12/01 12:43:14]

I'd ASSERT() on hasOverrideContainingBlockXXX() and force the caller to check it
before calling this function. Returning LayoutUnit() is not correct because 0 is
a valid override value.

[jfernandez, 2015/12/01 15:13:39]

I really wanted to return 0 (actually, it was the original implementation),
since we use this method to compare such value with the one computed via
gridAreaBreadthForChild. 

We wouldn't need a new assert in any case, since
overrideContainingBlockContentLogicalX has got it already. What we could do,
instead, is to call hasXXX before, so if it returns false, we will directly use
the computed value, but I think such logic is equivalent and it provides no
advantage. 

+    if (direction == ForRows)
+        return child.hasOverrideContainingBlockLogicalHeight() ? child.overrideContainingBlockContentLogicalHeight() : LayoutUnit();
+    return child.hasOverrideContainingBlockLogicalWidth() ? child.overrideContainingBlockContentLogicalWidth() : LayoutUnit();
+}
+
+void LayoutGrid::setOverrideContainingBlockBreadthForChild(LayoutBox& child, GridTrackSizingDirection direction, LayoutUnit breadth)
+{
+    if (direction == ForRows)
+        child.setOverrideContainingBlockContentLogicalHeight(breadth);
+    else
+        child.setOverrideContainingBlockContentLogicalWidth(breadth);
+}
+
 GridTrackSize LayoutGrid::gridTrackSize(GridTrackSizingDirection direction, size_t i) const
 {
     bool isForColumns = direction == ForColumns;
     const Vector<GridTrackSize>& trackStyles = isForColumns ? style()->gridTemplateColumns() : style()->gridTemplateRows();
     const GridTrackSize& trackSize = (i >= trackStyles.size()) ? (isForColumns ? style()->gridAutoColumns() : style()->gridAutoRows()) : trackStyles[i];
 
     GridLength minTrackBreadth = trackSize.minTrackBreadth();
     GridLength maxTrackBreadth = trackSize.maxTrackBreadth();
 
     // If the logical width/height of the grid container is indefinite, percentage values are treated as <auto>
     if (minTrackBreadth.hasPercentage() || maxTrackBreadth.hasPercentage()) {
         if (!hasDefiniteLogicalSize(direction)) {
             if (minTrackBreadth.hasPercentage())
                 minTrackBreadth = Length(Auto);
             if (maxTrackBreadth.hasPercentage())
                 maxTrackBreadth = Length(Auto);
         }
     }
 
     return GridTrackSize(minTrackBreadth, maxTrackBreadth);
 }
 
-LayoutUnit LayoutGrid::logicalHeightForChild(LayoutBox& child, Vector<GridTrack>& columnTracks)
+LayoutUnit LayoutGrid::logicalHeightForChild(LayoutBox& child, const GridSizingData& sizingData)
 {
+    bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();

[svillar, 2015/12/01 12:43:14]

This is repeated so many times that we should have a private method to handle
it.

[jfernandez, 2015/12/01 15:13:39]

Acknowledged.

+    GridTrackSizingDirection childInlineDirection = hasOrthogonalWritingMode ? ForRows : ForColumns;
     SubtreeLayoutScope layoutScope(child);
-    LayoutUnit oldOverrideContainingBlockContentLogicalWidth = child.hasOverrideContainingBlockLogicalWidth() ? child.overrideContainingBlockContentLogicalWidth() : LayoutUnit();
-    LayoutUnit overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(child, ForColumns, columnTracks);
-    if (child.hasRelativeLogicalHeight() || oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth) {
+    // Child's min-content contribution depends on the container's size along the child's inline-axis.
+    LayoutUnit oldGridAreaBreadth = overrideContainingBlockBreadthForChild(child, childInlineDirection);
+    LayoutUnit gridAreaBreadth = gridAreaBreadthForChild(child, childInlineDirection, sizingData);
+    if (child.hasRelativeLogicalHeight() || oldGridAreaBreadth != gridAreaBreadth) {
         layoutScope.setNeedsLayout(&child, LayoutInvalidationReason::GridChanged);
     }
 
     bool hasOverrideHeight = child.hasOverrideLogicalContentHeight();
     // We need to clear the stretched height to properly compute logical height during layout.
     if (hasOverrideHeight && child.needsLayout())
         child.clearOverrideLogicalContentHeight();
 
-    child.setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
+    setOverrideContainingBlockBreadthForChild(child, childInlineDirection, gridAreaBreadth);
     // If |child| has a relative logical height, we shouldn't let it override its intrinsic height, which is
-    // what we are interested in here. Thus we need to set the override logical height to -1 (no possible resolution).
+    // what we are interested in here. Thus we need to set the block-axis override breadth to -1 (no possible resolution).
     if (child.hasRelativeLogicalHeight())
-        child.setOverrideContainingBlockContentLogicalHeight(-1);
+        setOverrideContainingBlockBreadthForChild(child, childInlineDirection == ForColumns ? ForRows : ForColumns, -1);
     child.layoutIfNeeded();
     // If the child was stretched we should use its intrinsic height.
     return (hasOverrideHeight ? childIntrinsicHeight(child) : child.logicalHeight()) + child.marginLogicalHeight();
 }
 
-LayoutUnit LayoutGrid::minSizeForChild(LayoutBox& child, GridTrackSizingDirection direction, Vector<GridTrack>& columnTracks)
+static inline GridTrackSizingDirection flowAwareDirection(bool hasOrthogonalWritingMode, GridTrackSizingDirection direction)
+{
+    if (hasOrthogonalWritingMode)
+        return direction == ForColumns ? ForRows : ForColumns;
+    return direction;
+}

[svillar, 2015/12/01 12:43:14]

With the private method I mentioned above we could transform this in another
private method which does not need the bool (would call the other one
internally).

Also this can be a one-liner using another ternary operator.

[jfernandez, 2015/12/01 15:13:39]

We would change an static inline method for a class method; are you really sure
it'd be better ? 
I'm not sure whether a nested ternary operator results n a clearer code.
actually, I was asked several times to avoid it. 

+
+LayoutUnit LayoutGrid::minSizeForChild(LayoutBox& child, GridTrackSizingDirection direction, const GridSizingData& sizingData)
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    // TODO(svillar): Properly support orthogonal writing mode.
-    if (hasOrthogonalWritingMode)
-        return LayoutUnit();
-
-    const Length& childMinSize = direction == ForColumns ? child.style()->logicalMinWidth() : child.style()->logicalMinHeight();
+    const Length& childMinSize = flowAwareDirection(hasOrthogonalWritingMode, direction) == ForColumns ? child.style()->logicalMinWidth() : child.style()->logicalMinHeight();
     if (childMinSize.isAuto()) {
         // TODO(svillar): Implement intrinsic aspect ratio support (transferred size in specs).
-        return minContentForChild(child, direction, columnTracks);
+        return minContentForChild(child, direction, sizingData);
     }
 
-    if (direction == ForColumns)
+    if (flowAwareDirection(hasOrthogonalWritingMode, direction) == ForColumns)
         return child.computeLogicalWidthUsing(MinSize, childMinSize, contentLogicalWidth(), this);
 
     return child.computeContentLogicalHeight(MinSize, childMinSize, child.logicalHeight()) + child.scrollbarLogicalHeight();
 }
 
-LayoutUnit LayoutGrid::minContentForChild(LayoutBox& child, GridTrackSizingDirection direction, Vector<GridTrack>& columnTracks)
+LayoutUnit LayoutGrid::minContentForChild(LayoutBox& child, GridTrackSizingDirection direction, const GridSizingData& sizingData)
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    // FIXME: Properly support orthogonal writing mode.
-    if (hasOrthogonalWritingMode)
-        return 0;
-
-    if (direction == ForColumns) {
+    if (flowAwareDirection(hasOrthogonalWritingMode, direction) == ForColumns) {
         // If |child| has a relative logical width, we shouldn't let it override its intrinsic width, which is
-        // what we are interested in here. Thus we need to set the override logical width to -1 (no possible resolution).
+        // what we are interested in here. Thus we need to set the inline-axis override breadth to -1 (no possible resolution).
+        GridTrackSizingDirection childInlineDirection = hasOrthogonalWritingMode ? ForRows : ForColumns;
         if (child.hasRelativeLogicalWidth())
-            child.setOverrideContainingBlockContentLogicalWidth(-1);
+            setOverrideContainingBlockBreadthForChild(child, childInlineDirection, -1);
 
         // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
         // See http://lists.w3.org/Archives/Public/www-style/2013Jan/0245.html
         return child.minPreferredLogicalWidth() + marginIntrinsicLogicalWidthForChild(child);
     }
 
-    return logicalHeightForChild(child, columnTracks);
+    return logicalHeightForChild(child, sizingData);
 }
 
-LayoutUnit LayoutGrid::maxContentForChild(LayoutBox& child, GridTrackSizingDirection direction, Vector<GridTrack>& columnTracks)
+LayoutUnit LayoutGrid::maxContentForChild(LayoutBox& child, GridTrackSizingDirection direction, const GridSizingData& sizingData)
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    // FIXME: Properly support orthogonal writing mode.
-    if (hasOrthogonalWritingMode)
-        return LayoutUnit();
-
-    if (direction == ForColumns) {
+    if (flowAwareDirection(hasOrthogonalWritingMode, direction) == ForColumns) {
         // If |child| has a relative logical width, we shouldn't let it override its intrinsic width, which is
         // what we are interested in here. Thus we need to set the override logical width to -1 (no possible resolution).
+        GridTrackSizingDirection childInlineDirection = hasOrthogonalWritingMode ? ForRows : ForColumns;
         if (child.hasRelativeLogicalWidth())
-            child.setOverrideContainingBlockContentLogicalWidth(-1);
+            setOverrideContainingBlockBreadthForChild(child, childInlineDirection, -1);
 
         // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
         // See http://lists.w3.org/Archives/Public/www-style/2013Jan/0245.html
         return child.maxPreferredLogicalWidth() + marginIntrinsicLogicalWidthForChild(child);
     }
 
-    return logicalHeightForChild(child, columnTracks);
+    return logicalHeightForChild(child, sizingData);
 }
 
 // We're basically using a class instead of a std::pair because of accessing gridItem() or gridSpan() is much more
 // self-explanatory that using .first or .second members in the pair. Having a std::pair<LayoutBox*, size_t>
 // does not work either because we still need the GridSpan so we'd have to add an extra hash lookup for each item
 // at the beginning of LayoutGrid::resolveContentBasedTrackSizingFunctionsForItems().
 class GridItemWithSpan {
 public:
     GridItemWithSpan(LayoutBox& gridItem, const GridSpan& gridSpan)
         : m_gridItem(&gridItem)
@@ skipping 26 matching lines @@
 {
     sizingData.itemsSortedByIncreasingSpan.shrink(0);
     HashSet<LayoutBox*> itemsSet;
     for (const auto& trackIndex : sizingData.contentSizedTracksIndex) {
         GridIterator iterator(m_grid, direction, trackIndex);
         GridTrack& track = (direction == ForColumns) ? sizingData.columnTracks[trackIndex] : sizingData.rowTracks[trackIndex];
         while (LayoutBox* gridItem = iterator.nextGridItem()) {
             if (itemsSet.add(gridItem).isNewEntry) {
                 const GridSpan& span = cachedGridSpan(*gridItem, direction);
                 if (span.integerSpan() == 1) {
-                    resolveContentBasedTrackSizingFunctionsForNonSpanningItems(direction, span, *gridItem, track, sizingData.columnTracks);
+                    resolveContentBasedTrackSizingFunctionsForNonSpanningItems(direction, span, *gridItem, track, sizingData);
                 } else if (!spanningItemCrossesFlexibleSizedTracks(span, direction)) {
                     sizingData.itemsSortedByIncreasingSpan.append(GridItemWithSpan(*gridItem, span));
                 }
             }
         }
     }
     std::sort(sizingData.itemsSortedByIncreasingSpan.begin(), sizingData.itemsSortedByIncreasingSpan.end());
 
     auto it = sizingData.itemsSortedByIncreasingSpan.begin();
     auto end = sizingData.itemsSortedByIncreasingSpan.end();
     while (it != end) {
         GridItemsSpanGroupRange spanGroupRange = { it, std::upper_bound(it, end, *it) };
         resolveContentBasedTrackSizingFunctionsForItems<ResolveIntrinsicMinimums>(direction, sizingData, spanGroupRange);
         resolveContentBasedTrackSizingFunctionsForItems<ResolveContentBasedMinimums>(direction, sizingData, spanGroupRange);
         resolveContentBasedTrackSizingFunctionsForItems<ResolveMaxContentMinimums>(direction, sizingData, spanGroupRange);
         resolveContentBasedTrackSizingFunctionsForItems<ResolveIntrinsicMaximums>(direction, sizingData, spanGroupRange);
         resolveContentBasedTrackSizingFunctionsForItems<ResolveMaxContentMaximums>(direction, sizingData, spanGroupRange);
         it = spanGroupRange.rangeEnd;
     }
 
     for (const auto& trackIndex : sizingData.contentSizedTracksIndex) {
         GridTrack& track = (direction == ForColumns) ? sizingData.columnTracks[trackIndex] : sizingData.rowTracks[trackIndex];
         if (track.growthLimitIsInfinite())
             track.setGrowthLimit(track.baseSize());
     }
 }
 
-void LayoutGrid::resolveContentBasedTrackSizingFunctionsForNonSpanningItems(GridTrackSizingDirection direction, const GridSpan& span, LayoutBox& gridItem, GridTrack& track, Vector<GridTrack>& columnTracks)
+void LayoutGrid::resolveContentBasedTrackSizingFunctionsForNonSpanningItems(GridTrackSizingDirection direction, const GridSpan& span, LayoutBox& gridItem, GridTrack& track, const GridSizingData& sizingData)
 {
     const GridResolvedPosition trackPosition = span.resolvedInitialPosition;
     GridTrackSize trackSize = gridTrackSize(direction, trackPosition.toInt());
 
     if (trackSize.hasMinContentMinTrackBreadth())
-        track.setBaseSize(std::max(track.baseSize(), minContentForChild(gridItem, direction, columnTracks)));
+        track.setBaseSize(std::max(track.baseSize(), minContentForChild(gridItem, direction, sizingData)));
     else if (trackSize.hasMaxContentMinTrackBreadth())
-        track.setBaseSize(std::max(track.baseSize(), maxContentForChild(gridItem, direction, columnTracks)));
+        track.setBaseSize(std::max(track.baseSize(), maxContentForChild(gridItem, direction, sizingData)));
     else if (trackSize.hasAutoMinTrackBreadth())
-        track.setBaseSize(std::max(track.baseSize(), minSizeForChild(gridItem, direction, columnTracks)));
+        track.setBaseSize(std::max(track.baseSize(), minSizeForChild(gridItem, direction, sizingData)));
 
     if (trackSize.hasMinContentMaxTrackBreadth())
-        track.setGrowthLimit(std::max(track.growthLimit(), minContentForChild(gridItem, direction, columnTracks)));
+        track.setGrowthLimit(std::max(track.growthLimit(), minContentForChild(gridItem, direction, sizingData)));
     else if (trackSize.hasMaxContentOrAutoMaxTrackBreadth())
-        track.setGrowthLimit(std::max(track.growthLimit(), maxContentForChild(gridItem, direction, columnTracks)));
+        track.setGrowthLimit(std::max(track.growthLimit(), maxContentForChild(gridItem, direction, sizingData)));
 }
 
 static const LayoutUnit& trackSizeForTrackSizeComputationPhase(TrackSizeComputationPhase phase, const GridTrack& track, TrackSizeRestriction restriction)
 {
     switch (phase) {
     case ResolveIntrinsicMinimums:
     case ResolveContentBasedMinimums:
     case ResolveMaxContentMinimums:
     case MaximizeTracks:
         return track.baseSize();
@@ skipping 87 matching lines @@
         track.setGrowthLimit(track.plannedSize());
         return;
     case MaximizeTracks:
         ASSERT_NOT_REACHED();
         return;
     }
 
     ASSERT_NOT_REACHED();
 }
 
-LayoutUnit LayoutGrid::currentItemSizeForTrackSizeComputationPhase(TrackSizeComputationPhase phase, LayoutBox& gridItem, GridTrackSizingDirection direction, Vector<GridTrack>& columnTracks)
+LayoutUnit LayoutGrid::currentItemSizeForTrackSizeComputationPhase(TrackSizeComputationPhase phase, LayoutBox& gridItem, GridTrackSizingDirection direction, const GridSizingData& sizingData)
 {
     switch (phase) {
     case ResolveIntrinsicMinimums:
-        return minSizeForChild(gridItem, direction, columnTracks);
+        return minSizeForChild(gridItem, direction, sizingData);
     case ResolveContentBasedMinimums:
     case ResolveIntrinsicMaximums:
-        return minContentForChild(gridItem, direction, columnTracks);
+        return minContentForChild(gridItem, direction, sizingData);
     case ResolveMaxContentMinimums:
     case ResolveMaxContentMaximums:
-        return maxContentForChild(gridItem, direction, columnTracks);
+        return maxContentForChild(gridItem, direction, sizingData);
     case MaximizeTracks:
         ASSERT_NOT_REACHED();
         return 0;
     }
 
     ASSERT_NOT_REACHED();
     return 0;
 }
 
 template <TrackSizeComputationPhase phase>
@@ skipping 24 matching lines @@
 
             if (trackShouldGrowBeyondGrowthLimitsForTrackSizeComputationPhase(phase, trackSize))
                 sizingData.growBeyondGrowthLimitsTracks.append(&track);
         }
 
         if (sizingData.filteredTracks.isEmpty())
             continue;
 
         spanningTracksSize += guttersSize(direction, itemSpan.integerSpan());
 
-        LayoutUnit extraSpace = currentItemSizeForTrackSizeComputationPhase(phase, gridItemWithSpan.gridItem(), direction, sizingData.columnTracks) - spanningTracksSize;
+        LayoutUnit extraSpace = currentItemSizeForTrackSizeComputationPhase(phase, gridItemWithSpan.gridItem(), direction, sizingData) - spanningTracksSize;
         extraSpace = std::max<LayoutUnit>(extraSpace, 0);
         auto& tracksToGrowBeyondGrowthLimits = sizingData.growBeyondGrowthLimitsTracks.isEmpty() ? sizingData.filteredTracks : sizingData.growBeyondGrowthLimitsTracks;
         distributeSpaceToTracks<phase>(sizingData.filteredTracks, &tracksToGrowBeyondGrowthLimits, sizingData, extraSpace);
     }
 
     for (const auto& trackIndex : sizingData.contentSizedTracksIndex) {
         GridTrack& track = tracks[trackIndex];
         markAsInfinitelyGrowableForTrackSizeComputationPhase(phase, track);
         updateTrackSizeForTrackSizeComputationPhase(phase, track);
     }
@@ skipping 497 matching lines @@
     ASSERT(m_gridItemCoordinate.contains(&gridItem));
     return m_gridItemCoordinate.get(&gridItem);
 }
 
 GridSpan LayoutGrid::cachedGridSpan(const LayoutBox& gridItem, GridTrackSizingDirection direction) const
 {
     GridCoordinate coordinate = cachedGridCoordinate(gridItem);
     return direction == ForColumns ? coordinate.columns : coordinate.rows;
 }
 
-LayoutUnit LayoutGrid::gridAreaBreadthForChild(const LayoutBox& child, GridTrackSizingDirection direction, const Vector<GridTrack>& tracks) const
+bool LayoutGrid::gridLengthIsIndefinite(const GridLength& length, GridTrackSizingDirection direction) const
 {
+    return length.isContentSized() || length.isFlex() || (length.hasPercentage() && !hasDefiniteLogicalSize(direction));
+}

[svillar, 2015/12/01 12:43:14]

I panicked when I saw this one :). We should not use hasDefiniteLogicalXXX in
new code as it is completely wrong whenever we specify a min|max content
restriction (like width: 10px; min-width: min-content;) as it only checks
width/height properties.

Couldn't you just check that the growthLimit() of the track is not -1?

[jfernandez, 2015/12/01 15:13:39]

Umm, can you confirm that checking that out is enough to fulfill the
specification statement of "a track having a definite max track sizing function"
? 

https://drafts.csswg.org/css-grid/#algo-overview

+
+LayoutUnit LayoutGrid::assumedRowsBreadthForOrthogonalChild(const LayoutBox& child) const
+{
+    ASSERT(child.isHorizontalWritingMode() != isHorizontalWritingMode());
+    const GridSpan& span = cachedGridSpan(child, ForRows);
+    LayoutUnit gridAreaBreadth = 0;
+    for (GridSpan::iterator trackPosition = span.begin(); trackPosition != span.end(); ++trackPosition) {

[svillar, 2015/12/01 12:43:14]

Better do:

for (auto trackPosition : span)

since GridSpan is iteratable

[jfernandez, 2015/12/01 15:13:39]

Acknowledged.

+        const GridLength& maxTrackBreadth = gridTrackSize(ForRows, trackPosition.toInt()).maxTrackBreadth();
+        if (gridLengthIsIndefinite(maxTrackBreadth, ForRows)) {
+            gridAreaBreadth = child.maxPreferredLogicalWidth() + marginIntrinsicLogicalWidthForChild(child);
+            break;
+        }
+        gridAreaBreadth += valueForLength(maxTrackBreadth.length(), 0);
+    }
+
+    gridAreaBreadth += guttersSize(ForRows, span.integerSpan());
+
+    return gridAreaBreadth;
+}
+
+LayoutUnit LayoutGrid::gridAreaBreadthForChild(const LayoutBox& child, GridTrackSizingDirection direction, const GridSizingData& sizingData) const
+{
+    const Vector<GridTrack>& tracks = (direction == ForColumns) ? sizingData.columnTracks : sizingData.rowTracks;
+    if (child.isHorizontalWritingMode() != isHorizontalWritingMode() && tracks.isEmpty())

[svillar, 2015/12/01 12:43:14]

Don't get why you need the isEmpty().

[jfernandez, 2015/12/01 15:13:39]

I needed a way to detect whether row tracks were already set when computing
column tracks. The only/easier way was to avoid initializing the row tracks
vector. That's why I use "isEmpty".

+        return assumedRowsBreadthForOrthogonalChild(child);
     const GridSpan& span = cachedGridSpan(child, direction);
     LayoutUnit gridAreaBreadth = 0;
     for (GridSpan::iterator trackPosition = span.begin(); trackPosition != span.end(); ++trackPosition)
         gridAreaBreadth += tracks[trackPosition.toInt()].baseSize();
 
     gridAreaBreadth += guttersSize(direction, span.integerSpan());
 
     return gridAreaBreadth;
 }
 
@@ skipping 19 matching lines @@
     // using these positions to compute them.
 
     unsigned numberOfTracks = sizingData.columnTracks.size();
     unsigned numberOfLines = numberOfTracks + 1;
     unsigned lastLine = numberOfLines - 1;
     unsigned nextToLastLine = numberOfLines - 2;
     ContentAlignmentData offset = computeContentPositionAndDistributionOffset(ForColumns, sizingData.freeSpaceForDirection(ForColumns), numberOfTracks);
     LayoutUnit trackGap = guttersSize(ForColumns, 2);
     m_columnPositions.resize(numberOfLines);
     m_columnPositions[0] = borderAndPaddingStart() + offset.positionOffset;
-    for (unsigned i = 0; i < lastLine; ++i)
+    for (unsigned i = 0; i < nextToLastLine; ++i)

[cbiesinger, 2015/11/26 02:26:16]

Why this change now?

[jfernandez, 2015/11/26 08:58:27]

I admit that this change is unrelated to the issue, but it's a silly mistake we
made long time ago and,. being one line, I  like to keep it. The mistake didn't
cause a bug because the same value is overwritten in the next line, but I wanted
to avoid doing an unnecessary lopp iteration.

         m_columnPositions[i + 1] = m_columnPositions[i] + offset.distributionOffset + sizingData.columnTracks[i].baseSize() + trackGap;
     m_columnPositions[lastLine] = m_columnPositions[nextToLastLine] + sizingData.columnTracks[nextToLastLine].baseSize();
 
     numberOfTracks = sizingData.rowTracks.size();
     numberOfLines = numberOfTracks + 1;
     lastLine = numberOfLines - 1;
     nextToLastLine = numberOfLines - 2;
     offset = computeContentPositionAndDistributionOffset(ForRows, sizingData.freeSpaceForDirection(ForRows), numberOfTracks);
     trackGap = guttersSize(ForRows, 2);
     m_rowPositions.resize(numberOfLines);
     m_rowPositions[0] = borderAndPaddingBefore() + offset.positionOffset;
-    for (unsigned i = 0; i < lastLine; ++i)
+    for (unsigned i = 0; i < nextToLastLine; ++i)
         m_rowPositions[i + 1] = m_rowPositions[i] + offset.distributionOffset + sizingData.rowTracks[i].baseSize() + trackGap;
     m_rowPositions[lastLine] = m_rowPositions[nextToLastLine] + sizingData.rowTracks[nextToLastLine].baseSize();
 }
 
 static LayoutUnit computeOverflowAlignmentOffset(OverflowAlignment overflow, LayoutUnit trackBreadth, LayoutUnit childBreadth)
 {
     LayoutUnit offset = trackBreadth - childBreadth;
     switch (overflow) {
     case OverflowAlignmentSafe:
         // If overflow is 'safe', we have to make sure we don't overflow the 'start'
@@ skipping 431 matching lines @@
     case ContentPositionAuto:
         break;
     }
 
     ASSERT_NOT_REACHED();
     return {0, 0};
 }
 
 LayoutPoint LayoutGrid::findChildLogicalPosition(const LayoutBox& child, GridSizingData& sizingData) const
 {
+    LayoutUnit columnAxisOffset = columnAxisOffsetForChild(child);
     LayoutUnit rowAxisOffset = rowAxisOffsetForChild(child);
-    // We stored m_columnPosition s's data ignoring the direction, hence we might need now
+    // 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()) {
         LayoutUnit alignmentOffset =  m_columnPositions[0] - borderAndPaddingStart();
         LayoutUnit rightGridEdgePosition = m_columnPositions[m_columnPositions.size() - 1] + alignmentOffset + borderAndPaddingLogicalLeft();
         rowAxisOffset = rightGridEdgePosition - (rowAxisOffset + child.logicalWidth());
     }
 
-    return LayoutPoint(rowAxisOffset, columnAxisOffsetForChild(child));
+    // "In the positioning phase [...] calculations are performed according to the writing mode
+    // of the containing block of the box establishing the orthogonal flow." However, the
+    // resulting LayoutPoint will be used in 'setLogicalPosition' in order to set the child's
+    // logical position, which will only take into account the child's writing-mode.
+    LayoutPoint childLocation(rowAxisOffset, columnAxisOffset);
+    bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
+    return hasOrthogonalWritingMode ? childLocation.transposedPoint() : childLocation;
 }
 
 void LayoutGrid::paintChildren(const PaintInfo& paintInfo, const LayoutPoint& paintOffset) const
 {
     GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 } // namespace blink