Move size property 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 aaec7b836d0bf41a224eb9fc49b96912aba03c01..12667b09ba29cc4f6261ec25e255f8ac1b396b80 100644
--- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -641,6 +641,76 @@ static PassRefPtrWillBeRawPtr<CSSValueList> consumeRotation(CSSParserTokenRange&
     return list.release();
 }
 
+enum class SizeParameter {
+    None,
+    Auto,
+    Length,
+    PageSize,
+    Orientation,
+};
+
+static SizeParameter parseSizeParameter(CSSParserTokenRange& range, CSSParserMode cssParserMode, CSSValueList* parsedValues, SizeParameter prevParamType)
+{
+    switch (range.peek().id()) {
+    case CSSValueAuto:
+        if (prevParamType == SizeParameter::None) {
+            parsedValues->append(consumeIdent(range));
+            return SizeParameter::Auto;
+        }
+        return SizeParameter::None;
+    case CSSValueLandscape:
+    case CSSValuePortrait:
+        if (prevParamType == SizeParameter::None || prevParamType == SizeParameter::PageSize) {
+            parsedValues->append(consumeIdent(range));
+            return SizeParameter::Orientation;
+        }
+        return SizeParameter::None;
+    case CSSValueA3:
+    case CSSValueA4:
+    case CSSValueA5:
+    case CSSValueB4:
+    case CSSValueB5:
+    case CSSValueLedger:
+    case CSSValueLegal:
+    case CSSValueLetter:
+        if (prevParamType == SizeParameter::None || prevParamType == SizeParameter::Orientation) {
+            // Normalize to Page Size then Orientation order by prepending for simpler StyleBuilder handling
+            parsedValues->prepend(consumeIdent(range));
+            return SizeParameter::PageSize;
+        }
+        return SizeParameter::None;
+    case 0: {
+        RefPtrWillBeRawPtr<CSSValue> parsedValue = consumeLength(range, cssParserMode, ValueRangeNonNegative);
+        if (parsedValue && (prevParamType == SizeParameter::None || prevParamType == SizeParameter::Length)) {
+            parsedValues->append(parsedValue.release());
+            return SizeParameter::Length;
+        }
+        return SizeParameter::None;
+    }
+    default:
+        return SizeParameter::None;
+    }
+}
+
+static PassRefPtrWillBeRawPtr<CSSValueList> consumeSize(CSSParserTokenRange& range, CSSParserMode cssParserMode)
+{
+    RefPtrWillBeRawPtr<CSSValueList> parsedValues = CSSValueList::createSpaceSeparated();
+
+    // First parameter.

[Timothy Loh, 2015/10/08 23:36:38]

This can be lots simpler I think. Sorry for code dump, I wasn't sure how to word
this in text.

if (range.peek().id() == CSSValueAuto) {
    result->append(consumeIdent(range));
    return result.release();
}

if (RefPtrWillBeRawPtr<CSSValue> width = consumeLength(range, cssParserMode,
ValueRangeNonNegative)) {
    RefPtrWillBeRawPtr<CSSValue> height = consumeLength(range, cssParserMode,
ValueRangeNonNegative);
    if (!height)
        height = width;
    result->append(width.release());
    result->append(height.release());
}

RefPtrWillBeRawPtr<CSSValue> pageSize = consumePageSize(range);
RefPtrWillBeRawPtr<CSSValue> orientation = nullptr;
if (range.peek().id() == CSSValuePortrait || range.peek().id() ==
CSSValueLandscape)
    orientation = consumeIdent(range);
if (!pageSize)
    pageSize = consumePageSize(range);

if (!orientation && !pageSize)
    return nullptr;
if (pageSize)
    result->append(pageSize.release());
if (orientation)
    result->append(orientation.release());
return result.release();


(I'm thinking of making a templated consumeIdent which will make a whole bunch
of code a little nicer, e.g. consumeIdent<CSSValuePortrait, CSSValueLandscape>)

[rwlbuis, 2015/10/12 17:13:19]

I like it! But the part where height is set to width in case only one length is
given leads to a test difference. Also I think if we'd do that, then we could
remove some code in applyValueCSSPropertySize, so it sounds like a bigger
change...

+    SizeParameter paramType = parseSizeParameter(range, cssParserMode, parsedValues.get(), SizeParameter::None);
+    if (paramType == SizeParameter::None)
+        return nullptr;
+
+    // Second parameter, if any.
+    if (!range.atEnd()) {
+        paramType = parseSizeParameter(range, cssParserMode, parsedValues.get(), paramType);
+        if (paramType == SizeParameter::None)
+            return nullptr;
+    }
+
+    return parsedValues.release();
+}
+
 PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSPropertyID propId)
 {
     m_range.consumeWhitespace();
@@ -677,6 +747,8 @@ PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSProperty
     case CSSPropertyWebkitBorderHorizontalSpacing:
     case CSSPropertyWebkitBorderVerticalSpacing:
         return consumeLength(m_range, m_context.mode(), ValueRangeNonNegative);
+    case CSSPropertySize:
+        return consumeSize(m_range, m_context.mode());
     default:
         return nullptr;
     }