[CSS Grid Layout] Add 'auto' fallback for non existent named grid lines

Source/core/rendering/style/GridResolvedPosition.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/rendering/style/GridResolvedPosition.h"
 
 #include "core/rendering/RenderBox.h"
 #include "core/rendering/style/GridCoordinate.h"
 
 namespace WebCore {
 
 GridSpan GridResolvedPosition::resolveGridPositionsFromAutoPlacementPosition(const RenderBox&, GridTrackSizingDirection, const GridResolvedPosition& initialPosition)
 {
     // FIXME: We don't support spanning with auto positions yet. Once we do, this is wrong. Also we should make
     // sure the grid can accomodate the new item as we only grow 1 position in a given direction.
     return GridSpan(initialPosition, initialPosition);
 }
 
 static const NamedGridLinesMap& gridLinesForSide(const RenderStyle& style, GridPositionSide side)
 {
     return (side == ColumnStartSide || side == ColumnEndSide) ? style.namedGridColumnLines() : style.namedGridRowLines();
 }
 
-static inline bool isNonExistentNamedLineOrArea(const String& lineName, const RenderStyle& style, GridPositionSide side)
+static inline const String implicitNamedGridLineForSide(const String& lineName, GridPositionSide side)

[Julien - ping for review, 2014/06/11 22:24:14]

Not sure if returning a const gives us much, especially with C++11 rvalue
coming.

 {
-    return !style.namedGridArea().contains(lineName) && !gridLinesForSide(style, side).contains(lineName);
+    return lineName + ((side == ColumnStartSide || side == RowStartSide) ? "-start" : "-end");
+}
+
+static bool isNonExistentNamedLineOrArea(const String& lineName, const RenderStyle& style, GridPositionSide side)

[Julien - ping for review, 2014/06/11 22:24:14]

We should probably just flip the logic as we are doing a double negative
(ASSERT(!isNonExistentNamedLineOrArea())):

ASSERT(isValidNamedLineOrArea())

+{
+    const NamedGridLinesMap& gridLineNames = gridLinesForSide(style, side);
+
+    return !gridLineNames.contains(implicitNamedGridLineForSide(lineName, side)) && !gridLineNames.contains(lineName);
 }
 
 PassOwnPtr<GridSpan> GridResolvedPosition::resolveGridPositionsFromStyle(const RenderStyle& gridContainerStyle, const RenderBox& gridItem, GridTrackSizingDirection direction)
 {
     GridPosition initialPosition = (direction == ForColumns) ? gridItem.style()->gridColumnStart() : gridItem.style()->gridRowStart();
     const GridPositionSide initialPositionSide = (direction == ForColumns) ? ColumnStartSide : RowStartSide;
     GridPosition finalPosition = (direction == ForColumns) ? gridItem.style()->gridColumnEnd() : gridItem.style()->gridRowEnd();
     const GridPositionSide finalPositionSide = (direction == ForColumns) ? ColumnEndSide : RowEndSide;
 
     // We must handle the placement error handling code here instead of in the StyleAdjuster because we don't want to
     // overwrite the specified values.
     if (initialPosition.isSpan() && finalPosition.isSpan())
         finalPosition.setAutoPosition();
 
+    // Try to early detect the case of non existing named grid lines. This way we could assume later that
+    // GridResolvedPosition::resolveGrisPositionFromStyle() always return a valid resolved position.
     if (initialPosition.isNamedGridArea() && isNonExistentNamedLineOrArea(initialPosition.namedGridLine(), gridContainerStyle, initialPositionSide))
         initialPosition.setAutoPosition();
 
     if (finalPosition.isNamedGridArea() && isNonExistentNamedLineOrArea(finalPosition.namedGridLine(), gridContainerStyle, finalPositionSide))
         finalPosition.setAutoPosition();
 
     if (initialPosition.shouldBeResolvedAgainstOppositePosition() && finalPosition.shouldBeResolvedAgainstOppositePosition()) {
         if (gridContainerStyle.gridAutoFlow() == AutoFlowNone)
             return adoptPtr(new GridSpan(0, 0));
 
@@ skipping 80 matching lines @@
             return GridResolvedPosition(0);
 
         return adjustGridPositionForSide(endOfTrack - resolvedPosition, side);
     }
     case NamedGridAreaPosition:
     {
         // First attempt to match the grid area’s edge to a named grid area: if there is a named line with the name
         // ''<custom-ident>-start (for grid-*-start) / <custom-ident>-end'' (for grid-*-end), contributes the first such
         // line to the grid item’s placement.
         String namedGridLine = position.namedGridLine();
-        String implicitNamedGridLine = namedGridLine + ((side == ColumnStartSide || side == RowStartSide) ? "-start" : "-end");
+        ASSERT(!isNonExistentNamedLineOrArea(namedGridLine, gridContainerStyle, side));
+
         const NamedGridLinesMap& gridLineNames = gridLinesForSide(gridContainerStyle, side);
-        NamedGridLinesMap::const_iterator implicitLineIter = gridLineNames.find(implicitNamedGridLine);
+        NamedGridLinesMap::const_iterator implicitLineIter = gridLineNames.find(implicitNamedGridLineForSide(namedGridLine, side));
         if (implicitLineIter != gridLineNames.end())
             return adjustGridPositionForSide(implicitLineIter->value[0], side);
 
         // Otherwise, if there is a named line with the specified name, contributes the first such line to the grid
         // item’s placement.
         NamedGridLinesMap::const_iterator explicitLineIter = gridLineNames.find(namedGridLine);
         if (explicitLineIter != gridLineNames.end())
             return adjustGridPositionForSide(explicitLineIter->value[0], side);
 
-        // FIXME: if none of the above works specs mandate us to treat it as auto. We cannot return auto right here
-        // right now because callers expect a resolved position. We need deeper changes to support this use case.
+        // If none of the above works specs mandate us to treat it as auto BUT we should have detected it before calling
+        // this function in GridResolvedPosition::resolveGridPositionsFromStyle(). We should be also covered by the
+        // ASSERT at the beginning of this block.
+        ASSERT_NOT_REACHED();
         return GridResolvedPosition(0);
     }
     case AutoPosition:
     case SpanPosition:
         // 'auto' and span depend on the opposite position for resolution (e.g. grid-row: auto / 1 or grid-column: span 3 / "myHeader").
         ASSERT_NOT_REACHED();
         return GridResolvedPosition(0);
     }
     ASSERT_NOT_REACHED();
     return GridResolvedPosition(0);
@@ skipping 27 matching lines @@
 
     // If there is no named grid line of that name, we resolve the position to 'auto' (which is equivalent to 'span 1' in this case).
     // See http://lists.w3.org/Archives/Public/www-style/2013Jun/0394.html.
     if (it == gridLinesNames.end())
         return GridSpan::create(resolvedOppositePosition, resolvedOppositePosition);
 
     return GridSpan::createWithNamedSpanAgainstOpposite(resolvedOppositePosition, position, side, it->value);
 }
 
 } // namespace WebCore