Fix parsing of negative values for 'r', 'rx', 'ry' and 'stroke-width'

For all of these properties[1][2][3][4], a negative value is considered
illegal and should fail in the parser. (The value range is already
corectly specified for instance for animation, and also for the
corresponing presentation attributes.)

[1] https://svgwg.org/svg2-draft/geometry.html#R
[2] https://svgwg.org/svg2-draft/geometry.html#RX
[3] https://svgwg.org/svg2-draft/geometry.html#RY
[4] https://svgwg.org/svg2-draft/painting.html#StrokeWidthProperty

Bug: 902346
Change-Id: I4b073288b97151d7a9df4b1c2ce99341dca9c8ee
Reviewed-on: https://chromium-review.googlesource.com/c/1304561
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609077}

third_party/WebKit/LayoutTests/external/wpt/svg/geometry/parsing/r-invalid-expected.txt

@@ -2,7 +2,7 @@ This is a testharness.js-based test.
 FAIL e.style['r'] = "10" should not set the property value assert_equals: expected "" but got "10"
 PASS e.style['r'] = "auto" should not set the property value
 PASS e.style['r'] = "10px 20px" should not set the property value
-FAIL e.style['r'] = "-1px" should not set the property value assert_equals: expected "" but got "-1px"
-FAIL e.style['r'] = "-10%" should not set the property value assert_equals: expected "" but got "-10%"
+PASS e.style['r'] = "-1px" should not set the property value
+PASS e.style['r'] = "-10%" should not set the property value
 Harness: the test ran to completion.
 

third_party/WebKit/LayoutTests/external/wpt/svg/geometry/parsing/rx-invalid-expected.txt

@@ -2,6 +2,6 @@ This is a testharness.js-based test.
 FAIL e.style['rx'] = "10" should not set the property value assert_equals: expected "" but got "10"
 PASS e.style['rx'] = "none" should not set the property value
 PASS e.style['rx'] = "10px 20px" should not set the property value
-FAIL e.style['rx'] = "-1px" should not set the property value assert_equals: expected "" but got "-1px"
+PASS e.style['rx'] = "-1px" should not set the property value
 Harness: the test ran to completion.
 

third_party/WebKit/LayoutTests/external/wpt/svg/geometry/parsing/ry-invalid-expected.txt

@@ -2,6 +2,6 @@ This is a testharness.js-based test.
 FAIL e.style['rx'] = "10" should not set the property value assert_equals: expected "" but got "10"
 PASS e.style['rx'] = "none" should not set the property value
 PASS e.style['rx'] = "10px 20px" should not set the property value
-FAIL e.style['rx'] = "-1px" should not set the property value assert_equals: expected "" but got "-1px"
+PASS e.style['rx'] = "-1px" should not set the property value
 Harness: the test ran to completion.
 

third_party/WebKit/LayoutTests/external/wpt/svg/painting/parsing/stroke-width-invalid.svg

+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:h="http://www.w3.org/1999/xhtml"
+     width="800px" height="600px">
+  <title>SVG Painting: parsing stroke-width with invalid values</title>
+  <metadata>
+    <h:link rel="help" href="https://svgwg.org/svg2-draft/painting.html#StrokeWidth"/>
+    <h:meta name="assert" content="stroke-width supports only the grammar '&lt;length-percentage&gt;'."/>
+  </metadata>
+  <g id="target"></g>
+  <h:script src="/resources/testharness.js"/>
+  <h:script src="/resources/testharnessreport.js"/>
+  <h:script src="/css/support/parsing-testcommon.js"/>
+  <script><![CDATA[
+
+test_invalid_value("stroke-width", "auto");
+test_invalid_value("stroke-width", "10px 20px");
+test_invalid_value("stroke-width", "-1px");
+test_invalid_value("stroke-width", "-10%");
+
+  ]]></script>
+</svg>

third_party/WebKit/LayoutTests/external/wpt/svg/painting/parsing/stroke-width-valid.svg

+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:h="http://www.w3.org/1999/xhtml"
+     width="800px" height="600px">
+  <title>SVG Painting: parsing stroke-width with valid values</title>
+  <metadata>
+    <h:link rel="help" href="https://svgwg.org/svg2-draft/painting.html#StrokeWidth"/>
+    <h:meta name="assert" content="stroke-width supports the full grammar '&lt;length-percentage&gt;' and unitless."/>
+  </metadata>
+  <g id="target"></g>
+  <h:script src="/resources/testharness.js"/>
+  <h:script src="/resources/testharnessreport.js"/>
+  <h:script src="/css/support/parsing-testcommon.js"/>
+  <script><![CDATA[
+
+test_valid_value("stroke-width", "0");
+test_valid_value("stroke-width", "10");
+test_valid_value("stroke-width", "1px");
+test_valid_value("stroke-width", "calc(2em + 3ex)");
+test_valid_value("stroke-width", "4%");
+test_valid_value("stroke-width", "5vmin");
+
+  ]]></script>
+</svg>

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

@@ -433,10 +433,10 @@ bool IsNonZeroUserUnitsValue(const CSSPrimitiveValue* value) {
 
 CSSPrimitiveValue* ConsumeSVGGeometryPropertyLength(
     CSSParserTokenRange& range,
-    const CSSParserContext& context) {
-  // TODO(fs): Not all callers should be using kValueRangeAll.
+    const CSSParserContext& context,
+    ValueRange value_range) {
   CSSPrimitiveValue* value = ConsumeLengthOrPercent(
-      range, kSVGAttributeMode, kValueRangeAll, UnitlessQuirk::kForbid);
+      range, kSVGAttributeMode, value_range, UnitlessQuirk::kForbid);
   if (IsNonZeroUserUnitsValue(value))
     context.Count(WebFeature::kSVGGeometryPropertyHasNonZeroUnitlessValue);
   return value;

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

@@ -60,7 +60,8 @@ CSSPrimitiveValue* ConsumeLengthOrPercent(
     ValueRange,
     UnitlessQuirk = UnitlessQuirk::kForbid);
 CSSPrimitiveValue* ConsumeSVGGeometryPropertyLength(CSSParserTokenRange&,
-                                                    const CSSParserContext&);
+                                                    const CSSParserContext&,
+                                                    ValueRange);
 
 CSSPrimitiveValue* ConsumeAngle(
     CSSParserTokenRange&,

third_party/blink/renderer/core/css/properties/longhands/cx_custom.cc

@@ -13,8 +13,8 @@ namespace CSSLonghand {
 const CSSValue* Cx::ParseSingleValue(CSSParserTokenRange& range,
                                      const CSSParserContext& context,
                                      const CSSParserLocalContext&) const {
-  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(range,
-                                                                       context);
+  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(
+      range, context, kValueRangeAll);
 }
 
 const CSSValue* Cx::CSSValueFromComputedStyleInternal(

third_party/blink/renderer/core/css/properties/longhands/cy_custom.cc

@@ -13,8 +13,8 @@ namespace CSSLonghand {
 const CSSValue* Cy::ParseSingleValue(CSSParserTokenRange& range,
                                      const CSSParserContext& context,
                                      const CSSParserLocalContext&) const {
-  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(range,
-                                                                       context);
+  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(
+      range, context, kValueRangeAll);
 }
 
 const CSSValue* Cy::CSSValueFromComputedStyleInternal(

third_party/blink/renderer/core/css/properties/longhands/r_custom.cc

@@ -13,8 +13,8 @@ namespace CSSLonghand {
 const CSSValue* R::ParseSingleValue(CSSParserTokenRange& range,
                                     const CSSParserContext& context,
                                     const CSSParserLocalContext&) const {
-  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(range,
-                                                                       context);
+  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(
+      range, context, kValueRangeNonNegative);
 }
 
 const CSSValue* R::CSSValueFromComputedStyleInternal(

third_party/blink/renderer/core/css/properties/longhands/rx_custom.cc

@@ -15,8 +15,8 @@ const CSSValue* Rx::ParseSingleValue(CSSParserTokenRange& range,
                                      const CSSParserLocalContext&) const {
   if (range.Peek().Id() == CSSValueAuto)
     return css_property_parser_helpers::ConsumeIdent(range);
-  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(range,
-                                                                       context);
+  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(
+      range, context, kValueRangeNonNegative);
 }
 
 const CSSValue* Rx::CSSValueFromComputedStyleInternal(

third_party/blink/renderer/core/css/properties/longhands/ry_custom.cc

@@ -15,8 +15,8 @@ const CSSValue* Ry::ParseSingleValue(CSSParserTokenRange& range,
                                      const CSSParserLocalContext&) const {
   if (range.Peek().Id() == CSSValueAuto)
     return css_property_parser_helpers::ConsumeIdent(range);
-  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(range,
-                                                                       context);
+  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(
+      range, context, kValueRangeNonNegative);
 }
 
 const CSSValue* Ry::CSSValueFromComputedStyleInternal(

third_party/blink/renderer/core/css/properties/longhands/stroke_width_custom.cc

@@ -15,7 +15,7 @@ const CSSValue* StrokeWidth::ParseSingleValue(
     const CSSParserContext&,
     const CSSParserLocalContext&) const {
   return css_property_parser_helpers::ConsumeLengthOrPercent(
-      range, kSVGAttributeMode, kValueRangeAll,
+      range, kSVGAttributeMode, kValueRangeNonNegative,
       css_property_parser_helpers::UnitlessQuirk::kForbid);
 }
 

third_party/blink/renderer/core/css/properties/longhands/x_custom.cc

@@ -13,8 +13,8 @@ namespace CSSLonghand {
 const CSSValue* X::ParseSingleValue(CSSParserTokenRange& range,
                                     const CSSParserContext& context,
                                     const CSSParserLocalContext&) const {
-  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(range,
-                                                                       context);
+  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(
+      range, context, kValueRangeAll);
 }
 
 const CSSValue* X::CSSValueFromComputedStyleInternal(

third_party/blink/renderer/core/css/properties/longhands/y_custom.cc

@@ -13,8 +13,8 @@ namespace CSSLonghand {
 const CSSValue* Y::ParseSingleValue(CSSParserTokenRange& range,
                                     const CSSParserContext& context,
                                     const CSSParserLocalContext&) const {
-  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(range,
-                                                                       context);
+  return css_property_parser_helpers::ConsumeSVGGeometryPropertyLength(
+      range, context, kValueRangeAll);
 }
 
 const CSSValue* Y::CSSValueFromComputedStyleInternal(