Generate fewer redundant parentheses when serializing summations

This patch allows summations like "a + b + c" to be serialized as is
without inserting redundant parentheses like "(a + b) + c" that the
current implementation does.

This doesn't fix all the serialization issues, but at least we are one
step closer.

Bug: 1008146
Change-Id: I87350fe6fb43710974cfefb21d8d136258115c9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825732Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700253}

third_party/blink/renderer/core/css/css_math_expression_node.cc

@@ -662,26 +662,30 @@ bool CSSMathExpressionBinaryOperation::IsComputationallyIndependent() const {
          right_side_->IsComputationallyIndependent();
 }
 
-// static
-String CSSMathExpressionBinaryOperation::BuildCSSText(
-    const String& left_expression,
-    const String& right_expression,
-    CSSMathOperator op) {
+String CSSMathExpressionBinaryOperation::CustomCSSText() const {
   StringBuilder result;
-  result.Append('(');
-  result.Append(left_expression);
+
+  const bool left_side_needs_parentheses =
+      left_side_->IsBinaryOperation() && operator_ != CSSMathOperator::kAdd;
+  if (left_side_needs_parentheses)
+    result.Append('(');
+  result.Append(left_side_->CustomCSSText());
+  if (left_side_needs_parentheses)
+    result.Append(')');
+
   result.Append(' ');
-  result.Append(ToString(op));
+  result.Append(ToString(operator_));
   result.Append(' ');
-  result.Append(right_expression);
-  result.Append(')');
 
-  return result.ToString();
-}
+  const bool right_side_needs_parentheses =
+      right_side_->IsBinaryOperation() && operator_ != CSSMathOperator::kAdd;
+  if (right_side_needs_parentheses)
+    result.Append('(');
+  result.Append(right_side_->CustomCSSText());
+  if (right_side_needs_parentheses)
+    result.Append(')');
 
-String CSSMathExpressionBinaryOperation::CustomCSSText() const {
-  return BuildCSSText(left_side_->CustomCSSText(), right_side_->CustomCSSText(),
-                      operator_);
+  return result.ToString();
 }
 
 bool CSSMathExpressionBinaryOperation::operator==(

third_party/blink/renderer/core/css/css_math_expression_node.h

@@ -260,10 +260,6 @@ class CORE_EXPORT CSSMathExpressionBinaryOperation final
       const CSSMathExpressionNode* left_side,
       const CSSMathExpressionNode* right_side);
 
-  static String BuildCSSText(const String& left_expression,
-                             const String& right_expression,
-                             CSSMathOperator op);
-
   double Evaluate(double left_side, double right_side) const {
     return EvaluateOperator(left_side, right_side, operator_);
   }

third_party/blink/renderer/core/css/css_math_function_value.cc

@@ -97,12 +97,9 @@ Length CSSMathFunctionValue::ConvertToLength(
 static String BuildCSSText(const String& expression) {
   StringBuilder result;
   result.Append("calc");
-  bool expression_has_single_term = expression[0] != '(';
-  if (expression_has_single_term)
-    result.Append('(');
+  result.Append('(');
   result.Append(expression);
-  if (expression_has_single_term)
-    result.Append(')');
+  result.Append(')');
   return result.ToString();
 }
 

third_party/blink/web_tests/css3/calc/cssom-expected.txt

@@ -5,9 +5,9 @@ This tests calc() and the CSSOM
 100% => calc(100%)
 100% - 10px => calc(100% - 10px)
 10px + 10px * 5 => calc(60px)
-5px + 2em + 6in => calc((5px + 2em) + 6in)
+5px + 2em + 6in => calc(5px + 2em + 6in)
 100% - 10px / 2 => calc(100% - 5px)
-1px + 2em - 3rem + 4in => calc(((1px + 2em) - 3rem) + 4in)
+1px + 2em - 3rem + 4in => calc((1px + 2em) - 3rem + 4in)
 100px * (1 + 2 * 3 - 4 / 5) => calc(620px)
 (100px) + 200px => calc(300px)
 ((((((((((100px)))))))))) => calc(100px)

third_party/blink/web_tests/css3/calc/simplification-expected.txt

@@ -7,7 +7,7 @@ This tests parse time simplification in calc()
 100mm * (2.4 * 5 - 8 / 5) => calc(1040mm)
 100in * (6 * (5 - 4) / 8) => calc(75in)
 1px * (3 + 1/(7 + 1/(15 + 1/(1 + 1/(292 + 1/(1 + 1/(1 + 1/(1 + 1)))))))) => calc(3.14159px)
-100pc * 20 + 100rem * 10 - 100ch * 5 + 100pc => calc(((2000pc + 1000rem) - 500ch) + 100pc)
+100pc * 20 + 100rem * 10 - 100ch * 5 + 100pc => calc((2000pc + 1000rem) - 500ch + 100pc)
 ((100px + 20 * 5px) * 10 - 5 * (10em * 5 + 10em)) * 2 => calc((2000px - 300em) * 2)
 100px + 1in => calc(196px)
 10 * 10px + 0.5 * 2in => calc(196px)
@@ -16,7 +16,7 @@ This tests parse time simplification in calc()
 50cm + 50cm => calc(100cm)
 50cm + 10in + 100mm => calc(3227.72px)
 100px + 1em => calc(100px + 1em)
-100px + 1em + 100px => calc((100px + 1em) + 100px)
+100px + 1em + 100px => calc(100px + 1em + 100px)
 1em + 1rem => calc(1em + 1rem)
 1q + 10q => calc(11q)
 100q * (10 + 5) => calc(1500q)

third_party/blink/web_tests/external/wpt/css/css-values/calc-serialization-002-expected.txt

 This is a testharness.js-based test.
-FAIL testing calc(1vh + 2px + 3%) assert_equals: expected "calc(3% + 2px + 1vh)" but got "calc((1vh + 2px) + 3%)"
+FAIL testing calc(1vh + 2px + 3%) assert_equals: expected "calc(3% + 2px + 1vh)" but got "calc(1vh + 2px + 3%)"
 PASS testing calc(4px + 1vh)
-FAIL testing calc(5px + 6em + 1vh) assert_equals: expected "calc(6em + 5px + 1vh)" but got "calc((5px + 6em) + 1vh)"
+FAIL testing calc(5px + 6em + 1vh) assert_equals: expected "calc(6em + 5px + 1vh)" but got "calc(5px + 6em + 1vh)"
 FAIL testing calc(1vh - 7px) assert_equals: expected "calc(-7px + 1vh)" but got "calc(1vh - 7px)"
-FAIL testing calc(-8px + 9em + 1vh) assert_equals: expected "calc(9em - 8px + 1vh)" but got "calc((-8px + 9em) + 1vh)"
-FAIL testing calc(1pc + 1in + 1vh + 10%) assert_equals: expected "calc(10% + 112px + 1vh)" but got "calc((112px + 1vh) + 10%)"
-FAIL testing calc(25.4q + 1vh + 12%) assert_equals: expected "calc(12% + 24px + 1vh)" but got "calc((25.4q + 1vh) + 12%)"
-FAIL testing calc(1em + 1.27cm + 13% + 3em) assert_equals: expected "calc(13% + 4em + 48px)" but got "calc(((1em + 1.27cm) + 13%) + 3em)"
+FAIL testing calc(-8px + 9em + 1vh) assert_equals: expected "calc(9em - 8px + 1vh)" but got "calc(-8px + 9em + 1vh)"
+FAIL testing calc(1pc + 1in + 1vh + 10%) assert_equals: expected "calc(10% + 112px + 1vh)" but got "calc(112px + 1vh + 10%)"
+FAIL testing calc(25.4q + 1vh + 12%) assert_equals: expected "calc(12% + 24px + 1vh)" but got "calc(25.4q + 1vh + 12%)"
+FAIL testing calc(1em + 1.27cm + 13% + 3em) assert_equals: expected "calc(13% + 4em + 48px)" but got "calc(1em + 1.27cm + 13% + 3em)"
 FAIL testing calc(1vmin - 14%) assert_equals: expected "calc(-14% + 1vmin)" but got "calc(1vmin - 14%)"
 FAIL testing calc(15vw + 16vmin - 17vh) assert_equals: expected "calc(-17vh + 16vmin + 15vw)" but got "calc((15vw + 16vmin) - 17vh)"
 PASS testing calc(-80px + 25.4mm)

third_party/blink/web_tests/external/wpt/css/css-values/calc-serialization-expected.txt

 This is a testharness.js-based test.
-FAIL calc() serialization assert_equals: Serialization of calc(10px + 1vmin + 10%) expected "calc(10% + 10px + 1vmin)" but got "calc((10px + 1vmin) + 10%)"
+FAIL calc() serialization assert_equals: Serialization of calc(10px + 1vmin + 10%) expected "calc(10% + 10px + 1vmin)" but got "calc(10px + 1vmin + 10%)"
 Harness: the test ran to completion.