[css-grid] Allow percentage values for column and row gutters

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 1ac1d2d35d3cc99dcec4971c84193397ebd0a8ea..786388161c150f34d2f6d28363e7bdce7349e9c0 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -249,12 +249,17 @@ public:
     Vector<GridTrack*> growBeyondGrowthLimitsTracks;
 
     LayoutUnit& freeSpaceForDirection(GridTrackSizingDirection direction) { return direction == ForColumns ? freeSpaceForColumns : freeSpaceForRows; }
+    void setTrackGapsForDirection(GridTrackSizingDirection direction, LayoutUnit gap) { direction == ForColumns ? gapsForColumns = gap : gapsForRows = gap; }

[cbiesinger, 2016/06/22 18:46:13]

Please don't use the ternary operator instead of an if when there's side-effects
in the expressions like here

[jfernandez, 2016/06/27 23:56:47]

Acknowledged.

+    LayoutUnit trackGapsForDirection(GridTrackSizingDirection direction) const { return direction == ForColumns ? gapsForColumns : gapsForRows; }
+    LayoutUnit guttersSize(GridTrackSizingDirection direction, size_t span) const { return span <= 1 ? LayoutUnit() : trackGapsForDirection(direction) * (span - 1); }
 
     SizingOperation sizingOperation { TrackSizing };
 
 private:
     LayoutUnit freeSpaceForColumns { };
     LayoutUnit freeSpaceForRows { };
+    LayoutUnit gapsForColumns { };
+    LayoutUnit gapsForRows { };
 };
 
 struct GridItemsSpanGroupRange {
@@ -348,14 +353,14 @@ LayoutUnit LayoutGrid::computeTrackBasedLogicalHeight(const GridSizingData& sizi
     for (const auto& row : sizingData.rowTracks)
         logicalHeight += row.baseSize();
 
-    logicalHeight += guttersSize(ForRows, sizingData.rowTracks.size());
+    logicalHeight += sizingData.guttersSize(ForRows, sizingData.rowTracks.size());
 
     return logicalHeight;
 }
 
 void LayoutGrid::computeTrackSizesForDirection(GridTrackSizingDirection direction, GridSizingData& sizingData, LayoutUnit freeSpace)
 {
-    sizingData.freeSpaceForDirection(direction) = freeSpace - guttersSize(direction, direction == ForRows ? gridRowCount() : gridColumnCount());
+    sizingData.freeSpaceForDirection(direction) = freeSpace - sizingData.guttersSize(direction, direction == ForRows ? gridRowCount() : gridColumnCount());
     sizingData.sizingOperation = TrackSizing;
 
     LayoutUnit baseSizes, growthLimits;
@@ -393,14 +398,20 @@ void LayoutGrid::layoutBlock(bool relayoutChildren)
         // 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 availableWidth = availableLogicalWidth();
+        sizingData.setTrackGapsForDirection(ForColumns, guttersSize(ForColumns, 2, availableWidth));
+        computeTrackSizesForDirection(ForColumns, sizingData, availableWidth);
 
         // 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)
+        if (logicalHeightWasIndefinite) {
+            sizingData.setTrackGapsForDirection(ForRows, guttersSize(ForRows, 2, LayoutUnit()));
             computeIntrinsicLogicalHeight(sizingData);
-        else
-            computeTrackSizesForDirection(ForRows, sizingData, availableLogicalHeight(ExcludeMarginBorderPadding));
+        } else {
+            LayoutUnit availableHeight = availableLogicalHeight(ExcludeMarginBorderPadding);

[cbiesinger, 2016/06/22 18:46:13]

Does this code do something reasonable with an unspecified height?

[jfernandez, 2016/06/27 23:56:48]

This branch of code supposes height is "definite", hence specified. 

+            sizingData.setTrackGapsForDirection(ForRows, guttersSize(ForRows, 2, availableHeight));
+            computeTrackSizesForDirection(ForRows, sizingData, availableHeight);
+        }
         setLogicalHeight(computeTrackBasedLogicalHeight(sizingData) + borderAndPaddingLogicalHeight() + scrollbarLogicalHeight());
 
         LayoutUnit oldClientAfterEdge = clientLogicalBottom();
@@ -434,13 +445,13 @@ void LayoutGrid::layoutBlock(bool relayoutChildren)
     clearNeedsLayout();
 }
 
-LayoutUnit LayoutGrid::guttersSize(GridTrackSizingDirection direction, size_t span) const
+LayoutUnit LayoutGrid::guttersSize(GridTrackSizingDirection direction, size_t span, LayoutUnit availableSize) const
 {
     if (span <= 1)
         return LayoutUnit();
 
     const Length& trackGap = direction == ForColumns ? styleRef().gridColumnGap() : styleRef().gridRowGap();
-    return valueForLength(trackGap, LayoutUnit()) * (span - 1);
+    return valueForLength(trackGap, availableSize) * (span - 1);
 }
 
 void LayoutGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const
@@ -450,9 +461,10 @@ void LayoutGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, Layo
     GridSizingData sizingData(gridColumnCount(), gridRowCount());
     sizingData.freeSpaceForDirection(ForColumns) = LayoutUnit();
     sizingData.sizingOperation = IntrinsicSizeComputation;
+    sizingData.setTrackGapsForDirection(ForColumns, guttersSize(ForColumns, 2, LayoutUnit()));
     const_cast<LayoutGrid*>(this)->computeUsedBreadthOfGridTracks(ForColumns, sizingData, minLogicalWidth, maxLogicalWidth);
 
-    LayoutUnit totalGuttersSize = guttersSize(ForColumns, sizingData.columnTracks.size());
+    LayoutUnit totalGuttersSize = sizingData.guttersSize(ForColumns, sizingData.columnTracks.size());
     minLogicalWidth += totalGuttersSize;
     maxLogicalWidth += totalGuttersSize;
 
@@ -466,9 +478,10 @@ void LayoutGrid::computeIntrinsicLogicalHeight(GridSizingData& sizingData)
     ASSERT(tracksAreWiderThanMinTrackBreadth(ForColumns, sizingData));
     sizingData.freeSpaceForDirection(ForRows) = LayoutUnit();
     sizingData.sizingOperation = IntrinsicSizeComputation;
+    sizingData.setTrackGapsForDirection(ForRows, guttersSize(ForRows, 2, LayoutUnit()));
     computeUsedBreadthOfGridTracks(ForRows, sizingData, m_minContentHeight, m_maxContentHeight);
 
-    LayoutUnit totalGuttersSize = guttersSize(ForRows, gridRowCount());
+    LayoutUnit totalGuttersSize = sizingData.guttersSize(ForRows, gridRowCount());
     m_minContentHeight += totalGuttersSize;
     m_maxContentHeight += totalGuttersSize;
 
@@ -505,7 +518,8 @@ void LayoutGrid::computeUsedBreadthOfGridTracks(GridTrackSizingDirection directi
 {
     LayoutUnit& freeSpace = sizingData.freeSpaceForDirection(direction);
     const LayoutUnit initialFreeSpace = freeSpace;
-    Vector<GridTrack>& tracks = (direction == ForColumns) ? sizingData.columnTracks : sizingData.rowTracks;
+    bool isRowAxis = direction == ForColumns;
+    Vector<GridTrack>& tracks = isRowAxis ? sizingData.columnTracks : sizingData.rowTracks;
     Vector<size_t> flexibleSizedTracksIndex;
     sizingData.contentSizedTracksIndex.shrink(0);
 
@@ -513,7 +527,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 += sizingData.guttersSize(direction, isRowAxis ? gridColumnCount() : gridRowCount());
 
     // 1. Initialize per Grid track variables.
     for (size_t i = 0; i < tracks.size(); ++i) {
@@ -1096,7 +1110,8 @@ LayoutUnit LayoutGrid::currentItemSizeForTrackSizeComputationPhase(TrackSizeComp
 template <TrackSizeComputationPhase phase>
 void LayoutGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizingDirection direction, GridSizingData& sizingData, const GridItemsSpanGroupRange& gridItemsWithSpan)
 {
-    Vector<GridTrack>& tracks = (direction == ForColumns) ? sizingData.columnTracks : sizingData.rowTracks;
+    bool isRowAxis = direction == ForColumns;
+    Vector<GridTrack>& tracks = isRowAxis ? sizingData.columnTracks : sizingData.rowTracks;
     for (const auto& trackIndex : sizingData.contentSizedTracksIndex) {
         GridTrack& track = tracks[trackIndex];
         track.setPlannedSize(trackSizeForTrackSizeComputationPhase(phase, track, AllowInfinity));
@@ -1112,7 +1127,7 @@ void LayoutGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizing
         LayoutUnit spanningTracksSize;
         for (const auto& trackPosition : itemSpan) {
             GridTrackSize trackSize = gridTrackSize(direction, trackPosition);
-            GridTrack& track = (direction == ForColumns) ? sizingData.columnTracks[trackPosition] : sizingData.rowTracks[trackPosition];
+            GridTrack& track = isRowAxis ? sizingData.columnTracks[trackPosition] : sizingData.rowTracks[trackPosition];
             spanningTracksSize += trackSizeForTrackSizeComputationPhase(phase, track, ForbidInfinity);
             if (!shouldProcessTrackForTrackSizeComputationPhase(phase, trackSize))
                 continue;
@@ -1126,7 +1141,7 @@ void LayoutGrid::resolveContentBasedTrackSizingFunctionsForItems(GridTrackSizing
         if (sizingData.filteredTracks.isEmpty())
             continue;
 
-        spanningTracksSize += guttersSize(direction, itemSpan.integerSpan());
+        spanningTracksSize += sizingData.guttersSize(direction, itemSpan.integerSpan());
 
         LayoutUnit extraSpace = currentItemSizeForTrackSizeComputationPhase(phase, gridItemWithSpan.gridItem(), direction, sizingData) - spanningTracksSize;
         extraSpace = extraSpace.clampNegativeToZero();
@@ -1289,7 +1304,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 = guttersSize(direction, 2, availableSize);
     tracksSize += gapSize * trackSizes.size();
 
     LayoutUnit freeSpace = availableSize - tracksSize;
@@ -1729,6 +1744,7 @@ void LayoutGrid::offsetAndBreadthForPositionedChild(const LayoutBox& child, Grid
         }
     }
 
+    bool logicalHeightWasIndefinite = computeContentLogicalHeight(MainOrPreferredSize, style()->logicalHeight(), LayoutUnit(-1)) == LayoutUnit(-1);
     LayoutUnit end = isForColumns ? clientLogicalWidth() : clientLogicalHeight();
     if (!endIsAuto) {
         if (isForColumns) {
@@ -1742,7 +1758,7 @@ void LayoutGrid::offsetAndBreadthForPositionedChild(const LayoutBox& child, Grid
 
         // 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);
+            end -= guttersSize(direction, 2, isForColumns ? logicalWidth() : logicalHeightWasIndefinite ? LayoutUnit() : logicalHeight());

[cbiesinger, 2016/06/22 18:46:13]

Again, I think this needs to be contentLogicalHeight or availableLogicalHeight
(and width).

Instead of using the logicalHeightWasIndefinite, why not make a LayoutUnit
availableWidthToUse that stores the result of this check? Nested ternary
expressions are hard to read.

[jfernandez, 2016/06/27 23:56:47]

Done.

             end -= isForColumns ? m_offsetBetweenColumns : m_offsetBetweenRows;
         }
     }
@@ -1759,7 +1775,7 @@ void LayoutGrid::offsetAndBreadthForPositionedChild(const LayoutBox& child, Grid
             offset = translateRTLCoordinate(m_columnPositions[endLine]) - borderLogicalLeft();
 
             if (endLine > 0 && endLine < lastLine) {
-                offset += guttersSize(direction, 2);
+                offset += guttersSize(direction, 2, isForColumns ? logicalWidth() : logicalHeightWasIndefinite ? LayoutUnit() : logicalHeight());
                 offset += isForColumns ? m_offsetBetweenColumns : m_offsetBetweenRows;
             }
         }
@@ -1781,13 +1797,14 @@ GridSpan LayoutGrid::cachedGridSpan(const LayoutBox& gridItem, GridTrackSizingDi
 
 LayoutUnit LayoutGrid::gridAreaBreadthForChild(const LayoutBox& child, GridTrackSizingDirection direction, const GridSizingData& sizingData) const
 {
-    const Vector<GridTrack>& tracks = direction == ForColumns ? sizingData.columnTracks : sizingData.rowTracks;
+    bool isRowAxis = direction == ForColumns;
+    const Vector<GridTrack>& tracks = isRowAxis ? 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());
+    gridAreaBreadth += sizingData.guttersSize(direction, span.integerSpan());
 
     return gridAreaBreadth;
 }
@@ -1820,7 +1837,7 @@ 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);
+    LayoutUnit trackGap = sizingData.trackGapsForDirection(direction);
     auto& positions = isRowAxis ? m_columnPositions : m_rowPositions;
     positions.resize(numberOfLines);
     auto borderAndPadding = isRowAxis ? borderAndPaddingLogicalLeft() : borderAndPaddingBefore();
@@ -2080,7 +2097,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 = sizingData.trackGapsForDirection(ForRows);
         if (childEndLine < m_rowPositions.size() - 1) {
             endOfRow -= trackGap;
             endOfRow -= m_offsetBetweenRows;
@@ -2115,7 +2132,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 = sizingData.trackGapsForDirection(ForColumns);
         if (childEndLine < m_columnPositions.size() - 1) {
             endOfColumn -= trackGap;
             endOfColumn -= m_offsetBetweenColumns;