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

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/css/parser/CSSPropertyParser.h"
 
 #include "core/StylePropertyShorthand.h"
 #include "core/css/CSSBasicShapeValues.h"
 #include "core/css/CSSBorderImage.h"
 #include "core/css/CSSContentDistributionValue.h"
@@ skipping 3251 matching lines @@
     RefPtrWillBeRawPtr<CSSPrimitiveValue> positionKeyword = consumeIdent<CSSValueCenter, CSSValueLeft, CSSValueRight>(rangeCopy);
     if (!legacy)
         legacy = consumeIdent<CSSValueLegacy>(rangeCopy);
     if (legacy && positionKeyword) {
         range = rangeCopy;
         return CSSValuePair::create(legacy.release(), positionKeyword.release(), CSSValuePair::DropIdenticalValues);
     }
     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);
     if (CSSParserFastPaths::isKeywordPropertyID(property)) {
         if (!CSSParserFastPaths::isValidKeywordPropertyAndValue(property, m_range.peek().id()))
             return nullptr;
         return consumeIdent(m_range);
     }
     switch (property) {
     case CSSPropertyWillChange:
@@ skipping 342 matching lines @@
     case CSSPropertyWebkitMaskRepeatY:
         return nullptr;
     case CSSPropertyJustifySelf:
     case CSSPropertyAlignSelf:
     case CSSPropertyAlignItems:
         ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
         return consumeSelfPositionOverflowPosition(m_range);
     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()) {
             m_valueList = &valueList;
             if (RefPtrWillBeRawPtr<CSSValue> result = legacyParseValue(unresolvedProperty)) {
                 while (!m_range.atEnd())
                     m_range.consume();
                 return result.release();
             }
         }
@@ skipping 923 matching lines @@
         m_currentShorthand = oldShorthand;
         CSSParserValueList valueList(m_range);
         if (!valueList.size())
             return false;
         m_valueList = &valueList;
         return legacyParseShorthand(unresolvedProperty, important);
     }
 }
 
 } // namespace blink