[css-typed-om] Handle out-of-range values correctly.

Currently, when we set width to something like -1, we would get an
invalid value for width. According to the spec [1], we should be
wrapping the invalid value in a calc.

This patch wraps invalid values in calc, and changes the test harness
to allow us to test this.

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

third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-width.html

@@ -21,10 +21,23 @@ for (const suffix of ['top', 'left', 'right', 'bottom']) {
   runPropertyTests('border-' + suffix + '-width', [
     // Computed value is 0 when border-style is 'none'.
     // FIXME: Add separate test where border-style is not 'none' or 'hidden'.
-    { syntax: 'thin', computed: assert_is_zero_px },
-    { syntax: 'medium', computed: assert_is_zero_px },
-    { syntax: 'thick', computed: assert_is_zero_px },
-    { syntax: '<length>' },
+    {
+      syntax: 'thin',
+      computed: (_, result) => assert_is_zero_px(result)
+    },
+    {
+      syntax: 'medium',
+      computed: (_, result) => assert_is_zero_px(result)
+    },
+    {
+      syntax: 'thick',
+      computed: (_, result) => assert_is_zero_px(result)
+    },
+    {
+      syntax: '<length>',
+      specified: assert_is_equal_with_range_handling,
+      computed: (_, result) => assert_is_zero_px(result)
+    },
   ]);
 }
 

third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/height.html

@@ -15,8 +15,8 @@
 
 runPropertyTests('height', [
   { syntax: 'auto' },
-  { syntax: '<percentage>' },
-  { syntax: '<length>' },
+  { syntax: '<percentage>', specified: assert_is_equal_with_range_handling },
+  { syntax: '<length>', specified: assert_is_equal_with_range_handling },
 ]);
 
 </script>

third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/padding.html

@@ -15,8 +15,14 @@
 
 for (const suffix of ['top', 'left', 'right', 'bottom']) {
   runPropertyTests('padding-' + suffix, [
-    { syntax: '<percentage>' },
-    { syntax: '<length>' },
+    {
+      syntax: '<percentage>',
+      specified: assert_is_equal_with_range_handling
+    },
+    {
+      syntax: '<length>',
+      specified: assert_is_equal_with_range_handling
+    },
   ]);
 }
 

third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/resources/testsuite.js

@@ -9,6 +9,13 @@ function assert_is_calc_sum(result) {
     'specified calc must be a CSSMathSum');
 }
 
+function assert_is_equal_with_range_handling(input, result) {
+  if (input instanceof CSSUnitValue && input.value < 0)
+    assert_style_value_equals(result, new CSSMathSum(input));
+  else
+    assert_style_value_equals(result, input);
+}
+
 const gTestSyntaxExamples = {
   '<length>': {
     description: 'a length',
@@ -21,21 +28,21 @@ const gTestSyntaxExamples = {
         description: "a negative em",
         input: new CSSUnitValue(-3.14, 'em'),
         // 'ems' are relative units, so just check that it computes to px
-        defaultComputed: result => assert_is_unit('px', result)
+        defaultComputed: (_, result) => assert_is_unit('px', result)
       },
       {
         description: "a positive cm",
         input: new CSSUnitValue(3.14, 'cm'),
         // 'cms' are relative units, so just check that it computes to px
-        defaultComputed: result => assert_is_unit('px', result)
+        defaultComputed: (_, result) => assert_is_unit('px', result)
       },
       {
         description: "a calc length",
         input: new CSSMathSum(new CSSUnitValue(0, 'px'), new CSSUnitValue(0, 'em')),
         // Specified/computed calcs are usually simplified.
         // FIXME: Test this properly
-        defaultSpecified: assert_is_calc_sum,
-        defaultComputed: result => assert_is_unit('px', result)
+        defaultSpecified: (_, result) => assert_is_calc_sum(result),
+        defaultComputed: (_, result) => assert_is_unit('px', result)
       }
     ],
   },
@@ -59,8 +66,8 @@ const gTestSyntaxExamples = {
         input: new CSSMathSum(new CSSUnitValue(0, 'percent'), new CSSUnitValue(0, 'percent')),
         // Specified/computed calcs are usually simplified.
         // FIXME: Test this properly
-        defaultSpecified: assert_is_calc_sum,
-        defaultComputed: result => assert_is_unit('percent', result)
+        defaultSpecified: (_, result) => assert_is_calc_sum(result),
+        defaultComputed: (_, result) => assert_is_unit('percent', result)
       }
     ],
   },
@@ -84,8 +91,8 @@ const gTestSyntaxExamples = {
         input: new CSSMathSum(new CSSUnitValue(0, 's'), new CSSUnitValue(0, 'ms')),
         // Specified/computed calcs are usually simplified.
         // FIXME: Test this properly
-        defaultSpecified: assert_is_calc_sum,
-        defaultComputed: result => assert_is_unit('s', result)
+        defaultSpecified: (_, result) => assert_is_calc_sum(result),
+        defaultComputed: (_, result) => assert_is_unit('s', result)
       }
     ],
   },
@@ -104,7 +111,7 @@ const gTestSyntaxExamples = {
       {
         description: "a PNG image",
         input: new CSSURLImageValue('/media/1x1.png'),
-        defaultComputed: result => {
+        defaultComputed: (_, result) => {
           // URLs compute to absolute URLs
           assert_true(result instanceof CSSURLImageValue,
             'Computed value should be a CSSURLImageValue');
@@ -143,7 +150,7 @@ function testPropertyValid(propertyName, examples, specified, computed, descript
       // specified style
       const specifiedResult = element.attributeStyleMap.get(propertyName);
       if (specified || example.defaultSpecified) {
-        (specified || example.defaultSpecified)(specifiedResult);
+        (specified || example.defaultSpecified)(example.input, specifiedResult);
       } else {
         assert_not_equals(specifiedResult, null,
           'Specified value must not be null');
@@ -156,7 +163,7 @@ function testPropertyValid(propertyName, examples, specified, computed, descript
       // computed style
       const computedResult = element.computedStyleMap().get(propertyName);
       if (computed || example.defaultComputed) {
-        (computed || example.defaultComputed)(computedResult);
+        (computed || example.defaultComputed)(example.input, computedResult);
       } else {
         assert_not_equals(computedResult, null,
           'Computed value must not be null');
@@ -196,12 +203,20 @@ function createKeywordExample(keyword) {
 // }
 //
 // If a callback is passed to |specified|, then the callback will be passed
-// the result of calling get() on the inline style map (specified values).
+// two arguments:
+// 1. The input test case
+// 2. The result of calling get() on the inline style map (specified values).
+//
 // The callback should check if the result is expected using assert_* functions.
 // If no callback is passed, then we assert that the result is the same as
 // the input.
 //
 // Same goes for |computed|, but with the computed style map (computed values).
+//
+// FIXME: The reason we pass argument #2 is that it's sometimes difficult to
+// compute exactly what the expected result should be (e.g. browser-specific
+// values). Once we can do that, we can remove argument #2 and just return
+// the expected result.
 function runPropertyTests(propertyName, testCases) {
   let syntaxTested = new Set();
 

third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/width.html

@@ -15,8 +15,14 @@
 
 runPropertyTests('width', [
   { syntax: 'auto' },
-  { syntax: '<percentage>' },
-  { syntax: '<length>' },
+  {
+    syntax: '<percentage>',
+    specified: assert_is_equal_with_range_handling
+  },
+  {
+    syntax: '<length>',
+    specified: assert_is_equal_with_range_handling
+  },
 ]);
 
 </script>

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

@@ -64,6 +64,7 @@ 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 {
     return ToCSSValue();
   }

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

@@ -5,6 +5,7 @@
 #include "core/css/cssom/CSSUnitValue.h"
 
 #include "bindings/core/v8/ExceptionState.h"
+#include "core/animation/LengthPropertyFunctions.h"
 #include "core/css/CSSCalculationValue.h"
 #include "core/css/CSSResolutionUnits.h"
 #include "core/css/cssom/CSSMathInvert.h"
@@ -13,6 +14,7 @@
 #include "core/css/cssom/CSSMathProduct.h"
 #include "core/css/cssom/CSSMathSum.h"
 #include "core/css/cssom/CSSNumericSumValue.h"
+#include "core/css/properties/CSSProperty.h"
 #include "platform/wtf/MathExtras.h"
 
 namespace blink {
@@ -117,6 +119,22 @@ const CSSPrimitiveValue* CSSUnitValue::ToCSSValue() const {
   return CSSPrimitiveValue::Create(value_, unit_);
 }
 
+const CSSPrimitiveValue* CSSUnitValue::ToCSSValueWithProperty(
+    CSSPropertyID property_id) const {
+  // 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)) ==
+          kValueRangeNonNegative &&
+      value_ < 0) {
+    // Wrap out of range values with a calc.
+    CSSCalcExpressionNode* node = ToCalcExpressionNode();
+    node->SetIsNestedCalc();
+    return CSSPrimitiveValue::Create(CSSCalcValue::Create(node));
+  }
+
+  return CSSPrimitiveValue::Create(value_, unit_);
+}
+
 CSSCalcExpressionNode* CSSUnitValue::ToCalcExpressionNode() const {
   return CSSCalcValue::CreateExpressionNode(
       CSSPrimitiveValue::Create(value_, unit_));

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

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