[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)

[svillar, 2016/03/23 11:02:00]

This is weird, you're totally ignoring the passed row count

[jfernandez, 2016/04/01 16:39:56]

Yeah, not the best way to do it but since we are going to perform several
iterations of the tracks sizing algorithm we need a way to detect that we
haven't executed the first cycle on the row tracks. 

I wanted to explicitly spot this change so we can figure out better ways to do
it. If we eventually decide to go with this solution, we obviously have to
remove the gridRowCount argument.

[svillar, 2016/04/04 07:58:28]

I'd prefer to have a state machine where to track the different iterations
(perhaps just a single enum attribute?).

     {
     }
 
     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(), LayoutUnit(-1)) == LayoutUnit(-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, contentLogicalHeight());
 
+        // 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 ?"

[svillar, 2016/03/23 11:02:00]

I think the answer is yes. You have total control of the items in your grid
because you insert them one by one. It'd be easy just to keep a boolean
attribute in LayoutGrid.

[jfernandez, 2016/04/01 16:39:56]

Ok, we can do that.

+        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);
+            computeTrackSizesForDirection(ForRows, sizingData, logicalHeight());

[svillar, 2016/03/23 11:02:00]

I guess this should be contentLogicalHeight() ?

[jfernandez, 2016/04/01 16:39:56]

Why ? I'm not totally sure. We have just called to udpateLogicalHeight after
step 2, so logicalHeight should return the right value. I'm open to discuss it,
though.

[svillar, 2016/04/04 07:58:28]

logical height includes border padding.... We cannot use those for track sizing
just the content size.

+        }
+
         // 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()));
 
         applyStretchAlignmentToTracksIfNeeded(ForColumns, sizingData);
         applyStretchAlignmentToTracksIfNeeded(ForRows, sizingData);
 
         layoutGridItems(sizingData);
 
         if (size() != previousSize)
@@ skipping 78 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);
 
+    // The row tracks vector was not initialized as part of the GridSizingData instantiation because we want to detect
+    // that we will have to estimate the row track's breadth to figure out an orthogonal grid item's logical height.
+    // TODO (lajava) are there better ways to do this ?
+    if (direction == ForRows && tracks.size() == 0)
+        tracks.grow(gridRowCount());

[svillar, 2016/03/23 11:02:00]

I don't think this is the right way to do this.

[jfernandez, 2016/04/01 16:39:56]

Remember that we only do this to be able to detect a particular situation in the
track sizing alg lifecycle. 

[svillar, 2016/04/04 07:58:28]

Again, an state machine is much better IMO.

+
     LayoutUnit maxSize = std::max(LayoutUnit(), initialFreeSpace);
     // Grid gutters were removed from freeSpace by the caller, but we must use them to compute relative (i.e. percentages) sizes.
     if (availableSpaceType == AvailableSpaceDefinite)
         maxSize += guttersSize(direction, direction == ForRows ? gridRowCount() : gridColumnCount());
 
     // 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();
@@ skipping 57 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() <= 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(oldBaseSize, LayoutUnit(flexFraction * trackSize.maxTrackBreadth().flex()));
         if (LayoutUnit increment = baseSize - oldBaseSize) {
@@ skipping 76 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 (isOrthogonalChild(*child))
+            return true;
+    }
+    return false;
+}

[svillar, 2016/03/23 11:02:00]

See my comment above about not needing this method.

[jfernandez, 2016/04/01 16:39:56]

Acknowledged.

+
 bool LayoutGrid::hasDefiniteLogicalSize(GridTrackSizingDirection direction) const
 {
     return (direction == ForRows) ? hasDefiniteLogicalHeight() : hasDefiniteLogicalWidth();
 }
 
+LayoutUnit LayoutGrid::overrideContainingBlockBreadthForChild(const LayoutBox& child, GridTrackSizingDirection direction)
+{
+    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& autoTrackSize = isForColumns ? style()->gridAutoColumns() : style()->gridAutoRows();
     int translatedIndex = i + (isForColumns ? m_smallestColumnStart : m_smallestRowStart);
     const GridTrackSize& trackSize = (translatedIndex < 0 || translatedIndex >= static_cast<int>(trackStyles.size())) ? autoTrackSize : trackStyles[translatedIndex];
 
     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)
+bool LayoutGrid::isOrthogonalChild(const LayoutBox& child) const
 {
+    return child.isHorizontalWritingMode() != isHorizontalWritingMode();
+}
+
+LayoutUnit LayoutGrid::logicalHeightForChild(LayoutBox& child, const GridSizingData& sizingData)
+{
+    GridTrackSizingDirection childInlineDirection = isOrthogonalChild(child) ? 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(LayoutUnit(-1));
+        setOverrideContainingBlockBreadthForChild(child, childInlineDirection == ForColumns ? ForRows : ForColumns, LayoutUnit(-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)
+GridTrackSizingDirection LayoutGrid::flowAwareDirectionForChild(const LayoutBox& child, GridTrackSizingDirection direction) const
+{
+    return isOrthogonalChild(child) ? (direction == ForColumns ? ForRows : ForColumns) : direction;
+}
+
+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();
-
-    bool isRowAxis = direction == ForColumns;
+    bool isRowAxis = flowAwareDirection(hasOrthogonalWritingMode, direction) == ForColumns;
     const Length& childSize = isRowAxis ? child.styleRef().logicalWidth() : child.styleRef().logicalHeight();
-    const Length& childMinSize = isRowAxis ? child.styleRef().logicalMinWidth() : child.styleRef().logicalMinHeight();
-    if (!childSize.isAuto() || childMinSize.isAuto())
-        return minContentForChild(child, direction, columnTracks);
+    const Length& childMinSize = flowAwareDirection(hasOrthogonalWritingMode, direction) == ForColumns ? child.style()->logicalMinWidth() : child.style()->logicalMinHeight();
+    if (!childSize.isAuto() || childMinSize.isAuto()) {
+        // TODO(svillar): Implement intrinsic aspect ratio support (transferred size in specs).
+        return minContentForChild(child, direction, sizingData);
+    }
 
     if (isRowAxis)
         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 LayoutUnit();
-
-    if (direction == ForColumns) {
+    if (flowAwareDirectionForChild(child, 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 = isOrthogonalChild(child) ? ForRows : ForColumns;
         if (child.hasRelativeLogicalWidth())
-            child.setOverrideContainingBlockContentLogicalWidth(LayoutUnit(-1));
+            setOverrideContainingBlockBreadthForChild(child, childInlineDirection, LayoutUnit(-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 (flowAwareDirectionForChild(child, 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 = isOrthogonalChild(child) ? ForRows : ForColumns;
         if (child.hasRelativeLogicalWidth())
-            child.setOverrideContainingBlockContentLogicalWidth(LayoutUnit(-1));
+            setOverrideContainingBlockBreadthForChild(child, childInlineDirection, LayoutUnit(-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 getGridSpan() 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 size_t trackPosition = span.resolvedInitialPosition();
     GridTrackSize trackSize = gridTrackSize(direction, trackPosition);
 
     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);

[svillar, 2016/04/04 07:58:28]

Perhaps you could refactor this patch as the sizingData refactoring was already
done. That will simplify and reduce the size of this patch.

     case MaximizeTracks:
         ASSERT_NOT_REACHED();
         return LayoutUnit();
     }
 
     ASSERT_NOT_REACHED();
     return LayoutUnit();
 }
 
 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 = extraSpace.clampNegativeToZero();
         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 383 matching lines @@
 
         // 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 = gridAreaBreadthForChildIncludingAlignmentOffsets(*child, ForColumns, sizingData);
         LayoutUnit overrideContainingBlockContentLogicalHeight = gridAreaBreadthForChildIncludingAlignmentOffsets(*child, ForRows, sizingData);
 
         SubtreeLayoutScope layoutScope(*child);
-        if (oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && child->hasRelativeLogicalHeight()))
+        if (oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth || (oldOverrideContainingBlockContentLogicalHeight != overrideContainingBlockContentLogicalHeight && (child->hasRelativeLogicalHeight() || isOrthogonalChild(*child))))
             layoutScope.setNeedsLayout(child, LayoutInvalidationReason::GridChanged);
 
         child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
         child->setOverrideContainingBlockContentLogicalHeight(overrideContainingBlockContentLogicalHeight);
 
         // Stretching logic might force a child layout, so we need to run it before the layoutIfNeeded
         // call to avoid unnecessary relayouts. This might imply that child margins, needed to correctly
         // determine the available space before stretching, are not set yet.
         applyStretchAlignmentToChildIfNeeded(*child);
 
@@ skipping 27 matching lines @@
     childLayer->setStaticInlinePosition(borderAndPaddingStart());
     childLayer->setStaticBlockPosition(borderAndPaddingBefore());
 }
 
 void LayoutGrid::layoutPositionedObjects(bool relayoutChildren, PositionedLayoutBehavior info)
 {
     TrackedLayoutBoxListHashSet* positionedDescendants = positionedObjects();
     if (!positionedDescendants)
         return;
 
-    bool containerHasHorizontalWritingMode = isHorizontalWritingMode();
     for (auto* child : *positionedDescendants) {
-        bool hasOrthogonalWritingMode = child->isHorizontalWritingMode() != containerHasHorizontalWritingMode;
-        if (hasOrthogonalWritingMode) {
+        if (isOrthogonalChild(*child)) {
             // FIXME: Properly support orthogonal writing mode.
             continue;
         }
 
         LayoutUnit columnOffset = LayoutUnit();
         LayoutUnit columnBreadth = LayoutUnit();
         offsetAndBreadthForPositionedChild(*child, ForColumns, columnOffset, columnBreadth);
         LayoutUnit rowOffset = LayoutUnit();
         LayoutUnit rowBreadth = LayoutUnit();
         offsetAndBreadthForPositionedChild(*child, ForRows, rowOffset, rowBreadth);
 
         child->setOverrideContainingBlockContentLogicalWidth(columnBreadth);
         child->setOverrideContainingBlockContentLogicalHeight(rowBreadth);
         child->setExtraInlineOffset(columnOffset);
         child->setExtraBlockOffset(rowOffset);
     }
 
     LayoutBlock::layoutPositionedObjects(relayoutChildren, info);
 }
 
 void LayoutGrid::offsetAndBreadthForPositionedChild(const LayoutBox& child, GridTrackSizingDirection direction, LayoutUnit& offset, LayoutUnit& breadth)
 {
-    ASSERT(child.isHorizontalWritingMode() == isHorizontalWritingMode());
+    ASSERT(!isOrthogonalChild(child));
     bool isForColumns = direction == ForColumns;
 
     GridSpan positions = GridPositionsResolver::resolveGridPositionsFromStyle(*style(), child, direction);
     if (positions.isIndefinite()) {
         offset = LayoutUnit();
         breadth = isForColumns ? clientLogicalWidth() : clientLogicalHeight();
         return;
     }
 
     // For positioned items we cannot use GridSpan::translate(). Because we could end up with negative values, as the positioned items do not create implicit tracks per spec.
@@ skipping 50 matching lines @@
     ASSERT(m_gridItemArea.contains(&gridItem));
     return m_gridItemArea.get(&gridItem);
 }
 
 GridSpan LayoutGrid::cachedGridSpan(const LayoutBox& gridItem, GridTrackSizingDirection direction) const
 {
     GridArea area = cachedGridArea(gridItem);
     return direction == ForColumns ? area.columns : area.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, 2016/03/23 11:02:00]

Really dangerous. We should not use hasDefiniteXXX methods, they're basically
wrong.

[jfernandez, 2016/04/01 16:39:56]

I've just used what it's being done for the case of
minTrackBreadth.hasPercentage() || maxTrackBreadth.hasPercentage() in the
gridTrackSize function. This is just to consider the same case. If this is wrong
we might have to fix it there as well and we need a consistent way to determine
whether a Length is Indefinite or not.

[svillar, 2016/04/04 07:58:28]

Yes, we know that gridTrackSize() is wrong. The hasDefiniteXXX methods are not
the right way to detect what we want.

+}
+
+LayoutUnit LayoutGrid::assumedRowsBreadthForOrthogonalChild(const LayoutBox& child) const
+{
+    ASSERT(isOrthogonalChild(child));
+    const GridSpan& span = cachedGridSpan(child, ForRows);
+    LayoutUnit gridAreaBreadth;
+    for (auto trackPosition : span) {
+        const GridLength& maxTrackBreadth = gridTrackSize(ForRows, trackPosition).maxTrackBreadth();
+        if (gridLengthIsIndefinite(maxTrackBreadth, ForRows)) {
+            gridAreaBreadth = child.maxPreferredLogicalWidth() + marginIntrinsicLogicalWidthForChild(child);
+            break;
+        }
+        gridAreaBreadth += valueForLength(maxTrackBreadth.length(), LayoutUnit());
+    }
+
+    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;
+    // To determine the column track's breadth based on an orthogonal grid item we need it's logical height, which
+    // may depend on the row track's breadth. It's possible that row tracks sizing logic has not been performed yet,
+    // so we will need to do an estimation. We can detect this situation checking whether the row tracks vector is
+    // empty or not.
+    // TODO (lajava) are there better ways to do this ?
+    if (isOrthogonalChild(child) && tracks.isEmpty())
+        return assumedRowsBreadthForOrthogonalChild(child);
     const GridSpan& span = cachedGridSpan(child, direction);
     LayoutUnit gridAreaBreadth;
     for (const auto& trackPosition : span)
         gridAreaBreadth += tracks[trackPosition].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)
         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 164 matching lines @@
 GridAxisPosition LayoutGrid::columnAxisPositionForChild(const LayoutBox& child) const
 {
     bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
     bool hasSameWritingMode = child.styleRef().getWritingMode() == styleRef().getWritingMode();
 
     switch (ComputedStyle::resolveAlignment(styleRef(), child.styleRef(), ItemPositionStretch)) {
     case ItemPositionSelfStart:
         // If orthogonal writing-modes, this computes to 'start'.
         // FIXME: grid track sizing and positioning do not support orthogonal modes yet.
         // self-start is based on the child's block axis direction. That's why we need to check against the grid container's block flow.
-        return (hasOrthogonalWritingMode || hasSameWritingMode) ? GridAxisStart : GridAxisEnd;
+        return (isOrthogonalChild(child) || hasSameWritingMode) ? GridAxisStart : GridAxisEnd;
     case ItemPositionSelfEnd:
         // If orthogonal writing-modes, this computes to 'end'.
         // FIXME: grid track sizing and positioning do not support orthogonal modes yet.
         // self-end is based on the child's block axis direction. That's why we need to check against the grid container's block flow.
-        return (hasOrthogonalWritingMode || hasSameWritingMode) ? GridAxisEnd : GridAxisStart;
+        return (isOrthogonalChild(child) || hasSameWritingMode) ? GridAxisEnd : GridAxisStart;
     case ItemPositionLeft:
         // The alignment axis (column axis) and the inline axis are parallell in
         // orthogonal writing mode. Otherwise this this is equivalent to 'start'.
         // FIXME: grid track sizing and positioning do not support orthogonal modes yet.
         return GridAxisStart;
     case ItemPositionRight:
         // The alignment axis (column axis) and the inline axis are parallell in
         // orthogonal writing mode. Otherwise this this is equivalent to 'start'.
         // FIXME: grid track sizing and positioning do not support orthogonal modes yet.
-        return hasOrthogonalWritingMode ? GridAxisEnd : GridAxisStart;
+        return isOrthogonalChild(child) ? GridAxisEnd : GridAxisStart;
     case ItemPositionCenter:
         return GridAxisCenter;
     case ItemPositionFlexStart: // Only used in flex layout, otherwise equivalent to 'start'.
     case ItemPositionStart:
         return GridAxisStart;
     case ItemPositionFlexEnd: // Only used in flex layout, otherwise equivalent to 'end'.
     case ItemPositionEnd:
         return GridAxisEnd;
     case ItemPositionStretch:
         return GridAxisStart;
     case ItemPositionBaseline:
     case ItemPositionLastBaseline:
         // FIXME: These two require implementing Baseline Alignment. For now, we always 'start' align the child.
         // crbug.com/234191
         return GridAxisStart;
     case ItemPositionAuto:
         break;
     }
 
     ASSERT_NOT_REACHED();
     return GridAxisStart;
 }
 
 GridAxisPosition LayoutGrid::rowAxisPositionForChild(const LayoutBox& child) const
 {
-    bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
     bool hasSameDirection = child.styleRef().direction() == styleRef().direction();
     bool isLTR = styleRef().isLeftToRightDirection();
 
     switch (ComputedStyle::resolveJustification(styleRef(), child.styleRef(), ItemPositionStretch)) {
     case ItemPositionSelfStart:
         // For orthogonal writing-modes, this computes to 'start'
         // FIXME: grid track sizing and positioning do not support orthogonal modes yet.
         // self-start is based on the child's direction. That's why we need to check against the grid container's direction.
-        return (hasOrthogonalWritingMode || hasSameDirection) ? GridAxisStart : GridAxisEnd;
+        return (isOrthogonalChild(child) || hasSameDirection) ? GridAxisStart : GridAxisEnd;
     case ItemPositionSelfEnd:
         // For orthogonal writing-modes, this computes to 'start'
         // FIXME: grid track sizing and positioning do not support orthogonal modes yet.
-        return (hasOrthogonalWritingMode || hasSameDirection) ? GridAxisEnd : GridAxisStart;
+        return (isOrthogonalChild(child) || hasSameDirection) ? GridAxisEnd : GridAxisStart;
     case ItemPositionLeft:
         return isLTR ? GridAxisStart : GridAxisEnd;
     case ItemPositionRight:
         return isLTR ? GridAxisEnd : GridAxisStart;
     case ItemPositionCenter:
         return GridAxisCenter;
     case ItemPositionFlexStart: // Only used in flex layout, otherwise equivalent to 'start'.
     case ItemPositionStart:
         return GridAxisStart;
     case ItemPositionFlexEnd: // Only used in flex layout, otherwise equivalent to 'end'.
@@ skipping 189 matching lines @@
     case ContentPositionNormal:
         break;
     }
 
     ASSERT_NOT_REACHED();
     return {LayoutUnit(), LayoutUnit()};
 }
 
 LayoutPoint LayoutGrid::findChildLogicalPosition(const LayoutBox& child, GridSizingData& sizingData) const
 {
+    LayoutUnit columnAxisOffset = columnAxisOffsetForChild(child, sizingData);
     LayoutUnit rowAxisOffset = rowAxisOffsetForChild(child, sizingData);
-    // 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, sizingData));
+    // "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);
+    return isOrthogonalChild(child) ? childLocation.transposedPoint() : childLocation;
 }
 
 void LayoutGrid::paintChildren(const PaintInfo& paintInfo, const LayoutPoint& paintOffset) const
 {
     GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 } // namespace blink