Split CSSPrimitiveValue into CSSPrimitiveValue and CSSIdentifierValue

third_party/WebKit/Source/core/css/resolver/ViewportStyleResolver.cpp

Index: third_party/WebKit/Source/core/css/resolver/ViewportStyleResolver.cpp
diff --git a/third_party/WebKit/Source/core/css/resolver/ViewportStyleResolver.cpp b/third_party/WebKit/Source/core/css/resolver/ViewportStyleResolver.cpp
index 3f248994d466458ee3368f57787a1095980c3979..673a1f3e7d632df2453062b4c01e784db3a6ceb9 100644
--- a/third_party/WebKit/Source/core/css/resolver/ViewportStyleResolver.cpp
+++ b/third_party/WebKit/Source/core/css/resolver/ViewportStyleResolver.cpp
@@ -147,9 +147,28 @@ float ViewportStyleResolver::viewportArgumentValue(CSSPropertyID id) const
         defaultValue = 1;
 
     const CSSValue* value = m_propertySet->getPropertyCSSValue(id);
-    if (!value || !value->isPrimitiveValue())
+    if (!value || !value->isPrimitiveValue() || !value->isIdentifierValue())

[Timothy Loh, 2016/09/21 07:55:06]

This logic (and below in the file) is wrong since now the if condition will
always be false.

[sashab, 2016/09/23 01:01:10]

Ack, I hate these confusing if-statements!! I want to write a patch to remove
them all. Fixed.

         return defaultValue;
 
+    if (value->isIdentifierValue()) {
+        switch (toCSSIdentifierValue(value)->getValueID()) {
+        case CSSValueAuto:
+            return defaultValue;
+        case CSSValueLandscape:
+            return ViewportDescription::ValueLandscape;
+        case CSSValuePortrait:
+            return ViewportDescription::ValuePortrait;
+        case CSSValueZoom:
+            return defaultValue;
+        case CSSValueInternalExtendToZoom:
+            return ViewportDescription::ValueExtendToZoom;
+        case CSSValueFixed:
+            return 0;
+        default:
+            return defaultValue;
+        }
+    }
+
     const CSSPrimitiveValue* primitiveValue = toCSSPrimitiveValue(value);
 
     if (primitiveValue->isNumber() || primitiveValue->isPx())
@@ -166,27 +185,13 @@ float ViewportStyleResolver::viewportArgumentValue(CSSPropertyID id) const
         case CSSPropertyZoom:
             return percentValue;
         default:
-            ASSERT_NOT_REACHED();
+            NOTREACHED();
             break;
         }
     }
 
-    switch (primitiveValue->getValueID()) {
-    case CSSValueAuto:
-        return defaultValue;
-    case CSSValueLandscape:
-        return ViewportDescription::ValueLandscape;
-    case CSSValuePortrait:
-        return ViewportDescription::ValuePortrait;
-    case CSSValueZoom:
-        return defaultValue;
-    case CSSValueInternalExtendToZoom:
-        return ViewportDescription::ValueExtendToZoom;
-    case CSSValueFixed:
-        return 0;
-    default:
-        return defaultValue;
-    }
+    NOTREACHED();
+    return defaultValue;
 }
 
 Length ViewportStyleResolver::viewportLengthValue(CSSPropertyID id) const
@@ -197,14 +202,18 @@ Length ViewportStyleResolver::viewportLengthValue(CSSPropertyID id) const
         || id == CSSPropertyMinWidth);
 
     const CSSValue* value = m_propertySet->getPropertyCSSValue(id);
-    if (!value || !value->isPrimitiveValue())
+    if (!value || !value->isPrimitiveValue() || !value->isIdentifierValue())
         return Length(); // auto
 
-    const CSSPrimitiveValue* primitiveValue = toCSSPrimitiveValue(value);
-
-    if (primitiveValue->getValueID() == CSSValueInternalExtendToZoom)
-        return Length(ExtendToZoom);
+    if (value->isIdentifierValue()) {
+        CSSValueID valueID = toCSSIdentifierValue(value)->getValueID();
+        if (valueID == CSSValueInternalExtendToZoom)
+            return Length(ExtendToZoom);
+        if (valueID == CSSValueAuto)
+            return Length(Auto);
+    }
 
+    const CSSPrimitiveValue* primitiveValue = toCSSPrimitiveValue(value);
     ComputedStyle* documentStyle = m_document->mutableComputedStyle();
 
     // If we have viewport units the conversion will mark the document style as having viewport units.
@@ -214,9 +223,6 @@ Length ViewportStyleResolver::viewportLengthValue(CSSPropertyID id) const
     CSSToLengthConversionData::FontSizes fontSizes(documentStyle, documentStyle);
     CSSToLengthConversionData::ViewportSize viewportSize(m_document->layoutViewItem());
 
-    if (primitiveValue->getValueID() == CSSValueAuto)
-        return Length(Auto);
-
     Length result = primitiveValue->convertToLength(CSSToLengthConversionData(documentStyle, fontSizes, viewportSize, 1.0f));
     if (documentStyle->hasViewportUnits())
         m_document->setHasViewportUnits();