[css-typed-om] Allow StylePropertyMaps to take string arguments.

Currently StylePropertyMaps don't support strings e.g.

  styleMap.set('width', CSS.px(1)) works, but
  styleMap.set('width', '1px') doesn't.

This patch adds string support by moving parsing code to
StyleValueFactory so that both CSSStyleValue.parse and
StylePropertyMaps can share the parsing logic.

We also do some refactoring to make handling both CSSStyleValue
and strings easier.

There are still some code paths that we're not sure is correct,
pending clarification on GitHub:
https://github.com/w3c/css-houdini-drafts/issues/512

Spec: https://drafts.css-houdini.org/css-typed-om-1/#the-stylepropertymap

Bug: 545318
Change-Id: Iaef3cfad69789fc115b7e98a296eec5cb4480cd8
Reviewed-on: https://chromium-review.googlesource.com/769797
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518443}

third_party/WebKit/LayoutTests/typedcssom/inlinestyle/properties/animation-direction-expected.txt

-This is a testharness.js-based test.
-PASS Setting animation-direction to normal
-PASS Setting animation-direction to reverse
-PASS Setting animation-direction to alternate
-PASS Setting animation-direction to alternate-reverse
-PASS Setting animation-direction to initial
-PASS Setting animation-direction to inherit
-PASS Setting animation-direction to unset
-PASS Setting animation-direction to invalid value CSSUnitValue "4px" throws
-PASS Setting animation-direction to invalid value null throws
-PASS Setting animation-direction to invalid value undefined throws
-PASS Setting animation-direction to invalid value true throws
-PASS Setting animation-direction to invalid value false throws
-PASS Setting animation-direction to invalid value 1 throws
-PASS Setting animation-direction to invalid value hello throws
-PASS Setting animation-direction to invalid value [object Object] throws
-PASS Setting animation-direction to invalid value CSSKeywordValue "notAKeyword" throws
-PASS Getting animation-direction when it is set to normal
-PASS Getting animation-direction when it is set to reverse
-PASS Getting animation-direction when it is set to alternate
-PASS Getting animation-direction when it is set to alternate-reverse
-PASS Getting animation-direction when it is set to initial
-PASS Getting animation-direction when it is set to inherit
-PASS Getting animation-direction when it is set to unset
-PASS getAll for single-valued animation-direction
-FAIL getAll for list-valued animation-direction Failed to execute 'set' on 'StylePropertyMap': Not implemented yet
-PASS Delete animation-direction removes the value from the styleMap
-PASS animation-direction shows up in getProperties
-FAIL Set animation-direction to a sequence Failed to execute 'set' on 'StylePropertyMap': Not implemented yet
-PASS Set animation-direction to a sequence containing an invalid type
-PASS Appending a CSSKeywordValue to animation-direction
-FAIL Append a sequence to animation-direction Failed to execute 'append' on 'StylePropertyMap': Not implemented yet
-PASS Appending an invalid value to animation-direction
-PASS Append a sequence containing an invalid value to animation-direction
-Harness: the test ran to completion.
-

third_party/WebKit/LayoutTests/typedcssom/inlinestyle/properties/property-suite.js

@@ -153,7 +153,7 @@ function runSequenceSetterTests(
     propertyName, validObject, invalidObject, element) {
   test(function() {
     element.style = '';
-    element.attributeStyleMap.set(propertyName, [validObject, validObject]);
+    element.attributeStyleMap.set(propertyName, validObject, validObject);
     assert_equals(
         element.style[propertyName], validObject.toString() + ', ' +
         validObject.toString());
@@ -165,9 +165,8 @@ function runSequenceSetterTests(
   }, 'Set ' + propertyName + ' to a sequence');
 
   test(function() {
-    let sequence = [validObject, invalidObject];
     assert_throws(new TypeError(), function() {
-      element.attributeStyleMap.set(propertyName, sequence);
+      element.attributeStyleMap.set(propertyName, validObject, invalidObject);
     });
   }, 'Set ' + propertyName + ' to a sequence containing an invalid type');
 }
@@ -194,7 +193,7 @@ function runAppendTests(
   test(function() {
     element.style = '';
 
-    element.attributeStyleMap.append(propertyName, [validObject, validObject]);
+    element.attributeStyleMap.append(propertyName, validObject, validObject);
     assert_equals(
         element.style[propertyName], validObject.toString() + ', ' +
         validObject.toString());
@@ -213,9 +212,8 @@ function runAppendTests(
   }, 'Appending an invalid value to ' + propertyName);
 
   test(function() {
-    let sequence = [validObject, invalidObject];
     assert_throws(new TypeError(), function() {
-      element.attributeStyleMap.append(propertyName, sequence);
+      element.attributeStyleMap.append(propertyName, validObject, invalidObject);
     });
   }, 'Append a sequence containing an invalid value to ' + propertyName);
 }
@@ -239,7 +237,7 @@ function runGetAllTests(
   if (supportsMultiple) {
     test(function() {
       element.style = '';
-      element.attributeStyleMap.set(propertyName, [validObject, validObject]);
+      element.attributeStyleMap.set(propertyName, validObject, validObject);
       let result = element.attributeStyleMap.getAll(propertyName);
       assert_equals(result.length, 2,
           'Expected getAll to return an array containing two instances ' +
@@ -285,7 +283,7 @@ function runMultipleValuesNotSupportedTests(
   test(function() {
     element.style = '';
     assert_throws(new TypeError(), function() {
-      element.attributeStyleMap.set(propertyName, [validObject, validObject]);
+      element.attributeStyleMap.set(propertyName, validObject, validObject);
     });
   }, 'Setting ' + propertyName + ' to a sequence throws');
 

third_party/WebKit/LayoutTests/typedcssom/the-stylepropertymap/append-expected.txt

-This is a testharness.js-based test.
-PASS Calling StylePropertyMap.append with an unsupported property name throws TypeError
-PASS Calling StylePropertyMap.append with an null property name throws TypeError
-PASS Calling StylePropertyMap.append with a property that is not list valued throws TypeError
-PASS Calling StylePropertyMap.append with an invalid CSSStyleValue throws TypeError
-PASS Calling StylePropertyMap.append with an invalid String value throws TypeError
-PASS Calling StylePropertyMap.append with a mix of valid and invalid values throws TypeError
-FAIL Appending a list-valued property with CSSStyleValue or String updates its values Failed to execute 'append' on 'StylePropertyMap': Invalid type for property
-FAIL StylePropertyMap.append is case-insensitive Failed to execute 'append' on 'StylePropertyMap': Not implemented yet
-Harness: the test ran to completion.
-

third_party/WebKit/LayoutTests/typedcssom/the-stylepropertymap/set-expected.txt

@@ -3,9 +3,9 @@ PASS Setting a StylePropertyMap with an unsupported property name throws TypeErr
 PASS Setting a StylePropertyMap with an null property name throws TypeError
 PASS Setting a StylePropertyMap with an invalid CSSStyleValue throws TypeError
 PASS Setting a StylePropertyMap with an invalid String throws TypeError
-FAIL Setting a property with CSSStyleValue or String updates its value Failed to execute 'set' on 'StylePropertyMap': Not implemented yet
-FAIL Setting a list-valued property with CSSStyleValue or String updates its values Failed to execute 'set' on 'StylePropertyMap': Invalid type for property
+PASS Setting a property with CSSStyleValue or String updates its value
+PASS Setting a list-valued property with CSSStyleValue or String updates its values
 FAIL Setting a custom property with CSSStyleValue or String updates its value Failed to execute 'set' on 'StylePropertyMap': Invalid propertyName: --foo
-FAIL StylePropertyMap.set is case-insensitive Failed to execute 'set' on 'StylePropertyMap': Not implemented yet
+PASS StylePropertyMap.set is case-insensitive
 Harness: the test ran to completion.
 

third_party/WebKit/Source/core/css/CSSProperties.json5

@@ -368,6 +368,7 @@
       name: "animation-duration",
       property_class: "Duration",
       property_methods: ["ParseSingleValue"],
+      typedom_types: ["Time"],
       separator: ",",
       custom_apply_functions_all: true,
       priority: "Animation",
@@ -420,6 +421,7 @@
       name: "transition-duration",
       property_class: "Duration",
       property_methods: ["ParseSingleValue"],
+      typedom_types: ["Time"],
       separator: ",",
       custom_apply_functions_all: true,
       priority: "Animation",

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

@@ -9,7 +9,7 @@
 #include "bindings/core/v8/ToV8ForCore.h"
 #include "core/StylePropertyShorthand.h"
 #include "core/css/cssom/StyleValueFactory.h"
-#include "core/css/parser/CSSParser.h"
+#include "core/css/properties/CSSProperty.h"
 
 namespace blink {
 
@@ -34,13 +34,9 @@ CSSStyleValueVector ParseCSSStyleValue(
     return CSSStyleValueVector();
   }
 
-  // TODO(crbug.com/783031): This should probably use an existing parser context
-  // (e.g. from execution context) to parse relative URLs correctly.
-  const CSSValue* css_value = CSSParser::ParseSingleValue(
-      property_id, value,
-      StrictCSSParserContext(execution_context->SecureContextMode()));
-
-  if (!css_value) {
+  const auto style_values = StyleValueFactory::FromString(
+      property_id, value, execution_context->SecureContextMode());
+  if (style_values.IsEmpty()) {
     exception_state.ThrowDOMException(
         kSyntaxError, "The value provided ('" + value +
                           "') could not be parsed as a '" + property_name +
@@ -48,10 +44,7 @@ CSSStyleValueVector ParseCSSStyleValue(
     return CSSStyleValueVector();
   }
 
-  CSSStyleValueVector style_value_vector =
-      StyleValueFactory::CssValueToStyleValueVector(property_id, *css_value);
-  DCHECK(!style_value_vector.IsEmpty());
-  return style_value_vector;
+  return style_values;
 }
 
 }  // namespace

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

@@ -16,6 +16,7 @@
 #include "core/css/cssom/StyleValueFactory.h"
 #include "core/css/properties/CSSProperty.h"
 
+#include "core/css/cssom/CSSKeywordValue.h"
 namespace blink {
 
 namespace {
@@ -43,22 +44,25 @@ const CSSValue* StyleValueToCSSValue(CSSPropertyID property_id,
   return style_value.ToCSSValueWithProperty(property_id, secure_context_mode);
 }
 
-const CSSValue* SingleStyleValueAsCSSValue(
+const CSSValue* CoerceStyleValueOrStringToCSSValue(
     CSSPropertyID property_id,
-    const CSSStyleValue& style_value,
+    const CSSStyleValueOrString& value,
     SecureContextMode secure_context_mode) {
-  const CSSValue* css_value =
-      StyleValueToCSSValue(property_id, style_value, secure_context_mode);
-  if (!css_value)
-    return nullptr;
+  if (value.IsCSSStyleValue()) {
+    if (!value.GetAsCSSStyleValue())
+      return nullptr;
 
-  if (!CSSProperty::Get(property_id).IsRepeated() ||
-      css_value->IsCSSWideKeyword())
-    return css_value;
+    return StyleValueToCSSValue(property_id, *value.GetAsCSSStyleValue(),
+                                secure_context_mode);
+  }
 
-  CSSValueList* value_list = CssValueListForPropertyID(property_id);
-  value_list->Append(*css_value);
-  return value_list;
+  DCHECK(value.IsString());
+  const auto values = StyleValueFactory::FromString(
+      property_id, value.GetAsString(), secure_context_mode);
+  // TODO(785132): What should we do here?
+  if (values.size() != 1)
+    return nullptr;
+  return StyleValueToCSSValue(property_id, *values[0], secure_context_mode);
 }
 
 }  // namespace
@@ -112,25 +116,38 @@ void InlineStylePropertyMap::set(const ExecutionContext* execution_context,
   if (values.IsEmpty())
     return;
 
-  // TODO(545318): Implement correctly for both list and non-list properties
-  const auto& item = values[0];
-  const CSSValue* css_value = nullptr;
-  if (item.IsCSSStyleValue()) {
-    css_value =
-        SingleStyleValueAsCSSValue(property_id, *item.GetAsCSSStyleValue(),
-                                   execution_context->SecureContextMode());
+  if (CSSProperty::Get(property_id).IsRepeated()) {
+    CSSValueList* result = CssValueListForPropertyID(property_id);
+    for (const auto& value : values) {
+      const CSSValue* css_value = CoerceStyleValueOrStringToCSSValue(
+          property_id, value, execution_context->SecureContextMode());
+      if (!css_value || (css_value->IsCSSWideKeyword() && values.size() > 1)) {
+        exception_state.ThrowTypeError("Invalid type for property");
+        return;
+      }
+      result->Append(*css_value);
+    }
+
+    if (result->length() == 1 && result->Item(0).IsCSSWideKeyword())
+      owner_element_->SetInlineStyleProperty(property_id, &result->Item(0));
+    else
+      owner_element_->SetInlineStyleProperty(property_id, result);
   } else {
-    // Parse it.
-    DCHECK(item.IsString());
-    // TODO(meade): Implement this.
-    exception_state.ThrowTypeError("Not implemented yet");
-    return;
-  }
-  if (!css_value) {
-    exception_state.ThrowTypeError("Invalid type for property");
-    return;
+    if (values.size() != 1) {
+      // FIXME: Is this actually the correct behaviour?
+      exception_state.ThrowTypeError("Not supported");
+      return;
+    }
+
+    const CSSValue* result = CoerceStyleValueOrStringToCSSValue(
+        property_id, values[0], execution_context->SecureContextMode());
+    if (!result) {
+      exception_state.ThrowTypeError("Invalid type for property");
+      return;
+    }
+
+    owner_element_->SetInlineStyleProperty(property_id, result);
   }
-  owner_element_->SetInlineStyleProperty(property_id, css_value);
 }
 
 void InlineStylePropertyMap::append(const ExecutionContext* execution_context,
@@ -152,27 +169,19 @@ void InlineStylePropertyMap::append(const ExecutionContext* execution_context,
     css_value_list = ToCSSValueList(css_value)->Copy();
   } else {
     // TODO(meade): Figure out what the correct behaviour here is.
+    NOTREACHED();
     exception_state.ThrowTypeError("Property is not already list valued");
     return;
   }
 
-  for (auto& item : values) {
-    if (item.IsCSSStyleValue()) {
-      const CSSValue* css_value =
-          StyleValueToCSSValue(property_id, *item.GetAsCSSStyleValue(),
-                               execution_context->SecureContextMode());
-      if (!css_value) {
-        exception_state.ThrowTypeError("Invalid type for property");
-        return;
-      }
-      css_value_list->Append(*css_value);
-    } else {
-      // Parse it.
-      DCHECK(item.IsString());
-      // TODO(meade): Implement this.
-      exception_state.ThrowTypeError("Not implemented yet");
+  for (auto& value : values) {
+    const CSSValue* css_value = CoerceStyleValueOrStringToCSSValue(
+        property_id, value, execution_context->SecureContextMode());
+    if (!css_value) {
+      exception_state.ThrowTypeError("Invalid type for property");
       return;
     }
+    css_value_list->Append(*css_value);
   }
 
   owner_element_->SetInlineStyleProperty(property_id, css_value_list);

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

@@ -15,6 +15,8 @@
 #include "core/css/cssom/CSSURLImageValue.h"
 #include "core/css/cssom/CSSUnparsedValue.h"
 #include "core/css/cssom/CSSUnsupportedStyleValue.h"
+#include "core/css/parser/CSSParser.h"
+#include "core/css/properties/CSSProperty.h"
 
 namespace blink {
 
@@ -65,6 +67,31 @@ CSSStyleValueVector UnsupportedCSSValue(const CSSValue& value) {
 
 }  // namespace
 
+CSSStyleValueVector StyleValueFactory::FromString(
+    CSSPropertyID property_id,
+    const String& value,
+    SecureContextMode secure_context_mode) {
+  DCHECK_NE(property_id, CSSPropertyInvalid);
+  DCHECK(!CSSProperty::Get(property_id).IsShorthand());
+
+  // TODO(775804): Handle custom properties
+  if (property_id == CSSPropertyVariable) {
+    return CSSStyleValueVector();
+  }
+
+  // TODO(crbug.com/783031): This should probably use an existing parser context
+  // (e.g. from execution context) to parse relative URLs correctly.
+  const CSSValue* css_value = CSSParser::ParseSingleValue(
+      property_id, value, StrictCSSParserContext(secure_context_mode));
+  if (!css_value)
+    return CSSStyleValueVector();
+
+  CSSStyleValueVector style_value_vector =
+      StyleValueFactory::CssValueToStyleValueVector(property_id, *css_value);
+  DCHECK(!style_value_vector.IsEmpty());
+  return style_value_vector;
+}
+
 CSSStyleValueVector StyleValueFactory::CssValueToStyleValueVector(
     CSSPropertyID property_id,
     const CSSValue& css_value) {

third_party/WebKit/Source/core/css/cssom/StyleValueFactory.h

@@ -17,6 +17,9 @@ class CORE_EXPORT StyleValueFactory {
   STATIC_ONLY(StyleValueFactory);
 
  public:
+  static CSSStyleValueVector FromString(CSSPropertyID,
+                                        const String&,
+                                        SecureContextMode);
   static CSSStyleValueVector CssValueToStyleValueVector(CSSPropertyID,
                                                         const CSSValue&);
   // If you don't have complex CSS properties, use this one.