Auto merge of #17337 - servo:derive-all-the-things, r=emilio

Use CustomIdent in more properties

<!-- 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/17337)
<!-- Reviewable:end -->

5 files changed, 54 insertions, 81 deletions

diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index 8c4cdb4a063..c7a46b5e560 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -63,8 +63,8 @@ use std::mem::{forget, transmute, zeroed};
 use std::ptr;
 use stylearc::Arc;
 use std::cmp;
+use values::{Auto, CustomIdent, Either, KeyframesName};
 use values::computed::{Shadow, ToComputedValue};
-use values::{Either, Auto, KeyframesName};
 use computed_values::border_style;
 
 pub mod style_structs {
@@ -1190,8 +1190,8 @@ fn static_assert() {
     pub fn set_${value.name}(&mut self, v: longhands::${value.name}::computed_value::T) {
         use gecko_bindings::structs::{nsStyleGridLine_kMinLine, nsStyleGridLine_kMaxLine};
 
-        let ident = v.ident.unwrap_or(String::new());
-        self.gecko.${value.gecko}.mLineName.assign_utf8(&ident);
+        let ident = v.ident.as_ref().map_or(&[] as &[_], |ident| ident.0.as_slice());
+        self.gecko.${value.gecko}.mLineName.assign(ident);
         self.gecko.${value.gecko}.mHasSpan = v.is_span;
         self.gecko.${value.gecko}.mInteger = v.line_num.map(|i| {
             // clamping the integer between a range
@@ -2430,8 +2430,8 @@ fn static_assert() {
             self.gecko.mTransitionPropertyCount = v.len() as u32;
             for (servo, gecko) in v.zip(self.gecko.mTransitions.iter_mut()) {
                 match servo {
-                    TransitionProperty::Unsupported(ref atom) => unsafe {
-                        Gecko_StyleTransition_SetUnsupportedProperty(gecko, atom.as_ptr())
+                    TransitionProperty::Unsupported(ref ident) => unsafe {
+                        Gecko_StyleTransition_SetUnsupportedProperty(gecko, ident.0.as_ptr())
                     },
                     _ => gecko.mProperty = (&servo).into(),
                 }
@@ -2466,11 +2466,11 @@ fn static_assert() {
         if property == eCSSProperty_UNKNOWN || property == eCSSPropertyExtra_variable {
             let atom = self.gecko.mTransitions[index].mUnknownProperty.raw::<nsIAtom>();
             debug_assert!(!atom.is_null());
-            TransitionProperty::Unsupported(atom.into())
+            TransitionProperty::Unsupported(CustomIdent(atom.into()))
         } else if property == eCSSPropertyExtra_no_properties {
             // Actually, we don't expect TransitionProperty::Unsupported also represents "none",
             // but if the caller wants to convert it, it is fine. Please use it carefully.
-            TransitionProperty::Unsupported(atom!("none"))
+            TransitionProperty::Unsupported(CustomIdent(atom!("none")))
         } else {
             property.into()
         }
@@ -2683,19 +2683,19 @@ fn static_assert() {
                 }
 
                 for feature in features.iter() {
-                    if feature == &atom!("scroll-position") {
+                    if feature.0 == atom!("scroll-position") {
                         self.gecko.mWillChangeBitField |= NS_STYLE_WILL_CHANGE_SCROLL as u8;
-                    } else if feature == &atom!("opacity") {
+                    } else if feature.0 == atom!("opacity") {
                         self.gecko.mWillChangeBitField |= NS_STYLE_WILL_CHANGE_OPACITY as u8;
-                    } else if feature == &atom!("transform") {
+                    } else if feature.0 == atom!("transform") {
                         self.gecko.mWillChangeBitField |= NS_STYLE_WILL_CHANGE_TRANSFORM as u8;
                     }
 
                     unsafe {
-                        Gecko_AppendWillChange(&mut self.gecko, feature.as_ptr());
+                        Gecko_AppendWillChange(&mut self.gecko, feature.0.as_ptr());
                     }
 
-                    if let Ok(prop_id) = PropertyId::parse(feature.to_string().into()) {
+                    if let Ok(prop_id) = PropertyId::parse(feature.0.to_string().into()) {
                         match prop_id.as_shorthand() {
                             Ok(shorthand) => {
                                 for longhand in shorthand.longhands() {
diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs
index dfc55107435..6e787cafaa3 100644
--- a/components/style/properties/helpers/animated_properties.mako.rs
+++ b/components/style/properties/helpers/animated_properties.mako.rs
@@ -7,7 +7,7 @@
 <% from data import SYSTEM_FONT_LONGHANDS %>
 
 use app_units::Au;
-use cssparser::{Parser, RGBA, serialize_identifier};
+use cssparser::{Parser, RGBA};
 use euclid::{Point2D, Size2D};
 #[cfg(feature = "gecko")] use gecko_bindings::bindings::RawServoAnimationValueMap;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::RawGeckoGfxMatrix4x4;
@@ -28,15 +28,12 @@ use properties::longhands::vertical_align::computed_value::T as VerticalAlign;
 use properties::longhands::visibility::computed_value::T as Visibility;
 #[cfg(feature = "gecko")] use properties::{PropertyDeclarationId, LonghandId};
 use selectors::parser::SelectorParseError;
-#[cfg(feature = "servo")] use servo_atoms::Atom;
 use smallvec::SmallVec;
 use std::cmp;
 #[cfg(feature = "gecko")] use std::collections::HashMap;
-use std::fmt;
-use style_traits::{ToCss, ParseError};
+use style_traits::ParseError;
 use super::ComputedValues;
-use values::CSSFloat;
-use values::{Auto, Either};
+use values::{Auto, CSSFloat, CustomIdent, Either};
 use values::computed::{Angle, LengthOrPercentageOrAuto, LengthOrPercentageOrNone};
 use values::computed::{BorderCornerRadius, ClipRect};
 use values::computed::{CalcLengthOrPercentage, Color, Context, ComputedValueAsSpecified};
@@ -180,8 +177,8 @@ pub fn nscsspropertyid_is_animatable(property: nsCSSPropertyID) -> bool {
 /// a shorthand with at least one transitionable longhand component, or an unsupported property.
 // NB: This needs to be here because it needs all the longhands generated
 // beforehand.
-#[derive(Clone, Debug, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Clone, Debug, Eq, Hash, PartialEq, ToCss)]
 pub enum TransitionProperty {
     /// All, any transitionable property changing should generate a transition.
     All,
@@ -193,7 +190,7 @@ pub enum TransitionProperty {
     % endfor
     /// Unrecognized property which could be any non-transitionable, custom property, or
     /// unknown property.
-    Unsupported(Atom)
+    Unsupported(CustomIdent)
 }
 
 no_viewport_percentage!(TransitionProperty);
@@ -226,21 +223,22 @@ impl TransitionProperty {
     /// Parse a transition-property value.
     pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         let ident = try!(input.expect_ident());
-        (match_ignore_ascii_case! { &ident,
-            "all" => Ok(TransitionProperty::All),
+        let supported = match_ignore_ascii_case! { &ident,
+            "all" => Ok(Some(TransitionProperty::All)),
             % for prop in data.longhands + data.shorthands_except_all():
                 % if prop.transitionable:
-                    "${prop.name}" => Ok(TransitionProperty::${prop.camel_case}),
+                    "${prop.name}" => Ok(Some(TransitionProperty::${prop.camel_case})),
                 % endif
             % endfor
             "none" => Err(()),
-            _ => {
-                match CSSWideKeyword::from_ident(&ident) {
-                    Some(_) => Err(()),
-                    None => Ok(TransitionProperty::Unsupported((&*ident).into()))
-                }
-            }
-        }).map_err(|()| SelectorParseError::UnexpectedIdent(ident.into()).into())
+            _ => Ok(None),
+        };
+
+        match supported {
+            Ok(Some(property)) => Ok(property),
+            Ok(None) => CustomIdent::from_ident(ident, &[]).map(TransitionProperty::Unsupported),
+            Err(()) => Err(SelectorParseError::UnexpectedIdent(ident).into()),
+        }
     }
 
     /// Return transitionable longhands of this shorthand TransitionProperty, except for "all".
@@ -279,26 +277,6 @@ impl TransitionProperty {
     }
 }
 
-impl ToCss for TransitionProperty {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
-        where W: fmt::Write,
-    {
-        match *self {
-            TransitionProperty::All => dest.write_str("all"),
-            % for prop in data.longhands + data.shorthands_except_all():
-                % if prop.transitionable:
-                    TransitionProperty::${prop.camel_case} => dest.write_str("${prop.name}"),
-                % endif
-            % endfor
-            #[cfg(feature = "gecko")]
-            TransitionProperty::Unsupported(ref atom) => serialize_identifier(&atom.to_string(),
-                                                                              dest),
-            #[cfg(feature = "servo")]
-            TransitionProperty::Unsupported(ref atom) => serialize_identifier(atom, dest),
-        }
-    }
-}
-
 /// Convert to nsCSSPropertyID.
 #[cfg(feature = "gecko")]
 #[allow(non_upper_case_globals)]
@@ -329,7 +307,7 @@ impl From<nsCSSPropertyID> for TransitionProperty {
                         => TransitionProperty::${prop.camel_case},
                 % else:
                     ${helpers.to_nscsspropertyid(prop.ident)}
-                        => TransitionProperty::Unsupported(Atom::from("${prop.ident}")),
+                        => TransitionProperty::Unsupported(CustomIdent(Atom::from("${prop.ident}"))),
                 % endif
             % endfor
             nsCSSPropertyID::eCSSPropertyExtra_all_properties => TransitionProperty::All,
diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs
index 551b10f96ae..e839985f950 100644
--- a/components/style/properties/longhand/box.mako.rs
+++ b/components/style/properties/longhand/box.mako.rs
@@ -1866,9 +1866,9 @@ ${helpers.single_keyword("-moz-orient",
 
 <%helpers:longhand name="will-change" products="gecko" animation_value_type="none"
                    spec="https://drafts.csswg.org/css-will-change/#will-change">
-    use cssparser::serialize_identifier;
     use std::fmt;
     use style_traits::ToCss;
+    use values::CustomIdent;
     use values::computed::ComputedValueAsSpecified;
 
     impl ComputedValueAsSpecified for SpecifiedValue {}
@@ -1882,7 +1882,7 @@ ${helpers.single_keyword("-moz-orient",
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
     pub enum SpecifiedValue {
         Auto,
-        AnimateableFeatures(Vec<Atom>),
+        AnimateableFeatures(Vec<CustomIdent>),
     }
 
     impl ToCss for SpecifiedValue {
@@ -1891,12 +1891,10 @@ ${helpers.single_keyword("-moz-orient",
                 SpecifiedValue::Auto => dest.write_str("auto"),
                 SpecifiedValue::AnimateableFeatures(ref features) => {
                     let (first, rest) = features.split_first().unwrap();
-                    // handle head element
-                    serialize_identifier(&*first.to_string(), dest)?;
-                    // handle tail, precede each with a delimiter
+                    first.to_css(dest)?;
                     for feature in rest {
                         dest.write_str(", ")?;
-                        serialize_identifier(&*feature.to_string(), dest)?;
+                        feature.to_css(dest)?;
                     }
                     Ok(())
                 }
@@ -1916,17 +1914,12 @@ ${helpers.single_keyword("-moz-orient",
             Ok(computed_value::T::Auto)
         } else {
             input.parse_comma_separated(|i| {
-                let ident = i.expect_ident()?;
-                let bad_keyword = match_ignore_ascii_case! { &ident,
-                    "will-change" | "none" | "all" | "auto" |
-                    "initial" | "inherit" | "unset" | "default" => true,
-                    _ => false,
-                };
-                if bad_keyword {
-                    Err(SelectorParseError::UnexpectedIdent(ident.into()).into())
-                } else {
-                    Ok(Atom::from(ident))
-                }
+                CustomIdent::from_ident(i.expect_ident()?, &[
+                    "will-change",
+                    "none",
+                    "all",
+                    "auto",
+                ])
             }).map(SpecifiedValue::AnimateableFeatures)
         }
     }
diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs
index f56941645fb..3cb10439598 100644
--- a/components/style/values/generics/grid.rs
+++ b/components/style/values/generics/grid.rs
@@ -24,7 +24,7 @@ pub struct GridLine {
     /// A custom identifier for named lines.
     ///
     /// https://drafts.csswg.org/css-grid/#grid-placement-slot
-    pub ident: Option<String>,
+    pub ident: Option<CustomIdent>,
     /// Denotes the nth grid line from grid item's placement.
     pub line_num: Option<Integer>,
 }
@@ -62,7 +62,7 @@ impl ToCss for GridLine {
 
         if let Some(ref s) = self.ident {
             dest.write_str(" ")?;
-            serialize_identifier(s, dest)?;
+            s.to_css(dest)?;
         }
 
         Ok(())
@@ -100,12 +100,10 @@ impl Parse for GridLine {
 
                 grid_line.line_num = Some(i);
             } else if let Ok(name) = input.try(|i| i.expect_ident()) {
-                if val_before_span || grid_line.ident.is_some() ||
-                   CustomIdent::from_ident((&*name).into(), &[]).is_err() {
-                    return Err(StyleParseError::UnspecifiedError.into())
+                if val_before_span || grid_line.ident.is_some() {
+                    return Err(StyleParseError::UnspecifiedError.into());
                 }
-
-                grid_line.ident = Some(name.into_owned());
+                grid_line.ident = Some(CustomIdent::from_ident(name, &[])?);
             } else {
                 break
             }
diff --git a/tests/unit/style/parsing/transition_property.rs b/tests/unit/style/parsing/transition_property.rs
index 5929489651c..68663b3cb19 100644
--- a/tests/unit/style/parsing/transition_property.rs
+++ b/tests/unit/style/parsing/transition_property.rs
@@ -7,6 +7,7 @@ use servo_atoms::Atom;
 use style::properties::animated_properties::TransitionProperty;
 use style::properties::longhands::transition_property;
 use style::properties::shorthands::transition;
+use style::values::CustomIdent;
 use style_traits::ToCss;
 
 #[test]
@@ -19,12 +20,15 @@ fn test_longhand_properties() {
     assert_roundtrip_with_context!(transition_property::parse, "-other-unsupported-property");
     assert_roundtrip_with_context!(transition_property::parse, "--var");
 
-    assert_eq!(parse_longhand!(transition_property, "margin-left, transition-delay, width, --var"),
-               transition_property::SpecifiedValue(
-                   vec![TransitionProperty::MarginLeft,
-                        TransitionProperty::Unsupported(Atom::from("transition-delay")),
-                        TransitionProperty::Width,
-                        TransitionProperty::Unsupported(Atom::from("--var"))]));
+    assert_eq!(
+        parse_longhand!(transition_property, "margin-left, transition-delay, width, --var"),
+        transition_property::SpecifiedValue(vec![
+            TransitionProperty::MarginLeft,
+            TransitionProperty::Unsupported(CustomIdent(Atom::from("transition-delay"))),
+            TransitionProperty::Width,
+            TransitionProperty::Unsupported(CustomIdent(Atom::from("--var"))),
+        ])
+    );
 
     assert!(parse(transition_property::parse, ".width").is_err());
     assert!(parse(transition_property::parse, "1width").is_err());