[CSS Grid Layout] Implementation of the grid-template shorthand.

Source/core/css/parser/BisonCSSParser-in.cpp

Index: Source/core/css/parser/BisonCSSParser-in.cpp
diff --git a/Source/core/css/parser/BisonCSSParser-in.cpp b/Source/core/css/parser/BisonCSSParser-in.cpp
index 6376586c1e11b19d6cbf1c9c0d74d64d47b98c9c..cb6b8796d6ee5f8b6e2a284d64d7b81fa6467b59 100644
--- a/Source/core/css/parser/BisonCSSParser-in.cpp
+++ b/Source/core/css/parser/BisonCSSParser-in.cpp
@@ -2384,7 +2384,8 @@ bool BisonCSSParser::parseValue(CSSPropertyID propId, bool important)
     case CSSPropertyGridTemplateRows:
         if (!RuntimeEnabledFeatures::cssGridLayoutEnabled())
             return false;
-        return parseGridTrackList(propId, important);
+        parsedValue = parseGridTrackList(important);
+        break;
 
     case CSSPropertyGridColumnEnd:
     case CSSPropertyGridColumnStart:
@@ -2412,6 +2413,11 @@ bool BisonCSSParser::parseValue(CSSPropertyID propId, bool important)
         parsedValue = parseGridTemplateAreas();
         break;
 
+    case CSSPropertyGridTemplate:
+        if (!RuntimeEnabledFeatures::cssGridLayoutEnabled())
+            return false;
+        return parseGridTemplateShorthand(important);
+
     case CSSPropertyWebkitMarginCollapse: {
         if (num == 1) {
             ShorthandScope scope(this, CSSPropertyWebkitMarginCollapse);
@@ -4600,6 +4606,109 @@ bool BisonCSSParser::parseGridItemPositionShorthand(CSSPropertyID shorthandId, b
     return true;
 }
 
+
+bool BisonCSSParser::parseGridTemplateRowsAndAreas(bool important)
+{
+    NamedGridAreaMap gridAreaMap;
+    size_t rowCount = 0;
+    size_t columnCount = 0;
+    bool traillingIdentAdded = false;
+    RefPtr<CSSValueList> templateRows = CSSValueList::createSpaceSeparated();
+
+    // At least template-areas strings must be defined.
+    if (!m_valueList->current())
+        return false;
+
+    while (m_valueList->current()) {
+        // Handle leading <custom-ident>*.
+        if (m_valueList->current()->unit == CSSParserValue::ValueList) {
+            if (traillingIdentAdded)
+                parseGridLineNames(m_valueList.get(), *templateRows, static_cast<CSSGridLineNamesValue*>(templateRows->item(templateRows->length() - 1)));
+            else
+                parseGridLineNames(m_valueList.get(), *templateRows);
+        }
+
+        // Handle a template-area's row.
+        if (!parseGridTemplateAreasRow(gridAreaMap, rowCount, columnCount))
+            return false;
+        ++rowCount;
+
+        // Handle template-rows's track-size.
+        if (m_valueList->current() && m_valueList->current()->unit != CSSParserValue::ValueList && m_valueList->current()->unit != CSSPrimitiveValue::CSS_STRING) {
+            RefPtr<CSSValue> value = parseGridTrackSize(*m_valueList);
+            if (!value)
+                return false;
+            templateRows->append(value);
+        } else {
+            templateRows->append(cssValuePool().createIdentifierValue(CSSValueAuto));
+        }
+
+        // This will handle the trailing/leading <custom-ident>* in the grammar.
+        traillingIdentAdded = false;
+        if (m_valueList->current() && m_valueList->current()->unit == CSSParserValue::ValueList) {
+            parseGridLineNames(m_valueList.get(), *templateRows);
+            traillingIdentAdded = true;
+        }
+    }
+
+    RefPtr<CSSValue> templateAreas = CSSGridTemplateAreasValue::create(gridAreaMap, rowCount, columnCount);
+    addProperty(CSSPropertyGridTemplateAreas, templateAreas.release(), important);
+    addProperty(CSSPropertyGridTemplateRows, templateRows.release(), important);

[Julien - ping for review, 2014/02/26 21:53:38]

This is confusing as we set CSSPropertyGridTemplateColumns in another function.
I would rather unify the settings so that it's obvious that we always set all of
the values (even to empty values).

[jfernandez, 2014/02/27 11:18:15]

Done.

+
+    return true;
+}
+
+
+bool BisonCSSParser::parseGridTemplateShorthand(bool important)
+{
+    ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
+
+    ShorthandScope scope(this, CSSPropertyGridTemplate);
+    ASSERT(gridTemplateShorthand().length() == 3);
+    unsigned index = 0;
+
+    if (!m_valueList->current())
+        return false;
+
+    // none or <grid-template-columns> or [<track-list> / ] ?
+    if (parseValue(CSSPropertyGridTemplateColumns, important)) {

[Julien - ping for review, 2014/02/26 21:53:38]

Based on the comment below about not handling the slash, I wouldn't recurse into
parseValue but just call parseGridTrackList from here.

The fact of calling recursively parseValue matches other shorthands more closely
but here it is confusing as it magically sets CSSPropertyGridTemplateColumns,
which makes it non obvious if we missed setting it below.

[jfernandez, 2014/02/27 11:18:15]

Done.

+        if (!m_valueList->current()) {
+            if (m_valueList->previous()->id != CSSValueNone)

[Julien - ping for review, 2014/02/26 21:53:38]

This check seems unneeded as the grid-template-columns code should take care of
it.

[jfernandez, 2014/02/27 11:18:15]

After the change related to the parserValue recursion this code would be
slightly different, but I think it's still needed. It covers the case of
defining "grid-template: none", which is a valid syntax for parseGridTrackList,
but at the same time we have to be sure we don't allow "grid-template: 10px",
which is also valid for parseGridTrackList.

+                return false;
+            addProperty(CSSPropertyGridTemplateRows, cssValuePool().createIdentifierValue(CSSValueNone), important);
+            addProperty(CSSPropertyGridTemplateAreas, cssValuePool().createIdentifierValue(CSSValueNone), important);
+            return true;
+        }
+        if (!isForwardSlashOperator(m_valueList->current()))
+            return false;
+        if (!m_valueList->next())
+            return false;
+        // <grid-template-rows>
+        index = m_valueList->currentIndex();
+        if (parseValue(CSSPropertyGridTemplateRows, important)) {
+            addProperty(CSSPropertyGridTemplateAreas, cssValuePool().createIdentifierValue(CSSValueNone), important);
+            if (m_valueList->next())
+                return false;
+            return true;
+        }
+    } else {
+        index = 0;
+        addProperty(CSSPropertyGridTemplateColumns, cssValuePool().createIdentifierValue(CSSValueNone), important);
+    }
+
+    // Need to rewind parsing to explore the complex syntax of this shorthand.
+    while (m_valueList->currentIndex() > index)
+        m_valueList->previous();

[Julien - ping for review, 2014/02/26 21:53:38]

This is fairly inefficient, I would rather add the right primitive to rewind.

+
+    // [ <line-names>? <string> [ <track-size> <line-names> ]? ]+
+    if (!parseGridTemplateRowsAndAreas(important))

[Julien - ping for review, 2014/02/26 21:53:38]

return parseGridTemplateRowsAndAreas(important);

[jfernandez, 2014/02/27 11:18:15]

Done.

+        return false;
+
+
+    return true;
+}
+
+
 bool BisonCSSParser::parseGridAreaShorthand(bool important)
 {
     ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
@@ -4655,7 +4764,7 @@ bool BisonCSSParser::parseSingleGridAreaLonghand(RefPtr<CSSValue>& property)
     return true;
 }
 
-void BisonCSSParser::parseGridLineNames(CSSParserValueList* parserValueList, CSSValueList& valueList)
+void BisonCSSParser::parseGridLineNames(CSSParserValueList* parserValueList, CSSValueList& valueList, CSSGridLineNamesValue* lineNamesToConcat)
 {
     ASSERT(parserValueList->current() && parserValueList->current()->unit == CSSParserValue::ValueList);
 
@@ -4665,29 +4774,33 @@ void BisonCSSParser::parseGridLineNames(CSSParserValueList* parserValueList, CSS
         return;
     }
 
-    RefPtrWillBeRawPtr<CSSGridLineNamesValue> lineNames = CSSGridLineNamesValue::create();
+    // Need to ensure the identList is at the heading index, since the parserList might have been rewound.
+    while (identList->currentIndex() > 0)
+        identList->previous();
+
+    RefPtr<CSSGridLineNamesValue> lineNames = lineNamesToConcat ? lineNamesToConcat : CSSGridLineNamesValue::create();
     while (CSSParserValue* identValue = identList->current()) {
         ASSERT(identValue->unit == CSSPrimitiveValue::CSS_IDENT);
         RefPtrWillBeRawPtr<CSSPrimitiveValue> lineName = createPrimitiveStringValue(identValue);
         lineNames->append(lineName.release());
         identList->next();
     }
-    valueList.append(lineNames.release());
+    if (!lineNamesToConcat)
+        valueList.append(lineNames.release());
 
     parserValueList->next();
 }
 
-bool BisonCSSParser::parseGridTrackList(CSSPropertyID propId, bool important)
+PassRefPtr<CSSValue> BisonCSSParser::parseGridTrackList(bool important)
 {
     ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
 
     CSSParserValue* value = m_valueList->current();
     if (value->id == CSSValueNone) {
-        if (m_valueList->next())
-            return false;
+        if (m_valueList->next() && (!isForwardSlashOperator(m_valueList->current())))

[Julien - ping for review, 2014/02/26 21:53:38]

I would remove this check and push it to the callers. The main parsing function
(CSSPropertyParser::parseValue) will already fail if there is a following item
in m_valueList.

[jfernandez, 2014/02/27 11:18:15]

Done.

+            return 0;
 
-        addProperty(propId, cssValuePool().createIdentifierValue(value->id), important);
-        return true;
+        return cssValuePool().createIdentifierValue(value->id);
     }
 
     RefPtrWillBeRawPtr<CSSValueList> values = CSSValueList::createSpaceSeparated();
@@ -4698,14 +4811,16 @@ bool BisonCSSParser::parseGridTrackList(CSSPropertyID propId, bool important)
 
     bool seenTrackSizeOrRepeatFunction = false;
     while (CSSParserValue* currentValue = m_valueList->current()) {
+        if (isForwardSlashOperator(currentValue))
+            break;
         if (currentValue->unit == CSSParserValue::Function && equalIgnoringCase(currentValue->function->name, "repeat(")) {
             if (!parseGridTrackRepeatFunction(*values))
-                return false;
+                return 0;
             seenTrackSizeOrRepeatFunction = true;
         } else {
             RefPtr<CSSValue> value = parseGridTrackSize(*m_valueList);
             if (!value)
-                return false;
+                return 0;
             values->append(value);
             seenTrackSizeOrRepeatFunction = true;
         }
@@ -4717,10 +4832,9 @@ bool BisonCSSParser::parseGridTrackList(CSSPropertyID propId, bool important)
 
     // We should have found a <track-size> or else it is not a valid <track-list>
     if (!seenTrackSizeOrRepeatFunction)
-        return false;
+        return 0;
 
-    addProperty(propId, values.release(), important);
-    return true;
+    return values;
 }
 
 bool BisonCSSParser::parseGridTrackRepeatFunction(CSSValueList& list)
@@ -4817,71 +4931,79 @@ PassRefPtrWillBeRawPtr<CSSPrimitiveValue> BisonCSSParser::parseGridBreadth(CSSPa
     return createPrimitiveNumericValue(currentValue);
 }
 
-PassRefPtr<CSSValue> BisonCSSParser::parseGridTemplateAreas()
+bool BisonCSSParser::parseGridTemplateAreasRow(NamedGridAreaMap& gridAreaMap, const size_t rowCount, size_t& columnCount)
 {
-    NamedGridAreaMap gridAreaMap;
-    size_t rowCount = 0;
-    size_t columnCount = 0;
-
-    while (CSSParserValue* currentValue = m_valueList->current()) {
-        if (currentValue->unit != CSSPrimitiveValue::CSS_STRING)
-            return 0;
+    CSSParserValue* currentValue = m_valueList->current();
+    if (!currentValue || currentValue->unit != CSSPrimitiveValue::CSS_STRING)
+        return false;
 
-        String gridRowNames = currentValue->string;
-        if (!gridRowNames.length())
-            return 0;
+    String gridRowNames = currentValue->string;
+    if (!gridRowNames.length())
+        return false;
 
-        Vector<String> columnNames;
-        gridRowNames.split(' ', columnNames);
+    Vector<String> columnNames;
+    gridRowNames.split(' ', columnNames);
 
-        if (!columnCount) {
-            columnCount = columnNames.size();
-            ASSERT(columnCount);
-        } else if (columnCount != columnNames.size()) {
-            // The declaration is invalid is all the rows don't have the number of columns.
-            return 0;
-        }
+    if (!columnCount) {
+        columnCount = columnNames.size();
+        ASSERT(columnCount);
+    } else if (columnCount != columnNames.size()) {
+        // The declaration is invalid is all the rows don't have the number of columns.
+        return false;
+    }
 
-        for (size_t currentCol = 0; currentCol < columnCount; ++currentCol) {
-            const String& gridAreaName = columnNames[currentCol];
+    for (size_t currentCol = 0; currentCol < columnCount; ++currentCol) {
+        const String& gridAreaName = columnNames[currentCol];
 
-            // Unamed areas are always valid (we consider them to be 1x1).
-            if (gridAreaName == ".")
-                continue;
+        // Unamed areas are always valid (we consider them to be 1x1).
+        if (gridAreaName == ".")
+            continue;
 
-            // We handle several grid areas with the same name at once to simplify the validation code.
-            size_t lookAheadCol;
-            for (lookAheadCol = currentCol; lookAheadCol < (columnCount - 1); ++lookAheadCol) {
-                if (columnNames[lookAheadCol + 1] != gridAreaName)
-                    break;
-            }
+        // We handle several grid areas with the same name at once to simplify the validation code.
+        size_t lookAheadCol;
+        for (lookAheadCol = currentCol; lookAheadCol < (columnCount - 1); ++lookAheadCol) {
+            if (columnNames[lookAheadCol + 1] != gridAreaName)
+                break;
+        }
 
-            NamedGridAreaMap::iterator gridAreaIt = gridAreaMap.find(gridAreaName);
-            if (gridAreaIt == gridAreaMap.end()) {
-                gridAreaMap.add(gridAreaName, GridCoordinate(GridSpan(rowCount, rowCount), GridSpan(currentCol, lookAheadCol)));
-            } else {
-                GridCoordinate& gridCoordinate = gridAreaIt->value;
+        NamedGridAreaMap::iterator gridAreaIt = gridAreaMap.find(gridAreaName);
+        if (gridAreaIt == gridAreaMap.end()) {
+            gridAreaMap.add(gridAreaName, GridCoordinate(GridSpan(rowCount, rowCount), GridSpan(currentCol, lookAheadCol)));
+        } else {
+            GridCoordinate& gridCoordinate = gridAreaIt->value;
 
-                // The following checks test that the grid area is a single filled-in rectangle.
-                // 1. The new row is adjacent to the previously parsed row.
-                if (rowCount != gridCoordinate.rows.initialPositionIndex + 1)
-                    return 0;
+            // The following checks test that the grid area is a single filled-in rectangle.
+            // 1. The new row is adjacent to the previously parsed row.
+            if (rowCount != gridCoordinate.rows.initialPositionIndex + 1)
+                return false;
 
-                // 2. The new area starts at the same position as the previously parsed area.
-                if (currentCol != gridCoordinate.columns.initialPositionIndex)
-                    return 0;
+            // 2. The new area starts at the same position as the previously parsed area.
+            if (currentCol != gridCoordinate.columns.initialPositionIndex)
+                return false;
 
-                // 3. The new area ends at the same position as the previously parsed area.
-                if (lookAheadCol != gridCoordinate.columns.finalPositionIndex)
-                    return 0;
+            // 3. The new area ends at the same position as the previously parsed area.
+            if (lookAheadCol != gridCoordinate.columns.finalPositionIndex)
+                return false;
 
-                ++gridCoordinate.rows.finalPositionIndex;
-            }
-            currentCol = lookAheadCol;
+            ++gridCoordinate.rows.finalPositionIndex;
         }
+        currentCol = lookAheadCol;
+    }
+
+    m_valueList->next();
+    return true;
+}
 
+PassRefPtr<CSSValue> BisonCSSParser::parseGridTemplateAreas()
+{
+    NamedGridAreaMap gridAreaMap;
+    size_t rowCount = 0;
+    size_t columnCount = 0;
+
+    while (m_valueList->current()) {
+        if (!parseGridTemplateAreasRow(gridAreaMap, rowCount, columnCount))
+            return 0;
         ++rowCount;
-        m_valueList->next();
     }
 
     if (!rowCount || !columnCount)