[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..5d1f621d75d6eedaa16c468471a6d95894f6d6da 100644
--- a/Source/core/css/parser/BisonCSSParser-in.cpp
+++ b/Source/core/css/parser/BisonCSSParser-in.cpp
@@ -2412,6 +2412,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 +4605,130 @@ bool BisonCSSParser::parseGridItemPositionShorthand(CSSPropertyID shorthandId, b
     return true;
 }
 
+bool BisonCSSParser::parseGridTemplateShorthand(bool important)
+{
+    ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
+
+    ShorthandScope scope(this, CSSPropertyGridTemplate);
+    const StylePropertyShorthand& shorthand = gridTemplateShorthand();
+    ASSERT_UNUSED(shorthand, shorthand.length() == 3);

[Julien - ping for review, 2014/02/21 20:04:50]

ASSERT(gridTemplateShorthand().length() == 3) ?

+
+    if (!m_valueList->current())
+        return false;
+
+    RefPtr<CSSValue> columnsValue;
+    RefPtr<CSSValue> rowsValue;
+    RefPtr<CSSValue> areasValue;
+    NamedGridAreaMap gridAreaMap;
+    size_t areaRowCount = 0;
+    size_t areaColumnCount = 0;
+    size_t numberOfClauses = 1;

[Julien - ping for review, 2014/02/21 20:04:50]

This really should be a state, not a plain int. You are basically building a
state machine without naming it.

+    bool seenTemplateAreas = false;
+    bool seenTrackSizeOrRepeat = false;

[Julien - ping for review, 2014/02/21 20:04:50]

Per our style guide, this should be hasSeenTrackSizeOrRepeat. Same for
seenTemplateAreas.

+
+    RefPtr<CSSValueList> values = CSSValueList::createSpaceSeparated();
+
+    while (m_valueList->current()) {
+        if (isForwardSlashOperator(m_valueList->current())) {
+            // Slash not allowed in the second clause. Two clauses are required if slash used.
+            if (++numberOfClauses > 2 || !m_valueList->next())
+                return false;
+            if (values->length() > 0)
+                columnsValue = values.release();
+            else
+                columnsValue = cssValuePool().createIdentifierValue(CSSValueNone);
+            values = CSSValueList::createSpaceSeparated();
+            seenTrackSizeOrRepeat = false;
+        }
+
+        if (m_valueList->current()->id == CSSValueNone) {
+            if (seenTemplateAreas)
+                return false; // The 'none' value is not allowed in complex form.
+            if (seenTrackSizeOrRepeat)
+                return false; // The 'none' value is not allowed as part of a track-size list.
+            if (m_valueList->next()) {
+                if (!isForwardSlashOperator(m_valueList->current()))
+                    return false;
+                continue;
+            }
+            break;
+        }
+
+        // Handle leading <ident>*.

[Julien - ping for review, 2014/02/21 20:04:50]

Let's be precise here. First the specification calls these <custom-ident> not
<ident> (this is an open bug in our implementation that we need to convert
them).

But more to the point, you are handling a leading <track-list>? which IMO is
more meaningful.

+        if (m_valueList->current()->unit == CSSParserValue::ValueList)
+            parseGridLineNames(m_valueList.get(), *values);
+
+        // Handle a template-area's row.
+        if (m_valueList->current() && m_valueList->current()->unit == CSSPrimitiveValue::CSS_STRING) {
+            if (!parseGridTemplateAreasRow(gridAreaMap, areaRowCount, areaColumnCount))
+                return false;
+            if (!areaRowCount && seenTrackSizeOrRepeat)
+                return false; // No TrackSize allowed before the area's first row definition.
+            ++areaRowCount;
+            seenTemplateAreas = true;
+        }
+
+        // Handle template-{columns/rows}'s track-size, repeat or auto.

[Julien - ping for review, 2014/02/21 20:04:50]

I am concerned by that. We have now effectively duplicated our
grid-template-{columns|rows} parsing.

This is a limitation of our implementation that we can't rewind m_valueList but
I think it would be worthwhile to be able to do that here. That would require:
- refactoring parseGridTrackList so that it returns a CSSPrimitiveValue.
- trying to parse grid-template as the grid-template-columns /
grid-template-rows pair.
- If that fails, we are manually parsing the new syntax.

In practice, that would mean parsing grid-template twice for the last case but I
think that's a good trade-off as it would make the code way cleaner.

+        if (m_valueList->current() && m_valueList->current()->unit == CSSParserValue::Function && equalIgnoringCase(m_valueList->current()->function->name, "repeat(")) {
+            if (seenTemplateAreas)
+                return false; // Not allowed for template-row in complex form.
+            if (!parseGridTrackRepeatFunction(*values))
+                return false;
+            seenTrackSizeOrRepeat = true;
+        } else 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;
+            values->append(value);
+            seenTrackSizeOrRepeat = true;
+        } else {
+            if (!seenTemplateAreas)
+                return false; // track-size mandatory in template-columns, "auto" otherwise.
+            values->append(cssValuePool().createIdentifierValue(CSSValueAuto));
+        }
+
+        // This will handle the trailing <ident>* in the grammar.
+        if (m_valueList->current() && m_valueList->current()->unit == CSSParserValue::ValueList) {
+            parseGridLineNames(m_valueList.get(), *values);
+            if (m_valueList->current() && m_valueList->current()->unit == CSSParserValue::ValueList) {
+                if (!seenTemplateAreas)
+                    return false;
+                // Concat the last row's <ident>* with the new row's line name.
+                parseGridLineNames(m_valueList.get(), *values, static_cast<CSSGridLineNamesValue*>(values->item(values->length() - 1)));
+            }
+            if (seenTemplateAreas && m_valueList->current() && m_valueList->current()->unit != CSSPrimitiveValue::CSS_STRING)
+                return false; // area string mandatory (if additional elements) after line-name in complex form.
+        }
+    }
+
+    // Handle template-areas property value.
+    if (areaRowCount > 0)
+        areasValue = CSSGridTemplateAreasValue::create(gridAreaMap, areaRowCount, areaColumnCount);
+    else
+        areasValue = cssValuePool().createIdentifierValue(CSSValueNone);
+
+    // Handle template-columns property value.
+    if (numberOfClauses == 1) {
+        if (!areaRowCount && values->length() > 0)
+            return false;
+        columnsValue = cssValuePool().createIdentifierValue(CSSValueNone);
+    }
+
+    // Handle template-rows property value.
+    if (values->length() > 0)
+        rowsValue = values.release();
+    else if (areaRowCount > 0)
+        return false;
+    else
+        rowsValue = cssValuePool().createIdentifierValue(CSSValueNone);
+
+    addProperty(CSSPropertyGridTemplateColumns, columnsValue, important);
+    addProperty(CSSPropertyGridTemplateRows, rowsValue, important);
+    addProperty(CSSPropertyGridTemplateAreas, areasValue, important);
+
+    return true;
+}
+
 bool BisonCSSParser::parseGridAreaShorthand(bool important)
 {
     ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
@@ -4655,7 +4784,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,14 +4794,15 @@ void BisonCSSParser::parseGridLineNames(CSSParserValueList* parserValueList, CSS
         return;
     }
 
-    RefPtrWillBeRawPtr<CSSGridLineNamesValue> lineNames = CSSGridLineNamesValue::create();
+    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();
 }
@@ -4817,71 +4947,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)