Move columns related properties 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 40ef3b88d61ac53915a02cbddf325afe72601d35..6bd629c4fac357547aa2982d01245d4f98d70766 100644
--- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -663,6 +663,47 @@ static PassRefPtrWillBeRawPtr<CSSValue> consumeCounter(CSSParserTokenRange& rang
     return list.release();
 }
 
+static PassRefPtrWillBeRawPtr<CSSValue> consumeColumnWidth(CSSParserTokenRange& range)
+{
+    if (range.peek().id() == CSSValueAuto)
+        return consumeIdent(range);
+    // Always parse lengths in strict mode here, since it would be ambiguous otherwise when used in
+    // the 'columns' shorthand property.
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> columnWidth = consumeLength(range, HTMLStandardMode, ValueRangeNonNegative);
+    if (!columnWidth || (!columnWidth->isCalculated() && columnWidth->getDoubleValue() == 0))
+        return nullptr;
+    return columnWidth.release();
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeColumnCount(CSSParserTokenRange& range, CSSParserMode cssParserMode)
+{
+    if (range.peek().id() == CSSValueAuto)
+        return consumeIdent(range);
+    return consumeInteger(range, cssParserMode, 1);
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeColumnGap(CSSParserTokenRange& range, CSSParserMode cssParserMode)
+{
+    if (range.peek().id() == CSSValueNormal)
+        return consumeIdent(range);
+    return consumeLength(range, cssParserMode, ValueRangeNonNegative);
+}
+
+static PassRefPtrWillBeRawPtr<CSSValue> consumeColumnSpan(CSSParserTokenRange& range, CSSParserMode cssParserMode)
+{
+    CSSValueID id = range.peek().id();
+    if (id == CSSValueAll || id == CSSValueNone)
+        return consumeIdent(range);
+    if (range.peek().type() != NumberToken)

[Timothy Loh, 2015/10/13 00:30:03]

I probably wouldn't bother with this check

[rwlbuis, 2015/10/13 02:27:02]

The check was done to filter out any calc() usage.

+        return nullptr;
+    if (RefPtrWillBeRawPtr<CSSPrimitiveValue> spanValue = consumeInteger(range, cssParserMode)) {
+        // 1 (will be dropped in the unprefixed property).
+        if (spanValue->getIntValue() == 1)
+            return spanValue.release();
+    }
+    return nullptr;
+}
+
 PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSPropertyID propId)
 {
     m_range.consumeWhitespace();
@@ -702,6 +743,14 @@ PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSProperty
     case CSSPropertyCounterIncrement:
     case CSSPropertyCounterReset:
         return consumeCounter(m_range, m_context.mode(), propId == CSSPropertyCounterIncrement ? 1 : 0);
+    case CSSPropertyWebkitColumnWidth:
+        return consumeColumnWidth(m_range);
+    case CSSPropertyWebkitColumnCount:
+        return consumeColumnCount(m_range, m_context.mode());
+    case CSSPropertyWebkitColumnGap:
+        return consumeColumnGap(m_range, m_context.mode());
+    case CSSPropertyWebkitColumnSpan:
+        return consumeColumnSpan(m_range, m_context.mode());
     default:
         return nullptr;
     }
@@ -1036,6 +1085,37 @@ bool CSSPropertyParser::parseViewportDescriptor(CSSPropertyID propId, bool impor
     }
 }
 
+static void consumeColumnWidthOrCount(CSSParserTokenRange& range, CSSParserMode cssParserMode, RefPtrWillBeRawPtr<CSSValue> &columnWidth, RefPtrWillBeRawPtr<CSSValue> &columnCount)
+{
+    if (range.peek().id() == CSSValueAuto) {
+        consumeIdent(range);
+        return;
+    }
+    if (!columnWidth) {
+        if ((columnWidth = consumeColumnWidth(range)))
+            return;
+    }
+    if (!columnCount)
+        columnCount = consumeColumnCount(range, cssParserMode);
+}
+
+bool CSSPropertyParser::consumeColumns(bool important)

[Timothy Loh, 2015/10/13 00:30:03]

Do we check somewhere that the input isn't blank (whitespace-only)? Otherwise
this might allow "-webkit-columns: ;"

[rwlbuis, 2015/10/13 02:27:01]

Good point, I'll check tomorrow, likely with this patch we do accept that, I
wonder if we accepted it before too.

+{
+    RefPtrWillBeRawPtr<CSSValue> columnWidth = nullptr;
+    RefPtrWillBeRawPtr<CSSValue> columnCount = nullptr;
+    consumeColumnWidthOrCount(m_range, m_context.mode(), columnWidth, columnCount);
+    consumeColumnWidthOrCount(m_range, m_context.mode(), columnWidth, columnCount);
+    if (!m_range.atEnd())
+        return false;
+    if (!columnWidth)
+        columnWidth = cssValuePool().createIdentifierValue(CSSValueAuto);
+    if (!columnCount)
+        columnCount = cssValuePool().createIdentifierValue(CSSValueAuto);
+    addProperty(CSSPropertyWebkitColumnWidth, columnWidth.release(), important);
+    addProperty(CSSPropertyWebkitColumnCount, columnCount.release(), important);
+    return true;
+}
+
 bool CSSPropertyParser::parseShorthand(CSSPropertyID propId, bool important)
 {
     m_range.consumeWhitespace();
@@ -1089,6 +1169,10 @@ bool CSSPropertyParser::parseShorthand(CSSPropertyID propId, bool important)
     }
     case CSSPropertyBorderSpacing:
         return consumeBorderSpacing(important);
+    case CSSPropertyWebkitColumns: {
+        m_currentShorthand = oldShorthand;

[Timothy Loh, 2015/10/13 00:30:03]

?

[rwlbuis, 2015/10/13 02:27:02]

Columns did not use ShorthandScope, causing the output in
inspector/elements/styles-2/media-emulation.html to only state the longhands. I
am ok either way, i.e. removing above line and using the new expected result.

+        return consumeColumns(important);
+    }
     default:
         m_currentShorthand = oldShorthand;
         return false;