Deduplicate and sort font-variation-settings when computing style.

Make interpolation type always run CSSValues through the converter for
this property.

Update remaining WPT tests to use order-invariant comparison (as most
tests for this property already use).


spec https://drafts.csswg.org/css-fonts-4/#font-variation-settings-def

If the same axis name appears more than once, the value associated with
the last appearance supersedes any previous value for that axis. This
deduplication is observable by accessing the computed value of this
property.

resolution https://github.com/w3c/csswg-drafts/issues/1959
RESOLVED: For both font-variation-settings and font-feature-settings,
the computed value is a map (and thus specified dupes are removed)

Bug: 1005355
Change-Id: I2f6f0f76fdcd69cfece283625cf42f517686dc8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2253248
Commit-Queue: George Steel <gtsteel@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786564}

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

@@ -62,6 +62,8 @@
 #include "third_party/blink/renderer/platform/fonts/opentype/open_type_math_support.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
+#include "third_party/blink/renderer/platform/wtf/hash_map.h"
+#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
 
@@ -276,6 +278,10 @@ StyleBuilderConverter::ConvertFontFeatureSettings(StyleResolverState& state,
   return settings;
 }
 
+static bool CompareTags(FontVariationAxis a, FontVariationAxis b) {
+  return a.Tag() < b.Tag();
+}
+
 scoped_refptr<FontVariationSettings>
 StyleBuilderConverter::ConvertFontVariationSettings(
     const StyleResolverState& state,
@@ -285,13 +291,20 @@ StyleBuilderConverter::ConvertFontVariationSettings(
     return FontBuilder::InitialVariationSettings();
 
   const auto& list = To<CSSValueList>(value);
-  scoped_refptr<FontVariationSettings> settings =
-      FontVariationSettings::Create();
   int len = list.length();
+  HashMap<uint32_t, float> axes;
+  // Use a temporary HashMap to remove duplicate tags, keeping the last
+  // occurrence of each.
   for (int i = 0; i < len; ++i) {
     const auto& feature = To<cssvalue::CSSFontVariationValue>(list.Item(i));
-    settings->Append(FontVariationAxis(feature.Tag(), feature.Value()));
+    axes.Set(AtomicStringToFourByteTag(feature.Tag()), feature.Value());
+  }
+  scoped_refptr<FontVariationSettings> settings =
+      FontVariationSettings::Create();
+  for (auto& axis : axes) {
+    settings->Append(FontVariationAxis(axis.key, axis.value));
   }
+  std::sort(settings->begin(), settings->end(), CompareTags);
   return settings;
 }
 

third_party/blink/web_tests/external/wpt/css/css-fonts/font-variation-settings-serialization-001-expected.txt

-This is a testharness.js-based test.
-FAIL font-feature-settings should be serialized to not include duplicates assert_in_array: value "\"bldA\" 1, \"bldA\" 2" not in array ["\"bldA\" 2", "'bldA' 2"]
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/css/css-fonts/parsing/font-variation-settings-computed.html

@@ -15,7 +15,13 @@
 test_computed_value('font-variation-settings', 'normal');
 
 test_computed_value('font-variation-settings', '"wght" 700');
-test_computed_value('font-variation-settings', '"wght" 700, "XHGT" 0.7');
+test_computed_value('font-variation-settings', '"AB@D" 0.5');
+
+test_computed_value('font-variation-settings', '"wght" 700, "wght" 500', '"wght" 500',
+    "duplicate values should be removed, keeping the rightmost occurrence.");
+
+test_computed_value('font-variation-settings', '"wght" 700, "XHGT" 0.7',
+    ['"wght" 700, "XHGT" 0.7', '"XHGT" 0.7, "wght" 700']);
 
 test_computed_value('font-variation-settings', '"XHGT" calc(0.4 + 0.3)', '"XHGT" 0.7');
 </script>

third_party/blink/web_tests/external/wpt/css/css-fonts/parsing/font-variation-settings-invalid.html

@@ -22,6 +22,17 @@ test_invalid_value('font-variation-settings', '"XHGTX" 0.7');
 test_invalid_value('font-variation-settings', '"abc\1F" 0.5');
 test_invalid_value('font-variation-settings', '"abc\7F" 0.5');
 test_invalid_value('font-variation-settings', '"abc\A9" 0.5');
+
+test_invalid_value('font-variation-settings', "'wght' 200 'abcd' 400");
+test_invalid_value('font-variation-settings', "'a' 1234");
+test_invalid_value('font-variation-settings', "'abcde' 1234");
+test_invalid_value('font-variation-settings', "'wght' 200, ");
+test_invalid_value('font-variation-settings', "'abcd\" 123");
+
+test_invalid_value('font-variation-settings', "'wght' 100px");
+test_invalid_value('font-variation-settings', "'wght' calc(100px + 200px)");
+test_invalid_value('font-variation-settings', "'wght' 42%");
+test_invalid_value('font-variation-settings', "'wght' calc(100%)");
 </script>
 </body>
 </html>

third_party/blink/web_tests/external/wpt/css/css-fonts/parsing/font-variation-settings-valid.html

@@ -15,10 +15,12 @@
 test_valid_value('font-variation-settings', 'normal');
 
 test_valid_value('font-variation-settings', '"wght" 700');
+test_valid_value('font-variation-settings', "'wght' 700", '"wght" 700');
 test_valid_value('font-variation-settings', '"wght" 700, "XHGT" 0.7');
 
 test_valid_value('font-variation-settings', '"a cd" 0.5');
 test_valid_value('font-variation-settings', '"ab@d" 0.5');
+test_valid_value('font-variation-settings', "'wght' 1e3, 'slnt' -450.0e-1", '"wght" 1000, "slnt" -45');
 </script>
 </body>
 </html>

third_party/blink/web_tests/external/wpt/css/css-fonts/variations/font-variation-settings-parsing.html

-<!DOCTYPE html>
-<html>
-<head>
-    <title>Testing the parsing of the font-variation-settings property</title>
-    <link rel="help" href="https://www.w3.org/TR/css-fonts-4/#propdef-font-variation-settings" />
-    <script src="/resources/testharness.js"></script>
-    <script src="/resources/testharnessreport.js"></script>
-</head>
-<body>
-    <div id="value-parser-test"></div>
-    <script>
-
-        var valueParserTests = [
-            { value: "'wght' 1000, '9 ~A' -45",           expectedComputedValue: "\"wght\" 1000, \"9 ~A\" -45",    isValid: true,  message: "Axis tag with valid non-letter ascii characters" },
-            { value: "'\u001Fbdc' 123",                   expectedComputedValue: "",                               isValid: false, message: "Invalid character below allowed range (first char)"},
-            { value: "'abc\u007F' 123",                   expectedComputedValue: "",                               isValid: false, message: "Invalid character above allowed range (mid char)"},
-            { value: "'wght' 1e3, 'slnt' -450.0e-1 ",     expectedComputedValue: "\"wght\" 1000, \"slnt\" -45",    isValid: true,  message: "Axis values in scientific form are valid" },
-            { value: "normal",                            expectedComputedValue: "normal",                         isValid: true,  message: "'normal' value is valid" },
-            { value: "'a' 1234",                          expectedComputedValue: "",                               isValid: false, message: "Tag with less than 4 characters is invalid"},
-            { value: "'abcde' 1234",                      expectedComputedValue: "",                               isValid: false, message: "Tag with more than 4 characters is invalid"},
-            { value: "'wght' 200, ",                      expectedComputedValue: "",                               isValid: false, message: "Trailing comma is invalid"},
-            { value: "abcd 123",                          expectedComputedValue: "",                               isValid: false, message: "Unquoted tags are invalid"},
-            { value: "'abcd\" 123",                       expectedComputedValue: "",                               isValid: false, message: "Unmatched quotes around tag are invalid" },
-            { value: "'abcd'",                            expectedComputedValue: "",                               isValid: false, message: "Tag without value isinvalid"},
-            { value: "123",                               expectedComputedValue: "",                               isValid: false, message: "Value without tag is invalid"},
-            { value: "123 'abcd'",                        expectedComputedValue: "",                               isValid: false, message: "Value before tag is invalid"},
-            { value: "'wght' 200 'abcd' 400",             expectedComputedValue: "",                               isValid: false, message: "Missing comma between axes is invalid"},
-            { value: "'wght' calc(100 + 200)",            expectedComputedValue: "\"wght\" 300",                   isValid: true,  message: "Calculations should be supported" },
-            { value: "'wght' 100px",                      expectedComputedValue: "",                               isValid: false, message: "Units should not be supported" },
-            { value: "'wght' calc(100px + 200px)",        expectedComputedValue: "",                               isValid: false, message: "Units should not be supported (in calc)" },
-            { value: "'wght' 42%",                        expectedComputedValue: "",                               isValid: false, message: "Percentages should not be supported" },
-            { value: "'wght' calc(100%)",                 expectedComputedValue: "",                               isValid: false, message: "Percentages should not be supported (in calc)" },
-        ];
-
-        valueParserTests.forEach(function (testCase) {
-            test(() => {
-                var element = document.getElementById("value-parser-test");
-                // Reset to empty in order for testing to continue in case the next test would not parse as valid
-                element.style.fontVariationSettings = "";
-                element.style.fontVariationSettings = testCase.value;
-                var computed = window.getComputedStyle(element).fontVariationSettings;
-                if (testCase.isValid) {
-                    assert_equals(computed, testCase.expectedComputedValue, testCase.message);
-                }
-                else {
-                    assert_equals(computed, "normal", testCase.message);
-                }
-
-                element.style.fontVariationSettings = "";
-            },  "Property value: " + testCase.message);
-        });
-
-        valueParserTests.forEach(function (testCase) {
-            test(() => { assert_equals(window.CSS.supports("font-variation-settings", testCase.value), testCase.isValid, testCase.message); }, "@supports: " + testCase.message);
-        });
-
-    </script>
-</body>
-</html>

third_party/blink/web_tests/external/wpt/web-animations/animation-model/animation-types/accumulation-per-property-001-expected.txt

@@ -267,8 +267,8 @@ PASS font-variant-numeric: "lining-nums" onto "oldstyle-nums"
 PASS font-variation-settings (type: fontVariationSettings) has testAccumulation function
 PASS font-variation-settings with composite type accumulate
 PASS font-variation-settings (type: discrete) has testAccumulation function
-PASS font-variation-settings: ""wdth" 5" onto ""wght" 1.1, "wdth" 1"
-PASS font-variation-settings: ""wght" 1.1, "wdth" 1" onto ""wdth" 5"
+PASS font-variation-settings: ""wdth" 5" onto ""wdth" 1, "wght" 1.1"
+PASS font-variation-settings: ""wdth" 1, "wght" 1.1" onto ""wdth" 5"
 PASS font-variation-settings: "normal" onto ""wdth" 5"
 PASS font-variation-settings: ""wdth" 5" onto "normal"
 PASS grid-auto-columns (type: discrete) has testAccumulation function

third_party/blink/web_tests/external/wpt/web-animations/animation-model/animation-types/addition-per-property-001-expected.txt

@@ -267,8 +267,8 @@ PASS font-variant-numeric: "lining-nums" onto "oldstyle-nums"
 PASS font-variation-settings (type: fontVariationSettings) has testAddition function
 PASS font-variation-settings with composite type add
 PASS font-variation-settings (type: discrete) has testAddition function
-PASS font-variation-settings: ""wdth" 5" onto ""wght" 1.1, "wdth" 1"
-PASS font-variation-settings: ""wght" 1.1, "wdth" 1" onto ""wdth" 5"
+PASS font-variation-settings: ""wdth" 5" onto ""wdth" 1, "wght" 1.1"
+PASS font-variation-settings: ""wdth" 1, "wght" 1.1" onto ""wdth" 5"
 PASS font-variation-settings: "normal" onto ""wdth" 5"
 PASS font-variation-settings: ""wdth" 5" onto "normal"
 PASS grid-auto-columns (type: discrete) has testAddition function

third_party/blink/web_tests/external/wpt/web-animations/animation-model/animation-types/interpolation-per-property-001-expected.txt

 This is a testharness.js-based test.
-Found 373 tests; 359 PASS, 14 FAIL, 0 TIMEOUT, 0 NOTRUN.
+Found 373 tests; 361 PASS, 12 FAIL, 0 TIMEOUT, 0 NOTRUN.
 PASS Setup
 PASS align-content (type: discrete) has testInterpolation function
 PASS align-content uses discrete animation when animating between "flex-start" and "flex-end" with linear easing
@@ -324,12 +324,12 @@ PASS font-variant-numeric uses discrete animation when animating between "lining
 PASS font-variant-numeric uses discrete animation when animating between "lining-nums" and "oldstyle-nums" with keyframe easing
 PASS font-variation-settings (type: fontVariationSettings) has testInterpolation function
 PASS font-variation-settings supports animation as float
-FAIL font-variation-settings supports animation as float with multiple tags assert_array_equals: The computed values should be "wdth" 2,"wght" 1.2 at 250ms expected property 0 to be "\"wdth\" 2" but got "\"wdth\" 1" (expected array ["\"wdth\" 2", "\"wght\" 1.2"] got ["\"wdth\" 1", "\"wght\" 1.1"])
-FAIL font-variation-settings supports animation as float with multiple duplicate tags assert_array_equals: The computed values should be "wdth" 2,"wght" 1.2 at 250ms expected property 0 to be "\"wdth\" 2" but got "\"wdth\" 1" (expected array ["\"wdth\" 2", "\"wght\" 1.2"] got ["\"wdth\" 1", "\"wght\" 1.1"])
+PASS font-variation-settings supports animation as float with multiple tags
+PASS font-variation-settings supports animation as float with multiple duplicate tags
 PASS font-variation-settings (type: discrete) has testInterpolation function
-PASS font-variation-settings uses discrete animation when animating between ""wght" 1.1, "wdth" 1" and ""wdth" 5" with linear easing
-PASS font-variation-settings uses discrete animation when animating between ""wght" 1.1, "wdth" 1" and ""wdth" 5" with effect easing
-PASS font-variation-settings uses discrete animation when animating between ""wght" 1.1, "wdth" 1" and ""wdth" 5" with keyframe easing
+PASS font-variation-settings uses discrete animation when animating between ""wdth" 1, "wght" 1.1" and ""wdth" 5" with linear easing
+PASS font-variation-settings uses discrete animation when animating between ""wdth" 1, "wght" 1.1" and ""wdth" 5" with effect easing
+PASS font-variation-settings uses discrete animation when animating between ""wdth" 1, "wght" 1.1" and ""wdth" 5" with keyframe easing
 PASS font-variation-settings uses discrete animation when animating between ""wdth" 5" and "normal" with linear easing
 PASS font-variation-settings uses discrete animation when animating between ""wdth" 5" and "normal" with effect easing
 PASS font-variation-settings uses discrete animation when animating between ""wdth" 5" and "normal" with keyframe easing

third_party/blink/web_tests/external/wpt/web-animations/animation-model/animation-types/property-list.js

@@ -599,7 +599,7 @@ const gCSSProperties1 = {
     types: [
       'fontVariationSettings',
       { type: 'discrete',
-        options: [ ['"wght" 1.1, "wdth" 1', '"wdth" 5'],
+        options: [ ['"wdth" 1, "wght" 1.1', '"wdth" 5'],
                    ['"wdth" 5', 'normal']
                  ] },
     ]

third_party/blink/web_tests/fast/css/resources/font-variation-settings-css-support.js

@@ -2,8 +2,7 @@ var validWriteExpectations = [ "normal",
                                '"abcd" 1',
                                '"abcd" 0, "efgh" 1',
                                '"abcd" 0.3, "efgh" 1.4',
-                               '"abcd" -1000, "efgh" -10000',
-                               '"abcd" 0.3, "abcd" 0.4, "abcd" 0.5, "abcd" 0.6, "abcd" 0.7' ];
+                               '"abcd" -1000, "efgh" -10000'];
 
 var writeInvalidExpectations = ["none",
                                 "'abc' 1",