Use isComma more in CSSPropertyParser

Source/core/css/parser/CSSPropertyParser.cpp

 /*
  * Copyright (C) 2003 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Nicholas Shanks <webkit@nickshanks.com>
  * Copyright (C) 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2012 Adobe Systems Incorporated. All rights reserved.
  * Copyright (C) 2012 Intel Corporation. All rights reserved.
  *
@@ skipping 317 matching lines @@
 
 inline PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::createCSSImageValueWithReferrer(const String& rawValue, const KURL& url)
 {
     RefPtrWillBeRawPtr<CSSValue> imageValue = CSSImageValue::create(rawValue, url);
     toCSSImageValue(imageValue.get())->setReferrer(m_context.referrer());
     return imageValue;
 }
 
 static inline bool isComma(CSSParserValue* value)
 {
-    return value && value->unit == CSSParserValue::Operator && value->iValue == ',';
+    ASSERT(value);
+    return value->unit == CSSParserValue::Operator && value->iValue == ',';
 }
 
 static bool consumeComma(CSSParserValueList* valueList)
 {
-    if (!isComma(valueList->current()))
+    CSSParserValue* value = valueList->current();
+    if (!value || !isComma(value))
         return false;
     valueList->next();
     return true;
 }
 
 static inline bool isForwardSlashOperator(CSSParserValue* value)
 {
     ASSERT(value);
     return value->unit == CSSParserValue::Operator && value->iValue == '/';
 }
@@ skipping 1247 matching lines @@
 #endif
     RefPtrWillBeRawPtr<CSSValue> clipValue = nullptr;
     RefPtrWillBeRawPtr<CSSValue> positionYValue = nullptr;
     RefPtrWillBeRawPtr<CSSValue> repeatYValue = nullptr;
     bool foundClip = false;
     int i;
     bool foundPositionCSSProperty = false;
 
     while (m_valueList->current()) {
         CSSParserValue* val = m_valueList->current();
-        if (val->unit == CSSParserValue::Operator && val->iValue == ',') {
+        if (isComma(val)) {
             // We hit the end.  Fill in all remaining values with the initial value.
             m_valueList->next();
             for (i = 0; i < numProperties; ++i) {
                 if (properties[i] == CSSPropertyBackgroundColor && parsedProperty[i])
                     // Color is not allowed except as the last item in a list for backgrounds.
                     // Reject the entire property.
                     return false;
 
                 if (!parsedProperty[i] && properties[i] != CSSPropertyBackgroundColor) {
                     addFillValue(values[i], cssValuePool().createImplicitInitialValue());
@@ skipping 907 matching lines @@
 inline bool CSSPropertyParser::isPotentialPositionValue(CSSParserValue* value)
 {
     return isFillPositionKeyword(value->id) || validUnit(value, FPercent | FLength, ReleaseParsedCalcValue);
 }
 
 void CSSPropertyParser::parseFillPosition(CSSParserValueList* valueList, RefPtrWillBeRawPtr<CSSValue>& value1, RefPtrWillBeRawPtr<CSSValue>& value2, Units unitless)
 {
     unsigned numberOfValues = 0;
     for (unsigned i = valueList->currentIndex(); i < valueList->size(); ++i, ++numberOfValues) {
         CSSParserValue* current = valueList->valueAt(i);
-        if (isComma(current) || !current || isForwardSlashOperator(current) || !isPotentialPositionValue(current))
+        if (!current || isComma(current) || isForwardSlashOperator(current) || !isPotentialPositionValue(current))
             break;
     }
 
     if (numberOfValues > 4)
         return;
 
     // If we are parsing two values, we can safely call the CSS 2.1 parsing function and return.
     if (numberOfValues <= 2) {
         parse2ValuesFillPosition(valueList, value1, value2, unitless);
         return;
@@ skipping 53 matching lines @@
     value1 = parseFillPositionComponent(valueList, cumulativeFlags, value1Flag, ResolveValuesAsPercent, unitless);
     if (!value1)
         return;
 
     // It only takes one value for background-position to be correctly parsed if it was specified in a shorthand (since we
     // can assume that any other values belong to the rest of the shorthand).  If we're not parsing a shorthand, though, the
     // value was explicitly specified for our property.
     CSSParserValue* value = valueList->next();
 
     // First check for the comma.  If so, we are finished parsing this value or value pair.
-    if (isComma(value))
+    if (value && isComma(value))
         value = 0;
 
     if (value) {
         value2 = parseFillPositionComponent(valueList, cumulativeFlags, value2Flag, ResolveValuesAsPercent, unitless);
         if (value2)
             valueList->next();
         else {
             if (!inShorthand()) {
                 value1.clear();
                 return;
@@ skipping 1983 matching lines @@
 PassRefPtrWillBeRawPtr<CSSValueList> CSSPropertyParser::parseFontFamily()
 {
     RefPtrWillBeRawPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
     CSSParserValue* value = m_valueList->current();
 
     FontFamilyValueBuilder familyBuilder(list.get());
     bool inFamily = false;
 
     while (value) {
         CSSParserValue* nextValue = m_valueList->next();
-        bool nextValBreaksFont = !nextValue ||
-                                 (nextValue->unit == CSSParserValue::Operator && nextValue->iValue == ',');
+        bool nextValBreaksFont = !nextValue || isComma(nextValue);
         bool nextValIsFontName = nextValue &&
             ((nextValue->id >= CSSValueSerif && nextValue->id <= CSSValueWebkitBody) ||
             (nextValue->unit == CSSPrimitiveValue::CSS_STRING || nextValue->unit == CSSPrimitiveValue::CSS_IDENT));
 
         if (isCSSWideKeyword(*value) && !inFamily) {
             if (nextValBreaksFont)
                 value = m_valueList->next();
             else if (nextValIsFontName)
                 value = nextValue;
             continue;
@@ skipping 3536 matching lines @@
     bool validPrimitive = true;
     while (value) {
         validPrimitive = validUnit(value, FLength | FPercent | FNonNeg, SVGAttributeMode);
         if (!validPrimitive)
             break;
         if (value->id)
             ret->append(CSSPrimitiveValue::createIdentifier(value->id));
         else if (value->unit >= CSSPrimitiveValue::CSS_NUMBER && value->unit <= CSSPrimitiveValue::CSS_KHZ)
             ret->append(CSSPrimitiveValue::create(value->fValue, (CSSPrimitiveValue::UnitType) value->unit));
         value = m_valueList->next();
-        if (value && value->unit == CSSParserValue::Operator && value->iValue == ',')
+        if (value && isComma(value))

[Timothy Loh, 2015/02/17 23:37:02]

We should probably fix this case to work properly in the new parser (the Bison
parser does some rudimentary validation and disallows trailing commas, but the
new parser just assumes the CSSPropertyParser will do all this work). You can
probably find similar examples in the file using consumeComma.

             value = m_valueList->next();
     }
     if (!validPrimitive)
         return nullptr;
     return ret.release();
 }
 
 // normal | [ fill || stroke || markers ]
 PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parsePaintOrder() const
 {
@@ skipping 220 matching lines @@
         } else if (!validUnit(a, unit, HTMLStandardMode)) {
             return nullptr;
         }
 
         // Add the value to the current transform operation.
         transformValue->append(createPrimitiveNumericValue(a));
 
         a = args->next();
         if (!a)
             break;
-        if (a->unit != CSSParserValue::Operator || a->iValue != ',')
+        if (!isComma(a))
             return nullptr;
         a = args->next();
 
         argNumber++;
     }
 
     return transformValue.release();
 }
 
 PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseMotionPath()
@@ skipping 44 matching lines @@
         }
     }
 
     if (!list->length())
         return nullptr;
 
     return list.release();
 }
 
 } // namespace blink