Parse text-underline-position left/right

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/gXXMY1grZ-o/b0y3ENIaCQAJ

This CL implements parsing and serialization for the full spec grammar
auto | [ under || [ left | right ] ]

The new values left and right are hidden behind an experimental feature flag, TextUnderlinePositionLeftRight.
(Support for   auto | under   has shipped previously.)

This CL does not include painting support for the new values.

Bug: 313888
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ie1ba7ecb253e8966b0a9d6f0d0b15df310dfa0a1
Reviewed-on: https://chromium-review.googlesource.com/1135109
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575546}
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/text-underline-position-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/text-underline-position-expected.txt
deleted file mode 100644
index eb460df..0000000
--- a/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/properties/text-underline-position-expected.txt
+++ /dev/null
@@ -1,20 +0,0 @@
-This is a testharness.js-based test.
-PASS Can set 'text-underline-position' to CSS-wide keywords
-PASS Can set 'text-underline-position' to var() references
-PASS Can set 'text-underline-position' to the 'auto' keyword
-PASS Can set 'text-underline-position' to the 'under' keyword
-FAIL Can set 'text-underline-position' to the 'left' keyword Failed to execute 'set' on 'StylePropertyMap': Invalid type for property
-FAIL Can set 'text-underline-position' to the 'right' keyword Failed to execute 'set' on 'StylePropertyMap': Invalid type for property
-PASS Setting 'text-underline-position' to a length throws TypeError
-PASS Setting 'text-underline-position' to a percent throws TypeError
-PASS Setting 'text-underline-position' to a time throws TypeError
-PASS Setting 'text-underline-position' to an angle throws TypeError
-PASS Setting 'text-underline-position' to a flexible length throws TypeError
-PASS Setting 'text-underline-position' to a number throws TypeError
-PASS Setting 'text-underline-position' to a position throws TypeError
-PASS Setting 'text-underline-position' to a URL throws TypeError
-PASS Setting 'text-underline-position' to a transform throws TypeError
-FAIL 'text-underline-position' does not supported 'under left' assert_not_equals: Unsupported value must not be null got disallowed value null
-FAIL 'text-underline-position' does not supported 'right under' assert_not_equals: Unsupported value must not be null got disallowed value null
-Harness: the test ran to completion.
-
diff --git a/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/getComputedStyle/getComputedStyle-text-underline-position-expected.txt b/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/getComputedStyle/getComputedStyle-text-underline-position-expected.txt
index 4fdb7a7..e34d7d3 100644
--- a/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/getComputedStyle/getComputedStyle-text-underline-position-expected.txt
+++ b/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/getComputedStyle/getComputedStyle-text-underline-position-expected.txt
@@ -23,6 +23,22 @@
 PASS e.style.textUnderlinePosition is "under"
 PASS computedStyle.textUnderlinePosition is "under"
 
+Value 'left':
+PASS e.style.textUnderlinePosition is "left"
+PASS computedStyle.textUnderlinePosition is "left"
+
+Value 'right':
+PASS e.style.textUnderlinePosition is "right"
+PASS computedStyle.textUnderlinePosition is "right"
+
+Value 'under left':
+PASS e.style.textUnderlinePosition is "under left"
+PASS computedStyle.textUnderlinePosition is "under left"
+
+Value 'right under':
+PASS e.style.textUnderlinePosition is "under right"
+PASS computedStyle.textUnderlinePosition is "under right"
+
 Ancestor inherits values from parent:
 PASS e.style.textUnderlinePosition is ""
 PASS computedStyle.textUnderlinePosition is "under"
@@ -31,10 +47,22 @@
 PASS e.style.textUnderlinePosition is ""
 PASS computedStyle.textUnderlinePosition is "under"
 
+Value 'auto left':
+PASS e.style.textUnderlinePosition is ""
+PASS computedStyle.textUnderlinePosition is "under"
+
+Value 'right auto':
+PASS e.style.textUnderlinePosition is ""
+PASS computedStyle.textUnderlinePosition is "under"
+
 Value 'under under':
 PASS e.style.textUnderlinePosition is ""
 PASS computedStyle.textUnderlinePosition is "under"
 
+Value 'left right':
+PASS e.style.textUnderlinePosition is ""
+PASS computedStyle.textUnderlinePosition is "under"
+
 Value 'auto alphabetic under':
 PASS e.style.textUnderlinePosition is ""
 PASS computedStyle.textUnderlinePosition is "under"
diff --git a/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/getComputedStyle/script-tests/getComputedStyle-text-underline-position.js b/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/getComputedStyle/script-tests/getComputedStyle-text-underline-position.js
index 9c97358..da85461 100644
--- a/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/getComputedStyle/script-tests/getComputedStyle-text-underline-position.js
+++ b/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/getComputedStyle/script-tests/getComputedStyle-text-underline-position.js
@@ -36,6 +36,18 @@
 debug("Value 'under':");
 test("under", "under", "under");
 
+debug("Value 'left':");
+test("left", "left", "left");
+
+debug("Value 'right':");
+test("right", "right", "right");
+
+debug("Value 'under left':");
+test("under left", "under left", "under left");
+
+debug("Value 'right under':");
+test("right under", "under right", "under right");
+
 testContainer.innerHTML = '<div id="test-parent" style="text-underline-position: under;">hello <span id="test-ancestor">world</span></div>';
 debug("Ancestor inherits values from parent:");
 e = document.getElementById('test-ancestor');
@@ -44,9 +56,18 @@
 debug("Value 'auto under':");
 test("auto under", "", "under");
 
+debug("Value 'auto left':");
+test("auto left", "", "under");
+
+debug("Value 'right auto':");
+test("right auto", "", "under");
+
 debug("Value 'under under':");
 test("under under", "", "under");
 
+debug("Value 'left right':");
+test("left right", "", "under");
+
 debug("Value 'auto alphabetic under':");
 test("auto alphabetic under", "", "under");
 
diff --git a/third_party/WebKit/LayoutTests/virtual/stable/webexposed/nonstable-css-properties-expected.txt b/third_party/WebKit/LayoutTests/virtual/stable/webexposed/nonstable-css-properties-expected.txt
index 3e5c942..86e7a29 100644
--- a/third_party/WebKit/LayoutTests/virtual/stable/webexposed/nonstable-css-properties-expected.txt
+++ b/third_party/WebKit/LayoutTests/virtual/stable/webexposed/nonstable-css-properties-expected.txt
@@ -63,6 +63,22 @@
 el.style.getPropertyValue('text-justify') is 
 getComputedStyle(el).getPropertyValue('text-justify') is 
 
+el.style.setProperty('text-underline-position', 'left')
+el.style.getPropertyValue('text-underline-position') is 
+getComputedStyle(el).getPropertyValue('text-underline-position') is auto
+
+el.style.setProperty('text-underline-position', 'right')
+el.style.getPropertyValue('text-underline-position') is 
+getComputedStyle(el).getPropertyValue('text-underline-position') is auto
+
+el.style.setProperty('text-underline-position', 'under left')
+el.style.getPropertyValue('text-underline-position') is 
+getComputedStyle(el).getPropertyValue('text-underline-position') is auto
+
+el.style.setProperty('text-underline-position', 'right under')
+el.style.getPropertyValue('text-underline-position') is 
+getComputedStyle(el).getPropertyValue('text-underline-position') is auto
+
 PASS successfullyParsed is true
 
 TEST COMPLETE
diff --git a/third_party/WebKit/LayoutTests/webexposed/nonstable-css-properties-expected.txt b/third_party/WebKit/LayoutTests/webexposed/nonstable-css-properties-expected.txt
index f61be19..3694492 100644
--- a/third_party/WebKit/LayoutTests/webexposed/nonstable-css-properties-expected.txt
+++ b/third_party/WebKit/LayoutTests/webexposed/nonstable-css-properties-expected.txt
@@ -63,6 +63,22 @@
 el.style.getPropertyValue('text-justify') is distribute
 getComputedStyle(el).getPropertyValue('text-justify') is distribute
 
+el.style.setProperty('text-underline-position', 'left')
+el.style.getPropertyValue('text-underline-position') is left
+getComputedStyle(el).getPropertyValue('text-underline-position') is left
+
+el.style.setProperty('text-underline-position', 'right')
+el.style.getPropertyValue('text-underline-position') is right
+getComputedStyle(el).getPropertyValue('text-underline-position') is right
+
+el.style.setProperty('text-underline-position', 'under left')
+el.style.getPropertyValue('text-underline-position') is under left
+getComputedStyle(el).getPropertyValue('text-underline-position') is under left
+
+el.style.setProperty('text-underline-position', 'right under')
+el.style.getPropertyValue('text-underline-position') is under right
+getComputedStyle(el).getPropertyValue('text-underline-position') is under right
+
 PASS successfullyParsed is true
 
 TEST COMPLETE
diff --git a/third_party/WebKit/LayoutTests/webexposed/nonstable-css-properties.html b/third_party/WebKit/LayoutTests/webexposed/nonstable-css-properties.html
index 90f6d54..27dc3b1 100644
--- a/third_party/WebKit/LayoutTests/webexposed/nonstable-css-properties.html
+++ b/third_party/WebKit/LayoutTests/webexposed/nonstable-css-properties.html
@@ -38,6 +38,11 @@
 ['grid-template-areas', '"test"'],
 
 ['text-justify', 'distribute'],
+
+['text-underline-position', 'left'],
+['text-underline-position', 'right'],
+['text-underline-position', 'under left'],
+['text-underline-position', 'right under'],
 ];
 
 properties.forEach(function(args) {
diff --git a/third_party/blink/renderer/core/css/CSSProperties.json5 b/third_party/blink/renderer/core/css/CSSProperties.json5
index 2153f56..fcd5b73 100644
--- a/third_party/blink/renderer/core/css/CSSProperties.json5
+++ b/third_party/blink/renderer/core/css/CSSProperties.json5
@@ -3189,17 +3189,19 @@
       typedom_types: ["Keyword"],
       default_value: "none",
     },
-    // FIXME: Implement support for 'under left' and 'under right' values.
     {
       name: "text-underline-position",
       property_methods: ["ParseSingleValue", "CSSValueFromComputedStyleInternal"],
       inherited: true,
       field_group: "*",
-      field_template: "keyword",
-      keywords: ["auto", "under"],
-      default_value: "auto",
-      type_name: "TextUnderlinePosition",
-      typedom_types: ["Keyword"]
+      field_size: 3,
+      field_template: "primitive",
+      default_value: "kTextUnderlinePositionAuto",
+      name_for_methods: "TextUnderlinePosition",
+      type_name: "unsigned",
+      converter: "ConvertTextUnderlinePosition",
+      keywords: ["auto", "under", "left", "right"],
+      typedom_types: ["Keyword"],
     },
     {
       name: "top",
diff --git a/third_party/blink/renderer/core/css/css_primitive_value_mappings.h b/third_party/blink/renderer/core/css/css_primitive_value_mappings.h
index 144da32..cc46ccb 100644
--- a/third_party/blink/renderer/core/css/css_primitive_value_mappings.h
+++ b/third_party/blink/renderer/core/css/css_primitive_value_mappings.h
@@ -1982,6 +1982,43 @@
   return kContainsNone;
 }
 
+template <>
+inline CSSIdentifierValue::CSSIdentifierValue(TextUnderlinePosition position)
+    : CSSValue(kIdentifierClass) {
+  switch (position) {
+    case kTextUnderlinePositionAuto:
+      value_id_ = CSSValueAuto;
+      break;
+    case kTextUnderlinePositionUnder:
+      value_id_ = CSSValueUnder;
+      break;
+    case kTextUnderlinePositionLeft:
+      value_id_ = CSSValueLeft;
+      break;
+    case kTextUnderlinePositionRight:
+      value_id_ = CSSValueRight;
+      break;
+  }
+}
+
+template <>
+inline TextUnderlinePosition CSSIdentifierValue::ConvertTo() const {
+  switch (GetValueID()) {
+    case CSSValueAuto:
+      return kTextUnderlinePositionAuto;
+    case CSSValueUnder:
+      return kTextUnderlinePositionUnder;
+    case CSSValueLeft:
+      return kTextUnderlinePositionLeft;
+    case CSSValueRight:
+      return kTextUnderlinePositionRight;
+    default:
+      break;
+  }
+  NOTREACHED();
+  return kTextUnderlinePositionAuto;
+}
+
 }  // namespace blink
 
 #endif
diff --git a/third_party/blink/renderer/core/css/properties/longhands/text_underline_position_custom.cc b/third_party/blink/renderer/core/css/properties/longhands/text_underline_position_custom.cc
index 45c9d23..882eb40 100644
--- a/third_party/blink/renderer/core/css/properties/longhands/text_underline_position_custom.cc
+++ b/third_party/blink/renderer/core/css/properties/longhands/text_underline_position_custom.cc
@@ -4,20 +4,43 @@
 
 #include "third_party/blink/renderer/core/css/properties/longhands/text_underline_position.h"
 
+#include "third_party/blink/renderer/core/css/css_identifier_value.h"
+#include "third_party/blink/renderer/core/css/css_value_list.h"
 #include "third_party/blink/renderer/core/css/parser/css_property_parser_helpers.h"
 #include "third_party/blink/renderer/core/style/computed_style.h"
 
 namespace blink {
 namespace CSSLonghand {
 
+// auto | [ under || [ left | right ] ]
 const CSSValue* TextUnderlinePosition::ParseSingleValue(
     CSSParserTokenRange& range,
     const CSSParserContext& context,
     const CSSParserLocalContext&) const {
-  // auto | [ under || [ left | right ] ], but we only support auto | under
-  // for now
-  return CSSPropertyParserHelpers::ConsumeIdent<CSSValueAuto, CSSValueUnder>(
-      range);
+  if (range.Peek().Id() == CSSValueAuto)
+    return CSSPropertyParserHelpers::ConsumeIdent(range);
+
+  CSSIdentifierValue* under_value =
+      CSSPropertyParserHelpers::ConsumeIdent<CSSValueUnder>(range);
+  CSSIdentifierValue* left_or_right_value = nullptr;
+  if (RuntimeEnabledFeatures::TextUnderlinePositionLeftRightEnabled()) {
+    left_or_right_value =
+        CSSPropertyParserHelpers::ConsumeIdent<CSSValueLeft, CSSValueRight>(
+            range);
+    if (left_or_right_value && !under_value) {
+      under_value =
+          CSSPropertyParserHelpers::ConsumeIdent<CSSValueUnder>(range);
+    }
+  }
+  if (!under_value && !left_or_right_value) {
+    return nullptr;
+  }
+  CSSValueList* list = CSSValueList::CreateSpaceSeparated();
+  if (under_value)
+    list->Append(*under_value);
+  if (left_or_right_value)
+    list->Append(*left_or_right_value);
+  return list;
 }
 
 const CSSValue* TextUnderlinePosition::CSSValueFromComputedStyleInternal(
@@ -26,7 +49,25 @@
     const LayoutObject*,
     Node*,
     bool allow_visited_style) const {
-  return CSSIdentifierValue::Create(style.GetTextUnderlinePosition());
+  auto text_underline_position = style.TextUnderlinePosition();
+  if (text_underline_position == kTextUnderlinePositionAuto)
+    return CSSIdentifierValue::Create(CSSValueAuto);
+  if (text_underline_position == kTextUnderlinePositionUnder)
+    return CSSIdentifierValue::Create(CSSValueUnder);
+  if (text_underline_position == kTextUnderlinePositionLeft)
+    return CSSIdentifierValue::Create(CSSValueLeft);
+  if (text_underline_position == kTextUnderlinePositionRight)
+    return CSSIdentifierValue::Create(CSSValueRight);
+
+  CSSValueList* list = CSSValueList::CreateSpaceSeparated();
+  DCHECK(text_underline_position & kTextUnderlinePositionUnder);
+  list->Append(*CSSIdentifierValue::Create(CSSValueUnder));
+  if (text_underline_position & kTextUnderlinePositionLeft)
+    list->Append(*CSSIdentifierValue::Create(CSSValueLeft));
+  if (text_underline_position & kTextUnderlinePositionRight)
+    list->Append(*CSSIdentifierValue::Create(CSSValueRight));
+  DCHECK_EQ(list->length(), 2U);
+  return list;
 }
 
 }  // namespace CSSLonghand
diff --git a/third_party/blink/renderer/core/css/resolver/style_builder_converter.cc b/third_party/blink/renderer/core/css/resolver/style_builder_converter.cc
index d479f40..6d2e327 100644
--- a/third_party/blink/renderer/core/css/resolver/style_builder_converter.cc
+++ b/third_party/blink/renderer/core/css/resolver/style_builder_converter.cc
@@ -1478,6 +1478,26 @@
   return TextSizeAdjust(primitive_value.GetFloatValue() / 100.0f);
 }
 
+TextUnderlinePosition StyleBuilderConverter::ConvertTextUnderlinePosition(
+    StyleResolverState& state,
+    const CSSValue& value) {
+  TextUnderlinePosition flags = kTextUnderlinePositionAuto;
+
+  auto process = [&flags](const CSSValue& identifier) {
+    flags |=
+        ToCSSIdentifierValue(identifier).ConvertTo<TextUnderlinePosition>();
+  };
+
+  if (value.IsValueList()) {
+    for (auto& entry : ToCSSValueList(value)) {
+      process(*entry);
+    }
+  } else {
+    process(value);
+  }
+  return flags;
+}
+
 TransformOperations StyleBuilderConverter::ConvertTransformOperations(
     StyleResolverState& state,
     const CSSValue& value) {
diff --git a/third_party/blink/renderer/core/css/resolver/style_builder_converter.h b/third_party/blink/renderer/core/css/resolver/style_builder_converter.h
index 05d6e16..90e6340 100644
--- a/third_party/blink/renderer/core/css/resolver/style_builder_converter.h
+++ b/third_party/blink/renderer/core/css/resolver/style_builder_converter.h
@@ -204,6 +204,9 @@
   static float ConvertTextStrokeWidth(StyleResolverState&, const CSSValue&);
   static TextSizeAdjust ConvertTextSizeAdjust(StyleResolverState&,
                                               const CSSValue&);
+  static TextUnderlinePosition ConvertTextUnderlinePosition(
+      StyleResolverState& state,
+      const CSSValue& value);
   static TransformOperations ConvertTransformOperations(StyleResolverState&,
                                                         const CSSValue&);
   static TransformOrigin ConvertTransformOrigin(StyleResolverState&,
diff --git a/third_party/blink/renderer/core/paint/text_painter_base.cc b/third_party/blink/renderer/core/paint/text_painter_base.cc
index 965ced5..80a5e07 100644
--- a/third_party/blink/renderer/core/paint/text_painter_base.cc
+++ b/third_party/blink/renderer/core/paint/text_painter_base.cc
@@ -288,13 +288,11 @@
   // vertical text.
   switch (baseline_type) {
     case kAlphabeticBaseline:
-      switch (style.GetTextUnderlinePosition()) {
-        case TextUnderlinePosition::kAuto:
-          return ResolvedUnderlinePosition::kRoman;
-        case TextUnderlinePosition::kUnder:
-          return ResolvedUnderlinePosition::kUnder;
+      if (style.TextUnderlinePosition() == kTextUnderlinePositionAuto) {
+        return ResolvedUnderlinePosition::kRoman;
       }
-      break;
+      // TODO(https://crbug.com/313888) Support left, right.
+      return ResolvedUnderlinePosition::kUnder;
     case kIdeographicBaseline:
       // Compute language-appropriate default underline position.
       // https://drafts.csswg.org/css-text-decor-3/#default-stylesheet
diff --git a/third_party/blink/renderer/core/style/computed_style_constants.h b/third_party/blink/renderer/core/style/computed_style_constants.h
index 69060c9..c9ea1e2 100644
--- a/third_party/blink/renderer/core/style/computed_style_constants.h
+++ b/third_party/blink/renderer/core/style/computed_style_constants.h
@@ -192,6 +192,22 @@
   return a = a | b;
 }
 
+static const size_t kTextUnderlinePositionBits = 3;
+enum TextUnderlinePosition {
+  kTextUnderlinePositionAuto = 0x0,
+  kTextUnderlinePositionUnder = 0x1,
+  kTextUnderlinePositionLeft = 0x2,
+  kTextUnderlinePositionRight = 0x4
+};
+inline TextUnderlinePosition operator|(TextUnderlinePosition a,
+                                       TextUnderlinePosition b) {
+  return TextUnderlinePosition(int(a) | int(b));
+}
+inline TextUnderlinePosition& operator|=(TextUnderlinePosition& a,
+                                         TextUnderlinePosition b) {
+  return a = a | b;
+}
+
 enum class ItemPosition : unsigned {
   kLegacy,
   kAuto,
diff --git a/third_party/blink/renderer/platform/runtime_enabled_features.json5 b/third_party/blink/renderer/platform/runtime_enabled_features.json5
index 8aaee45..6665dfad 100644
--- a/third_party/blink/renderer/platform/runtime_enabled_features.json5
+++ b/third_party/blink/renderer/platform/runtime_enabled_features.json5
@@ -1229,6 +1229,10 @@
       status: "test",
     },
     {
+      name: "TextUnderlinePositionLeftRight",
+      status: "experimental",
+    },
+    {
       name: "TimerThrottlingForBackgroundTabs",
       status: "stable",
     },