[CSS Grid Layout] Named grid line span not allowed for automatic positions

From the grid layout spec we can extract the following sentence:
"If the grid item has an automatic position and a grid span for a named
line in a given dimension, instead treat the grid span as one."

The main reason is that as you don't know where the item is going to be
placed (because of it's auto-positioned), spanning to a particular line
makes no sense.

Updated GridResolvedPosition code to include this restriction when
resolving positions.

Adapted auto-placement algorithm implementation in RenderGrid as now
it's possible to pass row and column spans to
GridIterator::nextEmptyGridArea() simplifying the code.

Updated layout test to the new behavior.

BUG=353789
TEST=fast/css-grid-layout/grid-item-auto-placement-automatic-span.html

Review URL: https://codereview.chromium.org/320093002

git-svn-id: svn://svn.chromium.org/blink/trunk@176295 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-item-auto-placement-automatic-span.html

@@ -47,26 +47,26 @@
 
 .autoRowAutoColumnSpanningLine {
     background-color: indigo;
-    grid-column: span line;
+    grid-column: span line; /* This is treated as "span 1". */
     grid-row: auto;
 }
 
 .autoRowSpanningLineAutoColumn {
     background-color: moccasin;
     grid-column: auto;
-    grid-row: span line;
+    grid-row: span line; /* This is treated as "span 1". */
 }
 
 .autoRowAutoColumnSpanning2Line {
     background-color: sienna;
-    grid-column: span 2 line;
+    grid-column: span 2 line; /* This is treated as "span 1". */
     grid-row: auto;
 }
 
 .autoRowSpanning2LineAutoColumn {
     background-color: tomato;
     grid-column: auto;
-    grid-row: span 2 line;
+    grid-row: span 2 line; /* This is treated as "span 1". */
 }
 </style>
 <script src="../../resources/check-layout.js"></script>
@@ -121,18 +121,17 @@
 <div style="position: relative">
     <div class="grid gridAutoFlowRow gridNamedGridLinesColumns">
         <div class="sizedToGridArea autoRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowAutoColumnSpanning2Line" data-offset-x="50" data-offset-y="0" data-expected-width="300" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowAutoColumnSpanning2Line" data-offset-x="50" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
         <div class="sizedToGridArea autoRowSecondColumn" data-offset-x="50" data-offset-y="50" data-expected-width="100" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowAutoColumnSpanning2Line" data-offset-x="150" data-offset-y="50" data-expected-width="200" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowAutoColumnSpanning2Line" data-offset-x="150" data-offset-y="0" data-expected-width="200" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowThirdColumn" data-offset-x="150" data-offset-y="50" data-expected-width="200" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowAutoColumnSpanningLine" data-offset-x="0" data-offset-y="50" data-expected-width="50" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowAutoColumnSpanning2Line" data-offset-x="0" data-offset-y="100" data-expected-width="50" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowFirstColumn" data-offset-x="0" data-offset-y="150" data-expected-width="50" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowAutoColumnSpanningLine" data-offset-x="50" data-offset-y="100" data-expected-width="100" data-expected-height="50"></div>
         <div class="sizedToGridArea autoRowThirdColumn" data-offset-x="150" data-offset-y="100" data-expected-width="200" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowAutoColumnSpanningLine" data-offset-x="0" data-offset-y="100" data-expected-width="150" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowAutoColumnSpanning2Line" data-offset-x="0" data-offset-y="150" data-expected-width="350" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowFirstColumn" data-offset-x="0" data-offset-y="50" data-expected-width="50" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="50" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowAutoColumnSpanningLine" data-offset-x="50" data-offset-y="200" data-expected-width="100" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowThirdColumn" data-offset-x="150" data-offset-y="200" data-expected-width="200" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowAutoColumnSpanningLine" data-offset-x="0" data-offset-y="250" data-expected-width="150" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowAutoColumnSpanningLine" data-offset-x="150" data-offset-y="250" data-expected-width="200" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowAutoColumnSpanningLine" data-offset-x="50" data-offset-y="150" data-expected-width="100" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowAutoColumnSpanningLine" data-offset-x="150" data-offset-y="150" data-expected-width="200" data-expected-height="50"></div>
     </div>
 </div>
 
@@ -183,18 +182,17 @@
 <div style="position: relative">
     <div class="grid gridAutoFlowColumn gridNamedGridLinesRows">
         <div class="sizedToGridArea firstRowAutoColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowSpanning2LineAutoColumn" data-offset-x="0" data-offset-y="50" data-expected-width="100" data-expected-height="300"></div>
+        <div class="sizedToGridArea autoRowSpanning2LineAutoColumn" data-offset-x="0" data-offset-y="50" data-expected-width="100" data-expected-height="100"></div>
         <div class="sizedToGridArea secondRowAutoColumn" data-offset-x="100" data-offset-y="50" data-expected-width="100" data-expected-height="100"></div>
-        <div class="sizedToGridArea autoRowSpanning2LineAutoColumn" data-offset-x="100" data-offset-y="150" data-expected-width="100" data-expected-height="200"></div>
+        <div class="sizedToGridArea autoRowSpanning2LineAutoColumn" data-offset-x="0" data-offset-y="150" data-expected-width="100" data-expected-height="200"></div>
+        <div class="sizedToGridArea thirdRowAutoColumn" data-offset-x="100" data-offset-y="150" data-expected-width="100" data-expected-height="200"></div>
+        <div class="sizedToGridArea autoRowSpanningLineAutoColumn" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowSpanning2LineAutoColumn" data-offset-x="200" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+        <div class="sizedToGridArea firstRowAutoColumn" data-offset-x="300" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+        <div class="sizedToGridArea autoRowSpanningLineAutoColumn" data-offset-x="200" data-offset-y="50" data-expected-width="100" data-expected-height="100"></div>
         <div class="sizedToGridArea thirdRowAutoColumn" data-offset-x="200" data-offset-y="150" data-expected-width="100" data-expected-height="200"></div>
-        <div class="sizedToGridArea autoRowSpanningLineAutoColumn" data-offset-x="200" data-offset-y="0" data-expected-width="100" data-expected-height="150"></div>
-        <div class="sizedToGridArea autoRowSpanning2LineAutoColumn" data-offset-x="300" data-offset-y="0" data-expected-width="100" data-expected-height="350"></div>
-        <div class="sizedToGridArea firstRowAutoColumn" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
-        <div class="sizedToGridArea firstRowAutoColumn" data-offset-x="400" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
-        <div class="sizedToGridArea autoRowSpanningLineAutoColumn" data-offset-x="400" data-offset-y="50" data-expected-width="100" data-expected-height="100"></div>
-        <div class="sizedToGridArea thirdRowAutoColumn" data-offset-x="400" data-offset-y="150" data-expected-width="100" data-expected-height="200"></div>
-        <div class="sizedToGridArea autoRowSpanningLineAutoColumn" data-offset-x="500" data-offset-y="0" data-expected-width="100" data-expected-height="150"></div>
-        <div class="sizedToGridArea autoRowSpanningLineAutoColumn" data-offset-x="500" data-offset-y="150" data-expected-width="100" data-expected-height="200"></div>
+        <div class="sizedToGridArea autoRowSpanningLineAutoColumn" data-offset-x="300" data-offset-y="50" data-expected-width="100" data-expected-height="100"></div>
+        <div class="sizedToGridArea autoRowSpanningLineAutoColumn" data-offset-x="300" data-offset-y="150" data-expected-width="100" data-expected-height="200"></div-->
     </div>
 </div>
 

third_party/WebKit/Source/core/rendering/RenderGrid.cpp

@@ -122,16 +122,37 @@ public:
         return 0;
     }
 
-    PassOwnPtr<GridCoordinate> nextEmptyGridArea()
+    bool checkEmptyCells(size_t rowSpan, size_t columnSpan) const
+    {
+        // Ignore cells outside current grid as we will grow it later if needed.
+        size_t maxRows = std::min(m_rowIndex + rowSpan, m_grid.size());
+        size_t maxColumns = std::min(m_columnIndex + columnSpan, m_grid[0].size());
+
+        // This adds a O(N^2) behavior that shouldn't be a big deal as we expect spanning areas to be small.
+        for (size_t row = m_rowIndex; row < maxRows; ++row) {
+            for (size_t column = m_columnIndex; column < maxColumns; ++column) {
+                const GridCell& children = m_grid[row][column];
+                if (!children.isEmpty())
+                    return false;
+            }
+        }
+
+        return true;
+    }
+
+    PassOwnPtr<GridCoordinate> nextEmptyGridArea(size_t fixedTrackSpan, size_t varyingTrackSpan)
     {
         ASSERT(!m_grid.isEmpty());
+        ASSERT(fixedTrackSpan >= 1 && varyingTrackSpan >= 1);
+
+        size_t rowSpan = (m_direction == ForColumns) ? varyingTrackSpan : fixedTrackSpan;
+        size_t columnSpan = (m_direction == ForColumns) ? fixedTrackSpan : varyingTrackSpan;
 
         size_t& varyingTrackIndex = (m_direction == ForColumns) ? m_rowIndex : m_columnIndex;
         const size_t endOfVaryingTrackIndex = (m_direction == ForColumns) ? m_grid.size() : m_grid[0].size();
         for (; varyingTrackIndex < endOfVaryingTrackIndex; ++varyingTrackIndex) {
-            const GridCell& children = m_grid[m_rowIndex][m_columnIndex];
-            if (children.isEmpty()) {
-                OwnPtr<GridCoordinate> result = adoptPtr(new GridCoordinate(GridSpan(m_rowIndex, m_rowIndex), GridSpan(m_columnIndex, m_columnIndex)));
+            if (checkEmptyCells(rowSpan, columnSpan)) {
+                OwnPtr<GridCoordinate> result = adoptPtr(new GridCoordinate(GridSpan(m_rowIndex, m_rowIndex + rowSpan - 1), GridSpan(m_columnIndex, m_columnIndex + columnSpan - 1)));
                 // Advance the iterator to avoid an infinite loop where we would return the same grid area over and over.
                 ++varyingTrackIndex;
                 return result.release();
@@ -846,24 +867,6 @@ void RenderGrid::populateExplicitGridAndOrderIterator()
         m_grid[i].grow(maximumColumnIndex);
 }
 
-bool RenderGrid::checkEmptyCells(const GridCoordinate& coordinate) const
-{
-    // Ignore cells outside current grid as we will grow it later if needed.
-    size_t maxRows = std::min(coordinate.rows.resolvedFinalPosition.next().toInt(), gridRowCount());
-    size_t maxColumns = std::min(coordinate.columns.resolvedFinalPosition.next().toInt(), gridColumnCount());
-
-    // This adds a O(N^2) behavior that shouldn't be a big deal as we expect spanning areas to be small.
-    for (size_t row = coordinate.rows.resolvedInitialPosition.toInt(); row < maxRows; ++row) {
-        for (size_t column = coordinate.columns.resolvedInitialPosition.toInt(); column < maxColumns; ++column) {
-            const GridCell& children = m_grid[row][column];
-            if (!children.isEmpty())
-                return false;
-        }
-    }
-
-    return true;
-}
-
 PassOwnPtr<GridCoordinate> RenderGrid::createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(const RenderBox* gridItem, GridTrackSizingDirection specifiedDirection, const GridSpan& specifiedPositions) const
 {
     GridTrackSizingDirection crossDirection = specifiedDirection == ForColumns ? ForRows : ForColumns;
@@ -872,30 +875,14 @@ PassOwnPtr<GridCoordinate> RenderGrid::createEmptyGridAreaAtSpecifiedPositionsOu
     return adoptPtr(new GridCoordinate(specifiedDirection == ForColumns ? crossDirectionPositions : specifiedPositions, specifiedDirection == ForColumns ? specifiedPositions : crossDirectionPositions));
 }
 
-PassOwnPtr<GridCoordinate> RenderGrid::findEmptyGridAreaAtSpecifiedPositionsInsideGrid(const RenderBox* gridItem, GridTrackSizingDirection specifiedDirection, const GridSpan& specifiedPositions) const
-{
-    GridTrackSizingDirection crossDirection = specifiedDirection == ForColumns ? ForRows : ForColumns;
-
-    GridIterator iterator(m_grid, specifiedDirection, specifiedPositions.resolvedInitialPosition.toInt());
-    OwnPtr<GridCoordinate> emptyGridArea;
-    for (emptyGridArea = iterator.nextEmptyGridArea(); emptyGridArea; emptyGridArea = iterator.nextEmptyGridArea()) {
-        GridResolvedPosition crossDirectionInitialPositionIndex = crossDirection == ForColumns ? emptyGridArea->columns.resolvedInitialPosition : emptyGridArea->rows.resolvedInitialPosition;
-        GridSpan crossDirectionPositions = GridResolvedPosition::resolveGridPositionsFromAutoPlacementPosition(*style(), *gridItem, crossDirection, crossDirectionInitialPositionIndex);
-
-        emptyGridArea->rows = specifiedDirection == ForColumns ? crossDirectionPositions : specifiedPositions;
-        emptyGridArea->columns = specifiedDirection == ForColumns ? specifiedPositions : crossDirectionPositions;
-        if (checkEmptyCells(*emptyGridArea))
-            break;
-    }
-
-    return emptyGridArea.release();
-}
-
 void RenderGrid::placeSpecifiedMajorAxisItemsOnGrid(const Vector<RenderBox*>& autoGridItems)
 {
     for (size_t i = 0; i < autoGridItems.size(); ++i) {
         OwnPtr<GridSpan> majorAxisPositions = GridResolvedPosition::resolveGridPositionsFromStyle(*style(), *autoGridItems[i], autoPlacementMajorAxisDirection());
-        OwnPtr<GridCoordinate> emptyGridArea = findEmptyGridAreaAtSpecifiedPositionsInsideGrid(autoGridItems[i], autoPlacementMajorAxisDirection(), *majorAxisPositions);
+        GridSpan minorAxisPositions = GridResolvedPosition::resolveGridPositionsFromAutoPlacementPosition(*style(), *autoGridItems[i], autoPlacementMinorAxisDirection(), GridResolvedPosition(0));
+
+        GridIterator iterator(m_grid, autoPlacementMajorAxisDirection(), majorAxisPositions->resolvedInitialPosition.toInt());
+        OwnPtr<GridCoordinate> emptyGridArea = iterator.nextEmptyGridArea(majorAxisPositions->integerSpan(), minorAxisPositions.integerSpan());
         if (!emptyGridArea)
             emptyGridArea = createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(autoGridItems[i], autoPlacementMajorAxisDirection(), *majorAxisPositions);
         insertItemIntoGrid(autoGridItems[i], *emptyGridArea);
@@ -912,18 +899,20 @@ void RenderGrid::placeAutoMajorAxisItemOnGrid(RenderBox* gridItem)
 {
     OwnPtr<GridSpan> minorAxisPositions = GridResolvedPosition::resolveGridPositionsFromStyle(*style(), *gridItem, autoPlacementMinorAxisDirection());
     ASSERT(!GridResolvedPosition::resolveGridPositionsFromStyle(*style(), *gridItem, autoPlacementMajorAxisDirection()));
+    GridSpan majorAxisPositions = GridResolvedPosition::resolveGridPositionsFromAutoPlacementPosition(*style(), *gridItem, autoPlacementMajorAxisDirection(), GridResolvedPosition(0));
     OwnPtr<GridCoordinate> emptyGridArea;
     if (minorAxisPositions) {
-        emptyGridArea = findEmptyGridAreaAtSpecifiedPositionsInsideGrid(gridItem, autoPlacementMinorAxisDirection(), *minorAxisPositions);
+        GridIterator iterator(m_grid, autoPlacementMinorAxisDirection(), minorAxisPositions->resolvedInitialPosition.toInt());
+        emptyGridArea = iterator.nextEmptyGridArea(minorAxisPositions->integerSpan(), majorAxisPositions.integerSpan());
         if (!emptyGridArea)
             emptyGridArea = createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(gridItem, autoPlacementMinorAxisDirection(), *minorAxisPositions);
     } else {
+        GridSpan minorAxisPositions = GridResolvedPosition::resolveGridPositionsFromAutoPlacementPosition(*style(), *gridItem, autoPlacementMinorAxisDirection(), GridResolvedPosition(0));
+
         const size_t endOfMajorAxis = (autoPlacementMajorAxisDirection() == ForColumns) ? gridColumnCount() : gridRowCount();
         for (size_t majorAxisIndex = 0; majorAxisIndex < endOfMajorAxis; ++majorAxisIndex) {
-            // We need to resolve the position for every different index as the number of cells might vary depending on it.
-            // This will happen when we have "span <custom-indent>", which has a different resolution based on the position.
-            GridSpan majorAxisPositions = GridResolvedPosition::resolveGridPositionsFromAutoPlacementPosition(*style(), *gridItem, autoPlacementMajorAxisDirection(), majorAxisIndex);
-            emptyGridArea = findEmptyGridAreaAtSpecifiedPositionsInsideGrid(gridItem, autoPlacementMajorAxisDirection(), majorAxisPositions);
+            GridIterator iterator(m_grid, autoPlacementMajorAxisDirection(), majorAxisIndex);
+            emptyGridArea = iterator.nextEmptyGridArea(majorAxisPositions.integerSpan(), minorAxisPositions.integerSpan());
 
             if (emptyGridArea) {
                 // Check that it fits in the minor axis direction, as we shouldn't grow in that direction here (it was already managed in populateExplicitGridAndOrderIterator()).
@@ -938,10 +927,8 @@ void RenderGrid::placeAutoMajorAxisItemOnGrid(RenderBox* gridItem)
             }
         }
 
-        if (!emptyGridArea) {
-            GridSpan minorAxisPositions = GridResolvedPosition::resolveGridPositionsFromAutoPlacementPosition(*style(), *gridItem, autoPlacementMinorAxisDirection(), GridResolvedPosition(0));
+        if (!emptyGridArea)
             emptyGridArea = createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(gridItem, autoPlacementMinorAxisDirection(), minorAxisPositions);
-        }
     }
 
     insertItemIntoGrid(gridItem, *emptyGridArea);

third_party/WebKit/Source/core/rendering/RenderGrid.h

@@ -81,9 +81,7 @@ private:
     void insertItemIntoGrid(RenderBox*, const GridCoordinate&);
     void placeItemsOnGrid();
     void populateExplicitGridAndOrderIterator();
-    bool checkEmptyCells(const GridCoordinate&) const;
     PassOwnPtr<GridCoordinate> createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(const RenderBox*, GridTrackSizingDirection, const GridSpan& specifiedPositions) const;
-    PassOwnPtr<GridCoordinate> findEmptyGridAreaAtSpecifiedPositionsInsideGrid(const RenderBox*, GridTrackSizingDirection, const GridSpan& specifiedPositions) const;
     void placeSpecifiedMajorAxisItemsOnGrid(const Vector<RenderBox*>&);
     void placeAutoMajorAxisItemsOnGrid(const Vector<RenderBox*>&);
     void placeAutoMajorAxisItemOnGrid(RenderBox*);

third_party/WebKit/Source/core/rendering/style/GridCoordinate.h

@@ -114,6 +114,11 @@ struct GridSpan {
         return resolvedInitialPosition == o.resolvedInitialPosition && resolvedFinalPosition == o.resolvedFinalPosition;
     }
 
+    size_t integerSpan() const
+    {
+        return resolvedFinalPosition.toInt() - resolvedInitialPosition.toInt() + 1;
+    }
+
     GridResolvedPosition resolvedInitialPosition;
     GridResolvedPosition resolvedFinalPosition;
 

third_party/WebKit/Source/core/rendering/style/GridResolvedPosition.cpp

@@ -56,6 +56,12 @@ void GridResolvedPosition::initialAndFinalPositionsFromStyle(const RenderStyle&
 
     if (finalPosition.isNamedGridArea() && !isValidNamedLineOrArea(finalPosition.namedGridLine(), gridContainerStyle, finalPositionSide))
         finalPosition.setAutoPosition();
+
+    // If the grid item has an automatic position and a grid span for a named line in a given dimension, instead treat the grid span as one.
+    if (initialPosition.isAuto() && finalPosition.isSpan() && !finalPosition.namedGridLine().isNull())
+        finalPosition.setSpanPosition(1, String());
+    if (finalPosition.isAuto() && initialPosition.isSpan() && !initialPosition.namedGridLine().isNull())
+        initialPosition.setSpanPosition(1, String());
 }
 
 GridSpan GridResolvedPosition::resolveGridPositionsFromAutoPlacementPosition(const RenderStyle& gridContainerStyle, const RenderBox& gridItem, GridTrackSizingDirection direction, const GridResolvedPosition& resolvedInitialPosition)