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

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 369 matching lines @@
     return m_grid.size();
 }
 
 LayoutUnit LayoutGrid::computeTrackBasedLogicalHeight(const GridSizingData& sizingData) const
 {
     LayoutUnit logicalHeight;
 
     for (const auto& row : sizingData.rowTracks)
         logicalHeight += row.baseSize();
 
-    logicalHeight += guttersSize(ForRows, sizingData.rowTracks.size());
+    logicalHeight += guttersSize(ForRows, 0, sizingData.rowTracks.size());
 
     return logicalHeight;
 }
 
 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;
     computeUsedBreadthOfGridTracks(direction, sizingData, baseSizes, growthLimits);
     ASSERT(tracksAreWiderThanMinTrackBreadth(direction, sizingData));
     sizingData.nextState();
 }
 
 void LayoutGrid::repeatTracksSizingIfNeeded(GridSizingData& sizingData, LayoutUnit availableSpaceForColumns, LayoutUnit availableSpaceForRows)
 {
@@ skipping 84 matching lines @@
 
         computeOverflow(oldClientAfterEdge);
     }
 
     updateLayerTransformAfterLayout();
     updateAfterLayout();
 
     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
 {
     const_cast<LayoutGrid*>(this)->placeItemsOnGrid();
 
     GridSizingData sizingData(gridColumnCount(), gridRowCount());
     sizingData.freeSpaceForDirection(ForColumns) = LayoutUnit();
     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;
 
     LayoutUnit scrollbarWidth = LayoutUnit(scrollbarLogicalWidth());
     minLogicalWidth += scrollbarWidth;
     maxLogicalWidth += scrollbarWidth;
 }
 
 void LayoutGrid::computeIntrinsicLogicalHeight(GridSizingData& sizingData)
 {
     ASSERT(tracksAreWiderThanMinTrackBreadth(ForColumns, sizingData));
     sizingData.freeSpaceForDirection(ForRows) = LayoutUnit();
     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;
 
     ASSERT(tracksAreWiderThanMinTrackBreadth(ForRows, sizingData));
 }
 
 LayoutUnit LayoutGrid::computeIntrinsicLogicalContentHeightUsing(const Length& logicalHeightLength, LayoutUnit intrinsicContentHeight, LayoutUnit borderAndPadding) const
 {
     if (logicalHeightLength.isMinContent())
         return m_minContentHeight;
@@ skipping 24 matching lines @@
     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);
 
     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.
     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) {
         GridTrack& track = tracks[i];
         GridTrackSize trackSize = gridTrackSize(direction, i, sizingData.sizingOperation);
         const GridLength& minTrackBreadth = trackSize.minTrackBreadth();
         const GridLength& maxTrackBreadth = trackSize.maxTrackBreadth();
 
         track.setBaseSize(computeUsedBreadthOfMinLength(minTrackBreadth, maxSize));
         track.setGrowthLimit(computeUsedBreadthOfMaxLength(maxTrackBreadth, track.baseSize(), maxSize));
@@ skipping 209 matching lines @@
         return trackStyles[untranslatedIndex];
 
     if (untranslatedIndex < (insertionPoint + repetitions))
         return autoRepeatTrackStyles[0];
 
     return trackStyles[untranslatedIndex - repetitions];
 }
 
 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();
     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()) {
         // For the inline axis this only happens when we're computing the intrinsic sizes (AvailableSpaceIndefinite).
         // For the block axis we check that the percentage height is resolvable on the first in-flow child.
         // TODO (lajava) This condition for determining whether a size is indefinite or not is not working correctly for orthogonal flows.
@@ skipping 374 matching lines @@
 
             sizingData.filteredTracks.append(&track);
 
             if (trackShouldGrowBeyondGrowthLimitsForTrackSizeComputationPhase(phase, trackSize))
                 sizingData.growBeyondGrowthLimitsTracks.append(&track);
         }
 
         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();
         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);
@@ skipping 142 matching lines @@
     const Vector<GridTrackSize>& trackSizes = isRowAxis ? styleRef().gridTemplateColumns() : styleRef().gridTemplateRows();
 
     for (const auto& track : trackSizes) {
         bool hasDefiniteMaxTrackBreadth = track.maxTrackBreadth().isLength() && !track.maxTrackBreadth().isContentSized();
         DCHECK(hasDefiniteMaxTrackBreadth || (track.minTrackBreadth().isLength() && !track.minTrackBreadth().isContentSized()));
         tracksSize += valueForLength(hasDefiniteMaxTrackBreadth ? track.maxTrackBreadth().length() : track.minTrackBreadth().length(), availableSize);
     }
 
     // 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;
     if (freeSpace <= 0)
         return 1;
 
     size_t repetitions = 1 + (freeSpace / (autoRepeatTrackSize + gapSize)).toInt();
 
     // Provided the grid container does not have a definite size or max-size in the relevant axis,
     // if the min size is definite then the number of repetitions is the largest possible positive
     // integer that fulfills that minimum requirement.
     if (needsToFulfillMinimumSize)
         ++repetitions;
 
     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)
         return;
 
     ASSERT(m_gridItemArea.isEmpty());
 
     m_autoRepeatColumns = computeAutoRepeatTracksCount(ForColumns);
     m_autoRepeatRows = computeAutoRepeatTracksCount(ForRows);
 
@@ skipping 30 matching lines @@
     }
 
     DCHECK_GE(gridRowCount(), GridPositionsResolver::explicitGridRowCount(*style(), m_autoRepeatRows));
     DCHECK_GE(gridColumnCount(), GridPositionsResolver::explicitGridColumnCount(*style(), m_autoRepeatColumns));
 
     placeSpecifiedMajorAxisItemsOnGrid(specifiedMajorAxisAutoGridItems);
     placeAutoMajorAxisItemsOnGrid(autoMajorAxisAutoGridItems);
 
     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())
             continue;
 
         GridArea area = cachedGridArea(*child);
         ASSERT(area.rows.isTranslatedDefinite() && area.columns.isTranslatedDefinite());
     }
 #endif
 }
@@ skipping 178 matching lines @@
     // is still ongoing.
     // Forcing a new layout for the Grid layout would cancel any ongoing painting and ensure
     // the grid and its children are correctly laid out according to the new style rules.
     setNeedsLayout(LayoutInvalidationReason::GridChanged);
 
     m_grid.resize(0);
     m_gridItemArea.clear();
     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()
 {
     static const StyleContentAlignmentData normalBehavior = {ContentPositionNormal, ContentDistributionStretch};
     return normalBehavior;
 }
 
 void LayoutGrid::applyStretchAlignmentToTracksIfNeeded(GridTrackSizingDirection direction, GridSizingData& sizingData)
 {
@@ skipping 168 matching lines @@
     if (!endIsAuto) {
         if (isForColumns) {
             if (styleRef().isLeftToRightDirection())
                 end = m_columnPositions[endLine] - borderLogicalLeft();
             else
                 end = logicalWidth() - translateRTLCoordinate(m_columnPositions[endLine]) - borderLogicalRight();
         } else {
             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;
         }
     }
 
     breadth = end - start;
     offset = start;
 
     if (isForColumns && !styleRef().isLeftToRightDirection() && !child.styleRef().hasStaticInlinePosition(child.isHorizontalWritingMode())) {
         // If the child doesn't have a static inline position (i.e. "left" and/or "right" aren't "auto",
         // we need to calculate the offset from the left (even if we're in RTL).
         if (endIsAuto) {
             offset = LayoutUnit();
         } else {
             offset = translateRTLCoordinate(m_columnPositions[endLine]) - borderLogicalLeft();
 
             if (endLine > 0 && endLine < lastLine) {
-                offset += guttersSize(direction, 2);
+                offset += gridGapForDirection(direction);
                 offset += isForColumns ? m_offsetBetweenColumns : m_offsetBetweenRows;
             }
         }
     }
 
 }
 
 GridArea LayoutGrid::cachedGridArea(const LayoutBox& gridItem) const
 {
     ASSERT(m_gridItemArea.contains(&gridItem));
@@ skipping 14 matching lines @@
     bool gridAreaIsIndefinite = false;
     LayoutUnit containingBlockAvailableSize = containingBlockLogicalHeightForContent(ExcludeMarginBorderPadding);
     for (auto trackPosition : span) {
         const GridLength& maxTrackSize = gridTrackSize(ForRows, trackPosition, sizingOperation).maxTrackBreadth();
         if (maxTrackSize.isContentSized() || maxTrackSize.isFlex())
             gridAreaIsIndefinite = true;
         else
             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;
 }
 
 LayoutUnit LayoutGrid::gridAreaBreadthForChild(const LayoutBox& child, GridTrackSizingDirection direction, const GridSizingData& sizingData) const
 {
     // To determine the column track's size based on an orthogonal grid item we need it's logical height, which
     // may depend on the row track's size. It's possible that the row tracks sizing logic has not been performed yet,
     // so we will need to do an estimation.
     if (direction == ForRows && sizingData.sizingState == GridSizingData::ColumnSizingFirstIteration)
         return assumedRowsSizeForOrthogonalChild(child, sizingData.sizingOperation);
 
     const Vector<GridTrack>& tracks = direction == ForColumns ? sizingData.columnTracks : sizingData.rowTracks;
     const GridSpan& span = cachedGridSpan(child, direction);
     LayoutUnit gridAreaBreadth;
     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;
 }
 
 LayoutUnit LayoutGrid::gridAreaBreadthForChildIncludingAlignmentOffsets(const LayoutBox& child, GridTrackSizingDirection direction, const GridSizingData& sizingData) const
 {
     // We need the cached value when available because Content Distribution alignment properties
     // may have some influence in the final grid area breadth.
     const Vector<GridTrack>& tracks = (direction == ForColumns) ? sizingData.columnTracks : sizingData.rowTracks;
     const GridSpan& span = cachedGridSpan(child, direction);
@@ skipping 12 matching lines @@
 
     // The grid container's frame elements (border, padding and <content-position> offset) are sensible to the
     // inline-axis flow direction. However, column lines positions are 'direction' unaware. This simplification
     // allows us to use the same indexes to identify the columns independently on the inline-axis direction.
     bool isRowAxis = direction == ForColumns;
     auto& tracks = isRowAxis ? sizingData.columnTracks : sizingData.rowTracks;
     size_t numberOfTracks = tracks.size();
     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;
 }
 
 static LayoutUnit computeOverflowAlignmentOffset(OverflowAlignment overflow, LayoutUnit trackBreadth, LayoutUnit childBreadth)
 {
     LayoutUnit offset = trackBreadth - childBreadth;
     switch (overflow) {
     case OverflowAlignmentSafe:
@@ skipping 230 matching lines @@
     switch (axisPosition) {
     case GridAxisStart:
         return startPosition;
     case GridAxisEnd:
     case GridAxisCenter: {
         size_t childEndLine = rowsSpan.endLine();
         LayoutUnit endOfRow = m_rowPositions[childEndLine];
         // 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;
         }
         LayoutUnit childBreadth = child.logicalHeight() + child.marginLogicalHeight();
         OverflowAlignment overflow = child.styleRef().resolvedAlignment(styleRef(), ItemPositionStretch).overflow();
         LayoutUnit offsetFromStartPosition = computeOverflowAlignmentOffset(overflow, endOfRow - startOfRow, childBreadth);
         return startPosition + (axisPosition == GridAxisEnd ? offsetFromStartPosition : offsetFromStartPosition / 2);
     }
     }
@@ skipping 14 matching lines @@
     switch (axisPosition) {
     case GridAxisStart:
         return startPosition;
     case GridAxisEnd:
     case GridAxisCenter: {
         size_t childEndLine = columnsSpan.endLine();
         LayoutUnit endOfColumn = m_columnPositions[childEndLine];
         // 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;
         }
         LayoutUnit childBreadth = child.logicalWidth() + child.marginLogicalWidth();
         LayoutUnit offsetFromStartPosition = computeOverflowAlignmentOffset(child.styleRef().justifySelfOverflowAlignment(), endOfColumn - startOfColumn, childBreadth);
         return startPosition + (axisPosition == GridAxisEnd ? offsetFromStartPosition : offsetFromStartPosition / 2);
     }
     }
 
@@ skipping 128 matching lines @@
     return isOrthogonalChild(child) ? childLocation.transposedPoint() : childLocation;
 }
 
 void LayoutGrid::paintChildren(const PaintInfo& paintInfo, const LayoutPoint& paintOffset) const
 {
     if (!m_gridItemArea.isEmpty())
         GridPainter(*this).paintChildren(paintInfo, paintOffset);
 }
 
 } // namespace blink