[css-grid] repeat() syntax should take a <track-list> argument

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 c310533d05364a05b7b3dded3c7ce4408e07dc38..4e20e617f7335d6b9b226773739a423e7d5618ed 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -363,6 +363,13 @@ bool LayoutGrid::namedGridLinesDefinitionDidChange(const ComputedStyle& oldStyle
         || oldStyle.namedGridColumnLines() != styleRef().namedGridColumnLines();
 }
 
+size_t LayoutGrid::autoRepeatTracksCount(GridTrackSizingDirection direction) const
+{
+    if (direction == ForColumns)
+        return m_autoRepeatColumns * styleRef().gridAutoRepeatColumns().size();
+    return m_autoRepeatRows * styleRef().gridAutoRepeatRows().size();
+}
+
 size_t LayoutGrid::gridColumnCount() const
 {
     DCHECK(!m_gridIsDirty);
@@ -371,7 +378,7 @@ size_t LayoutGrid::gridColumnCount() const
     // to get it from the style. Note that we know for sure that there are't any implicit tracks,
     // because not having rows implies that there are no "normal" children (out-of-flow children are
     // not stored in m_grid).
-    return m_grid.size() ? m_grid[0].size() : GridPositionsResolver::explicitGridColumnCount(styleRef(), m_autoRepeatColumns);
+    return m_grid.size() ? m_grid[0].size() : GridPositionsResolver::explicitGridColumnCount(styleRef(), autoRepeatTracksCount(ForColumns));
 }
 
 size_t LayoutGrid::gridRowCount() const
@@ -850,12 +857,12 @@ const GridTrackSize& LayoutGrid::rawGridTrackSize(GridTrackSizingDirection direc
     const Vector<GridTrackSize>& autoRepeatTrackStyles = isRowAxis ? styleRef().gridAutoRepeatColumns() : styleRef().gridAutoRepeatRows();
     const GridTrackSize& autoTrackSize = isRowAxis ? styleRef().gridAutoColumns() : styleRef().gridAutoRows();
     size_t insertionPoint = isRowAxis ? styleRef().gridAutoRepeatColumnsInsertionPoint() : styleRef().gridAutoRepeatRowsInsertionPoint();
-    size_t repetitions = autoRepeatCountForDirection(direction);
+    size_t autoRepeatTracksCount = autoRepeatCountForDirection(direction) * autoRepeatTrackStyles.size();
 
     // We should not use GridPositionsResolver::explicitGridXXXCount() for this because the
     // explicit grid might be larger than the number of tracks in grid-template-rows|columns (if
     // grid-template-areas is specified for example).
-    size_t explicitTracksCount = trackStyles.size() + repetitions;
+    size_t explicitTracksCount = trackStyles.size() + autoRepeatTracksCount;
 
     int untranslatedIndexAsInt = translatedIndex + (isRowAxis ? m_smallestColumnStart : m_smallestRowStart);
     if (untranslatedIndexAsInt < 0)
@@ -865,13 +872,15 @@ const GridTrackSize& LayoutGrid::rawGridTrackSize(GridTrackSizingDirection direc
     if (untranslatedIndex >= explicitTracksCount)
         return autoTrackSize;
 
-    if (LIKELY(!repetitions) || untranslatedIndex < insertionPoint)
+    if (LIKELY(!autoRepeatTracksCount) || untranslatedIndex < insertionPoint)
         return trackStyles[untranslatedIndex];
 
-    if (untranslatedIndex < (insertionPoint + repetitions))
-        return autoRepeatTrackStyles[0];
+    if (untranslatedIndex < (insertionPoint + autoRepeatTracksCount)) {
+        size_t autoRepeatLocalIndex = untranslatedIndexAsInt - insertionPoint;
+        return autoRepeatTrackStyles[autoRepeatLocalIndex % autoRepeatTrackStyles.size()];
+    }
 
-    return trackStyles[untranslatedIndex - repetitions];
+    return trackStyles[untranslatedIndex - autoRepeatTracksCount];

[jfernandez, 2016/07/19 14:43:18]

Are you sure this can't lead to a negative value as vector index ?

[eae, 2016/07/19 20:59:48]

Please add a DCHECK here to ensure it cannot go negative.

[svillar, 2016/07/20 07:22:30]

Replying to both, I am not changing the code here, just renaming the variable.

That said we know from the if above that

    untraslatedIndex >= insertionPoint + autoRepeatTracksCount

so if my math is correct

    untranslatedIndex >= autoRepeatTracksCount

This means that the code flow guarantees the condition (which my patch is not
changing). IMHO we should not add a check there because it's pretty explicit in
the code.

[jfernandez, 2016/07/20 08:51:50]

Yes, you're right. 

 }
 
 GridTrackSize LayoutGrid::gridTrackSize(GridTrackSizingDirection direction, size_t translatedIndex, SizingOperation sizingOperation) const
@@ -1388,11 +1397,6 @@ size_t LayoutGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection directi
     if (!autoRepeatTracks.size())
         return 0;
 
-    DCHECK_EQ(autoRepeatTracks.size(), static_cast<size_t>(1));
-    auto autoTrackSize = autoRepeatTracks.at(0);
-    DCHECK(autoTrackSize.minTrackBreadth().isLength());
-    DCHECK(!autoTrackSize.minTrackBreadth().isFlex());
-
     LayoutUnit availableSize = isRowAxis ? availableLogicalWidth() : computeContentLogicalHeight(MainOrPreferredSize, styleRef().logicalHeight(), LayoutUnit(-1));
     if (availableSize == -1) {
         const Length& maxLength = isRowAxis ? styleRef().logicalMaxWidth() : styleRef().logicalMaxHeight();
@@ -1419,14 +1423,20 @@ size_t LayoutGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection directi
         needsToFulfillMinimumSize = true;
     }
 
-    bool hasDefiniteMaxTrackSizingFunction = autoTrackSize.maxTrackBreadth().isLength() && !autoTrackSize.maxTrackBreadth().isContentSized();
-    const Length trackLength = hasDefiniteMaxTrackSizingFunction ? autoTrackSize.maxTrackBreadth().length() : autoTrackSize.minTrackBreadth().length();
+    LayoutUnit autoRepeatTracksSize;
+    for (auto autoTrackSize : autoRepeatTracks) {
+        DCHECK(autoTrackSize.minTrackBreadth().isLength());
+        DCHECK(!autoTrackSize.minTrackBreadth().isFlex());
+        bool hasDefiniteMaxTrackSizingFunction = autoTrackSize.maxTrackBreadth().isLength() && !autoTrackSize.maxTrackBreadth().isContentSized();
+        auto trackLength = hasDefiniteMaxTrackSizingFunction ? autoTrackSize.maxTrackBreadth().length() : autoTrackSize.minTrackBreadth().length();
+        autoRepeatTracksSize += valueForLength(trackLength, availableSize);
+    }
     // For the purpose of finding the number of auto-repeated tracks, the UA must floor the track size to a UA-specified
     // value to avoid division by zero. It is suggested that this floor be 1px.
-    LayoutUnit autoRepeatTrackSize = std::max<LayoutUnit>(LayoutUnit(1), valueForLength(trackLength, availableSize));
+    autoRepeatTracksSize = std::max<LayoutUnit>(LayoutUnit(1), autoRepeatTracksSize);
 
     // There will be always at least 1 auto-repeat track, so take it already into account when computing the total track size.
-    LayoutUnit tracksSize = autoRepeatTrackSize;
+    LayoutUnit tracksSize = autoRepeatTracksSize;
     const Vector<GridTrackSize>& trackSizes = isRowAxis ? styleRef().gridTemplateColumns() : styleRef().gridTemplateRows();
 
     for (const auto& track : trackSizes) {
@@ -1444,7 +1454,7 @@ size_t LayoutGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection directi
     if (freeSpace <= 0)
         return 1;
 
-    size_t repetitions = 1 + (freeSpace / (autoRepeatTrackSize + gapSize)).toInt();
+    size_t repetitions = 1 + (freeSpace / (autoRepeatTracksSize + 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
@@ -1465,9 +1475,10 @@ std::unique_ptr<LayoutGrid::OrderedTrackIndexSet> LayoutGrid::computeEmptyTracks
 
     std::unique_ptr<OrderedTrackIndexSet> emptyTrackIndexes;
     size_t insertionPoint = isRowAxis ? styleRef().gridAutoRepeatColumnsInsertionPoint() : styleRef().gridAutoRepeatRowsInsertionPoint();
+    auto& autoRepeatTracks = isRowAxis ? styleRef().gridAutoRepeatColumns() : styleRef().gridAutoRepeatRows();
     size_t repetitions = autoRepeatCountForDirection(direction);
     size_t firstAutoRepeatTrack = insertionPoint + std::abs(isRowAxis ? m_smallestColumnStart : m_smallestRowStart);
-    size_t lastAutoRepeatTrack = firstAutoRepeatTrack + repetitions;
+    size_t lastAutoRepeatTrack = firstAutoRepeatTrack + repetitions * autoRepeatTracks.size();
 
     if (m_gridItemArea.isEmpty()) {
         emptyTrackIndexes = wrapUnique(new OrderedTrackIndexSet);
@@ -1531,8 +1542,8 @@ void LayoutGrid::placeItemsOnGrid(SizingOperation sizingOperation)
         insertItemIntoGrid(*child, area);
     }
 
-    DCHECK_GE(gridRowCount(), GridPositionsResolver::explicitGridRowCount(*style(), m_autoRepeatRows));
-    DCHECK_GE(gridColumnCount(), GridPositionsResolver::explicitGridColumnCount(*style(), m_autoRepeatColumns));
+    DCHECK_GE(gridRowCount(), GridPositionsResolver::explicitGridRowCount(*style(), autoRepeatTracksCount(ForRows)));

[jfernandez, 2016/07/19 14:43:18]

It seems that we are using the autoRepeatTracksCount(ForRows/ForColumns) mutiple
times. Wouldn't make sense to cache it, in some local variable, to avoid
repeating the arithmetic operations ?

[svillar, 2016/07/20 07:22:30]

I've been checking where we use m_autoRepeatColumns|Rows and it seems that we
just use them now for autoRepeatTracksCount() computation. So I'm going to
change their semantics and store in those attributes the total number of auto
repeat tracks, because we don't use the actual number of repetitions for
anything else.

+    DCHECK_GE(gridColumnCount(), GridPositionsResolver::explicitGridColumnCount(*style(), autoRepeatTracksCount(ForColumns)));
 
     placeSpecifiedMajorAxisItemsOnGrid(specifiedMajorAxisAutoGridItems);
     placeAutoMajorAxisItemsOnGrid(autoMajorAxisAutoGridItems);
@@ -1560,8 +1571,8 @@ void LayoutGrid::populateExplicitGridAndOrderIterator()
 
     m_smallestRowStart = m_smallestColumnStart = 0;
 
-    size_t maximumRowIndex = GridPositionsResolver::explicitGridRowCount(*style(), m_autoRepeatRows);
-    size_t maximumColumnIndex = GridPositionsResolver::explicitGridColumnCount(*style(), m_autoRepeatColumns);
+    size_t maximumRowIndex = GridPositionsResolver::explicitGridRowCount(*style(), autoRepeatTracksCount(ForRows));
+    size_t maximumColumnIndex = GridPositionsResolver::explicitGridColumnCount(*style(), autoRepeatTracksCount(ForColumns));
 
     ASSERT(m_gridItemsIndexesMap.isEmpty());
     size_t childIndex = 0;
@@ -1573,8 +1584,8 @@ void LayoutGrid::populateExplicitGridAndOrderIterator()
         m_gridItemsIndexesMap.set(child, childIndex++);
 
         // This function bypasses the cache (cachedGridArea()) as it is used to build it.
-        GridSpan rowPositions = GridPositionsResolver::resolveGridPositionsFromStyle(*style(), *child, ForRows, m_autoRepeatRows);
-        GridSpan columnPositions = GridPositionsResolver::resolveGridPositionsFromStyle(*style(), *child, ForColumns, m_autoRepeatColumns);
+        GridSpan rowPositions = GridPositionsResolver::resolveGridPositionsFromStyle(*style(), *child, ForRows, autoRepeatTracksCount(ForRows));
+        GridSpan columnPositions = GridPositionsResolver::resolveGridPositionsFromStyle(*style(), *child, ForColumns, autoRepeatTracksCount(ForColumns));
         m_gridItemArea.set(child, GridArea(rowPositions, columnPositions));
 
         // |positions| is 0 if we need to run the auto-placement algorithm.