Move viewport descriptor handling 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 "config.h"
 #include "core/css/parser/CSSPropertyParser.h"
 
 #include "core/StylePropertyShorthand.h"
 #include "core/css/CSSCalculationValue.h"
 #include "core/css/CSSCustomIdentValue.h"
@@ skipping 34 matching lines @@
 
     CSSParserValueList valueList(range);
     if (!valueList.size())
         return false; // Parser error
     CSSPropertyParser parser(&valueList, range, context, parsedProperties, ruleType);
     CSSPropertyID resolvedProperty = resolveCSSPropertyID(unresolvedProperty);
     bool parseSuccess;
 
     if (ruleType == StyleRule::Viewport) {
         parseSuccess = (RuntimeEnabledFeatures::cssViewportEnabled() || isUASheetBehavior(context.mode()))
-            && parser.parseViewportProperty(resolvedProperty, important);
+            && parser.parseViewportDescriptor(resolvedProperty, important);
     } else if (ruleType == StyleRule::FontFace) {
         parseSuccess = parser.parseFontFaceDescriptor(resolvedProperty);
     } else {
         parseSuccess = parser.parseValue(unresolvedProperty, important);
     }
 
     // This doesn't count UA style sheets
     if (parseSuccess && context.useCounter())
         context.useCounter()->count(context, unresolvedProperty);
 
@@ skipping 185 matching lines @@
             || (valueRange == ValueRangeNonNegative && token.numericValue() < 0))
             return nullptr;
         return cssValuePool().createValue(range.consumeIncludingWhitespace().numericValue(), CSSPrimitiveValue::UnitType::Pixels);
     }
     CalcParser calcParser(range, valueRange);
     if (calcParser.value() && calcParser.value()->category() == CalcLength)
         return calcParser.consumeValue();
     return nullptr;
 }
 
-static PassRefPtrWillBeRawPtr<CSSPrimitiveValue> consumeLengthOrPercent(CSSParserTokenRange& range, CSSParserMode cssParserMode, ValueRange valueRange, UnitlessQuirk unitless = UnitlessQuirk::Forbid)
+static PassRefPtrWillBeRawPtr<CSSPrimitiveValue> consumePercent(CSSParserTokenRange& range, ValueRange valueRange)
 {
     const CSSParserToken& token = range.peek();
-    if (token.type() == DimensionToken || token.type() == NumberToken)
-        return consumeLength(range, cssParserMode, valueRange, unitless);
     if (token.type() == PercentageToken) {
         if (valueRange == ValueRangeNonNegative && token.numericValue() < 0)
             return nullptr;
         return cssValuePool().createValue(range.consumeIncludingWhitespace().numericValue(), CSSPrimitiveValue::UnitType::Percentage);
     }
     CalcParser calcParser(range, valueRange);
     if (const CSSCalcValue* calculation = calcParser.value()) {
+        if (calculation->category() == CalcPercent)
+            return calcParser.consumeValue();
+    }
+    return nullptr;
+}
+
+static inline bool isCSSWideKeyword(const CSSValueID& id)

[Timothy Loh, 2015/10/08 01:05:21]

not sure if this was supposed to be moved?

+{
+    return id == CSSValueInitial || id == CSSValueInherit || id == CSSValueUnset || id == CSSValueDefault;
+}
+
+static PassRefPtrWillBeRawPtr<CSSPrimitiveValue> consumeLengthOrPercent(CSSParserTokenRange& range, CSSParserMode cssParserMode, ValueRange valueRange, UnitlessQuirk unitless = UnitlessQuirk::Forbid)
+{
+    const CSSParserToken& token = range.peek();
+    if (token.type() == DimensionToken || token.type() == NumberToken)
+        return consumeLength(range, cssParserMode, valueRange, unitless);
+    if (token.type() == PercentageToken)
+        return consumePercent(range, valueRange);
+    CalcParser calcParser(range, valueRange);
+    if (const CSSCalcValue* calculation = calcParser.value()) {
         if (calculation->category() == CalcLength || calculation->category() == CalcPercent || calculation->category() == CalcPercentLength)
             return calcParser.consumeValue();
     }
     return nullptr;
 }
 
 static PassRefPtrWillBeRawPtr<CSSPrimitiveValue> consumeAngle(CSSParserTokenRange& range)
 {
     const CSSParserToken& token = range.peek();
     if (token.type() == DimensionToken) {
         switch (token.unitType()) {
         case CSSPrimitiveValue::UnitType::Degrees:
         case CSSPrimitiveValue::UnitType::Radians:
         case CSSPrimitiveValue::UnitType::Gradians:
         case CSSPrimitiveValue::UnitType::Turns:
             return cssValuePool().createValue(range.consumeIncludingWhitespace().numericValue(), token.unitType());
         default:
             return nullptr;
         }
     }
     CalcParser calcParser(range, ValueRangeAll);
     if (const CSSCalcValue* calculation = calcParser.value()) {
         if (calculation->category() == CalcAngle)
             return calcParser.consumeValue();
     }
     return nullptr;
 }
 
-static inline bool isCSSWideKeyword(const CSSValueID& id)
-{
-    return id == CSSValueInitial || id == CSSValueInherit || id == CSSValueUnset || id == CSSValueDefault;
-}
-
 // 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();
     }
 
     // Every comma-separated list of identifiers is a valid will-change value,
@@ skipping 585 matching lines @@
     RefPtrWillBeRawPtr<CSSValue> verticalSpacing = horizontalSpacing;
     if (!m_range.atEnd())
         verticalSpacing = consumeLength(m_range, m_context.mode(), ValueRangeNonNegative, UnitlessQuirk::Allow);
     if (!verticalSpacing || !m_range.atEnd())
         return false;
     addProperty(CSSPropertyWebkitBorderHorizontalSpacing, horizontalSpacing.release(), important);
     addProperty(CSSPropertyWebkitBorderVerticalSpacing, verticalSpacing.release(), important);
     return true;
 }
 
+static PassRefPtrWillBeRawPtr<CSSValue> consumeSingleViewportDescriptor(CSSParserTokenRange& range, CSSPropertyID propId, CSSParserMode cssParserMode)
+{
+    CSSValueID id = range.peek().id();
+    switch (propId) {
+    case CSSPropertyMinWidth:
+    case CSSPropertyMaxWidth:
+    case CSSPropertyMinHeight:
+    case CSSPropertyMaxHeight:
+        if (id == CSSValueAuto || id == CSSValueInternalExtendToZoom)
+            return consumeIdent(range);
+        return consumeLengthOrPercent(range, cssParserMode, ValueRangeNonNegative);
+    case CSSPropertyMinZoom:
+    case CSSPropertyMaxZoom:
+    case CSSPropertyZoom: {
+        if (id == CSSValueAuto)
+            return consumeIdent(range);
+        RefPtrWillBeRawPtr<CSSValue> parsedValue = consumeNumber(range, ValueRangeNonNegative);

[Timothy Loh, 2015/10/08 01:05:21]

I prefer:

if (RefPtrWillBeRawPtr<CSSValue> parsedValue = consumeNumber(range,
ValueRangeNonNegative))
    return parsedValue.release();
return consumePercent(Range, ValueRangeNonNegative);

+        if (!parsedValue)
+            parsedValue = consumePercent(range, ValueRangeNonNegative);
+        return parsedValue.release();
+    }
+    case CSSPropertyUserZoom:
+        if (id == CSSValueZoom || id == CSSValueFixed)
+            return consumeIdent(range);
+        break;
+    case CSSPropertyOrientation:
+        if (id == CSSValueAuto || id == CSSValuePortrait || id == CSSValueLandscape)
+            return consumeIdent(range);
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+        break;
+    }
+
+    return nullptr;
+}
+
+bool CSSPropertyParser::parseViewportDescriptor(CSSPropertyID propId, bool important)
+{
+    ASSERT(RuntimeEnabledFeatures::cssViewportEnabled() || isUASheetBehavior(m_context.mode()));
+
+    m_range.consumeWhitespace();

[Timothy Loh, 2015/10/08 01:05:21]

We should probably do this up front somewhere so it doesn't need to be in
parseViewportDescriptor/parseFontFaceDescriptor/parseShorthand/parseSingleValue,
but there's no reason for that to be in this patch.

+    RefPtrWillBeRawPtr<CSSValue> parsedValue = nullptr;
+
+    switch (propId) {
+    case CSSPropertyWidth: {
+        RefPtrWillBeRawPtr<CSSValue> minWidth = consumeSingleViewportDescriptor(m_range, CSSPropertyMinWidth, m_context.mode());
+        if (!minWidth)
+            return false;
+        RefPtrWillBeRawPtr<CSSValue> maxWidth = minWidth;
+        if (!m_range.atEnd())
+            maxWidth = consumeSingleViewportDescriptor(m_range, CSSPropertyMaxWidth, m_context.mode());
+        if (!maxWidth || !m_range.atEnd())
+            return false;
+        addProperty(CSSPropertyMinWidth, minWidth.release(), important);
+        addProperty(CSSPropertyMaxWidth, maxWidth.release(), important);
+        return true;
+    }
+    case CSSPropertyHeight: {
+        RefPtrWillBeRawPtr<CSSValue> minHeight = consumeSingleViewportDescriptor(m_range, CSSPropertyMinHeight, m_context.mode());
+        if (!minHeight)
+            return false;
+        RefPtrWillBeRawPtr<CSSValue> maxHeight = minHeight;
+        if (!m_range.atEnd())
+            maxHeight = consumeSingleViewportDescriptor(m_range, CSSPropertyMaxHeight, m_context.mode());
+        if (!maxHeight || !m_range.atEnd())
+            return false;
+        addProperty(CSSPropertyMinHeight, minHeight.release(), important);
+        addProperty(CSSPropertyMaxHeight, maxHeight.release(), important);
+        return true;
+    }
+    case CSSPropertyMinWidth:
+    case CSSPropertyMaxWidth:
+    case CSSPropertyMinHeight:
+    case CSSPropertyMaxHeight:
+    case CSSPropertyMinZoom:
+    case CSSPropertyMaxZoom:
+    case CSSPropertyZoom:
+    case CSSPropertyUserZoom:
+    case CSSPropertyOrientation:
+        parsedValue = consumeSingleViewportDescriptor(m_range, propId, m_context.mode());

[Timothy Loh, 2015/10/08 01:05:21]

if we only set parsedValue here, we should just move the addProperty logic and
variable declaration into here

+        break;
+    default:
+        return false;
+    }
+
+    if (!parsedValue || !m_range.atEnd())
+        return false;
+
+    addProperty(propId, parsedValue.release(), important);
+    return true;
+}
+
 bool CSSPropertyParser::parseShorthand(CSSPropertyID propId, bool important)
 {
     m_range.consumeWhitespace();
     CSSPropertyID oldShorthand = m_currentShorthand;
     // TODO(rob.buis): Remove this when the legacy property parser is gone
     m_currentShorthand = propId;
     switch (propId) {
     case CSSPropertyWebkitMarginCollapse: {
         CSSValueID id = m_range.consumeIncludingWhitespace().id();
         if (!CSSParserFastPaths::isValidKeywordPropertyAndValue(CSSPropertyWebkitMarginBeforeCollapse, id))
@@ skipping 40 matching lines @@
     }
     case CSSPropertyBorderSpacing:
         return consumeBorderSpacing(important);
     default:
         m_currentShorthand = oldShorthand;
         return false;
     }
 }
 
 } // namespace blink