[css-properties-values-api] Support calc() values for <integer>.

For custom properties registered with <integer>, it should according to
CSS Values and Units Level 4 be possible to apply non-integral values,
and then have the value round to the nearest integer computed value time.

To do this, this CL ...:

 * Changes the parsing of <integer> values such that non-integral calc()
   expressions are allowed.
 * Recognizes such calc()-expressions computed-value time, resolves them,
   and rounds them to the nearest integer.
 * Wraps values such as CSS.number(1.5) in calc() if necessary, when
   setting such values using Typed OM.

R=futhark@chromium.org

Bug: 641877
Change-Id: I2d4d7c4d72c9b2069f2fe10b1be1b4b94d5900e2
Reviewed-on: https://chromium-review.googlesource.com/c/1280663
Commit-Queue: Anders Ruud <andruud@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600716}

third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/register-property-syntax-parsing-expected.txt

 This is a testharness.js-based test.
-Found 126 tests; 125 PASS, 1 FAIL, 0 TIMEOUT, 0 NOTRUN.
+Found 130 tests; 129 PASS, 1 FAIL, 0 TIMEOUT, 0 NOTRUN.
 PASS syntax:'*', initialValue:'a' is valid
 PASS syntax:' * ', initialValue:'b' is valid
 PASS syntax:'<length>', initialValue:'2px' is valid
@@ -30,6 +30,10 @@ PASS syntax:'<number>', initialValue:'-109' is valid
 PASS syntax:'<number>', initialValue:'2.3e4' is valid
 PASS syntax:'<integer>', initialValue:'-109' is valid
 PASS syntax:'<integer>', initialValue:'19' is valid
+PASS syntax:'<integer>', initialValue:'calc(1)' is valid
+PASS syntax:'<integer>', initialValue:'calc(1 + 2)' is valid
+PASS syntax:'<integer>', initialValue:'calc(3.1415)' is valid
+PASS syntax:'<integer>', initialValue:'calc(3.1415 + 3.1415)' is valid
 PASS syntax:'<angle>', initialValue:'10deg' is valid
 PASS syntax:'<angle>', initialValue:'20.5rad' is valid
 PASS syntax:'<angle>', initialValue:'calc(50grad + 3.14159rad)' is valid

third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/register-property-syntax-parsing.html

@@ -55,6 +55,10 @@ assert_valid("<number>", "-109");
 assert_valid("<number>", "2.3e4");
 assert_valid("<integer>", "-109");
 assert_valid("<integer>", "19");
+assert_valid("<integer>", "calc(1)");
+assert_valid("<integer>", "calc(1 + 2)");
+assert_valid("<integer>", "calc(3.1415)");
+assert_valid("<integer>", "calc(3.1415 + 3.1415)");
 
 assert_valid("<angle>", "10deg");
 assert_valid("<angle>", "20.5rad");

third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/registered-property-computation.html

@@ -111,5 +111,17 @@ for (let element of [divWithFontSizeSet, divWithFontSizeInherited]) {
         assert_computed_value('<transform-function>', 'translateX(calc(11em + 10%))', 'translateX(calc(110px + 10%))');
         assert_computed_value('<transform-function>+', 'translateX(10%) scale(2)', 'translateX(10%) scale(2)');
     }, "<transform-function> values are computed correctly for " + id);
+
+    test(function() {
+        assert_computed_value('<integer>', '15', '15');
+        assert_computed_value('<integer>', 'calc(15 + 15)', '30');
+        assert_computed_value('<integer>', 'calc(2.4)', '2');
+        assert_computed_value('<integer>', 'calc(2.6)', '3');
+        assert_computed_value('<integer>', 'calc(2.6 + 3.1)', '6');
+    }, "<integer> values are computed correctly for " + id);
+
+    test(function() {
+        assert_computed_value('<integer>+', '15 calc(2.4) calc(2.6)', '15 2 3');
+    }, "<integer>+ values are computed correctly for " + id);
 }
 </script>

third_party/WebKit/LayoutTests/external/wpt/css/css-properties-values-api/typedom.tentative.html

@@ -382,8 +382,8 @@ test_style_property_map_set({
 test_style_property_map_set({
     syntax: '<integer>',
     initialValue: '0',
-    shouldAccept: [CSS.number(1), CSS.number(-42), '1', '-42'],
-    shouldReject: [unparsed('42'), CSS.px(100), '50px', [CSS.number(42), '42']],
+    shouldAccept: [CSS.number(1), CSS.number(-42), '1', '-42', 'calc(2.4)'],
+    shouldReject: [unparsed('42'), CSS.px(100), '50px', [CSS.number(42), '42'], 'calc(2px + 1px)'],
 });
 
 test_style_property_map_set({
@@ -482,8 +482,8 @@ test_style_property_map_set({
 test_style_property_map_set({
     syntax: '<integer>+',
     initialValue: '0',
-    shouldAccept: [CSS.number(42), [CSS.number(42), '42'], '42 42'],
-    shouldReject: [[CSS.number(42), keyword('noint')], '42 noint'],
+    shouldAccept: [CSS.number(42), [CSS.number(42), '42'], '42 42', 'calc(2.4) calc(2.6)'],
+    shouldReject: [[CSS.number(42), keyword('noint')], '42 noint', 'calc(2px + 2px)'],
 });
 
 test_style_property_map_set({

third_party/blink/renderer/build/scripts/core/css/templates/cssom_types.cc.tmpl

@@ -93,7 +93,8 @@ bool CSSOMTypes::IsPropertySupported(CSSPropertyID id) {
 bool CSSOMTypes::PropertyCanTake(CSSPropertyID id,
                                  const AtomicString& custom_property_name,
                                  const PropertyRegistration* registration,
-                                 const CSSStyleValue& value) {
+                                 const CSSStyleValue& value,
+                                 const CSSSyntaxComponent*& match) {
   DCHECK_EQ(id == CSSPropertyVariable, !custom_property_name.IsNull());
 
   if (id == CSSPropertyVariable && registration) {
@@ -101,7 +102,8 @@ bool CSSOMTypes::PropertyCanTake(CSSPropertyID id,
       return ToCSSUnsupportedStyleValue(value).GetCustomPropertyName() ==
           custom_property_name;
     }
-    return registration->Syntax().CanTake(value);
+    match = registration->Syntax().Match(value);
+    return match != nullptr;
   }
 
   if (value.GetType() == CSSStyleValue::kKeywordType) {

third_party/blink/renderer/core/css/css_syntax_descriptor.cc

@@ -179,7 +179,7 @@ const CSSValue* ConsumeSingleType(const CSSSyntaxComponent& syntax,
     case CSSSyntaxType::kUrl:
       return ConsumeUrl(range, context);
     case CSSSyntaxType::kInteger:
-      return ConsumeInteger(range);
+      return ConsumeIntegerOrNumberCalc(range);
     case CSSSyntaxType::kAngle:
       return ConsumeAngle(range, context, base::Optional<WebFeature>());
     case CSSSyntaxType::kTime:
@@ -278,12 +278,17 @@ bool CSSSyntaxComponent::CanTake(const CSSStyleValue& value) const {
   }
 }
 
-bool CSSSyntaxDescriptor::CanTake(const CSSStyleValue& value) const {
+const CSSSyntaxComponent* CSSSyntaxDescriptor::Match(
+    const CSSStyleValue& value) const {
   for (const CSSSyntaxComponent& component : syntax_components_) {
     if (component.CanTake(value))
-      return true;
+      return &component;
   }
-  return false;
+  return nullptr;
+}
+
+bool CSSSyntaxDescriptor::CanTake(const CSSStyleValue& value) const {
+  return Match(value);
 }
 
 const CSSValue* CSSSyntaxDescriptor::Parse(CSSParserTokenRange range,

third_party/blink/renderer/core/css/css_syntax_descriptor.h

@@ -49,6 +49,7 @@ class CSSSyntaxComponent {
   const String& GetString() const { return string_; }
   CSSSyntaxRepeat GetRepeat() const { return repeat_; }
   bool IsRepeatable() const { return repeat_ != CSSSyntaxRepeat::kNone; }
+  bool IsInteger() const { return type_ == CSSSyntaxType::kInteger; }
   char Separator() const {
     DCHECK(IsRepeatable());
     return repeat_ == CSSSyntaxRepeat::kSpaceSeparated ? ' ' : ',';
@@ -69,6 +70,7 @@ class CORE_EXPORT CSSSyntaxDescriptor {
   const CSSValue* Parse(CSSParserTokenRange,
                         const CSSParserContext*,
                         bool is_animation_tainted) const;
+  const CSSSyntaxComponent* Match(const CSSStyleValue&) const;
   bool CanTake(const CSSStyleValue&) const;
   bool IsValid() const { return !syntax_components_.IsEmpty(); }
   bool IsTokenStream() const {

third_party/blink/renderer/core/css/cssom/css_style_value.h

@@ -15,6 +15,7 @@
 
 namespace blink {
 
+class CSSSyntaxComponent;
 class ExceptionState;
 class ExecutionContext;
 enum class SecureContextMode;
@@ -66,7 +67,9 @@ class CORE_EXPORT CSSStyleValue : public ScriptWrappable {
 
   virtual const CSSValue* ToCSSValue() const = 0;
   // FIXME: We should make this a method on CSSProperty instead.
-  virtual const CSSValue* ToCSSValueWithProperty(CSSPropertyID) const {
+  virtual const CSSValue* ToCSSValueWithProperty(
+      CSSPropertyID,
+      const CSSSyntaxComponent*) const {
     return ToCSSValue();
   }
   virtual String toString() const;

third_party/blink/renderer/core/css/cssom/css_unit_value.cc

@@ -7,6 +7,7 @@
 #include "third_party/blink/renderer/core/animation/length_property_functions.h"
 #include "third_party/blink/renderer/core/css/css_calculation_value.h"
 #include "third_party/blink/renderer/core/css/css_resolution_units.h"
+#include "third_party/blink/renderer/core/css/css_syntax_descriptor.h"
 #include "third_party/blink/renderer/core/css/cssom/css_math_invert.h"
 #include "third_party/blink/renderer/core/css/cssom/css_math_max.h"
 #include "third_party/blink/renderer/core/css/cssom/css_math_min.h"
@@ -37,7 +38,8 @@ CSSPrimitiveValue::UnitType ToCanonicalUnitIfPossible(
 
 bool IsValueOutOfRangeForProperty(CSSPropertyID property_id,
                                   double value,
-                                  CSSPrimitiveValue::UnitType unit) {
+                                  CSSPrimitiveValue::UnitType unit,
+                                  const CSSSyntaxComponent* match) {
   // FIXME: Avoid this CSSProperty::Get call as it can be costly.
   // The caller often has a CSSProperty already, so we can just pass it here.
   if (LengthPropertyFunctions::GetValueRange(CSSProperty::Get(property_id)) ==
@@ -47,6 +49,10 @@ bool IsValueOutOfRangeForProperty(CSSPropertyID property_id,
 
   // For non-length properties and special cases.
   switch (property_id) {
+    case CSSPropertyVariable:
+      if (match && match->IsInteger())
+        return round(value) != value;
+      return false;
     case CSSPropertyOrder:
     case CSSPropertyZIndex:
       return round(value) != value;
@@ -167,8 +173,9 @@ const CSSPrimitiveValue* CSSUnitValue::ToCSSValue() const {
 }
 
 const CSSPrimitiveValue* CSSUnitValue::ToCSSValueWithProperty(
-    CSSPropertyID property_id) const {
-  if (IsValueOutOfRangeForProperty(property_id, value_, unit_)) {
+    CSSPropertyID property_id,
+    const CSSSyntaxComponent* match) const {
+  if (IsValueOutOfRangeForProperty(property_id, value_, unit_, match)) {
     // Wrap out of range values with a calc.
     CSSCalcExpressionNode* node = ToCalcExpressionNode();
     node->SetIsNestedCalc();

third_party/blink/renderer/core/css/cssom/css_unit_value.h

@@ -46,7 +46,9 @@ class CORE_EXPORT CSSUnitValue final : public CSSNumericValue {
   // From CSSStyleValue.
   StyleValueType GetType() const final;
   const CSSPrimitiveValue* ToCSSValue() const final;
-  const CSSPrimitiveValue* ToCSSValueWithProperty(CSSPropertyID) const final;
+  const CSSPrimitiveValue* ToCSSValueWithProperty(
+      CSSPropertyID,
+      const CSSSyntaxComponent*) const final;
   CSSCalcExpressionNode* ToCalcExpressionNode() const final;
 
  private:

third_party/blink/renderer/core/css/cssom/cssom_types.h

@@ -33,10 +33,15 @@ class CSSOMTypes {
   static bool IsCSSStyleValuePosition(const CSSStyleValue&);
 
   static bool IsPropertySupported(CSSPropertyID);
+  // For registered custom properties, if the CSSStyleValue is accepted
+  // because it matches the registered grammar (and not because it is
+  // a CSSUnsupportedStyleValue with matching name), 'match' will be set
+  // to the component that was matched.
   static bool PropertyCanTake(CSSPropertyID,
                               const AtomicString& custom_property_name,
                               const PropertyRegistration*,
-                              const CSSStyleValue&);
+                              const CSSStyleValue&,
+                              const CSSSyntaxComponent*& match);
 };
 
 }  // namespace blink

third_party/blink/renderer/core/css/cssom/style_property_map.cc

@@ -53,6 +53,16 @@ CSSValueList* CssValueListForPropertyID(CSSPropertyID property_id) {
   }
 }
 
+String StyleValueToString(const CSSProperty& property,
+                          const CSSStyleValue& style_value,
+                          const CSSSyntaxComponent* syntax_component) {
+  if (style_value.GetType() == CSSStyleValue::kUnknownType)
+    return style_value.toString();
+  return style_value
+      .ToCSSValueWithProperty(property.PropertyID(), syntax_component)
+      ->CssText();
+}
+
 const CSSVariableReferenceValue* CreateVariableReferenceValue(
     const String& value,
     const CSSParserContext& context) {
@@ -78,14 +88,16 @@ const CSSVariableReferenceValue* CreateVariableReferenceValue(
   StringBuilder builder;
 
   for (const auto& value : values) {
+    const CSSSyntaxComponent* syntax_component = nullptr;
+
     if (!CSSOMTypes::PropertyCanTake(property.PropertyID(),
                                      custom_property_name, &registration,
-                                     *value)) {
+                                     *value, syntax_component)) {
       return nullptr;
     }
     if (!builder.IsEmpty())
       builder.Append(separator);
-    builder.Append(value->toString());
+    builder.Append(StyleValueToString(property, *value, syntax_component));
   }
 
   return CreateVariableReferenceValue(builder.ToString(), context);
@@ -100,9 +112,12 @@ const CSSValue* StyleValueToCSSValue(
   DCHECK_EQ(property.IDEquals(CSSPropertyVariable),
             !custom_property_name.IsNull());
 
+  const CSSSyntaxComponent* syntax_component = nullptr;
+
   const CSSPropertyID property_id = property.PropertyID();
   if (!CSSOMTypes::PropertyCanTake(property_id, custom_property_name,
-                                   registration, style_value)) {
+                                   registration, style_value,
+                                   syntax_component)) {
     return nullptr;
   }
 
@@ -123,7 +138,9 @@ const CSSValue* StyleValueToCSSValue(
       if (registration &&
           style_value.GetType() != CSSStyleValue::kUnparsedType) {
         CSSParserContext* context = CSSParserContext::Create(execution_context);
-        return CreateVariableReferenceValue(style_value.toString(), *context);
+        String string =
+            StyleValueToString(property, style_value, syntax_component);
+        return CreateVariableReferenceValue(string, *context);
       }
       break;
     case CSSPropertyBorderBottomLeftRadius:
@@ -246,7 +263,7 @@ const CSSValue* StyleValueToCSSValue(
       break;
   }
 
-  return style_value.ToCSSValueWithProperty(property_id);
+  return style_value.ToCSSValueWithProperty(property_id, syntax_component);
 }
 
 const CSSValue* CoerceStyleValueOrString(
@@ -342,8 +359,10 @@ void StylePropertyMap::set(const ExecutionContext* execution_context,
     String css_text;
     if (values[0].IsCSSStyleValue()) {
       CSSStyleValue* style_value = values[0].GetAsCSSStyleValue();
-      if (style_value && CSSOMTypes::PropertyCanTake(property_id, g_null_atom,
-                                                     nullptr, *style_value)) {
+      const CSSSyntaxComponent* syntax_component = nullptr;
+      if (style_value &&
+          CSSOMTypes::PropertyCanTake(property_id, g_null_atom, nullptr,
+                                      *style_value, syntax_component)) {
         css_text = style_value->toString();
       }
     } else {

third_party/blink/renderer/core/css/parser/css_property_parser_helpers.cc

@@ -245,6 +245,30 @@ CSSPrimitiveValue* ConsumeInteger(CSSParserTokenRange& range,
   return nullptr;
 }
 
+// This implements the behavior defined in [1], where calc() expressions
+// are valid when <integer> is expected, even if the calc()-expression does
+// not result in an integral value.
+//
+// TODO(andruud): Eventually this behavior should just be part of
+// ConsumeInteger, and this function can be removed. For now, having a separate
+// function with this behavior allows us to implement [1] gradually.
+//
+// [1] https://drafts.csswg.org/css-values-4/#calc-type-checking
+CSSPrimitiveValue* ConsumeIntegerOrNumberCalc(CSSParserTokenRange& range) {
+  CSSParserTokenRange int_range(range);
+  if (CSSPrimitiveValue* value = ConsumeInteger(int_range)) {
+    range = int_range;
+    return value;
+  }
+  CalcParser calc_parser(range);
+  if (const CSSCalcValue* calculation = calc_parser.Value()) {
+    if (calculation->Category() != kCalcNumber)
+      return nullptr;
+    return calc_parser.ConsumeValue();
+  }
+  return nullptr;
+}
+
 CSSPrimitiveValue* ConsumePositiveInteger(CSSParserTokenRange& range) {
   return ConsumeInteger(range, 1);
 }

third_party/blink/renderer/core/css/parser/css_property_parser_helpers.h

@@ -45,6 +45,7 @@ enum class UnitlessQuirk { kAllow, kForbid };
 CSSPrimitiveValue* ConsumeInteger(
     CSSParserTokenRange&,
     double minimum_value = -std::numeric_limits<double>::max());
+CSSPrimitiveValue* ConsumeIntegerOrNumberCalc(CSSParserTokenRange&);
 CSSPrimitiveValue* ConsumePositiveInteger(CSSParserTokenRange&);
 bool ConsumeNumberRaw(CSSParserTokenRange&, double& result);
 CSSPrimitiveValue* ConsumeNumber(CSSParserTokenRange&, ValueRange);

third_party/blink/renderer/core/css/resolver/style_builder_converter.cc

@@ -1701,6 +1701,18 @@ static const CSSValue& ComputeRegisteredPropertyValue(
           css_to_length_conversion_data.CopyWithAdjustedZoom(1));
       return *CSSPrimitiveValue::Create(length, 1);
     }
+    // If we encounter a calculated number that was not resolved during
+    // parsing, it means that a calc()-expression was allowed in place of
+    // an integer. Such calc()-for-integers must be rounded at computed value
+    // time.
+    // https://drafts.csswg.org/css-values-4/#calc-type-checking
+    if (primitive_value.IsCalculated() &&
+        (primitive_value.TypeWithCalcResolved() ==
+         CSSPrimitiveValue::UnitType::kNumber)) {
+      double double_value = primitive_value.CssCalcValue()->DoubleValue();
+      auto unit_type = CSSPrimitiveValue::UnitType::kInteger;
+      return *CSSPrimitiveValue::Create(std::round(double_value), unit_type);
+    }
   }
   return value;
 }