[Typed OM] Add CSSValue -> CSSNumberValue conversion

third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.cpp

Index: third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.cpp
diff --git a/third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.cpp b/third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.cpp
index 215887e5eeffbbc952ce234576e62f4eddd24744..934dae48a4c180188ca7e00308abada3e3024a42 100644
--- a/third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.cpp
+++ b/third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.cpp
@@ -25,6 +25,35 @@ CSSValue* styleValueToCSSValue(CSSPropertyID propertyID, const CSSStyleValue& st
     return styleValue.toCSSValueWithProperty(propertyID);
 }
 
+CSSValue* singleStyleValueAsCSSValue(CSSPropertyID propertyID, const CSSStyleValue& styleValue)
+{
+    if (!CSSPropertyMetadata::propertySupportsMultiple(propertyID))
+        return styleValueToCSSValue(propertyID, styleValue);
+
+    CSSValue* cssValue = styleValueToCSSValue(propertyID, styleValue);
+    if (!cssValue)
+        return nullptr;
+
+    // TODO(meade): This won't always work. Figure out what kind of CSSValueList to create properly.
+    CSSValueList* valueList = CSSValueList::createSpaceSeparated();
+    valueList->append(*cssValue);
+    return valueList;
+}
+
+CSSValueList* asCSSValueList(CSSPropertyID propertyID, const CSSStyleValueVector& styleValueVector)
+{
+    // TODO(meade): This won't always work. Figure out what kind of CSSValueList to create properly.
+    CSSValueList* valueList = CSSValueList::createSpaceSeparated();
+    for (const CSSStyleValue* value : styleValueVector) {
+        CSSValue* cssValue = styleValueToCSSValue(propertyID, *value);
+        if (!cssValue) {
+            return nullptr;
+        }
+        valueList->append(*cssValue);
+    }
+    return valueList;
+}
+
 } // namespace
 
 CSSStyleValueVector InlineStylePropertyMap::getAllInternal(CSSPropertyID propertyID)
@@ -58,38 +87,27 @@ Vector<String> InlineStylePropertyMap::getProperties()
 
 void InlineStylePropertyMap::set(CSSPropertyID propertyID, CSSStyleValueOrCSSStyleValueSequenceOrString& item, ExceptionState& exceptionState)
 {
+    CSSValue* cssValue = nullptr;
     if (item.isCSSStyleValue()) {
-        CSSValue* cssValue = styleValueToCSSValue(propertyID, *item.getAsCSSStyleValue());
-        if (!cssValue) {
-            exceptionState.throwTypeError("Invalid type for property");
-            return;
-        }
-        m_ownerElement->setInlineStyleProperty(propertyID, cssValue);
+        cssValue = singleStyleValueAsCSSValue(propertyID, *item.getAsCSSStyleValue());
     } else if (item.isCSSStyleValueSequence()) {
         if (!CSSPropertyMetadata::propertySupportsMultiple(propertyID)) {
             exceptionState.throwTypeError("Property does not support multiple values");
             return;
         }
-
-        // TODO(meade): This won't always work. Figure out what kind of CSSValueList to create properly.
-        CSSValueList* valueList = CSSValueList::createSpaceSeparated();
-        CSSStyleValueVector styleValueVector = item.getAsCSSStyleValueSequence();
-        for (const Member<CSSStyleValue> value : styleValueVector) {
-            CSSValue* cssValue = styleValueToCSSValue(propertyID, *value);
-            if (!cssValue) {
-                exceptionState.throwTypeError("Invalid type for property");
-                return;
-            }
-            valueList->append(*cssValue);
-        }
-
-        m_ownerElement->setInlineStyleProperty(propertyID, valueList);
+        cssValue = asCSSValueList(propertyID, item.getAsCSSStyleValueSequence());
     } else {
         // Parse it.
         DCHECK(item.isString());
         // TODO(meade): Implement this.
         exceptionState.throwTypeError("Not implemented yet");
+        return;
+    }
+    if (!cssValue) {
+        exceptionState.throwTypeError("Invalid type for property");
+        return;
     }
+    m_ownerElement->setInlineStyleProperty(propertyID, cssValue);
 }
 
 void InlineStylePropertyMap::append(CSSPropertyID propertyID, CSSStyleValueOrCSSStyleValueSequenceOrString& item, ExceptionState& exceptionState)
@@ -101,7 +119,10 @@ void InlineStylePropertyMap::append(CSSPropertyID propertyID, CSSStyleValueOrCSS
 
     const CSSValue* cssValue = m_ownerElement->ensureMutableInlineStyle().getPropertyCSSValue(propertyID);
     CSSValueList* cssValueList = nullptr;
-    if (cssValue->isValueList()) {
+    if (!cssValue) {
+        // TODO(meade): Figure out the correct separator.

[sashab, 2016/07/11 23:47:22]

There's a lot of these TODOs to figure out the correct behavior... Should you
have more ASSERTs or should we be erroring out in these cases? Is there an
alternative way we could implement this so more info is passed through?

[meade_UTC10, 2016/07/27 07:14:29]

So these are interesting - we don't check the separator when this is used
internally for calculating styles (so there is no visual effect here), but the
generated text is wrong. We definitely shouldn't ASSERT or error out here, it's
just not-quite-right yet.
I have a plan to fix this, but it involves changing CSSProperties.in to store
which separator is used. It's a decent sized change. Though, you might find that
change useful for your stuff eventually too :)

[sashab, 2016/07/27 23:23:57]

Ok, that sounds good, but I would make the TODO more descriptive then, like:
// TODO(meade): Store the correct separator for each property in
CSSProperties.in and use it to choose the list type.

[meade_UTC10, 2016/08/03 03:26:44]

Done.

+        cssValueList = CSSValueList::createSpaceSeparated();
+    } else if (cssValue->isValueList()) {
         cssValueList = toCSSValueList(cssValue)->copy();
     } else {
         // TODO(meade): Figure out what the correct behaviour here is.