[css-typed-om] Add support for border properties.

border-radius-* tests are failing because the computed value should always be a pair of values, but Blink currently returns a CSSValuePair with kDropIdenticalValue, so we sometimes compute to a single value. Fixing this requires either changing computed border-radius-* to use kKeepIdenticalValue, or overriding the computed value just for Typed OM [1] [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp?q=ComputedStylePropertyMap.cpp&dr&l=229 Bug: 820299 Change-Id: I4c67a7986a8e77d37e7936eabfa0b68b32de9df8 Reviewed-on: https://chromium-review.googlesource.com/994433 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#548697}

[css-typed-om] Add support for border properties.
border-radius-* tests are failing because the computed value should always be a pair of values, but Blink currently returns a CSSValuePair with kDropIdenticalValue, so we sometimes compute to a single value. Fixing this requires either changing computed border-radius-* to use kKeepIdenticalValue, or overriding the computed value just for Typed OM [1] [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp?q=ComputedStylePropertyMap.cpp&dr&l=229 Bug: 820299 Change-Id: I4c67a7986a8e77d37e7936eabfa0b68b32de9df8 Reviewed-on: https://chromium-review.googlesource.com/994433 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#548697}
db520aaa · Darren Shen · Commit Bot · 4123a5e9 · db520aaa · db520aaa
Commit db520aaa authored Apr 06, 2018 by Darren Shen Committed by Commit Bot Apr 06, 2018
11 changed files
--- a/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-image-outset.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-image-outset.html
+<!doctype html>
+<meta charset="utf-8">
+<title>'border-image-outset' property</title>
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../../resources/testhelper.js"></script>
+<script src="resources/testsuite.js"></script>
+<body>
+<div id="log"></div>
+<script>
+'use strict';
+
+runPropertyTests('border-image-outset', [
+  // Computed value is always four values, which are not supported in
+  // Typed OM level 1.
+  {
+    syntax: '<length>',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+  {
+    syntax: '<number>',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+]);
+
+runUnsupportedPropertyTests('border-image-outset', [
+  '1 1.2', '30px 2 45px', '7px 12px 14px 5px'
+]);
+
+</script>
--- a/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-image-repeat.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-image-repeat.html
+<!doctype html>
+<meta charset="utf-8">
+<title>'border-image-repeat' property</title>
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../../resources/testhelper.js"></script>
+<script src="resources/testsuite.js"></script>
+<body>
+<div id="log"></div>
+<script>
+'use strict';
+
+runPropertyTests('border-image-repeat', [
+  // Computed value is always a pair of values, which are not supported in
+  // Typed OM level 1.
+  {
+    syntax: 'stretch',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+  {
+    syntax: 'repeat',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+  {
+    syntax: 'round',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+  {
+    syntax: 'space',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+]);
+
+runUnsupportedPropertyTests('border-image-repeat', [
+  'stretch repeat', 'round space'
+]);
+
+</script>
--- a/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-image-slice.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-image-slice.html
+<!doctype html>
+<meta charset="utf-8">
+<title>'border-image-slice' property</title>
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../../resources/testhelper.js"></script>
+<script src="resources/testsuite.js"></script>
+<body>
+<div id="log"></div>
+<script>
+'use strict';
+
+runPropertyTests('border-image-slice', [
+  // Computed value is always four values, which are not supported in
+  // Typed OM level 1.
+  {
+    syntax: '<number>',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+  {
+    syntax: '<percentage>',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+]);
+
+runUnsupportedPropertyTests('border-image-slice', [
+  '30 fill', '30 40 50', '30 40 50 60 fill'
+]);
+
+</script>
--- a/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-image-width.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-image-width.html
+<!doctype html>
+<meta charset="utf-8">
+<title>'border-image-width' property</title>
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../../resources/testhelper.js"></script>
+<script src="resources/testsuite.js"></script>
+<body>
+<div id="log"></div>
+<script>
+'use strict';
+
+runPropertyTests('border-image-width', [
+  // Computed value is always four values, which are not supported in
+  // Typed OM level 1.
+  {
+    syntax: '<length>',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+  {
+    syntax: '<percentage>',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+  {
+    syntax: '<number>',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+  {
+    syntax: 'auto',
+    computed: (_, result) => assert_is_unsupported(result)
+  },
+]);
+
+runUnsupportedPropertyTests('border-image-width', [
+  '2em 3em', '5% 15% 10%', '5% 2em 10% auto'
+]);
+
+</script>
--- a/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-radius-expected.txt
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-radius-expected.txt
+This is a testharness.js-based test.
+PASS Can set 'border-top-left-radius' to CSS-wide keywords
+PASS Can set 'border-top-left-radius' to var() references
+FAIL Can set 'border-top-left-radius' to a length assert_equals: expected "[object CSSStyleValue]" but got "[object CSSUnitValue]"
+FAIL Can set 'border-top-left-radius' to a percent assert_equals: expected "[object CSSStyleValue]" but got "[object CSSUnitValue]"
+PASS Setting 'border-top-left-radius' to a time throws TypeError
+PASS Setting 'border-top-left-radius' to a flexible length throws TypeError
+PASS Setting 'border-top-left-radius' to a number throws TypeError
+PASS Setting 'border-top-left-radius' to a position throws TypeError
+PASS Setting 'border-top-left-radius' to a URL throws TypeError
+PASS Setting 'border-top-left-radius' to a transform throws TypeError
+PASS Can set 'border-top-right-radius' to CSS-wide keywords
+PASS Can set 'border-top-right-radius' to var() references
+FAIL Can set 'border-top-right-radius' to a length assert_equals: expected "[object CSSStyleValue]" but got "[object CSSUnitValue]"
+FAIL Can set 'border-top-right-radius' to a percent assert_equals: expected "[object CSSStyleValue]" but got "[object CSSUnitValue]"
+PASS Setting 'border-top-right-radius' to a time throws TypeError
+PASS Setting 'border-top-right-radius' to a flexible length throws TypeError
+PASS Setting 'border-top-right-radius' to a number throws TypeError
+PASS Setting 'border-top-right-radius' to a position throws TypeError
+PASS Setting 'border-top-right-radius' to a URL throws TypeError
+PASS Setting 'border-top-right-radius' to a transform throws TypeError
+PASS Can set 'border-bottom-left-radius' to CSS-wide keywords
+PASS Can set 'border-bottom-left-radius' to var() references
+FAIL Can set 'border-bottom-left-radius' to a length assert_equals: expected "[object CSSStyleValue]" but got "[object CSSUnitValue]"
+FAIL Can set 'border-bottom-left-radius' to a percent assert_equals: expected "[object CSSStyleValue]" but got "[object CSSUnitValue]"
+PASS Setting 'border-bottom-left-radius' to a time throws TypeError
+PASS Setting 'border-bottom-left-radius' to a flexible length throws TypeError
+PASS Setting 'border-bottom-left-radius' to a number throws TypeError
+PASS Setting 'border-bottom-left-radius' to a position throws TypeError
+PASS Setting 'border-bottom-left-radius' to a URL throws TypeError
+PASS Setting 'border-bottom-left-radius' to a transform throws TypeError
+PASS Can set 'border-bottom-right-radius' to CSS-wide keywords
+PASS Can set 'border-bottom-right-radius' to var() references
+FAIL Can set 'border-bottom-right-radius' to a length assert_equals: expected "[object CSSStyleValue]" but got "[object CSSUnitValue]"
+FAIL Can set 'border-bottom-right-radius' to a percent assert_equals: expected "[object CSSStyleValue]" but got "[object CSSUnitValue]"
+PASS Setting 'border-bottom-right-radius' to a time throws TypeError
+PASS Setting 'border-bottom-right-radius' to a flexible length throws TypeError
+PASS Setting 'border-bottom-right-radius' to a number throws TypeError
+PASS Setting 'border-bottom-right-radius' to a position throws TypeError
+PASS Setting 'border-bottom-right-radius' to a URL throws TypeError
+PASS Setting 'border-bottom-right-radius' to a transform throws TypeError
+PASS 'border-radius' does not supported '30px'
+PASS 'border-radius' does not supported '25% 10%'
+PASS 'border-radius' does not supported '10% / 50%'
+PASS 'border-radius' does not supported '50% 20% / 10% 40%'
+Harness: the test ran to completion.
+
--- a/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-radius.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/border-radius.html
+<!doctype html>
+<meta charset="utf-8">
+<title>border radius properties</title>
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
+<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../../resources/testhelper.js"></script>
+<script src="resources/testsuite.js"></script>
+<body>
+<div id="log"></div>
+<script>
+'use strict';
+
+for (const suffix of ['top-left', 'top-right', 'bottom-left', 'bottom-right']) {
+  // Computed value is always a pair of values, which are not supported in
+  // Typed OM level 1.
+  runPropertyTests('border-' + suffix + '-radius', [
+    {
+      syntax: '<length>',
+      computed: (_, result) => assert_is_unsupported(result)
+    },
+    {
+      syntax: '<percentage>',
+      computed: (_, result) => assert_is_unsupported(result)
+    },
+  ]);
+}
+
+// shorthand
+runUnsupportedPropertyTests('border-radius', [
+  '30px', '25% 10%', '10% / 50%', '50% 20% / 10% 40%'
+]);
+
+</script>
--- a/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/resources/testsuite.js
+++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/resources/testsuite.js
@@ -16,6 +16,10 @@ function assert_is_equal_with_range_handling(input, result) {
    assert_style_value_equals(result, input);
 }

+function assert_is_unsupported(result) {
+  assert_class_string(result, 'CSSStyleValue');
+}
+
 const gCssWideKeywordsExamples = [
  {
    description: 'initial keyword',

--- a/third_party/WebKit/Source/core/css/CSSProperties.json5
+++ b/third_party/WebKit/Source/core/css/CSSProperties.json5
@@ -871,6 +871,7 @@
      default_value: "LengthSize(Length(0, kFixed), Length(0, kFixed))",
      type_name: "LengthSize",
      converter: "ConvertRadius",
+      typedom_types: ["Length", "Percentage"],
    },
    {
      name: "border-bottom-right-radius",
@@ -882,6 +883,7 @@
      default_value: "LengthSize(Length(0, kFixed), Length(0, kFixed))",
      type_name: "LengthSize",
      converter: "ConvertRadius",
+      typedom_types: ["Length", "Percentage"],
    },
    {
      name: "border-bottom-style",
@@ -925,17 +927,21 @@
      property_methods: ["ParseSingleValue", "CSSValueFromComputedStyleInternal"],
      interpolable: true,
      custom_apply_functions_all: true,
+      typedom_types: ["Length", "Number"]
    },
    {
      name: "border-image-repeat",
      property_methods: ["ParseSingleValue", "CSSValueFromComputedStyleInternal"],
      custom_apply_functions_all: true,
+      keywords: ["stretch", "repeat", "round", "space"],
+      typedom_types: ["Keyword"]
    },
    {
      name: "border-image-slice",
      property_methods: ["ParseSingleValue", "CSSValueFromComputedStyleInternal"],
      interpolable: true,
      custom_apply_functions_all: true,
+      typedom_types: ["Number", "Percentage"],
    },
    {
      name: "border-image-source",
@@ -950,6 +956,8 @@
      property_methods: ["ParseSingleValue", "CSSValueFromComputedStyleInternal"],
      interpolable: true,
      custom_apply_functions_all: true,
+      keywords: ["auto"],
+      typedom_types: ["Keyword", "Length", "Percentage", "Number"],
    },
    {
      name: "border-left-color",
@@ -1057,6 +1065,7 @@
      default_value: "LengthSize(Length(0, kFixed), Length(0, kFixed))",
      type_name: "LengthSize",
      converter: "ConvertRadius",
+      typedom_types: ["Length", "Percentage"],
    },
    {
      name: "border-top-right-radius",
@@ -1068,6 +1077,7 @@
      default_value: "LengthSize(Length(0, kFixed), Length(0, kFixed))",
      type_name: "LengthSize",
      converter: "ConvertRadius",
+      typedom_types: ["Length", "Percentage"],
    },
    {
      name: "border-top-style",

--- a/third_party/WebKit/Source/core/css/CSSValuePair.h
+++ b/third_party/WebKit/Source/core/css/CSSValuePair.h
@@ -40,6 +40,10 @@ class CORE_EXPORT CSSValuePair : public CSSValue {
  const CSSValue& First() const { return *first_; }
  const CSSValue& Second() const { return *second_; }

+  bool KeepIdenticalValues() const {
+    return identical_values_policy_ == kKeepIdenticalValues;
+  };
+
  String CustomCSSText() const {
    String first = first_->CssText();
    String second = second_->CssText();

--- a/third_party/WebKit/Source/core/css/cssom/StylePropertyMap.cpp
+++ b/third_party/WebKit/Source/core/css/cssom/StylePropertyMap.cpp
@@ -7,6 +7,7 @@
 #include "bindings/core/v8/ExceptionState.h"
 #include "core/css/CSSIdentifierValue.h"
 #include "core/css/CSSValueList.h"
+#include "core/css/CSSValuePair.h"
 #include "core/css/cssom/CSSOMTypes.h"
 #include "core/css/cssom/CSSStyleValue.h"
 #include "core/css/cssom/StyleValueFactory.h"
@@ -53,6 +54,19 @@ const CSSValue* StyleValueToCSSValue(
  // TODO(https://crbug.com/545324): Move this into a method on
  // CSSProperty when there are more of these cases.
  switch (property_id) {
+    case CSSPropertyBorderBottomLeftRadius:
+    case CSSPropertyBorderBottomRightRadius:
+    case CSSPropertyBorderTopLeftRadius:
+    case CSSPropertyBorderTopRightRadius: {
+      // level 1 only accept single <length-percentages>, but border-radius-*
+      // expects pairs.
+      const auto* value = style_value.ToCSSValue();
+      if (value->IsPrimitiveValue()) {
+        return CSSValuePair::Create(value, value,
+                                    CSSValuePair::kDropIdenticalValues);
+      }
+      break;
+    }
    case CSSPropertyGridAutoFlow: {
      // level 1 only accepts single keywords
      const auto* value = style_value.ToCSSValue();

--- a/third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp
+++ b/third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp
@@ -8,6 +8,7 @@
 #include "core/css/CSSIdentifierValue.h"
 #include "core/css/CSSImageValue.h"
 #include "core/css/CSSValue.h"
+#include "core/css/CSSValuePair.h"
 #include "core/css/CSSVariableReferenceValue.h"
 #include "core/css/cssom/CSSKeywordValue.h"
 #include "core/css/cssom/CSSNumericValue.h"
@@ -45,6 +46,19 @@ CSSStyleValue* CreateStyleValueWithPropertyInternal(CSSPropertyID property_id,
  // FIXME: We should enforce/document what the possible CSSValue structures
  // are for each property.
  switch (property_id) {
+    case CSSPropertyBorderBottomLeftRadius:
+    case CSSPropertyBorderBottomRightRadius:
+    case CSSPropertyBorderTopLeftRadius:
+    case CSSPropertyBorderTopRightRadius: {
+      // border-radius-* are always stored as pairs, but when both values are
+      // the same, we should reify as a single value.
+      if (const CSSValuePair* pair = ToCSSValuePairOrNull(value)) {
+        if (pair->First() == pair->Second() && !pair->KeepIdenticalValues()) {
+          return CreateStyleValue(pair->First());
+        }
+      }
+      return nullptr;
+    }
    case CSSPropertyCaretColor:
      // caret-color also supports 'auto'
      if (value.IsIdentifierValue() &&