Fix small bugs in new CSS Property parser

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 f73940f7577063e3db07bfb25bd5ae789afb78ab..7bf006fb59bd7bc04468e4adfbcc5155647517cc 100644
--- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -871,12 +871,10 @@ static inline bool isCSSWideKeyword(const CSSValueID& id)
 // Methods for consuming non-shorthand properties starts here.
 static PassRefPtrWillBeRawPtr<CSSValue> consumeWillChange(CSSParserTokenRange& range)
 {
-    RefPtrWillBeRawPtr<CSSValueList> values = CSSValueList::createCommaSeparated();
-    if (range.peek().id() == CSSValueAuto) {
-        // FIXME: This will be read back as an empty string instead of auto
-        return values.release();
-    }
+    if (range.peek().id() == CSSValueAuto)
+        return consumeIdent(range);
 
+    RefPtrWillBeRawPtr<CSSValueList> values = CSSValueList::createCommaSeparated();
     // Every comma-separated list of identifiers is a valid will-change value,
     // unless the list includes an explicitly disallowed identifier.
     while (true) {
@@ -1229,8 +1227,7 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumeCounter(CSSParserTokenRange& rang
     if (range.peek().id() == CSSValueNone)
         return consumeIdent(range);
 
-    // TODO(rwlbuis): should be space separated list.
-    RefPtrWillBeRawPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
+    RefPtrWillBeRawPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
     do {
         RefPtrWillBeRawPtr<CSSCustomIdentValue> counterName = consumeCustomIdent(range);
         if (!counterName)
@@ -1239,7 +1236,7 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumeCounter(CSSParserTokenRange& rang
         if (RefPtrWillBeRawPtr<CSSPrimitiveValue> counterValue = consumeInteger(range))
             i = clampTo<int>(counterValue->getDoubleValue());
         list->append(CSSValuePair::create(counterName.release(),
-            cssValuePool().createValue(i, CSSPrimitiveValue::UnitType::Number),
+            cssValuePool().createValue(i, CSSPrimitiveValue::UnitType::Integer),
             CSSValuePair::DropIdenticalValues));
     } while (!range.atEnd());
     return list.release();
@@ -2387,7 +2384,7 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumeCursor(CSSParserTokenRange& range
             list = CSSValueList::createCommaSeparated();
 
         list->append(CSSCursorImageValue::create(image, hotSpotSpecified, hotSpot));
-        if (!consumeCommaIncludingWhitespace(range))
+        if (!consumeCommaIncludingWhitespace(range) || range.atEnd())

[Timothy Loh, 2016/01/06 00:14:52]

I think there's a better way to fix this (see below).

[rwlbuis, 2016/01/06 08:11:03]

Acknowledged.

             return nullptr;
     }
 
@@ -2400,8 +2397,9 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumeCursor(CSSParserTokenRange& range
     }
     RefPtrWillBeRawPtr<CSSValue> cursorType = nullptr;
     if (id == CSSValueHand) {
-        if (inQuirksMode) // Non-standard behavior
-            cursorType = cssValuePool().createIdentifierValue(CSSValuePointer);
+        if (!inQuirksMode) // Non-standard behavior
+            return nullptr;
+        cursorType = cssValuePool().createIdentifierValue(CSSValuePointer);
         range.consumeIncludingWhitespace();
     } else if ((id >= CSSValueAuto && id <= CSSValueWebkitZoomOut) || id == CSSValueCopy || id == CSSValueNone) {
         cursorType = consumeIdent(range);
@@ -3472,25 +3470,27 @@ bool CSSPropertyParser::parseViewportDescriptor(CSSPropertyID propId, bool impor
     }
 }
 
-static void consumeColumnWidthOrCount(CSSParserTokenRange& range, CSSParserMode cssParserMode, RefPtrWillBeRawPtr<CSSValue> &columnWidth, RefPtrWillBeRawPtr<CSSValue> &columnCount)
+static bool consumeColumnWidthOrCount(CSSParserTokenRange& range, CSSParserMode cssParserMode, RefPtrWillBeRawPtr<CSSValue> &columnWidth, RefPtrWillBeRawPtr<CSSValue> &columnCount)
 {
     if (range.peek().id() == CSSValueAuto) {
         consumeIdent(range);
-        return;
+        return true;
     }
     if (!columnWidth) {
         if ((columnWidth = consumeColumnWidth(range)))
-            return;
+            return true;
     }
     if (!columnCount)
         columnCount = consumeColumnCount(range);
+    return columnCount;
 }
 
 bool CSSPropertyParser::consumeColumns(bool important)
 {
     RefPtrWillBeRawPtr<CSSValue> columnWidth = nullptr;
     RefPtrWillBeRawPtr<CSSValue> columnCount = nullptr;
-    consumeColumnWidthOrCount(m_range, m_context.mode(), columnWidth, columnCount);
+    if (!consumeColumnWidthOrCount(m_range, m_context.mode(), columnWidth, columnCount))
+        return false;
     consumeColumnWidthOrCount(m_range, m_context.mode(), columnWidth, columnCount);
     if (!m_range.atEnd())
         return false;
@@ -3568,17 +3568,18 @@ bool CSSPropertyParser::consumeFlex(bool important)
                     return false;
             }
         }
+        if (index == 0)
+            return false;
+        if (flexGrow == unsetValue)
+            flexGrow = 1;
+        if (flexShrink == unsetValue)
+            flexShrink = 1;
+        if (!flexBasis)
+            flexBasis = cssValuePool().createValue(0, CSSPrimitiveValue::UnitType::Percentage);
     }
+
     if (!m_range.atEnd())
         return false;
-
-    if (flexGrow == unsetValue)
-        flexGrow = 1;
-    if (flexShrink == unsetValue)
-        flexShrink = 1;
-    if (!flexBasis)
-        flexBasis = cssValuePool().createValue(0, CSSPrimitiveValue::UnitType::Percentage);
-
     addProperty(CSSPropertyFlexGrow, cssValuePool().createValue(clampTo<float>(flexGrow), CSSPrimitiveValue::UnitType::Number), important);
     addProperty(CSSPropertyFlexShrink, cssValuePool().createValue(clampTo<float>(flexShrink), CSSPrimitiveValue::UnitType::Number), important);
     addProperty(CSSPropertyFlexBasis, flexBasis, important);