[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 7ed7414f92c47a11434b4c196c79d000310cd949..51cc20b881c61d99f38de5a09a80a60a3172398a 100644
--- a/Source/core/css/parser/BisonCSSParser-in.cpp
+++ b/Source/core/css/parser/BisonCSSParser-in.cpp
@@ -2488,6 +2488,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);
@@ -4718,6 +4723,169 @@ bool BisonCSSParser::parseGridItemPositionShorthand(CSSPropertyID shorthandId, b
     return true;
 }
 
+bool BisonCSSParser::parseGridTemplateRowsAndAreas(bool important)
+{
+    NamedGridAreaMap gridAreaMap;
+    size_t rowCount = 0;
+    size_t columnCount = 0;
+    bool traillingAdded = false;
+    RefPtr<CSSValueList> templateRows = CSSValueList::createSpaceSeparated();
+    CSSParserValue* currentValue = m_valueList->current();
+
+    // There must be either template-areas or template-rows.
+    if (!currentValue)
+        return false;
+
+    do {
+        // Handle leading/trailling <ident>*.
+        if (currentValue->unit == CSSParserValue::ValueList)
+            parseGridLineNames(m_valueList.get(), *templateRows, traillingAdded);
+
+        // Handle a template-area's row.
+        if (parseGridTemplateAreasRow(gridAreaMap, rowCount, columnCount))
+            ++rowCount;
+
+        // If no template-areas, template-rows must be defined.
+        if (!((currentValue = m_valueList->current()) || rowCount))
+            return false;
+
+        // Handle template-rows's track-size.
+        if (currentValue && currentValue->unit != CSSParserValue::ValueList) {
+            RefPtr<CSSValue> value = parseGridTrackSize(*m_valueList);
+            if (!(value || rowCount))
+                return false;
+            templateRows->append(value);
+        } else {
+            templateRows->append(cssValuePool().createIdentifierValue(CSSValueAuto));
+        }
+
+        // This will handle the trailing <ident>* in the grammar.
+        traillingAdded = false;
+        if (m_valueList->current() && m_valueList->current()->unit == CSSParserValue::ValueList) {
+            parseGridLineNames(m_valueList.get(), *templateRows);
+            traillingAdded = true;
+        }
+    } while ((currentValue = m_valueList->current()));
+
+    if (!templateRows->length())
+        addProperty(CSSPropertyGridTemplateRows, cssValuePool().createIdentifierValue(CSSValueNone), important);
+    else
+        addProperty(CSSPropertyGridTemplateRows, templateRows.release(), important);
+    if (!rowCount) {
+        addProperty(CSSPropertyGridTemplateAreas, cssValuePool().createIdentifierValue(CSSValueNone), important);
+    } else {
+        RefPtr<CSSValue> templateAreas = CSSGridTemplateAreasValue::create(gridAreaMap, rowCount, columnCount);
+        addProperty(CSSPropertyGridTemplateAreas, templateAreas.release(), important);
+    }
+
+    return true;
+}
+
+bool BisonCSSParser::parseGridTemplateShorthand(bool important)
+{
+    ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
+
+    ShorthandScope scope(this, CSSPropertyGridTemplate);
+    const StylePropertyShorthand& shorthand = gridTemplateShorthand();
+    ASSERT_UNUSED(shorthand, shorthand.length() == 3);
+
+    RefPtr<CSSValue> columnsValue = cssValuePool().createIdentifierValue(CSSValueNone);
+    RefPtr<CSSValue> rowsValue = cssValuePool().createIdentifierValue(CSSValueNone);
+    RefPtr<CSSValue> areasValue = cssValuePool().createIdentifierValue(CSSValueNone);
+
+    RefPtr<CSSValueList> values = CSSValueList::createSpaceSeparated();
+    CSSParserValue* current = m_valueList->current();
+
+    if (!current)
+        return false;

[svillar, 2014/02/07 12:02:49]

Better do this before creating all the above objects.

BTW maybe it's better not to initialize them to none since you're likely going
to replace those values later. We can just check if they're null when adding the
properties at the very end.

+
+    NamedGridAreaMap gridAreaMap;
+    size_t rowCount = 0;
+    size_t columnCount = 0;
+    bool forwardOp = false;

[svillar, 2014/02/07 12:02:49]

Use a more descriptive name like seenForwardSlashOperator.

+    bool traillingAdded = false;

[svillar, 2014/02/07 12:02:49]

trailing. Also traillingAdded is a very confusing name, I don't know what it
means unless I check the code.

+    bool simpleForm = true; // template-columns / template-rows

[svillar, 2014/02/07 12:02:49]

Use a more descriptive name for this variable, we need to know that it refers to
a grid-template without areas. After that remove the comment :)

+    do {
+        if (isForwardSlashOperator(current)) {
+            if (forwardOp)
+                return false; // Slash not allowed in the second clause.
+            if (!values->length())
+                return false; // First clause is mandatory.
+            if (!(current = m_valueList->next()))
+                return false; // Second clause is mandatory.

[svillar, 2014/02/07 12:02:49]

I guess you could put these three in a single row

+            forwardOp = true;
+            columnsValue = values.release();
+            values = CSSValueList::createSpaceSeparated();
+        }
+
+        // Handle leading/trailling <ident>*.

[svillar, 2014/02/07 12:02:49]

trailing

+        if (current->unit == CSSParserValue::ValueList) {
+            if (traillingAdded && simpleForm) // Not allowed in simple form.
+                return false;
+            parseGridLineNames(m_valueList.get(), *values, traillingAdded);
+            if (!(current = m_valueList->current()))
+                break;
+        }
+
+        // Handle a template-area's row.
+        if (current->unit == CSSPrimitiveValue::CSS_STRING) {
+            if (!parseGridTemplateAreasRow(gridAreaMap, rowCount, columnCount))
+                return false;
+            ++rowCount;
+            simpleForm = false;
+            if (!(current = m_valueList->current()))
+                break;
+        }
+
+        // Handle template-{columns/rows}'s track-size, repeat or auto.
+        if (current->unit == CSSParserValue::Function && equalIgnoringCase(current->function->name, "repeat(")) {
+            if (!simpleForm)
+                return false; // Not allowed for template-row in complex form.
+            if (!parseGridTrackRepeatFunction(*values))
+                return false;
+        } else if (current->unit != CSSParserValue::ValueList) {
+            RefPtr<CSSValue> value = parseGridTrackSize(*m_valueList);
+            if (!value && simpleForm) // Mandatory in template-columns, "auto" in 2nd form template-rows.
+                return false;
+            values->append(value);
+        } else {
+            if (simpleForm)
+                return false; // track-size mandatory in template-columns
+            values->append(cssValuePool().createIdentifierValue(CSSValueAuto));
+        }
+
+        if (!(current = m_valueList->current()))
+            break;
+
+        // This will handle the trailing <ident>* in the grammar.
+        traillingAdded = false;
+        if (current && current->unit == CSSParserValue::ValueList) {
+            parseGridLineNames(m_valueList.get(), *values);
+            traillingAdded = true;
+            current = m_valueList->current();
+        }
+    } while ((current = m_valueList->current()));
+
+    if (rowCount)
+        areasValue = CSSGridTemplateAreasValue::create(gridAreaMap, rowCount, columnCount);
+
+    if (forwardOp) {
+        if (values->length() > 0)
+            rowsValue = values;
+    } else {
+        if (simpleForm)
+            return false; // 2 mandatory clauses in simple form.

[svillar, 2014/02/07 12:02:49]

It looks like this should be an ASSERT but haven't checked the above algorithm
carefully enough to know if it guarantees that.

+        if (values->length() > 0)
+            columnsValue = values;
+    }
+
+    addProperty(CSSPropertyGridTemplateColumns, columnsValue, important);
+    addProperty(CSSPropertyGridTemplateRows, rowsValue, important);
+    addProperty(CSSPropertyGridTemplateAreas, areasValue, important);
+
+    return true;
+}
+
 bool BisonCSSParser::parseGridAreaShorthand(bool important)
 {
     ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
@@ -4773,7 +4941,7 @@ bool BisonCSSParser::parseSingleGridAreaLonghand(RefPtr<CSSValue>& property)
     return true;
 }
 
-void BisonCSSParser::parseGridLineNames(CSSParserValueList* parserValueList, CSSValueList& valueList)
+void BisonCSSParser::parseGridLineNames(CSSParserValueList* parserValueList, CSSValueList& valueList, bool concat)
 {
     ASSERT(parserValueList->current() && parserValueList->current()->unit == CSSParserValue::ValueList);
 
@@ -4783,14 +4951,15 @@ void BisonCSSParser::parseGridLineNames(CSSParserValueList* parserValueList, CSS
         return;
     }
 
-    RefPtr<CSSGridLineNamesValue> lineNames = CSSGridLineNamesValue::create();
+    RefPtr<CSSGridLineNamesValue> lineNames = concat && valueList.length() > 0 ? static_cast< CSSGridLineNamesValue* >(valueList.item(valueList.length() - 1)) : CSSGridLineNamesValue::create();
     while (CSSParserValue* identValue = identList->current()) {
         ASSERT(identValue->unit == CSSPrimitiveValue::CSS_IDENT);
         RefPtr<CSSPrimitiveValue> lineName = createPrimitiveStringValue(identValue);
         lineNames->append(lineName.release());
         identList->next();
     }
-    valueList.append(lineNames.release());
+    if (!concat)
+        valueList.append(lineNames.release());
 

[svillar, 2014/02/07 12:02:49]

So instead of doing this, I think it's better to pass a CSSGridLineNamesValue&
instead of a CSSValueList&. That way this function will always do the same
thing. The caller is the one that will either create a new CSSGridLineNamesValue
instance or just pass a reference to an already existing one.

     parserValueList->next();
 }
@@ -4935,71 +5104,79 @@ PassRefPtr<CSSPrimitiveValue> BisonCSSParser::parseGridBreadth(CSSParserValue* c
     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)