[css-grid] Implement repeat(auto-fit)

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 6d8864e4a0d6d97a786ebe9bb66b5cfebf6bd4b9..02cef0508f2d9b4696980bb36e078437ba16dce3 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -387,7 +387,7 @@ LayoutUnit LayoutGrid::computeTrackBasedLogicalHeight(const GridSizingData& sizi
     for (const auto& row : sizingData.rowTracks)
         logicalHeight += row.baseSize();
 
-    logicalHeight += guttersSize(ForRows, sizingData.rowTracks.size());
+    logicalHeight += guttersSize(ForRows, 0, sizingData.rowTracks.size());
 
     return logicalHeight;
 }
@@ -395,7 +395,7 @@ LayoutUnit LayoutGrid::computeTrackBasedLogicalHeight(const GridSizingData& sizi
 void LayoutGrid::computeTrackSizesForDirection(GridTrackSizingDirection direction, GridSizingData& sizingData, LayoutUnit freeSpace)
 {
     DCHECK(sizingData.isValidTransitionForDirection(direction));
-    sizingData.freeSpaceForDirection(direction) = freeSpace - guttersSize(direction, direction == ForRows ? gridRowCount() : gridColumnCount());
+    sizingData.freeSpaceForDirection(direction) = freeSpace - guttersSize(direction, 0, direction == ForRows ? gridRowCount() : gridColumnCount());
     sizingData.sizingOperation = TrackSizing;
 
     LayoutUnit baseSizes, growthLimits;
@@ -500,13 +500,65 @@ void LayoutGrid::layoutBlock(bool relayoutChildren)
     clearNeedsLayout();
 }
 
-LayoutUnit LayoutGrid::guttersSize(GridTrackSizingDirection direction, size_t span) const
+bool LayoutGrid::isEmptyAutoRepeatTrack(GridTrackSizingDirection direction, size_t lineNumber) const

[Manuel Rego, 2016/07/08 11:12:34]

We usually use just "line" to refer to the other line arguments
that store line indexes (starting on 0).
Otherwise I'd think this method expects a line number (from the spec),
the ones starting on 1.

[svillar, 2016/07/11 12:46:19]

Acknowledged.

+{
+    return direction == ForColumns ? m_autoRepeatEmptyColumns->contains(lineNumber) : m_autoRepeatEmptyRows->contains(lineNumber);
+}
+
+LayoutUnit LayoutGrid::gridGapForDirection(GridTrackSizingDirection direction) const
+{
+    return valueForLength(direction == ForColumns ? styleRef().gridColumnGap() : styleRef().gridRowGap(), LayoutUnit());
+}
+
+LayoutUnit LayoutGrid::guttersSize(GridTrackSizingDirection direction, size_t startLine, size_t span) const

[Manuel Rego, 2016/07/08 11:12:35]

In most of the cases startLine is simply 0.
I'd move it to the last parameter and make it optional.

[svillar, 2016/07/11 12:46:19]

That's correct, but I did it like this for API coherence. I'm calling
guttersSize with a starline and then a span. Having first the span and then the
start line looks weird to me from an API POV.

It's like if you define a function like this void f(end, start).

 {
     if (span <= 1)
         return LayoutUnit();
 
-    const Length& trackGap = direction == ForColumns ? styleRef().gridColumnGap() : styleRef().gridRowGap();
-    return valueForLength(trackGap, LayoutUnit()) * (span - 1);
+    bool isRowAxis = direction == ForColumns;
+    LayoutUnit gap = gridGapForDirection(direction);
+
+    // Fast path, no collapsing tracks.
+    if (isRowAxis ? !m_autoRepeatEmptyColumns : !m_autoRepeatEmptyRows)
+        return gap * (span - 1);
+
+    // If there are collapsing tracks we need to be sure that gutters are properly collapsed. Apart
+    // from that, if we have a collapsed track in the edges of the span we're considering, we need
+    // to move forward (or backwards) in order to know whether the collapsed tracks reach the end of
+    // the grid (so the gap becomes 0) or there is a non empty track before that.
+
+    LayoutUnit gutterAccumulator;

[Manuel Rego, 2016/07/08 11:12:34]

Nit: You call this gapAccumulator in a different method.

[svillar, 2016/07/11 12:46:19]

Acknowledged.

+    size_t endLine = startLine + span;
+
+    for (size_t line = startLine; line < endLine - 1; ++line) {
+        if (!isEmptyAutoRepeatTrack(direction, line))
+            gutterAccumulator += gap;
+    }
+
+    // If the startLine is the start line of a collapsed track we need to go backwards till we reach
+    // a non collapsed track. Should we find a non collapsed track we need to add that gap.

[Manuel Rego, 2016/07/08 11:12:34]

Nit: s/Should/If/ (it's clearer to me).

[svillar, 2016/07/11 12:46:18]

Acknowledged.

[Manuel Rego, 2016/07/14 09:10:51]

Ping ;)

+    if (startLine && isEmptyAutoRepeatTrack(direction, startLine)) {
+        size_t nonEmptyTracksBeforeStartLine = startLine;
+        auto begin = isRowAxis ? m_autoRepeatEmptyColumns->begin() : m_autoRepeatEmptyRows->begin();
+        for (auto it = begin; *it != startLine; ++it)
+            --nonEmptyTracksBeforeStartLine;
+        if (nonEmptyTracksBeforeStartLine)
+            gutterAccumulator += gap;
+    }
+
+    // If the endLine is the end line of a collapsed track we need to go forward till we reach a non
+    // collapsed track. Should we find a non collapsed track we need to add that gap.

[Manuel Rego, 2016/07/08 11:12:35]

Ditto.

[svillar, 2016/07/11 12:46:19]

Acknowledged.

+    if (isEmptyAutoRepeatTrack(direction, endLine - 1)) {
+        size_t nonEmptyTracksAfterEndLine = (isRowAxis ? gridColumnCount() : gridRowCount()) - endLine;
+        auto currentEmptyLine = isRowAxis ? m_autoRepeatEmptyColumns->find(endLine - 1) : m_autoRepeatEmptyRows->find(endLine - 1);
+        auto endEmptyLine = isRowAxis ? m_autoRepeatEmptyColumns->end() : m_autoRepeatEmptyRows->end();
+        for (auto it = currentEmptyLine; it != endEmptyLine; ++it)
+            --nonEmptyTracksAfterEndLine;
+        if (nonEmptyTracksAfterEndLine)
+            gutterAccumulator += gap;
+    }
+
+    return gutterAccumulator;
 }
 
 void LayoutGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const
@@ -518,7 +570,7 @@ void LayoutGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, Layo
     sizingData.sizingOperation = IntrinsicSizeComputation;
     const_cast<LayoutGrid*>(this)->computeUsedBreadthOfGridTracks(ForColumns, sizingData, minLogicalWidth, maxLogicalWidth);
 
-    LayoutUnit totalGuttersSize = guttersSize(ForColumns, sizingData.columnTracks.size());
+    LayoutUnit totalGuttersSize = guttersSize(ForColumns, 0, sizingData.columnTracks.size());
     minLogicalWidth += totalGuttersSize;
     maxLogicalWidth += totalGuttersSize;
 
@@ -534,7 +586,7 @@ void LayoutGrid::computeIntrinsicLogicalHeight(GridSizingData& sizingData)
     sizingData.sizingOperation = IntrinsicSizeComputation;
     computeUsedBreadthOfGridTracks(ForRows, sizingData, m_minContentHeight, m_maxContentHeight);
 
-    LayoutUnit totalGuttersSize = guttersSize(ForRows, gridRowCount());
+    LayoutUnit totalGuttersSize = guttersSize(ForRows, 0, gridRowCount());
     m_minContentHeight += totalGuttersSize;
     m_maxContentHeight += totalGuttersSize;
 
@@ -579,7 +631,7 @@ void LayoutGrid::computeUsedBreadthOfGridTracks(GridTrackSizingDirection directi
     // Grid gutters were removed from freeSpace by the caller, but we must use them to compute relative (i.e. percentages) sizes.
     bool hasDefiniteFreeSpace = sizingData.sizingOperation == TrackSizing;
     if (hasDefiniteFreeSpace)
-        maxSize += guttersSize(direction, direction == ForRows ? gridRowCount() : gridColumnCount());
+        maxSize += guttersSize(direction, 0, direction == ForRows ? gridRowCount() : gridColumnCount());
 
     // 1. Initialize per Grid track variables.
     for (size_t i = 0; i < tracks.size(); ++i) {
@@ -809,6 +861,11 @@ const GridTrackSize& LayoutGrid::rawGridTrackSize(GridTrackSizingDirection direc
 
 GridTrackSize LayoutGrid::gridTrackSize(GridTrackSizingDirection direction, size_t translatedIndex, SizingOperation sizingOperation) const
 {
+    // Collapse empty auto repeat tracks if auto-fit

[Manuel Rego, 2016/07/08 11:12:34]

Nit: Missing dot at the end.

[svillar, 2016/07/11 12:46:19]

Acknowledged.

+    if ((direction == ForColumns && m_autoRepeatEmptyColumns && m_autoRepeatEmptyColumns->contains(translatedIndex))
+        || (direction == ForRows && m_autoRepeatEmptyRows && m_autoRepeatEmptyRows->contains(translatedIndex)))

[Manuel Rego, 2016/07/08 11:12:34]

Cannot we use isEmptyAutoRepeatTrack() here?

[svillar, 2016/07/11 12:46:19]

Totally, but I still need the null check here (won't add it to the function as
it's always called when there are auto repetitions but in this case).

+        return GridTrackSize(Length(Fixed), Length(Fixed));
+
     const GridTrackSize& trackSize = rawGridTrackSize(direction, translatedIndex);
 
     GridLength minTrackBreadth = trackSize.minTrackBreadth();
@@ -1203,7 +1260,8 @@ void LayoutGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizing
         if (sizingData.filteredTracks.isEmpty())
             continue;
 
-        spanningTracksSize += guttersSize(direction, itemSpan.integerSpan());
+        DCHECK(itemSpan.isTranslatedDefinite());

[Manuel Rego, 2016/07/08 11:12:35]

I don't believe we need this here.

We've an ASSERT at the end of LayoutGrid::placeItemsOnGrid()
to verify that all the cached spans are "TranslatedDefinite".
Also GridSpan::startLine() method has already the same ASSERT inside.

[svillar, 2016/07/11 12:46:19]

Acknowledged.

+        spanningTracksSize += guttersSize(direction, itemSpan.startLine(), itemSpan.integerSpan());
 
         LayoutUnit extraSpace = currentItemSizeForTrackSizeComputationPhase(phase, gridItemWithSpan.gridItem(), direction, sizingData) - spanningTracksSize;
         extraSpace = extraSpace.clampNegativeToZero();
@@ -1366,7 +1424,7 @@ size_t LayoutGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection directi
 
     // Add gutters as if there where only 1 auto repeat track. Gaps between auto repeat tracks will be added later when
     // computing the repetitions.
-    LayoutUnit gapSize = guttersSize(direction, 2);
+    LayoutUnit gapSize = gridGapForDirection(direction);
     tracksSize += gapSize * trackSizes.size();
 
     LayoutUnit freeSpace = availableSize - tracksSize;
@@ -1384,6 +1442,37 @@ size_t LayoutGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection directi
     return repetitions;
 }
 
+
+std::unique_ptr<LayoutGrid::TrackIndexSet> LayoutGrid::computeEmptyTracksForAutoRepeat(GridTrackSizingDirection direction) const
+{
+    bool isRowAxis = direction == ForColumns;
+    if ((isRowAxis && styleRef().gridAutoRepeatColumnsType() != AutoFit)
+        || (!isRowAxis && styleRef().gridAutoRepeatRowsType() != AutoFit))
+        return nullptr;
+
+    std::unique_ptr<TrackIndexSet> emptyTrackIndexes;

[Manuel Rego, 2016/07/08 11:12:34]

I guess you can initialize it here already so you can avoid the if the for
below.

[svillar, 2016/07/11 12:46:19]

What if there is no empty track? I'd need to create and destroy the object.

[Manuel Rego, 2016/07/14 09:10:50]

True, I didn't realize before.

+    size_t insertionPoint = isRowAxis ? styleRef().gridAutoRepeatColumnsInsertionPoint() : styleRef().gridAutoRepeatRowsInsertionPoint();
+    size_t repetitions = autoRepeatCountForDirection(direction);
+    size_t firstAutoRepeatTrack = insertionPoint + std::abs(isRowAxis ? m_smallestColumnStart : m_smallestRowStart);
+    size_t lastAutoRepeatTrack = firstAutoRepeatTrack + repetitions;
+
+    if (m_gridItemArea.isEmpty()) {
+        emptyTrackIndexes = wrapUnique(new TrackIndexSet);
+        for (size_t trackIndex = firstAutoRepeatTrack; trackIndex < lastAutoRepeatTrack; ++trackIndex)
+            emptyTrackIndexes->add(trackIndex);
+    } else {
+        for (size_t trackIndex = firstAutoRepeatTrack; trackIndex < lastAutoRepeatTrack; ++trackIndex) {

[Manuel Rego, 2016/07/08 11:12:35]

The for is the same, maybe you can move the outer if inside it.

[svillar, 2016/07/11 12:46:18]

If I do that I'd evaluate the if() condition for every single iteration.
Although it's more code it's much more efficient. Also you cannot create a
GridIterator for empty grids.

+            GridIterator iterator(m_grid, direction, trackIndex);
+            if (!iterator.nextGridItem()) {
+                if (!emptyTrackIndexes)
+                    emptyTrackIndexes = wrapUnique(new TrackIndexSet);
+                emptyTrackIndexes->add(trackIndex);
+            }
+        }
+    }
+    return emptyTrackIndexes;
+}
+
 void LayoutGrid::placeItemsOnGrid()
 {
     if (!m_gridIsDirty)
@@ -1434,6 +1523,10 @@ void LayoutGrid::placeItemsOnGrid()
 
     m_grid.shrinkToFit();
 
+    // Compute collapsable tracks for auto-fit

[Manuel Rego, 2016/07/08 11:12:34]

Nit: Missing dot at the end.

+    m_autoRepeatEmptyColumns = computeEmptyTracksForAutoRepeat(ForColumns);
+    m_autoRepeatEmptyRows = computeEmptyTracksForAutoRepeat(ForRows);
+
 #if ENABLE(ASSERT)
     for (LayoutBox* child = m_orderIterator.first(); child; child = m_orderIterator.next()) {
         if (child->isOutOfFlowPositioned())
@@ -1632,6 +1725,47 @@ void LayoutGrid::dirtyGrid()
     m_gridItemsOverflowingGridArea.resize(0);
     m_gridItemsIndexesMap.clear();
     m_gridIsDirty = true;
+    m_autoRepeatEmptyColumns = nullptr;
+    m_autoRepeatEmptyRows = nullptr;
+}
+
+Vector<LayoutUnit> LayoutGrid::trackSizesForComputedStyle(GridTrackSizingDirection direction) const

[Manuel Rego, 2016/07/08 11:12:35]

We've been computing almost the same information in
LayoutGrid::populateGridPositionsForDirection().

It'd be great to be able to reuse it. Otherwise we might introduce bugs later
when we modify something in LayoutGrid::populateGridPositionsForDirection()
and forgot about changing this method.

[svillar, 2016/07/11 12:46:19]

In one method we're computing the line positions while in the other we're
computing the track sizes. I agree that they're related but they are not
definitely the same.

If we want to reuse the grid line positions then we have to store some extra
information, like the size of the gaps for each line.

Note that having a method like this already improves the current situation,
because the track sizes were computed outside the grid code by the
CSSValueMapping object which was already manipulating the offset and gaps
between tracks, something that should be kept inside the grid.

[Manuel Rego, 2016/07/14 09:10:51]

Yeah I agree. My concern was that there're some duplicated complex logic,
that might be hard to keep updated in both places at the same time.

One idea could be to generate this Vector on populateGridPositionsForDirection()
and just return it here. But I also don't like it a lot. :-/

+{
+    bool isRowAxis = direction == ForColumns;
+    auto& positions = isRowAxis ? m_columnPositions : m_rowPositions;
+    size_t numPositions = positions.size();

[Manuel Rego, 2016/07/08 11:12:34]

Maybe we can rename numPositions to numTracks:
  numTracks = positions.size() - 1;

Then we can use it later.

[svillar, 2016/07/11 12:46:19]

Hmm not sure what we would win with that, we still need numPositions - 2 in a
couple of places. Also the loop is iterating positions so it'd be a bit weird to
have a (i < numTracks) as the exit condition (we know about the close
relationship between tracks and positions but perhaps is not that evident for an
outsider).

+    LayoutUnit offsetBetweenTracks = isRowAxis ? m_offsetBetweenColumns : m_offsetBetweenRows;
+
+    Vector<LayoutUnit> tracks;
+    if (numPositions < 2)
+        return tracks;
+
+    bool hasCollapsedTracks = isRowAxis ? !!m_autoRepeatEmptyColumns : !!m_autoRepeatEmptyRows;
+    LayoutUnit gap = !hasCollapsedTracks ? gridGapForDirection(direction) : LayoutUnit();
+    tracks.reserveCapacity(numPositions - 1);
+    for (size_t i = 0; i < numPositions - 2; ++i)
+        tracks.append(positions[i + 1] - positions[i] - offsetBetweenTracks - gap);
+    tracks.append(positions[numPositions - 1] - positions[numPositions - 2]);
+
+    if (!hasCollapsedTracks)
+        return tracks;
+
+    size_t remainingEmptyTracks = isRowAxis ? m_autoRepeatEmptyColumns->size() : m_autoRepeatEmptyRows->size();
+    size_t lastLine = tracks.size();

[Manuel Rego, 2016/07/08 11:12:35]

E.g. Here we'd already have the numTracks stored.

+    gap = gridGapForDirection(direction);
+    for (size_t i = 1; i < lastLine; ++i) {
+        if (isEmptyAutoRepeatTrack(direction, i - 1)) {
+            --remainingEmptyTracks;
+        } else {
+            // Remove the gap between consecutive non empty tracks. Remove it also just once for an
+            // arbitrary number of empty tracks between two non empty ones.
+            bool allRemainingTracksAreEmpty = remainingEmptyTracks == (lastLine - i);
+            if (!allRemainingTracksAreEmpty || !isEmptyAutoRepeatTrack(direction, i))
+                tracks[i - 1] -= gap;
+        }

[Manuel Rego, 2016/07/08 11:12:35]

Could we early return if (remainingEmptyTracks == 0) or not?

[svillar, 2016/07/11 12:46:19]

Totally the opposite :). If none of the remaining tracks are empty this means
that you have to remove the gaps for all of them (note that the first loop of
this method does not remove gaps if we have empty auto repeat tracks so we have
to remove them here if needed).

+    }
+
+    return tracks;
 }
 
 static const StyleContentAlignmentData& normalValueBehavior()
@@ -1820,9 +1954,11 @@ void LayoutGrid::offsetAndBreadthForPositionedChild(const LayoutBox& child, Grid
             end = m_rowPositions[endLine] - borderBefore();
         }
 
-        // These vectors store line positions including gaps, but we shouldn't consider them for the edges of the grid.
-        if (endLine > 0 && endLine < lastLine) {
-            end -= guttersSize(direction, 2);
+        // These vectors store line positions including gaps, but we shouldn't consider them for the
+        // edges of the grid (note that a collapsed track at the end of the grid has its end line
+        // overlapping the grid'd edge).

[Manuel Rego, 2016/07/08 11:12:35]

Apart from the typo "grid'd" I don't get the part in parenthesis.
Could you clarify it?

[Manuel Rego, 2016/07/14 09:10:51]

Ping. I don't understand the part between parenthesis
"(note that a collapsed track at the end of the grid
 has its end line overlapping the grid'd edge)."

Imagine a grid with:
* width: 500px;
* grid-template-columns: repeat(auto-fit, [a] 200px [b]);

If the 2nd track collapse we'll have:
[a] 200px [a b] 0px [b]

The grid's edge is 500px not related at all with the collapsed track.

+        if (endLine && (endLine < lastLine)) {

[Manuel Rego, 2016/07/08 11:12:35]

Why you removed "> 0" for endLine?
endLine is an int, probably better to keep the "endLine > 0" here.

[svillar, 2016/07/11 12:46:19]

Oh my mistake, I did it due to the coding style

+            end -= guttersSize(direction, endLine - 1, 2);

[Manuel Rego, 2016/07/08 11:12:34]

Cannot we use gridGapForDirection(direction) like you do below?

It seems weird to me that this code is different to the if a few lines below.

[svillar, 2016/07/11 12:46:19]

No we cannot. gridGapForDirection just returns the gap from the style. Here we
need the actual gap between 2 tracks (it might have collapsed so it could be
0px)

It looks like I forgot to change the one bellow (and also that we need a test
for that case).

             end -= isForColumns ? m_offsetBetweenColumns : m_offsetBetweenRows;
         }
     }
@@ -1839,7 +1975,7 @@ void LayoutGrid::offsetAndBreadthForPositionedChild(const LayoutBox& child, Grid
             offset = translateRTLCoordinate(m_columnPositions[endLine]) - borderLogicalLeft();
 
             if (endLine > 0 && endLine < lastLine) {
-                offset += guttersSize(direction, 2);
+                offset += gridGapForDirection(direction);
                 offset += isForColumns ? m_offsetBetweenColumns : m_offsetBetweenRows;
             }
         }
@@ -1874,7 +2010,7 @@ LayoutUnit LayoutGrid::assumedRowsSizeForOrthogonalChild(const LayoutBox& child,
             gridAreaSize += valueForLength(maxTrackSize.length(), containingBlockAvailableSize);
     }
 
-    gridAreaSize += guttersSize(ForRows, span.integerSpan());
+    gridAreaSize += guttersSize(ForRows, span.startLine(), span.integerSpan());
 
     return gridAreaIsIndefinite ? std::max(child.maxPreferredLogicalWidth(), gridAreaSize) : gridAreaSize;
 }
@@ -1893,7 +2029,8 @@ LayoutUnit LayoutGrid::gridAreaBreadthForChild(const LayoutBox& child, GridTrack
     for (const auto& trackPosition : span)
         gridAreaBreadth += tracks[trackPosition].baseSize();
 
-    gridAreaBreadth += guttersSize(direction, span.integerSpan());
+    DCHECK(span.isTranslatedDefinite());

[Manuel Rego, 2016/07/08 11:12:34]

Ditto.

+    gridAreaBreadth += guttersSize(direction, span.startLine(), span.integerSpan());
 
     return gridAreaBreadth;
 }
@@ -1926,16 +2063,40 @@ void LayoutGrid::populateGridPositionsForDirection(GridSizingData& sizingData, G
     size_t numberOfLines = numberOfTracks + 1;
     size_t lastLine = numberOfLines - 1;
     ContentAlignmentData offset = computeContentPositionAndDistributionOffset(direction, sizingData.freeSpaceForDirection(direction), numberOfTracks);
-    LayoutUnit trackGap = guttersSize(direction, 2);
     auto& positions = isRowAxis ? m_columnPositions : m_rowPositions;
     positions.resize(numberOfLines);
     auto borderAndPadding = isRowAxis ? borderAndPaddingLogicalLeft() : borderAndPaddingBefore();
     positions[0] = borderAndPadding + offset.positionOffset;
     if (numberOfLines > 1) {
+        // If we have collapsed tracks we just ignore gaps here and add them later as we might not
+        // compute the gap between two consecutive tracks without examining the surrounding ones.
+        bool hasEmptyLinesForDirection = isRowAxis ? !!m_autoRepeatEmptyColumns : !!m_autoRepeatEmptyRows;

[Manuel Rego, 2016/07/08 11:12:35]

s/hasEmptyLinesForDirection/hasEmptyTracksForDirection/

I believe "track" is more accurate than "line" here.

[svillar, 2016/07/11 12:46:19]

Oh yeah, empty lines makes no sense indeed.

+        LayoutUnit gap = !hasEmptyLinesForDirection ? gridGapForDirection(direction) : LayoutUnit();
         size_t nextToLastLine = numberOfLines - 2;
         for (size_t i = 0; i < nextToLastLine; ++i)
-            positions[i + 1] = positions[i] + offset.distributionOffset + tracks[i].baseSize() + trackGap;
+            positions[i + 1] = positions[i] + offset.distributionOffset + tracks[i].baseSize() + gap;
         positions[lastLine] = positions[nextToLastLine] + tracks[nextToLastLine].baseSize();
+
+        // Adjust collapsed gaps. Collapsed tracks cause the surrounding gutters to collapse (they
+        // coincide exactly) except on the edges of the grid where they become 0.
+        if (hasEmptyLinesForDirection) {
+            gap = gridGapForDirection(direction);
+            size_t remainingEmptyTracks = isRowAxis ? m_autoRepeatEmptyColumns->size() : m_autoRepeatEmptyRows->size();
+            LayoutUnit gapAccumulator;
+            for (size_t i = 1; i < lastLine; ++i) {
+                if (isEmptyAutoRepeatTrack(direction, i - 1)) {
+                    --remainingEmptyTracks;
+                } else {
+                    // Add gap between consecutive non empty tracks. Add it also just once for an
+                    // arbitrary number of empty tracks between two non empty ones.
+                    bool allRemainingTracksAreEmpty = remainingEmptyTracks == (lastLine - i);
+                    if (!allRemainingTracksAreEmpty || !isEmptyAutoRepeatTrack(direction, i))
+                        gapAccumulator += gap;
+                }
+                positions[i] += gapAccumulator;
+            }
+            positions[lastLine] += gapAccumulator;
+        }
     }
     auto& offsetBetweenTracks = isRowAxis ? m_offsetBetweenColumns : m_offsetBetweenRows;
     offsetBetweenTracks = offset.distributionOffset;
@@ -2186,7 +2347,7 @@ LayoutUnit LayoutGrid::columnAxisOffsetForChild(const LayoutBox& child, GridSizi
         // m_rowPositions include distribution offset (because of content alignment) and gutters
         // so we need to subtract them to get the actual end position for a given row
         // (this does not have to be done for the last track as there are no more m_columnPositions after it).
-        LayoutUnit trackGap = guttersSize(ForRows, 2);
+        LayoutUnit trackGap = gridGapForDirection(ForRows);
         if (childEndLine < m_rowPositions.size() - 1) {
             endOfRow -= trackGap;
             endOfRow -= m_offsetBetweenRows;
@@ -2221,7 +2382,7 @@ LayoutUnit LayoutGrid::rowAxisOffsetForChild(const LayoutBox& child, GridSizingD
         // m_columnPositions include distribution offset (because of content alignment) and gutters
         // so we need to subtract them to get the actual end position for a given column
         // (this does not have to be done for the last track as there are no more m_columnPositions after it).
-        LayoutUnit trackGap = guttersSize(ForColumns, 2);
+        LayoutUnit trackGap = gridGapForDirection(ForColumns);
         if (childEndLine < m_columnPositions.size() - 1) {
             endOfColumn -= trackGap;
             endOfColumn -= m_offsetBetweenColumns;