Auto merge of #14421 - upsuper:patch-2, r=emilio

Treat top-level number in calc() invalid

<!-- Please describe your changes on the following line: -->
This should probably considered as a temporary fix (for [bug 1321206](https://bugzilla.mozilla.org/show_bug.cgi?id=1321206)), to avoid assertion when trying to serialize calc value with only numbers. Certain properties (e.g. `line-height`) would eventually need to keep numbers inside calc.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Currently, CalcLengthOrPercentage doesn't actually keep the number value. If we don't treat it invalid, we can end up generating empty `calc()` value when one contains numbers (e.g. `calc(1)`), which would violate assertion elsewhere that `calc` must not be empty.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14421)
<!-- Reviewable:end -->

2 files changed, 24 insertions, 4 deletions

diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs
index 7136c40fce2..f8f237ab401 100644
--- a/components/style/values/specified/length.rs
+++ b/components/style/values/specified/length.rs
@@ -554,8 +554,8 @@ impl CalcLengthOrPercentage {
         CalcLengthOrPercentage::parse(input, CalcUnit::LengthOrPercentage)
     }
 
-    fn parse(input: &mut Parser,
-             expected_unit: CalcUnit) -> Result<CalcLengthOrPercentage, ()> {
+    pub fn parse(input: &mut Parser,
+                 expected_unit: CalcUnit) -> Result<CalcLengthOrPercentage, ()> {
         let ast = try!(CalcLengthOrPercentage::parse_sum(input, expected_unit));
 
         let mut simplified = Vec::new();
@@ -576,7 +576,6 @@ impl CalcLengthOrPercentage {
         let mut ch = None;
         let mut rem = None;
         let mut percentage = None;
-        let mut number = None;
 
         for value in simplified {
             match value {
@@ -606,7 +605,7 @@ impl CalcLengthOrPercentage {
                         FontRelativeLength::Rem(val) =>
                             rem = Some(rem.unwrap_or(0.) + val),
                     },
-                SimplifiedValueNode::Number(val) => number = Some(number.unwrap_or(0.) + val),
+                // TODO Add support for top level number in calc(). See servo/servo#14421.
                 _ => return Err(()),
             }
         }
diff --git a/tests/unit/style/value.rs b/tests/unit/style/value.rs
index aff4b83c783..4f9944e324d 100644
--- a/tests/unit/style/value.rs
+++ b/tests/unit/style/value.rs
@@ -3,8 +3,10 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use app_units::Au;
+use cssparser::Parser;
 use style::values::HasViewportPercentage;
 use style::values::specified::{ViewportPercentageLength, Length};
+use style::values::specified::length::{CalcLengthOrPercentage, CalcUnit};
 
 #[test]
 fn length_has_viewport_percentage() {
@@ -13,3 +15,22 @@ fn length_has_viewport_percentage() {
     let l = Length::Absolute(Au(100));
     assert!(!l.has_viewport_percentage());
 }
+
+#[test]
+fn calc_top_level_number_with_unit() {
+    fn parse(text: &str, unit: CalcUnit) -> Result<CalcLengthOrPercentage, ()> {
+        let mut parser = Parser::new(text);
+        CalcLengthOrPercentage::parse(&mut parser, unit)
+    }
+    assert_eq!(parse("1", CalcUnit::Length), Err(()));
+    assert_eq!(parse("1", CalcUnit::LengthOrPercentage), Err(()));
+    assert_eq!(parse("1", CalcUnit::Angle), Err(()));
+    assert_eq!(parse("1", CalcUnit::Time), Err(()));
+    assert_eq!(parse("1px  + 1", CalcUnit::Length), Err(()));
+    assert_eq!(parse("1em  + 1", CalcUnit::Length), Err(()));
+    assert_eq!(parse("1px  + 1", CalcUnit::LengthOrPercentage), Err(()));
+    assert_eq!(parse("1%   + 1", CalcUnit::LengthOrPercentage), Err(()));
+    assert_eq!(parse("1rad + 1", CalcUnit::Angle), Err(()));
+    assert_eq!(parse("1deg + 1", CalcUnit::Angle), Err(()));
+    assert_eq!(parse("1s   + 1", CalcUnit::Time), Err(()));
+}