Bug 1843519 - Reuse merge logic from try_product_in_place when parsin…

…g. r=emilio

The merge logic in try_product_in_place could not be used because it
expected the rhs node to be a leaf already.  Now it tries to resolve the
rhs to a number before checking validity.  This allows the use of the
logic during parsing as well.  When simplifying the rhs is already a
node as it used to be, so the resolve should hit the Leaf case anyway
and return early, so the overhead is negligible.

Differential Revision: https://phabricator.services.mozilla.com/D183573

servo/components/style/values/generics/calc.rs

@@ -571,26 +571,29 @@ impl<L: CalcNodeLeaf> CalcNode<L> {
 
     /// Tries to merge one node into another using the product, that is, perform `x` * `y`.
     pub fn try_product_in_place(&mut self, other: &mut Self) -> bool {
-        if let CalcNode::Leaf(left) = self {
-            if let CalcNode::Leaf(right) = other {
-                return left.try_product_in_place(right);
+        if let Ok(resolved) = other.resolve() {
+            if let Some(number) = resolved.as_number() {
+                if number == 1.0 {
+                    return true;
+                }
+
+                if self.is_product_distributive() {
+                    self.map(|v| v * number);
+                    return true;
+                }
             }
         }
 
-        if let CalcNode::Leaf(left) = self {
-            if let Some(left) = left.as_number() {
-                if other.is_product_distributive() || left == 1.0 {
-                    other.map(|v| v * left);
+        if let Ok(resolved) = self.resolve() {
+            if let Some(number) = resolved.as_number() {
+                if number == 1.0 {
                     std::mem::swap(self, other);
                     return true;
                 }
-            }
-        }
 
-        if let CalcNode::Leaf(right) = other {
-            if let Some(right) = right.as_number() {
-                if self.is_product_distributive() || right == 1.0 {
-                    self.map(|v| v * right);
+                if other.is_product_distributive() {
+                    other.map(|v| v * number);
+                    std::mem::swap(self, other);
                     return true;
                 }
             }

servo/components/style/values/specified/calc.rs

@@ -780,46 +780,57 @@ impl CalcNode {
                 Ok(&Token::Delim('*')) => {
                     let mut rhs = Self::parse_one(context, input, allowed_units)?;
 
-                    let last = product.last_mut().unwrap();
+                    // We can unwrap here, becuase we start the function by adding a node to
+                    // the list.
+                    if !product.last_mut().unwrap().try_product_in_place(&mut rhs) {
+                        product.push(rhs);
+                    }
+                },
+                Ok(&Token::Delim('/')) => {
+                    let rhs = Self::parse_one(context, input, allowed_units)?;
 
-                    if let Ok(value) = rhs.to_number() {
-                        if last.is_product_distributive() || value == 1.0 {
-                            last.map(|v| v * value);
-                            continue;
-                        }
+                    enum InPlaceDivisionResult {
+                        /// The right was merged into the left.
+                        Merged,
+                        /// The right is not a number or could not be resolved, so the left is
+                        /// unchanged.
+                        Unchanged,
+                        /// The right was resolved, but was not a number, so the calculation is
+                        /// invalid.
+                        Invalid,
                     }
 
-                    if let Ok(value) = last.to_number() {
-                        if value == 1.0 {
-                            std::mem::swap(last, &mut rhs);
-                            continue;
-                        }
-                        if rhs.is_product_distributive() {
-                            rhs.map(|v| v * value);
-                            std::mem::swap(last, &mut rhs);
-                            continue;
+                    fn try_division_in_place(
+                        left: &mut CalcNode,
+                        right: &CalcNode,
+                    ) -> InPlaceDivisionResult {
+                        if let Ok(resolved) = right.resolve() {
+                            if let Some(number) = resolved.as_number() {
+                                if number != 1.0 && left.is_product_distributive() {
+                                    left.map(|l| l / number);
+                                    return InPlaceDivisionResult::Merged;
+                                }
+                            } else {
+                                return InPlaceDivisionResult::Invalid;
+                            }
                         }
+                        InPlaceDivisionResult::Unchanged
                     }
 
-                    product.push(rhs);
-                },
-                Ok(&Token::Delim('/')) => {
-                    let rhs = Self::parse_one(context, input, allowed_units)?;
-
-                    if let (CalcNode::Leaf(left), Ok(right)) =
-                        (product.last_mut().unwrap(), rhs.to_number())
-                    {
-                        left.map(|l| l / right);
-                    } else {
-                        // Right hand side is not resolvable to a number right now, but it must
-                        // still resolve to a number eventually, so check it.
-                        if !rhs.unit().map_or(false, |n| n.is_empty()) {
+                    // The right hand side of a division *must* be a number, so if we can
+                    // already resolve it, then merge it with the last node on the product list.
+                    // We can unwrap here, becuase we start the function by adding a node to
+                    // the list.
+                    match try_division_in_place(product.last_mut().unwrap(), &rhs) {
+                        InPlaceDivisionResult::Merged => {},
+                        InPlaceDivisionResult::Unchanged => {
+                            product.push(Self::Invert(Box::new(rhs)))
+                        },
+                        InPlaceDivisionResult::Invalid => {
                             return Err(
                                 input.new_custom_error(StyleParseErrorKind::UnspecifiedError)
-                            );
-                        }
-
-                        product.push(Self::Invert(Box::new(rhs)));
+                            )
+                        },
                     }
                 },
                 _ => {