Move some grid-column/grid-row related longhands into CSSPropertyParser

third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp

Index: third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
diff --git a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
index 6f247e4fabba6906dbe62ae6d183200e54bd65fe..06195b1cd7fa0688a0c29caaf432bc9f45abb21e 100644
--- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -3269,6 +3269,53 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumeJustifyItems(CSSParserTokenRange&
     return consumeSelfPositionOverflowPosition(range);
 }
 
+static PassRefPtrWillBeRawPtr<CSSCustomIdentValue> consumeValidCustomIdentValue(CSSParserTokenRange& range)
+{
+    if (isCSSWideKeyword(range.peek().id()) || range.peek().id() == CSSValueAuto || range.peek().id() == CSSValueSpan)

[Timothy Loh, 2016/03/16 04:24:25]

isCSSWideKeyword should be in consumeCustomIdent, can we fix this first?
Declarations like "animation: 2 inherit" should be invalid.

Also I'm not sure if span/auto are actually invalid here... Maybe if you just
write "span" it should match as a <custom-ident>? In any case, the function name
needs to make it clear it's specific to grid lines

[rwlbuis, 2016/03/16 17:02:02]

Sure, I moved that here:
https://codereview.chromium.org/1808753002/

+        return nullptr;
+    return consumeCustomIdent(range);
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeGridLine(CSSParserTokenRange& range)
+{
+    if (range.peek().id() == CSSValueAuto)
+        return consumeIdent(range);
+
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> spanValue = nullptr;
+    RefPtrWillBeRawPtr<CSSCustomIdentValue> gridLineName = nullptr;
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> numericValue = consumeInteger(range);
+    if (numericValue) {
+        gridLineName = consumeValidCustomIdentValue(range);
+        spanValue = consumeIdent<CSSValueSpan>(range);
+    } else if ((spanValue = consumeIdent<CSSValueSpan>(range))) {
+        numericValue = consumeInteger(range);
+        gridLineName = consumeValidCustomIdentValue(range);
+        if (!numericValue)
+            numericValue = consumeInteger(range);
+    } else if ((gridLineName = consumeValidCustomIdentValue(range))) {
+        numericValue = consumeInteger(range);
+        spanValue = consumeIdent<CSSValueSpan>(range);
+        if (!spanValue && !numericValue)
+            return gridLineName.release();
+    } else {
+        return nullptr;
+    }
+
+    // Negative numbers are not allowed for span (but are for <integer>).

[Timothy Loh, 2016/03/16 04:24:26]

Comment is misleading since <integer> is a type defined in css-values which can
be negative. Also IMO it's clearer if you break this into two separate if
statements, it sort of looks like the brackets are in the wrong place.

[rwlbuis, 2016/03/16 20:23:42]

Done.

+    if (numericValue && ((spanValue && numericValue->getIntValue() < 0) || (numericValue->getIntValue() == 0)))
+        return nullptr;
+
+    RefPtrWillBeRawPtr<CSSValueList> values = CSSValueList::createSpaceSeparated();
+    if (spanValue)
+        values->append(spanValue.release());
+    if (numericValue)
+        values->append(numericValue.release());
+    if (gridLineName)
+        values->append(gridLineName.release());
+    ASSERT(values->length());
+    return values.release();
+}
+
 PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSPropertyID unresolvedProperty)
 {
     CSSPropertyID property = resolveCSSPropertyID(unresolvedProperty);
@@ -3631,6 +3678,12 @@ PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSProperty
     case CSSPropertyJustifyItems:
         ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
         return consumeJustifyItems(m_range);
+    case CSSPropertyGridColumnEnd:
+    case CSSPropertyGridColumnStart:
+    case CSSPropertyGridRowEnd:
+    case CSSPropertyGridRowStart:
+        ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
+        return consumeGridLine(m_range);
     default:
         CSSParserValueList valueList(m_range);
         if (valueList.size()) {