Chromium Code Reviews| Index: Source/core/rendering/style/GridResolvedPosition.cpp |
| diff --git a/Source/core/rendering/style/GridResolvedPosition.cpp b/Source/core/rendering/style/GridResolvedPosition.cpp |
| index 263ed15edfecf8063bd81eafc851cd2b5bdbf859..a3fef45b711452921dfa8b8b9114a8aca56daf77 100644 |
| --- a/Source/core/rendering/style/GridResolvedPosition.cpp |
| +++ b/Source/core/rendering/style/GridResolvedPosition.cpp |
| @@ -22,9 +22,16 @@ static const NamedGridLinesMap& gridLinesForSide(const RenderStyle& style, GridP |
| 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, espec
|
| { |
| - 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 d
|
| +{ |
| + 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) |
| @@ -39,6 +46,8 @@ PassOwnPtr<GridSpan> GridResolvedPosition::resolveGridPositionsFromStyle(const R |
| 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(); |
| @@ -139,9 +148,10 @@ GridResolvedPosition GridResolvedPosition::resolveGridPositionFromStyle(const Re |
| // ''<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); |
| @@ -151,8 +161,10 @@ GridResolvedPosition GridResolvedPosition::resolveGridPositionFromStyle(const Re |
| 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: |