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

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

Index: third_party/WebKit/Source/core/layout/LayoutGrid.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
index 313e2ef9ede84f9cca3dce5929924e2043eacb2b..d1dcff83ea2aff9cbf3b43af29840969929c775b 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -232,7 +232,7 @@ struct LayoutGrid::GridSizingData {
 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?).

     {
     }
 
@@ -359,12 +359,16 @@ void LayoutGrid::layoutBlock(bool relayoutChildren)
 
         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
@@ -379,6 +383,19 @@ void LayoutGrid::layoutBlock(bool relayoutChildren)
         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()));
@@ -477,6 +494,12 @@ void LayoutGrid::computeUsedBreadthOfGridTracks(GridTrackSizingDirection directi
     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)
@@ -554,7 +577,7 @@ void LayoutGrid::computeUsedBreadthOfGridTracks(GridTrackSizingDirection directi
                 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)));
             }
         }
     }
@@ -651,11 +674,35 @@ double LayoutGrid::findFlexFactorUnitSize(const Vector<GridTrack>& tracks, const
     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;
@@ -680,12 +727,19 @@ GridTrackSize LayoutGrid::gridTrackSize(GridTrackSizingDirection direction, size
     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);
     }
 
@@ -694,28 +748,31 @@ LayoutUnit LayoutGrid::logicalHeightForChild(LayoutBox& child, Vector<GridTrack>
     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
 {
-    bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    // TODO(svillar): Properly support orthogonal writing mode.
-    if (hasOrthogonalWritingMode)
-        return LayoutUnit();
+    return isOrthogonalChild(child) ? (direction == ForColumns ? ForRows : ForColumns) : direction;
+}
 
-    bool isRowAxis = direction == ForColumns;
+LayoutUnit LayoutGrid::minSizeForChild(LayoutBox& child, GridTrackSizingDirection direction, const GridSizingData& sizingData)
+{
+    bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
+    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);
@@ -723,46 +780,38 @@ LayoutUnit LayoutGrid::minSizeForChild(LayoutBox& child, GridTrackSizingDirectio
     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
@@ -809,7 +858,7 @@ void LayoutGrid::resolveContentBasedTrackSizingFunctions(GridTrackSizingDirectio
             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));
                 }
@@ -837,22 +886,22 @@ void LayoutGrid::resolveContentBasedTrackSizingFunctions(GridTrackSizingDirectio
     }
 }
 
-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)
@@ -960,17 +1009,17 @@ static void updateTrackSizeForTrackSizeComputationPhase(TrackSizeComputationPhas
     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();
@@ -1015,7 +1064,7 @@ void LayoutGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizing
 
         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);
@@ -1419,7 +1468,7 @@ void LayoutGrid::layoutGridItems(GridSizingData& 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);
@@ -1467,10 +1516,8 @@ void LayoutGrid::layoutPositionedObjects(bool relayoutChildren, PositionedLayout
     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;
         }
@@ -1493,7 +1540,7 @@ void LayoutGrid::layoutPositionedObjects(bool relayoutChildren, PositionedLayout
 
 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);
@@ -1564,8 +1611,40 @@ GridSpan LayoutGrid::cachedGridSpan(const LayoutBox& gridItem, GridTrackSizingDi
     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)
@@ -1605,7 +1684,7 @@ void LayoutGrid::populateGridPositions(GridSizingData& sizingData)
     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();
 
@@ -1617,7 +1696,7 @@ void LayoutGrid::populateGridPositions(GridSizingData& sizingData)
     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();
 }
@@ -1802,12 +1881,12 @@ GridAxisPosition LayoutGrid::columnAxisPositionForChild(const LayoutBox& child)
         // 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'.
@@ -1817,7 +1896,7 @@ GridAxisPosition LayoutGrid::columnAxisPositionForChild(const LayoutBox& child)
         // 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'.
@@ -1843,7 +1922,6 @@ GridAxisPosition LayoutGrid::columnAxisPositionForChild(const LayoutBox& child)
 
 GridAxisPosition LayoutGrid::rowAxisPositionForChild(const LayoutBox& child) const
 {
-    bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
     bool hasSameDirection = child.styleRef().direction() == styleRef().direction();
     bool isLTR = styleRef().isLeftToRightDirection();
 
@@ -1852,11 +1930,11 @@ GridAxisPosition LayoutGrid::rowAxisPositionForChild(const LayoutBox& child) con
         // 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:
@@ -2066,8 +2144,9 @@ ContentAlignmentData LayoutGrid::computeContentPositionAndDistributionOffset(Gri
 
 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();
@@ -2075,7 +2154,12 @@ LayoutPoint LayoutGrid::findChildLogicalPosition(const LayoutBox& child, GridSiz
         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