servo: Merge #16283 - style: Multiple cleanups around parsing code (from emilio:cleanup-parse-non-negative); r=Wafflespeanut
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 06 Apr 2017 13:57:40 -0500
changeset 399766 9e5803e13afc789154e649f9a3e83622e9e4a124
parent 399765 4c06dea25ef7ddb7c6bcfa5be2c3d7b56240be60
child 399767 dc314cde29d390514937204a4413181b4a15f9b1
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWafflespeanut
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #16283 - style: Multiple cleanups around parsing code (from emilio:cleanup-parse-non-negative); r=Wafflespeanut Source-Repo: https://github.com/servo/servo Source-Revision: 208d5dbfb6d8d9eed5ee5f5f21641221fe16e819
servo/components/style/properties/longhand/background.mako.rs
servo/components/style/properties/longhand/font.mako.rs
servo/components/style/properties/longhand/inherited_text.mako.rs
servo/components/style/properties/longhand/position.mako.rs
servo/components/style/properties/shorthand/position.mako.rs
servo/components/style/stylesheets.rs
servo/components/style/values/specified/grid.rs
servo/components/style/values/specified/length.rs
servo/components/style/viewport.rs
servo/tests/unit/style/viewport.rs
--- a/servo/components/style/properties/longhand/background.mako.rs
+++ b/servo/components/style/properties/longhand/background.mako.rs
@@ -478,45 +478,32 @@
     pub fn get_initial_specified_value() -> SpecifiedValue {
         SpecifiedValue::Explicit(ExplicitSize {
             width: specified::LengthOrPercentageOrAuto::Auto,
             height: specified::LengthOrPercentageOrAuto::Auto,
         })
     }
 
     pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue,()> {
-        let width;
-        if let Ok(value) = input.try(|input| {
-            match input.next() {
-                Err(_) => Err(()),
-                Ok(Token::Ident(ref ident)) if ident.eq_ignore_ascii_case("cover") => {
-                    Ok(SpecifiedValue::Cover)
-                }
-                Ok(Token::Ident(ref ident)) if ident.eq_ignore_ascii_case("contain") => {
-                    Ok(SpecifiedValue::Contain)
-                }
-                Ok(_) => Err(()),
-            }
-        }) {
-            return Ok(value)
-        } else {
-            width = try!(specified::LengthOrPercentageOrAuto::parse_non_negative(input))
+        if input.try(|input| input.expect_ident_matching("cover")).is_ok() {
+            return Ok(SpecifiedValue::Cover);
+        }
+
+        if input.try(|input| input.expect_ident_matching("contain")).is_ok() {
+            return Ok(SpecifiedValue::Contain);
         }
 
-        let height;
-        if let Ok(value) = input.try(|input| {
-            match input.next() {
-                Err(_) => Ok(specified::LengthOrPercentageOrAuto::Auto),
-                Ok(_) => Err(()),
-            }
-        }) {
-            height = value
+        let width =
+            try!(specified::LengthOrPercentageOrAuto::parse_non_negative(input));
+
+        let height = if input.is_exhausted() {
+            specified::LengthOrPercentageOrAuto::Auto
         } else {
-            height = try!(specified::LengthOrPercentageOrAuto::parse_non_negative(input));
-        }
+            try!(specified::LengthOrPercentageOrAuto::parse_non_negative(input))
+        };
 
         Ok(SpecifiedValue::Explicit(ExplicitSize {
             width: width,
             height: height,
         }))
     }
 </%helpers:vector_longhand>
 
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -654,25 +654,27 @@
                 SpecifiedValue::Length(LengthOrPercentage::Length(
                         ToComputedValue::from_computed_value(computed)
                 ))
         }
     }
     /// <length> | <percentage> | <absolute-size> | <relative-size>
     pub fn parse(_: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
         if let Ok(lop) = input.try(specified::LengthOrPercentage::parse_non_negative) {
-            Ok(SpecifiedValue::Length(lop))
-        } else if let Ok(kw) = input.try(KeywordSize::parse) {
-            Ok(SpecifiedValue::Keyword(kw))
-        } else {
-            match_ignore_ascii_case! {&*input.expect_ident()?,
-                "smaller" => Ok(SpecifiedValue::Smaller),
-                "larger" => Ok(SpecifiedValue::Larger),
-                _ => Err(())
-            }
+            return Ok(SpecifiedValue::Length(lop))
+        }
+
+        if let Ok(kw) = input.try(KeywordSize::parse) {
+            return Ok(SpecifiedValue::Keyword(kw))
+        }
+
+        match_ignore_ascii_case! {&*input.expect_ident()?,
+            "smaller" => Ok(SpecifiedValue::Smaller),
+            "larger" => Ok(SpecifiedValue::Larger),
+            _ => Err(())
         }
     }
 </%helpers:longhand>
 
 <%helpers:longhand products="gecko" name="font-size-adjust" animatable="True"
                    spec="https://drafts.csswg.org/css-fonts/#propdef-font-size-adjust">
     use std::fmt;
     use style_traits::ToCss;
--- a/servo/components/style/properties/longhand/inherited_text.mako.rs
+++ b/servo/components/style/properties/longhand/inherited_text.mako.rs
@@ -40,41 +40,43 @@
                     SpecifiedValue::MozBlockHeight => dest.write_str("-moz-block-height"),
                 % endif
                 SpecifiedValue::LengthOrPercentage(ref value) => value.to_css(dest),
                 SpecifiedValue::Number(number) => number.to_css(dest),
             }
         }
     }
     /// normal | <number> | <length> | <percentage>
-    pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
+    pub fn parse(_: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
         use cssparser::Token;
         use std::ascii::AsciiExt;
-        // We try to parse as a Number first because, for 'line-height', we want "0" to be
-        // parsed as a plain Number rather than a Length (0px); this matches the behaviour
-        // of all major browsers
-        input.try(specified::Number::parse_non_negative)
-            .map(SpecifiedValue::Number)
-            .or_else(|()| {
-                input.try(specified::LengthOrPercentage::parse_non_negative)
-                .map(SpecifiedValue::LengthOrPercentage)
-                .or_else(|()| {
-                    match try!(input.next()) {
-                        Token::Ident(ref value) if value.eq_ignore_ascii_case("normal") => {
-                            Ok(SpecifiedValue::Normal)
-                        }
-                        % if product == "gecko":
-                        Token::Ident(ref value) if value.eq_ignore_ascii_case("-moz-block-height") => {
-                            Ok(SpecifiedValue::MozBlockHeight)
-                        }
-                        % endif
-                        _ => Err(()),
-                    }
-                })
-            })
+
+        // We try to parse as a Number first because, for 'line-height', we want
+        // "0" to be parsed as a plain Number rather than a Length (0px); this
+        // matches the behaviour of all major browsers
+        if let Ok(number) = input.try(specified::Number::parse_non_negative) {
+            return Ok(SpecifiedValue::Number(number))
+        }
+
+        if let Ok(lop) = input.try(specified::LengthOrPercentage::parse_non_negative) {
+            return Ok(SpecifiedValue::LengthOrPercentage(lop))
+        }
+
+
+        match try!(input.next()) {
+            Token::Ident(ref value) if value.eq_ignore_ascii_case("normal") => {
+                Ok(SpecifiedValue::Normal)
+            }
+            % if product == "gecko":
+            Token::Ident(ref value) if value.eq_ignore_ascii_case("-moz-block-height") => {
+                Ok(SpecifiedValue::MozBlockHeight)
+            }
+            % endif
+            _ => Err(()),
+        }
     }
     pub mod computed_value {
         use app_units::Au;
         use std::fmt;
         use values::CSSFloat;
         #[derive(PartialEq, Copy, Clone, Debug)]
         #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
         pub enum T {
--- a/servo/components/style/properties/longhand/position.mako.rs
+++ b/servo/components/style/properties/longhand/position.mako.rs
@@ -137,17 +137,18 @@
 
 // FIXME: Gecko doesn't support content value yet.
 // FIXME: This property should be animatable.
 ${helpers.predefined_type("flex-basis",
                           "LengthOrPercentageOrAuto" if product == "gecko" else
                           "LengthOrPercentageOrAutoOrContent",
                           "computed::LengthOrPercentageOrAuto::Auto" if product == "gecko" else
                           "computed::LengthOrPercentageOrAutoOrContent::Auto",
-                          "parse_non_negative_with_context",
+                          "parse_non_negative",
+                          needs_context=False,
                           spec="https://drafts.csswg.org/css-flexbox/#flex-basis-property",
                           extra_prefixes="webkit",
                           animatable=True if product == "gecko" else False)}
 
 % for (size, logical) in ALL_SIZES:
     <%
       spec = "https://drafts.csswg.org/css-box/#propdef-%s"
       if logical:
--- a/servo/components/style/properties/shorthand/position.mako.rs
+++ b/servo/components/style/properties/shorthand/position.mako.rs
@@ -43,90 +43,71 @@
             dest.write_str(" ")?;
             self.flex_wrap.to_css(dest)
         }
     }
 </%helpers:shorthand>
 
 <%helpers:shorthand name="flex" sub_properties="flex-grow flex-shrink flex-basis" extra_prefixes="webkit"
                     spec="https://drafts.csswg.org/css-flexbox/#flex-property">
-    use parser::Parse;
-    use values::specified::{Number, NoCalcLength};
-    % if product == "gecko":
-        use values::specified::LengthOrPercentageOrAuto;
-    % else:
-        use values::specified::LengthOrPercentageOrAutoOrContent;
-    % endif
+    use values::specified::Number;
 
-    pub fn parse_flexibility(input: &mut Parser)
-                             -> Result<(Number, Option<Number>),()> {
+    fn parse_flexibility(input: &mut Parser)
+                         -> Result<(Number, Option<Number>),()> {
         let grow = try!(Number::parse_non_negative(input));
         let shrink = input.try(Number::parse_non_negative).ok();
         Ok((grow, shrink))
     }
 
     pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> {
         let mut grow = None;
         let mut shrink = None;
         let mut basis = None;
 
         if input.try(|input| input.expect_ident_matching("none")).is_ok() {
             return Ok(Longhands {
                 flex_grow: Number::new(0.0),
                 flex_shrink: Number::new(0.0),
-                % if product == "gecko":
-                    flex_basis: LengthOrPercentageOrAuto::Auto
-                % else:
-                    flex_basis: LengthOrPercentageOrAutoOrContent::Auto
-                % endif
-
+                flex_basis: longhands::flex_basis::SpecifiedValue::auto(),
             })
         }
         loop {
             if grow.is_none() {
                 if let Ok((flex_grow, flex_shrink)) = input.try(parse_flexibility) {
                     grow = Some(flex_grow);
                     shrink = flex_shrink;
                     continue
                 }
             }
             if basis.is_none() {
-                % if product == "gecko":
-                    if let Ok(value) = input.try(|i| LengthOrPercentageOrAuto::parse(context, i)) {
-                % else:
-                    if let Ok(value) = input.try(|i| LengthOrPercentageOrAutoOrContent::parse(context, i)) {
-                % endif
+                if let Ok(value) = input.try(|input| longhands::flex_basis::parse(context, input)) {
                     basis = Some(value);
                     continue
                 }
             }
             break
         }
 
         if grow.is_none() && basis.is_none() {
             return Err(())
         }
         Ok(Longhands {
             flex_grow: grow.unwrap_or(Number::new(1.0)),
             flex_shrink: shrink.unwrap_or(Number::new(1.0)),
-            % if product == "gecko":
-                flex_basis: basis.unwrap_or(LengthOrPercentageOrAuto::Length(NoCalcLength::zero()))
-            % else:
-                flex_basis: basis.unwrap_or(LengthOrPercentageOrAutoOrContent::Length(NoCalcLength::zero()))
-            % endif
+            flex_basis: basis.unwrap_or(longhands::flex_basis::SpecifiedValue::zero()),
         })
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
             try!(self.flex_grow.to_css(dest));
-            try!(write!(dest, " "));
+            try!(dest.write_str(" "));
 
             try!(self.flex_shrink.to_css(dest));
-            try!(write!(dest, " "));
+            try!(dest.write_str(" "));
 
             self.flex_basis.to_css(dest)
         }
     }
 </%helpers:shorthand>
 
 <%helpers:shorthand name="grid-gap" sub_properties="grid-row-gap grid-column-gap"
                     spec="https://drafts.csswg.org/css-grid/#propdef-grid-gap"
--- a/servo/components/style/stylesheets.rs
+++ b/servo/components/style/stylesheets.rs
@@ -18,17 +18,17 @@ use font_face::parse_font_face_block;
 pub use gecko::rules::FontFaceRule;
 #[cfg(feature = "gecko")]
 use gecko_bindings::structs::URLExtraData;
 #[cfg(feature = "gecko")]
 use gecko_bindings::sugar::refptr::RefPtr;
 use keyframes::{Keyframe, parse_keyframe_list};
 use media_queries::{Device, MediaList, parse_media_query_list};
 use parking_lot::RwLock;
-use parser::{ParserContext, log_css_error};
+use parser::{Parse, ParserContext, log_css_error};
 use properties::{PropertyDeclarationBlock, parse_property_declaration_list};
 use selector_parser::{SelectorImpl, SelectorParser};
 use selectors::parser::SelectorList;
 use servo_config::prefs::PREFS;
 #[cfg(not(feature = "gecko"))]
 use servo_url::ServoUrl;
 use shared_lock::{SharedRwLock, Locked, ToCssWithGuard, SharedRwLockReadGuard};
 use std::cell::Cell;
@@ -1054,17 +1054,17 @@ impl<'a, 'b> AtRuleParser for NestedRule
                 Ok(CssRule::Supports(Arc::new(self.shared_lock.wrap(SupportsRule {
                     condition: cond,
                     rules: self.parse_nested_rules(input),
                     enabled: enabled,
                 }))))
             }
             AtRulePrelude::Viewport => {
                 Ok(CssRule::Viewport(Arc::new(self.shared_lock.wrap(
-                    try!(ViewportRule::parse(input, self.context))))))
+                    try!(ViewportRule::parse(self.context, input))))))
             }
             AtRulePrelude::Keyframes(name) => {
                 Ok(CssRule::Keyframes(Arc::new(self.shared_lock.wrap(KeyframesRule {
                     name: name,
                     keyframes: parse_keyframe_list(&self.context, input, self.shared_lock),
                 }))))
             }
         }
--- a/servo/components/style/values/specified/grid.rs
+++ b/servo/components/style/values/specified/grid.rs
@@ -1,16 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Necessary types for [grid](https://drafts.csswg.org/css-grid/).
 
 use cssparser::{Parser, Token};
 use parser::{Parse, ParserContext};
+use std::ascii::AsciiExt;
 use std::fmt;
 use style_traits::ToCss;
 use values::{CSSFloat, HasViewportPercentage};
 use values::computed::{ComputedValueAsSpecified, Context, ToComputedValue};
 use values::specified::LengthOrPercentage;
 
 #[derive(PartialEq, Clone, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
@@ -125,33 +126,33 @@ pub enum TrackBreadth<L> {
     Flex(CSSFloat),
     /// One of the track-sizing keywords (`auto`, `min-content`, `max-content`)
     Keyword(TrackKeyword),
 }
 
 /// Parse a single flexible length.
 pub fn parse_flex(input: &mut Parser) -> Result<CSSFloat, ()> {
     match try!(input.next()) {
-        Token::Dimension(ref value, ref unit) if unit.to_lowercase() == "fr" && value.value.is_sign_positive()
+        Token::Dimension(ref value, ref unit) if unit.eq_ignore_ascii_case("fr") && value.value.is_sign_positive()
             => Ok(value.value),
         _ => Err(()),
     }
 }
 
 impl Parse for TrackBreadth<LengthOrPercentage> {
-    fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+    fn parse(_: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
         if let Ok(lop) = input.try(LengthOrPercentage::parse_non_negative) {
-            Ok(TrackBreadth::Breadth(lop))
-        } else {
-            if let Ok(f) = input.try(parse_flex) {
-                Ok(TrackBreadth::Flex(f))
-            } else {
-                TrackKeyword::parse(input).map(TrackBreadth::Keyword)
-            }
+            return Ok(TrackBreadth::Breadth(lop))
         }
+
+        if let Ok(f) = input.try(parse_flex) {
+            return Ok(TrackBreadth::Flex(f))
+        }
+
+        TrackKeyword::parse(input).map(TrackBreadth::Keyword)
     }
 }
 
 impl<L: ToCss> ToCss for TrackBreadth<L> {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         match *self {
             TrackBreadth::Breadth(ref lop) => lop.to_css(dest),
             TrackBreadth::Flex(ref value) => write!(dest, "{}fr", value),
@@ -218,34 +219,38 @@ impl<L> Default for TrackSize<L> {
     fn default() -> Self {
         TrackSize::Breadth(TrackBreadth::Keyword(TrackKeyword::Auto))
     }
 }
 
 impl Parse for TrackSize<LengthOrPercentage> {
     fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
         if let Ok(b) = input.try(|i| TrackBreadth::parse(context, i)) {
-            Ok(TrackSize::Breadth(b))
-        } else {
-            if input.try(|i| i.expect_function_matching("minmax")).is_ok() {
-                Ok(try!(input.parse_nested_block(|input| {
-                    let inflexible_breadth = if let Ok(lop) = input.try(LengthOrPercentage::parse_non_negative) {
-                        Ok(TrackBreadth::Breadth(lop))
-                    } else {
-                        TrackKeyword::parse(input).map(TrackBreadth::Keyword)
+            return Ok(TrackSize::Breadth(b))
+        }
+
+        if input.try(|i| i.expect_function_matching("minmax")).is_ok() {
+            return input.parse_nested_block(|input| {
+                let inflexible_breadth =
+                    match input.try(LengthOrPercentage::parse_non_negative) {
+                        Ok(lop) => TrackBreadth::Breadth(lop),
+                        Err(..) => {
+                            let keyword = try!(TrackKeyword::parse(input));
+                            TrackBreadth::Keyword(keyword)
+                        }
                     };
 
-                    try!(input.expect_comma());
-                    Ok(TrackSize::MinMax(try!(inflexible_breadth), try!(TrackBreadth::parse(context, input))))
-                })))
-            } else {
-                try!(input.expect_function_matching("fit-content"));
-                Ok(try!(LengthOrPercentage::parse(context, input).map(TrackSize::FitContent)))
-            }
+                try!(input.expect_comma());
+                Ok(TrackSize::MinMax(inflexible_breadth, try!(TrackBreadth::parse(context, input))))
+            });
         }
+
+        try!(input.expect_function_matching("fit-content"));
+        // FIXME(emilio): This needs a parse_nested_block, doesn't it?
+        Ok(try!(LengthOrPercentage::parse(context, input).map(TrackSize::FitContent)))
     }
 }
 
 impl<L: ToCss> ToCss for TrackSize<L> {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         match *self {
             TrackSize::Breadth(ref b) => b.to_css(dest),
             TrackSize::MinMax(ref infexible, ref flexible) => {
--- a/servo/components/style/values/specified/length.rs
+++ b/servo/components/style/values/specified/length.rs
@@ -437,17 +437,17 @@ impl Length {
                 }),
             _ => Err(())
         }
     }
 
     /// Parse a non-negative length
     #[inline]
     pub fn parse_non_negative(input: &mut Parser) -> Result<Length, ()> {
-        Length::parse_internal(input, AllowedNumericType::NonNegative)
+        Self::parse_internal(input, AllowedNumericType::NonNegative)
     }
 
     /// Get an absolute length from a px value.
     #[inline]
     pub fn from_px(px_value: CSSFloat) -> Length {
         Length::NoCalc(NoCalcLength::from_px(px_value))
     }
 
@@ -457,17 +457,17 @@ impl Length {
     #[inline]
     pub fn take(&mut self) -> Self {
         mem::replace(self, Length::zero())
     }
 }
 
 impl Parse for Length {
     fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        Length::parse_internal(input, AllowedNumericType::All)
+        Self::parse_internal(input, AllowedNumericType::All)
     }
 }
 
 impl Either<Length, None_> {
     /// Parse a non-negative length or none
     #[inline]
     pub fn parse_non_negative_length(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
         if input.try(|input| None_::parse(context, input)).is_ok() {
@@ -1044,60 +1044,64 @@ impl LengthOrPercentage {
             },
             _ => Err(())
         }
     }
 
     /// Parse a non-negative length.
     #[inline]
     pub fn parse_non_negative(input: &mut Parser) -> Result<LengthOrPercentage, ()> {
-        LengthOrPercentage::parse_internal(input, AllowedNumericType::NonNegative)
+        Self::parse_internal(input, AllowedNumericType::NonNegative)
     }
 
     /// Parse a length, treating dimensionless numbers as pixels
     ///
     /// https://www.w3.org/TR/SVG2/types.html#presentation-attribute-css-value
     pub fn parse_numbers_are_pixels(input: &mut Parser) -> Result<LengthOrPercentage, ()> {
         if let Ok(lop) = input.try(|i| Self::parse_internal(i, AllowedNumericType::All)) {
-            Ok(lop)
-        } else {
-            let num = input.expect_number()?;
-            Ok(LengthOrPercentage::Length(NoCalcLength::Absolute(Au((AU_PER_PX * num) as i32))))
+            return Ok(lop)
         }
+
+        // TODO(emilio): Probably should use Number::parse_non_negative to
+        // handle calc()?
+        let num = input.expect_number()?;
+        Ok(LengthOrPercentage::Length(NoCalcLength::Absolute(Au((AU_PER_PX * num) as i32))))
     }
 
     /// Parse a non-negative length, treating dimensionless numbers as pixels
     ///
     /// This is nonstandard behavior used by Firefox for SVG
     pub fn parse_numbers_are_pixels_non_negative(input: &mut Parser) -> Result<LengthOrPercentage, ()> {
         if let Ok(lop) = input.try(|i| Self::parse_internal(i, AllowedNumericType::NonNegative)) {
-            Ok(lop)
+            return Ok(lop)
+        }
+
+        // TODO(emilio): Probably should use Number::parse_non_negative to
+        // handle calc()?
+        let num = input.expect_number()?;
+        if num >= 0. {
+            Ok(LengthOrPercentage::Length(NoCalcLength::Absolute(Au((AU_PER_PX * num) as i32))))
         } else {
-            let num = input.expect_number()?;
-            if num >= 0. {
-                Ok(LengthOrPercentage::Length(NoCalcLength::Absolute(Au((AU_PER_PX * num) as i32))))
-            } else {
-                Err(())
-            }
+            Err(())
         }
     }
 
     /// Extract value from ref without a clone, replacing it with a 0 Au
     ///
     /// Use when you need to move out of a length array without cloning
     #[inline]
     pub fn take(&mut self) -> Self {
         mem::replace(self, LengthOrPercentage::zero())
     }
 }
 
 impl Parse for LengthOrPercentage {
     #[inline]
     fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        LengthOrPercentage::parse_internal(input, AllowedNumericType::All)
+        Self::parse_internal(input, AllowedNumericType::All)
     }
 }
 
 /// TODO(emilio): Do the Length and Percentage variants make any sense with
 /// CalcLengthOrPercentage?
 #[derive(Clone, PartialEq, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[allow(missing_docs)]
@@ -1141,54 +1145,55 @@ impl ToCss for LengthOrPercentageOrAuto 
             LengthOrPercentageOrAuto::Auto => dest.write_str("auto"),
             LengthOrPercentageOrAuto::Calc(ref calc) => calc.to_css(dest),
         }
     }
 }
 
 impl LengthOrPercentageOrAuto {
     fn parse_internal(input: &mut Parser, context: AllowedNumericType)
-                      -> Result<LengthOrPercentageOrAuto, ()>
-    {
+                      -> Result<Self, ()> {
         match try!(input.next()) {
             Token::Dimension(ref value, ref unit) if context.is_ok(value.value) =>
                 NoCalcLength::parse_dimension(value.value, unit).map(LengthOrPercentageOrAuto::Length),
             Token::Percentage(ref value) if context.is_ok(value.unit_value) =>
                 Ok(LengthOrPercentageOrAuto::Percentage(Percentage(value.unit_value))),
             Token::Number(ref value) if value.value == 0. =>
-                Ok(LengthOrPercentageOrAuto::Length(NoCalcLength::zero())),
+                Ok(Self::zero()),
             Token::Ident(ref value) if value.eq_ignore_ascii_case("auto") =>
                 Ok(LengthOrPercentageOrAuto::Auto),
             Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
                 let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
                 Ok(LengthOrPercentageOrAuto::Calc(Box::new(calc)))
             },
             _ => Err(())
         }
     }
 
     /// Parse a non-negative length, percentage, or auto.
     #[inline]
     pub fn parse_non_negative(input: &mut Parser) -> Result<LengthOrPercentageOrAuto, ()> {
-        LengthOrPercentageOrAuto::parse_internal(input, AllowedNumericType::NonNegative)
+        Self::parse_internal(input, AllowedNumericType::NonNegative)
     }
 
-    /// Parse a non-negative length, percentage, or auto.
-    #[inline]
-    pub fn parse_non_negative_with_context(_context: &ParserContext,
-                                           input: &mut Parser)
-                                           -> Result<LengthOrPercentageOrAuto, ()> {
-        LengthOrPercentageOrAuto::parse_non_negative(input)
+    /// Returns the `auto` value.
+    pub fn auto() -> Self {
+        LengthOrPercentageOrAuto::Auto
+    }
+
+    /// Returns a value representing a `0` length.
+    pub fn zero() -> Self {
+        LengthOrPercentageOrAuto::Length(NoCalcLength::zero())
     }
 }
 
 impl Parse for LengthOrPercentageOrAuto {
     #[inline]
     fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        LengthOrPercentageOrAuto::parse_internal(input, AllowedNumericType::All)
+        Self::parse_internal(input, AllowedNumericType::All)
     }
 }
 
 /// TODO(emilio): Do the Length and Percentage variants make any sense with
 /// CalcLengthOrPercentage?
 #[derive(Clone, PartialEq, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[allow(missing_docs)]
@@ -1236,25 +1241,25 @@ impl LengthOrPercentageOrNone {
             },
             Token::Ident(ref value) if value.eq_ignore_ascii_case("none") =>
                 Ok(LengthOrPercentageOrNone::None),
             _ => Err(())
         }
     }
     /// Parse a non-negative LengthOrPercentageOrNone.
     #[inline]
-    pub fn parse_non_negative(input: &mut Parser) -> Result<LengthOrPercentageOrNone, ()> {
-        LengthOrPercentageOrNone::parse_internal(input, AllowedNumericType::NonNegative)
+    pub fn parse_non_negative(input: &mut Parser) -> Result<Self, ()> {
+        Self::parse_internal(input, AllowedNumericType::NonNegative)
     }
 }
 
 impl Parse for LengthOrPercentageOrNone {
     #[inline]
     fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        LengthOrPercentageOrNone::parse_internal(input, AllowedNumericType::All)
+        Self::parse_internal(input, AllowedNumericType::All)
     }
 }
 
 /// Either a `<length>` or the `none` keyword.
 pub type LengthOrNone = Either<Length, None_>;
 
 /// Either a `<length>` or the `normal` keyword.
 pub type LengthOrNormal = Either<Length, Normal>;
@@ -1277,22 +1282,46 @@ pub enum LengthOrPercentageOrAutoOrConte
     Calc(Box<CalcLengthOrPercentage>),
     /// The `auto` keyword.
     Auto,
     /// The `content` keyword.
     Content
 }
 
 impl LengthOrPercentageOrAutoOrContent {
-    /// Alias to `parse` so that Gecko and Servo can use the same method name for
-    /// both `LengthOrPercentageOrAuto` and `LengthOrPercentageOrAutoOrContent`.
-    ///
-    /// NOTE: `parse` already only accepts non-negative values.
-    pub fn parse_non_negative_with_context(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        Self::parse(context, input)
+    /// Parse a non-negative LengthOrPercentageOrAutoOrContent.
+    pub fn parse_non_negative(input: &mut Parser) -> Result<Self, ()> {
+        let context = AllowedNumericType::NonNegative;
+        match try!(input.next()) {
+            Token::Dimension(ref value, ref unit) if context.is_ok(value.value) =>
+                NoCalcLength::parse_dimension(value.value, unit).map(LengthOrPercentageOrAutoOrContent::Length),
+            Token::Percentage(ref value) if context.is_ok(value.unit_value) =>
+                Ok(LengthOrPercentageOrAutoOrContent::Percentage(Percentage(value.unit_value))),
+            Token::Number(ref value) if value.value == 0. =>
+                Ok(Self::zero()),
+            Token::Ident(ref value) if value.eq_ignore_ascii_case("auto") =>
+                Ok(LengthOrPercentageOrAutoOrContent::Auto),
+            Token::Ident(ref value) if value.eq_ignore_ascii_case("content") =>
+                Ok(LengthOrPercentageOrAutoOrContent::Content),
+            Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
+                let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
+                Ok(LengthOrPercentageOrAutoOrContent::Calc(Box::new(calc)))
+            },
+            _ => Err(())
+        }
+    }
+
+    /// Returns the `auto` value.
+    pub fn auto() -> Self {
+        LengthOrPercentageOrAutoOrContent::Auto
+    }
+
+    /// Returns a value representing a `0` length.
+    pub fn zero() -> Self {
+        LengthOrPercentageOrAutoOrContent::Length(NoCalcLength::zero())
     }
 }
 
 impl HasViewportPercentage for LengthOrPercentageOrAutoOrContent {
     fn has_viewport_percentage(&self) -> bool {
         match *self {
             LengthOrPercentageOrAutoOrContent::Length(ref length) => length.has_viewport_percentage(),
             LengthOrPercentageOrAutoOrContent::Calc(ref calc) => calc.has_viewport_percentage(),
@@ -1308,53 +1337,30 @@ impl ToCss for LengthOrPercentageOrAutoO
             LengthOrPercentageOrAutoOrContent::Percentage(perc) => perc.to_css(dest),
             LengthOrPercentageOrAutoOrContent::Auto => dest.write_str("auto"),
             LengthOrPercentageOrAutoOrContent::Content => dest.write_str("content"),
             LengthOrPercentageOrAutoOrContent::Calc(ref calc) => calc.to_css(dest),
         }
     }
 }
 
-impl Parse for LengthOrPercentageOrAutoOrContent {
-    fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        let context = AllowedNumericType::NonNegative;
-        match try!(input.next()) {
-            Token::Dimension(ref value, ref unit) if context.is_ok(value.value) =>
-                NoCalcLength::parse_dimension(value.value, unit).map(LengthOrPercentageOrAutoOrContent::Length),
-            Token::Percentage(ref value) if context.is_ok(value.unit_value) =>
-                Ok(LengthOrPercentageOrAutoOrContent::Percentage(Percentage(value.unit_value))),
-            Token::Number(ref value) if value.value == 0. =>
-                Ok(LengthOrPercentageOrAutoOrContent::Length(NoCalcLength::zero())),
-            Token::Ident(ref value) if value.eq_ignore_ascii_case("auto") =>
-                Ok(LengthOrPercentageOrAutoOrContent::Auto),
-            Token::Ident(ref value) if value.eq_ignore_ascii_case("content") =>
-                Ok(LengthOrPercentageOrAutoOrContent::Content),
-            Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
-                Ok(LengthOrPercentageOrAutoOrContent::Calc(Box::new(calc)))
-            },
-            _ => Err(())
-        }
-    }
-}
-
 /// Either a `<length>` or a `<number>`.
 pub type LengthOrNumber = Either<Length, Number>;
 
 impl LengthOrNumber {
     /// Parse a non-negative LengthOrNumber.
-    pub fn parse_non_negative(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        // We try to parse as a Number first because, for cases like LengthOrNumber,
-        // we want "0" to be parsed as a plain Number rather than a Length (0px); this
-        // matches the behaviour of all major browsers
+    pub fn parse_non_negative(_: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+        // We try to parse as a Number first because, for cases like
+        // LengthOrNumber, we want "0" to be parsed as a plain Number rather
+        // than a Length (0px); this matches the behaviour of all major browsers
         if let Ok(v) = input.try(Number::parse_non_negative) {
-            Ok(Either::Second(v))
-        } else {
-            Length::parse_non_negative(input).map(Either::First)
+            return Ok(Either::Second(v))
         }
+
+        Length::parse_non_negative(input).map(Either::First)
     }
 }
 
 /// A value suitable for a `min-width` or `min-height` property.
 /// Unlike `max-width` or `max-height` properties, a MinLength can be
 /// `auto`, and cannot be `none`.
 #[derive(Debug, Clone, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
--- a/servo/components/style/viewport.rs
+++ b/servo/components/style/viewport.rs
@@ -9,17 +9,17 @@
 
 #![deny(missing_docs)]
 
 use app_units::Au;
 use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser, parse_important};
 use cssparser::ToCss as ParserToCss;
 use euclid::size::TypedSize2D;
 use media_queries::Device;
-use parser::{ParserContext, log_css_error};
+use parser::{Parse, ParserContext, log_css_error};
 use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
 use std::ascii::AsciiExt;
 use std::borrow::Cow;
 use std::fmt;
 use std::iter::Enumerate;
 use std::str::Chars;
 use style_traits::{PinchZoomFactor, ToCss};
 use style_traits::viewport::{Orientation, UserZoom, ViewportConstraints, Zoom};
@@ -160,19 +160,19 @@ impl FromMeta for ViewportLength {
                     Err(_) => specified!(NoCalcLength::from_px(1.))
                 }
             }
         })
     }
 }
 
 impl ViewportLength {
-    fn parse(input: &mut Parser) -> Result<ViewportLength, ()> {
-        // we explicitly do not accept 'extend-to-zoom', since it is a UA internal value
-        // for <META> viewport translation
+    fn parse(input: &mut Parser) -> Result<Self, ()> {
+        // we explicitly do not accept 'extend-to-zoom', since it is a UA
+        // internal value for <META> viewport translation
         LengthOrPercentageOrAuto::parse_non_negative(input).map(ViewportLength::Specified)
     }
 }
 
 impl FromMeta for Zoom {
     fn from_meta(value: &str) -> Option<Zoom> {
         Some(match value {
             v if v.eq_ignore_ascii_case("yes") => Zoom::Number(1.),
@@ -241,17 +241,17 @@ impl ToCss for ViewportDescriptorDeclara
             try!(dest.write_str(" !important"));
         }
         dest.write_str(";")
     }
 }
 
 fn parse_shorthand(input: &mut Parser) -> Result<(ViewportLength, ViewportLength), ()> {
     let min = try!(ViewportLength::parse(input));
-    match input.try(|input| ViewportLength::parse(input)) {
+    match input.try(ViewportLength::parse) {
         Err(()) => Ok((min.clone(), min)),
         Ok(max) => Ok((min, max))
     }
 }
 
 impl<'a, 'b> AtRuleParser for ViewportRuleParser<'a, 'b> {
     type Prelude = ();
     type AtRule = Vec<ViewportDescriptorDeclaration>;
@@ -282,43 +282,28 @@ impl<'a, 'b> DeclarationParser for Viewp
                 let shorthand = try!(parse_shorthand(input));
                 let important = input.try(parse_important).is_ok();
 
                 Ok(vec![declaration!($min(value: shorthand.0, important: important)),
                         declaration!($max(value: shorthand.1, important: important))])
             }}
         }
 
-        match name {
-            n if n.eq_ignore_ascii_case("min-width") =>
-                ok!(MinWidth(ViewportLength::parse)),
-            n if n.eq_ignore_ascii_case("max-width") =>
-                ok!(MaxWidth(ViewportLength::parse)),
-            n if n.eq_ignore_ascii_case("width") =>
-                ok!(shorthand -> [MinWidth, MaxWidth]),
-
-            n if n.eq_ignore_ascii_case("min-height") =>
-                ok!(MinHeight(ViewportLength::parse)),
-            n if n.eq_ignore_ascii_case("max-height") =>
-                ok!(MaxHeight(ViewportLength::parse)),
-            n if n.eq_ignore_ascii_case("height") =>
-                ok!(shorthand -> [MinHeight, MaxHeight]),
-
-            n if n.eq_ignore_ascii_case("zoom") =>
-                ok!(Zoom(Zoom::parse)),
-            n if n.eq_ignore_ascii_case("min-zoom") =>
-                ok!(MinZoom(Zoom::parse)),
-            n if n.eq_ignore_ascii_case("max-zoom") =>
-                ok!(MaxZoom(Zoom::parse)),
-
-            n if n.eq_ignore_ascii_case("user-zoom") =>
-                ok!(UserZoom(UserZoom::parse)),
-            n if n.eq_ignore_ascii_case("orientation") =>
-                ok!(Orientation(Orientation::parse)),
-
+        match_ignore_ascii_case! { name,
+            "min-width" => ok!(MinWidth(ViewportLength::parse)),
+            "max-width" => ok!(MaxWidth(ViewportLength::parse)),
+            "width" => ok!(shorthand -> [MinWidth, MaxWidth]),
+            "min-height" => ok!(MinHeight(ViewportLength::parse)),
+            "max-height" => ok!(MaxHeight(ViewportLength::parse)),
+            "height" => ok!(shorthand -> [MinHeight, MaxHeight]),
+            "zoom" => ok!(Zoom(Zoom::parse)),
+            "min-zoom" => ok!(MinZoom(Zoom::parse)),
+            "max-zoom" => ok!(MaxZoom(Zoom::parse)),
+            "user-zoom" => ok!(UserZoom(UserZoom::parse)),
+            "orientation" => ok!(Orientation(Orientation::parse)),
             _ => Err(()),
         }
     }
 }
 
 /// A `@viewport` rule.
 #[derive(Debug, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
@@ -335,21 +320,19 @@ const WHITESPACE: &'static [char] = &['\
 // need to use \x2c instead of ',' due to test-tidy
 const SEPARATOR: &'static [char] = &['\x2c', ';'];
 
 #[inline]
 fn is_whitespace_separator_or_equals(c: &char) -> bool {
     WHITESPACE.contains(c) || SEPARATOR.contains(c) || *c == '='
 }
 
-impl ViewportRule {
+impl Parse for ViewportRule {
     #[allow(missing_docs)]
-    pub fn parse(input: &mut Parser, context: &ParserContext)
-                     -> Result<ViewportRule, ()>
-    {
+    fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
         let parser = ViewportRuleParser { context: context };
 
         let mut cascade = Cascade::new();
         let mut parser = DeclarationListParser::new(input, parser);
         while let Some(result) = parser.next() {
             match result {
                 Ok(declarations) => {
                     for declarations in declarations {
@@ -361,17 +344,19 @@ impl ViewportRule {
                     let message = format!("Unsupported @viewport descriptor declaration: '{}'",
                                           parser.input.slice(range));
                     log_css_error(parser.input, pos, &*message, &context);
                 }
             }
         }
         Ok(ViewportRule { declarations: cascade.finish() })
     }
+}
 
+impl ViewportRule {
     #[allow(missing_docs)]
     pub fn from_meta(content: &str) -> Option<ViewportRule> {
         let mut declarations = vec![None; VIEWPORT_DESCRIPTOR_VARIANTS];
         macro_rules! push_descriptor {
             ($descriptor:ident($value:expr)) => {{
                 let descriptor = ViewportDescriptor::$descriptor($value);
                 let discriminant = descriptor.discriminant_value();
                 declarations[discriminant] = Some(ViewportDescriptorDeclaration::new(
--- a/servo/tests/unit/style/viewport.rs
+++ b/servo/tests/unit/style/viewport.rs
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use cssparser::Parser;
 use euclid::size::TypedSize2D;
 use media_queries::CSSErrorReporterTest;
 use servo_config::prefs::{PREFS, PrefValue};
 use servo_url::ServoUrl;
 use style::media_queries::{Device, MediaType};
-use style::parser::ParserContext;
+use style::parser::{Parse, ParserContext};
 use style::shared_lock::SharedRwLock;
 use style::stylesheets::{Stylesheet, Origin};
 use style::values::specified::LengthOrPercentageOrAuto::{self, Auto};
 use style::values::specified::NoCalcLength::{self, ViewportPercentage};
 use style::values::specified::ViewportPercentageLength::Vw;
 use style::viewport::*;
 use style_traits::PinchZoomFactor;
 use style_traits::viewport::*;
@@ -289,17 +289,17 @@ fn multiple_stylesheets_cascading() {
 #[test]
 fn constrain_viewport() {
     let url = ServoUrl::parse("http://localhost").unwrap();
     let reporter = CSSErrorReporterTest;
     let context = ParserContext::new(Origin::Author, &url, &reporter);
 
     macro_rules! from_css {
         ($css:expr) => {
-            &ViewportRule::parse(&mut Parser::new($css), &context).unwrap()
+            &ViewportRule::parse(&context, &mut Parser::new($css)).unwrap()
         }
     }
 
     let initial_viewport = TypedSize2D::new(800., 600.);
     let device = Device::new(MediaType::Screen, initial_viewport);
     assert_eq!(ViewportConstraints::maybe_new(&device, from_css!("")), None);
 
     assert_eq!(ViewportConstraints::maybe_new(&device, from_css!("width: 320px auto")),