servo: Merge #19455 - style: Don't waste an allocation when failing to parse a CSSParserColor (from emilio:color-does-stupid-things-too); r=upsuper
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 01 Dec 2017 18:03:11 -0600
changeset 394765 581884fc2da1d5e9e8ea3653deb38d70a68571f5
parent 394764 8994162ee112a8c0a0b33f3404f04ebd9fad1285
child 394766 da64771ef8457997534d1b612388443ae3894c27
push id97969
push userbtara@mozilla.com
push dateMon, 04 Dec 2017 20:48:53 +0000
treeherdermozilla-inbound@b580f65bdb21 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersupsuper
bugs19455
milestone59.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 #19455 - style: Don't waste an allocation when failing to parse a CSSParserColor (from emilio:color-does-stupid-things-too); r=upsuper I see that allocation show up in the profiles, and it makes sense, because system colors and such are common in Firefox, and they're just wasting it. Note that the clone() added is refcounted. Source-Repo: https://github.com/servo/servo Source-Revision: 49e6594bc9f4c8178175232ab4a61581144dfa8e
servo/components/style/values/specified/color.rs
--- a/servo/components/style/values/specified/color.rs
+++ b/servo/components/style/values/specified/color.rs
@@ -208,37 +208,43 @@ impl Color {
 
     /// Parse a <color> value.
     pub fn parse_color<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         // Currently we only store authored value for color keywords,
         // because all browsers serialize those values as keywords for
         // specified value.
         let start = input.state();
         let authored = match input.next() {
-            Ok(&Token::Ident(ref s)) => Some(s.to_lowercase().into_boxed_str()),
+            Ok(&Token::Ident(ref s)) => Some(s.clone()),
             _ => None,
         };
         input.reset(&start);
         match input.try(CSSParserColor::parse) {
             Ok(value) =>
                 Ok(match value {
                     CSSParserColor::CurrentColor => Color::CurrentColor,
-                    CSSParserColor::RGBA(rgba) => Color::Numeric {
-                        parsed: rgba,
-                        authored: authored,
-                    },
+                    CSSParserColor::RGBA(rgba) => {
+                        Color::Numeric {
+                            parsed: rgba,
+                            authored: authored.map(|s| s.to_lowercase().into_boxed_str()),
+                        }
+                    }
                 }),
             Err(e) => {
-                #[cfg(feature = "gecko")] {
+                #[cfg(feature = "gecko")]
+                {
                     if let Ok(system) = input.try(SystemColor::parse) {
                         return Ok(Color::System(system));
-                    } else if let Ok(c) = gecko::SpecialColorKeyword::parse(input) {
+                    }
+
+                    if let Ok(c) = gecko::SpecialColorKeyword::parse(input) {
                         return Ok(Color::Special(c));
                     }
                 }
+
                 match e {
                     BasicParseError { kind: BasicParseErrorKind::UnexpectedToken(t), location } => {
                         Err(location.new_custom_error(
                             StyleParseErrorKind::ValueError(ValueParseErrorKind::InvalidColor(t))
                         ))
                     }
                     e => Err(e.into())
                 }