Bug 1371518 - Use AnimatableLonghand for AnimationValueMap and related code; r?hiro draft
authorBrian Birtles <birtles@gmail.com>
Wed, 14 Jun 2017 14:46:48 +0900
changeset 593854 c14e3c0c642f0a64657e9d668dbce6c1ba245a13
parent 593853 63b47754692cd1ec8251a1497ed442f6cc5d3d7e
child 593855 8d33d231ff0739beec28618e17402573b3b67221
push id63840
push userbbirtles@mozilla.com
push dateWed, 14 Jun 2017 07:29:38 +0000
reviewershiro
bugs1371518
milestone56.0a1
Bug 1371518 - Use AnimatableLonghand for AnimationValueMap and related code; r?hiro In the next few patches we move all non-transition related code over to using AnimatableLonghand instead of TransitionProperty. This will allow us to re-purpose TransitionProperty to represent only properties that can be transitioned (i.e. excluding discrete properties) as well as simplifying the code by removing the need to deal with shorthands and the "all" value in places that do not need to handle those values. MozReview-Commit-ID: AHRNOKgAOGV
servo/components/style/animation.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/properties/declaration_block.rs
servo/components/style/properties/helpers/animated_properties.mako.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/stylesheets/keyframes_rule.rs
servo/ports/geckolib/glue.rs
servo/tests/unit/style/keyframes.rs
--- a/servo/components/style/animation.rs
+++ b/servo/components/style/animation.rs
@@ -7,17 +7,17 @@
 
 use Atom;
 use bezier::Bezier;
 use context::SharedStyleContext;
 use dom::OpaqueNode;
 use euclid::point::Point2D;
 use font_metrics::FontMetricsProvider;
 use properties::{self, CascadeFlags, ComputedValues, Importance};
-use properties::animated_properties::{AnimatedProperty, TransitionProperty};
+use properties::animated_properties::{AnimatableLonghand, AnimatedProperty, TransitionProperty};
 use properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection;
 use properties::longhands::animation_iteration_count::single_value::computed_value::T as AnimationIterationCount;
 use properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState;
 use rule_tree::CascadeLevel;
 use std::sync::mpsc::Sender;
 use stylearc::Arc;
 use stylesheets::keyframes_rule::{KeyframesStep, KeyframesStepValue};
 use timer::Timer;
@@ -325,17 +325,37 @@ impl PropertyAnimation {
     fn from_transition_property(transition_property: &TransitionProperty,
                                 timing_function: TimingFunction,
                                 duration: Time,
                                 old_style: &ComputedValues,
                                 new_style: &ComputedValues)
                                 -> Option<PropertyAnimation> {
         debug_assert!(!transition_property.is_shorthand() &&
                       transition_property != &TransitionProperty::All);
-        let animated_property = AnimatedProperty::from_transition_property(transition_property,
+
+        // We're not expecting |transition_property| to be a shorthand (including 'all') and
+        // all other transitionable properties should be animatable longhands (since transitionable
+        // is a subset of animatable).
+        let animatable_longhand =
+            AnimatableLonghand::from_transition_property(transition_property).unwrap();
+
+        PropertyAnimation::from_animatable_longhand(&animatable_longhand,
+                                                    timing_function,
+                                                    duration,
+                                                    old_style,
+                                                    new_style)
+    }
+
+    fn from_animatable_longhand(animatable_longhand: &AnimatableLonghand,
+                                timing_function: TimingFunction,
+                                duration: Time,
+                                old_style: &ComputedValues,
+                                new_style: &ComputedValues)
+                                -> Option<PropertyAnimation> {
+        let animated_property = AnimatedProperty::from_animatable_longhand(animatable_longhand,
                                                                            old_style,
                                                                            new_style);
 
         let property_animation = PropertyAnimation {
             property: animated_property,
             timing_function: timing_function,
             duration: duration,
         };
@@ -738,32 +758,32 @@ pub fn update_style_for_animation(contex
             let target_style = compute_style_for_animation_step(context,
                                                                 target_keyframe,
                                                                 &from_style,
                                                                 &state.cascade_style,
                                                                 font_metrics_provider);
 
             let mut new_style = (*style).clone();
 
-            for transition_property in &animation.properties_changed {
+            for property in &animation.properties_changed {
                 debug!("update_style_for_animation: scanning prop {:?} for animation \"{}\"",
-                       transition_property, name);
-                match PropertyAnimation::from_transition_property(transition_property,
+                       property, name);
+                match PropertyAnimation::from_animatable_longhand(property,
                                                                   timing_function,
                                                                   Time::from_seconds(relative_duration as f32),
                                                                   &from_style,
                                                                   &target_style) {
                     Some(property_animation) => {
-                        debug!("update_style_for_animation: got property animation for prop {:?}", transition_property);
+                        debug!("update_style_for_animation: got property animation for prop {:?}", property);
                         debug!("update_style_for_animation: {:?}", property_animation);
                         property_animation.update(Arc::make_mut(&mut new_style), relative_progress);
                     }
                     None => {
                         debug!("update_style_for_animation: property animation {:?} not animating",
-                               transition_property);
+                               property);
                     }
                 }
             }
 
             debug!("update_style_for_animation: got style change in animation \"{}\"", name);
             *style = new_style;
         }
     }
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -63,17 +63,18 @@ use gecko_bindings::structs::ELEMENT_HAS
 use gecko_bindings::structs::EffectCompositor_CascadeLevel as CascadeLevel;
 use gecko_bindings::structs::NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE;
 use gecko_bindings::structs::NODE_IS_NATIVE_ANONYMOUS;
 use gecko_bindings::sugar::ownership::{HasArcFFI, HasSimpleFFI};
 use logical_geometry::WritingMode;
 use media_queries::Device;
 use properties::{ComputedValues, parse_style_attribute};
 use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock};
-use properties::animated_properties::{AnimationValue, AnimationValueMap, TransitionProperty};
+use properties::animated_properties::{AnimatableLonghand, AnimationValue, AnimationValueMap};
+use properties::animated_properties::TransitionProperty;
 use properties::style_structs::Font;
 use rule_tree::CascadeLevel as ServoCascadeLevel;
 use selector_parser::{AttrValue, ElementExt, PseudoClassStringArg};
 use selectors::Element;
 use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity, NamespaceConstraint};
 use selectors::matching::{ElementSelectorFlags, LocalMatchingContext, MatchingContext};
 use selectors::matching::{RelevantLinkStatus, VisitedHandlingMode};
 use shared_lock::Locked;
@@ -1133,18 +1134,21 @@ impl<'le> TElement for GeckoElement<'le>
             // If there is an existing transition, update only if the end value differs.
             // If the end value has not changed, we should leave the currently running
             // transition as-is since we don't want to interrupt its timing function.
             let after_value =
                 Arc::new(AnimationValue::from_computed_values(property, after_change_style));
             return existing_transitions.get(property).unwrap() != &after_value;
         }
 
+        // |property| should be an animatable longhand
+        let animatable_longhand = AnimatableLonghand::from_transition_property(property).unwrap();
+
         combined_duration > 0.0f32 &&
-        AnimatedProperty::from_transition_property(property,
+        AnimatedProperty::from_animatable_longhand(&animatable_longhand,
                                                    before_change_style,
                                                    after_change_style).does_animate()
     }
 
     #[inline]
     fn lang_attr(&self) -> Option<AttrValue> {
         let ptr = unsafe { bindings::Gecko_LangValue(self.0) };
         if ptr.is_null() {
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -120,31 +120,31 @@ impl<'a, 'cx, 'cx_a:'cx> AnimationValueI
             iter: declarations.declarations().iter(),
             context: context,
             default_values: default_values,
         }
     }
 }
 
 impl<'a, 'cx, 'cx_a:'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> {
-    type Item = (TransitionProperty, AnimationValue);
+    type Item = (AnimatableLonghand, AnimationValue);
     #[inline]
     fn next(&mut self) -> Option<Self::Item> {
         use properties::Importance;
 
         loop {
             let next = self.iter.next();
             match next {
                 Some(&(ref decl, importance)) => {
                     if importance == Importance::Normal {
-                        let property = TransitionProperty::from_declaration(decl);
+                        let property = AnimatableLonghand::from_declaration(decl);
                         let animation = AnimationValue::from_declaration(decl, &mut self.context,
                                                                          self.default_values);
                         debug_assert!(property.is_none() == animation.is_none(),
-                                      "The failure condition of TransitionProperty::from_declaration \
+                                      "The failure condition of AnimatableLonghand::from_declaration \
                                        and AnimationValue::from_declaration should be the same");
                         // Skip the property if either ::from_declaration fails.
                         match (property, animation) {
                             (Some(p), Some(a)) => return Some((p, a)),
                             (_, _) => {},
                         }
                     }
                 },
@@ -195,17 +195,17 @@ impl PropertyDeclarationBlock {
 
     /// Iterate over only PropertyDeclaration.
     pub fn declarations_iter(&self) -> PropertyDeclarationIterator {
         PropertyDeclarationIterator {
             iter: self.declarations.iter(),
         }
     }
 
-    /// Return an iterator of (TransitionProperty, AnimationValue).
+    /// Return an iterator of (AnimatableLonghand, AnimationValue).
     pub fn to_animation_value_iter<'a, 'cx, 'cx_a:'cx>(&'a self,
                                                        context: &'cx mut Context<'cx_a>,
                                                        default_values: &'a Arc<ComputedValues>)
                                                        -> AnimationValueIterator<'a, 'cx, 'cx_a> {
         AnimationValueIterator::new(self, context, default_values)
     }
 
     /// Returns whether this block contains any declaration with `!important`.
@@ -548,17 +548,17 @@ impl PropertyDeclarationBlock {
 
     /// Convert AnimationValueMap to PropertyDeclarationBlock.
     #[cfg(feature = "gecko")]
     pub fn from_animation_value_map(animation_value_map: &AnimationValueMap) -> Self {
         let mut declarations = vec![];
         let mut longhands = LonghandIdSet::new();
 
         for (property, animation_value) in animation_value_map.iter() {
-          longhands.set_transition_property_bit(property);
+          longhands.set_animatable_longhand_bit(property);
           declarations.push((animation_value.uncompute(), Importance::Normal));
         }
 
         PropertyDeclarationBlock {
             declarations: declarations,
             important_count: 0,
             longhands: longhands,
         }
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -46,25 +46,122 @@ use values::generics::border::BorderCorn
 use values::generics::position as generic_position;
 
 
 /// A longhand property whose animation type is not "none".
 ///
 /// NOTE: This includes the 'display' property since it is animatable from SMIL even though it is
 /// not animatable from CSS animations or Web Animations. CSS transitions also does not allow
 /// animating 'display', but for CSS transitions we have the separate TransitionProperty type.
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum AnimatableLonghand {
     % for prop in data.longhands:
         % if prop.animatable:
             /// ${prop.name}
             ${prop.camel_case},
         % endif
     % endfor
 }
 
+impl AnimatableLonghand {
+    /// Converts from an nsCSSPropertyID. Returns None if nsCSSPropertyID is not an animatable
+    /// longhand in Servo.
+    #[cfg(feature = "gecko")]
+    pub fn from_nscsspropertyid(css_property: nsCSSPropertyID) -> Option<Self> {
+        match css_property {
+            % for prop in data.longhands:
+                % if prop.animatable:
+                    ${helpers.to_nscsspropertyid(prop.ident)}
+                        => Some(AnimatableLonghand::${prop.camel_case}),
+                % endif
+            % endfor
+            _ => None
+        }
+    }
+
+    /// Converts from TransitionProperty. Returns None if the property is not an animatable
+    /// longhand.
+    pub fn from_transition_property(transition_property: &TransitionProperty) -> Option<Self> {
+        match *transition_property {
+            % for prop in data.longhands:
+                <%
+                    # TODO: Later in this patch series, once we introduce the 'transitionable'
+                    # definition, we will need to make the below test:
+                    #
+                    #    if prop.transitionable and prop.animatable:
+                %>
+                % if prop.animatable:
+                    TransitionProperty::${prop.camel_case}
+                        => Some(AnimatableLonghand::${prop.camel_case}),
+                % endif
+            % endfor
+            _ => None
+        }
+    }
+
+    /// Get an animatable longhand property from a property declaration.
+    pub fn from_declaration(declaration: &PropertyDeclaration) -> Option<Self> {
+        use properties::LonghandId;
+        match *declaration {
+            % for prop in data.longhands:
+                % if prop.animatable:
+                    PropertyDeclaration::${prop.camel_case}(..)
+                        => Some(AnimatableLonghand::${prop.camel_case}),
+                % endif
+            % endfor
+            PropertyDeclaration::CSSWideKeyword(id, _) |
+            PropertyDeclaration::WithVariables(id, _) => {
+                match id {
+                    % for prop in data.longhands:
+                        % if prop.animatable:
+                            LonghandId::${prop.camel_case} =>
+                                Some(AnimatableLonghand::${prop.camel_case}),
+                        % endif
+                    % endfor
+                    _ => None,
+                }
+            },
+            _ => None,
+        }
+    }
+}
+
+/// Convert to nsCSSPropertyID.
+#[cfg(feature = "gecko")]
+#[allow(non_upper_case_globals)]
+impl<'a> From< &'a AnimatableLonghand> for nsCSSPropertyID {
+    fn from(property: &'a AnimatableLonghand) -> nsCSSPropertyID {
+        match *property {
+            % for prop in data.longhands:
+                % if prop.animatable:
+                    AnimatableLonghand::${prop.camel_case}
+                        => ${helpers.to_nscsspropertyid(prop.ident)},
+                % endif
+            % endfor
+        }
+    }
+}
+
+/// Convert to PropertyDeclarationId.
+#[cfg(feature = "gecko")]
+#[allow(non_upper_case_globals)]
+impl<'a> From<AnimatableLonghand> for PropertyDeclarationId<'a> {
+    fn from(property: AnimatableLonghand) -> PropertyDeclarationId<'a> {
+        match property {
+            % for prop in data.longhands:
+                % if prop.animatable:
+                    AnimatableLonghand::${prop.camel_case}
+                        => PropertyDeclarationId::Longhand(LonghandId::${prop.camel_case}),
+                % endif
+            % endfor
+        }
+    }
+}
+
 /// A given transition property, that is either `All`, an animatable longhand property,
 /// a shorthand with at least one animatable 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))]
 pub enum TransitionProperty {
     /// All, any animatable property changing should generate a transition.
@@ -122,42 +219,16 @@ impl TransitionProperty {
                 match CSSWideKeyword::from_ident(&ident) {
                     Some(_) => Err(()),
                     None => Ok(TransitionProperty::Unsupported((&*ident).into()))
                 }
             }
         }).map_err(|()| SelectorParseError::UnexpectedIdent(ident.into()).into())
     }
 
-    /// Get a transition property from a property declaration.
-    pub fn from_declaration(declaration: &PropertyDeclaration) -> Option<Self> {
-        use properties::LonghandId;
-        match *declaration {
-            % for prop in data.longhands:
-                % if prop.animatable:
-                    PropertyDeclaration::${prop.camel_case}(..)
-                        => Some(TransitionProperty::${prop.camel_case}),
-                % endif
-            % endfor
-            PropertyDeclaration::CSSWideKeyword(id, _) |
-            PropertyDeclaration::WithVariables(id, _) => {
-                match id {
-                    % for prop in data.longhands:
-                        % if prop.animatable:
-                            LonghandId::${prop.camel_case} =>
-                                Some(TransitionProperty::${prop.camel_case}),
-                        % endif
-                    % endfor
-                    _ => None,
-                }
-            },
-            _ => None,
-        }
-    }
-
     /// Returns true if this TransitionProperty is one of the discrete animatable properties and
     /// this TransitionProperty should be a longhand property.
     pub fn is_discrete(&self) -> bool {
         match *self {
             % for prop in data.longhands:
                 % if prop.animation_value_type == "discrete":
                     TransitionProperty::${prop.camel_case} => true,
                 % endif
@@ -269,33 +340,16 @@ impl From<nsCSSPropertyID> for Transitio
                 % endif
             % endfor
             nsCSSPropertyID::eCSSPropertyExtra_all_properties => TransitionProperty::All,
             _ => panic!("Unconvertable nsCSSPropertyID: {:?}", property),
         }
     }
 }
 
-/// Convert to PropertyDeclarationId.
-#[cfg(feature = "gecko")]
-#[allow(non_upper_case_globals)]
-impl<'a> From<TransitionProperty> for PropertyDeclarationId<'a> {
-    fn from(transition_property: TransitionProperty) -> PropertyDeclarationId<'a> {
-        match transition_property {
-            % for prop in data.longhands:
-                % if prop.animatable:
-                    TransitionProperty::${prop.camel_case}
-                        => PropertyDeclarationId::Longhand(LonghandId::${prop.camel_case}),
-                % endif
-            % endfor
-            _ => panic!(),
-        }
-    }
-}
-
 /// An animated property interpolation between two computed values for that
 /// property.
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum AnimatedProperty {
     % for prop in data.longhands:
         % if prop.animatable:
             <%
@@ -372,41 +426,39 @@ impl AnimatedProperty {
                     }
                 % endif
             % endfor
         }
     }
 
     /// Get an animatable value from a transition-property, an old style, and a
     /// new style.
-    pub fn from_transition_property(transition_property: &TransitionProperty,
+    pub fn from_animatable_longhand(property: &AnimatableLonghand,
                                     old_style: &ComputedValues,
                                     new_style: &ComputedValues)
                                     -> AnimatedProperty {
-        match *transition_property {
-            TransitionProperty::All => panic!("Can't use TransitionProperty::All here."),
+        match *property {
             % for prop in data.longhands:
                 % if prop.animatable:
-                    TransitionProperty::${prop.camel_case} => {
+                    AnimatableLonghand::${prop.camel_case} => {
                         AnimatedProperty::${prop.camel_case}(
                             old_style.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}().into(),
                             new_style.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}().into())
                     }
                 % endif
             % endfor
-            ref other => panic!("Can't use TransitionProperty::{:?} here", other),
         }
     }
 }
 
 /// A collection of AnimationValue that were composed on an element.
 /// This HashMap stores the values that are the last AnimationValue to be
 /// composed for each TransitionProperty.
 #[cfg(feature = "gecko")]
-pub type AnimationValueMap = HashMap<TransitionProperty, AnimationValue>;
+pub type AnimationValueMap = HashMap<AnimatableLonghand, AnimationValue>;
 #[cfg(feature = "gecko")]
 unsafe impl HasFFI for AnimationValueMap {
     type FFIType = RawServoAnimationValueMap;
 }
 #[cfg(feature = "gecko")]
 unsafe impl HasSimpleFFI for AnimationValueMap {}
 
 /// An enum to represent a single computed value belonging to an animated
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -27,17 +27,17 @@ use computed_values;
 use context::QuirksMode;
 use font_metrics::FontMetricsProvider;
 #[cfg(feature = "gecko")] use gecko_bindings::bindings;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::{self, nsCSSPropertyID};
 #[cfg(feature = "servo")] use logical_geometry::{LogicalMargin, PhysicalSide};
 use logical_geometry::WritingMode;
 use media_queries::Device;
 use parser::{PARSING_MODE_DEFAULT, Parse, ParserContext};
-use properties::animated_properties::TransitionProperty;
+use properties::animated_properties::AnimatableLonghand;
 #[cfg(feature = "gecko")] use properties::longhands::system_font::SystemFont;
 use selectors::parser::SelectorParseError;
 #[cfg(feature = "servo")] use servo_config::prefs::PREFS;
 use shared_lock::StylesheetGuards;
 use style_traits::{HasViewportPercentage, ToCss, ParseError, PropertyDeclarationParseError};
 use stylesheets::{CssRuleType, MallocSizeOf, MallocSizeOfFn, Origin, UrlExtraData};
 #[cfg(feature = "servo")] use values::Either;
 use values::generics::text::LineHeight;
@@ -295,39 +295,35 @@ impl LonghandIdSet {
     /// Clear all bits
     #[inline]
     pub fn clear(&mut self) {
         for cell in &mut self.storage {
             *cell = 0
         }
     }
 
-    /// Set the corresponding bit of TransitionProperty.
-    /// This function will panic if TransitionProperty::All is given.
-    pub fn set_transition_property_bit(&mut self, property: &TransitionProperty) {
+    /// Set the corresponding bit of AnimatableLonghand.
+    pub fn set_animatable_longhand_bit(&mut self, property: &AnimatableLonghand) {
         match *property {
             % for prop in data.longhands:
                 % if prop.animatable:
-                    TransitionProperty::${prop.camel_case} => self.insert(LonghandId::${prop.camel_case}),
+                    AnimatableLonghand::${prop.camel_case} => self.insert(LonghandId::${prop.camel_case}),
                 % endif
             % endfor
-            ref other => unreachable!("Tried to set TransitionProperty::{:?} in a PropertyBitfield", other),
         }
     }
 
-    /// Return true if the corresponding bit of TransitionProperty is set.
-    /// This function will panic if TransitionProperty::All is given.
-    pub fn has_transition_property_bit(&self, property: &TransitionProperty) -> bool {
+    /// Return true if the corresponding bit of AnimatableLonghand is set.
+    pub fn has_animatable_longhand_bit(&self, property: &AnimatableLonghand) -> bool {
         match *property {
             % for prop in data.longhands:
                 % if prop.animatable:
-                    TransitionProperty::${prop.camel_case} => self.contains(LonghandId::${prop.camel_case}),
+                    AnimatableLonghand::${prop.camel_case} => self.contains(LonghandId::${prop.camel_case}),
                 % endif
             % endfor
-            ref other => unreachable!("Tried to get TransitionProperty::{:?} in a PropertyBitfield", other),
         }
     }
 }
 
 /// A specialized set of PropertyDeclarationId
 pub struct PropertyDeclarationIdSet {
     longhands: LonghandIdSet,
 
--- a/servo/components/style/stylesheets/keyframes_rule.rs
+++ b/servo/components/style/stylesheets/keyframes_rule.rs
@@ -6,17 +6,17 @@
 
 use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser, ParserInput};
 use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule, SourceLocation};
 use error_reporting::{NullReporter, ContextualParseError};
 use parser::{PARSING_MODE_DEFAULT, ParserContext, log_css_error};
 use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock, PropertyId};
 use properties::{PropertyDeclarationId, LonghandId, SourcePropertyDeclaration};
 use properties::LonghandIdSet;
-use properties::animated_properties::TransitionProperty;
+use properties::animated_properties::AnimatableLonghand;
 use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction;
 use selectors::parser::SelectorParseError;
 use shared_lock::{DeepCloneWithLock, SharedRwLock, SharedRwLockReadGuard, Locked, ToCssWithGuard};
 use std::borrow::Cow;
 use std::fmt;
 use style_traits::{ToCss, ParseError, StyleParseError};
 use stylearc::Arc;
 use stylesheets::{CssRuleType, Stylesheet};
@@ -332,52 +332,52 @@ impl KeyframesStep {
 ///
 /// It only takes into account animable properties.
 #[derive(Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct KeyframesAnimation {
     /// The difference steps of the animation.
     pub steps: Vec<KeyframesStep>,
     /// The properties that change in this animation.
-    pub properties_changed: Vec<TransitionProperty>,
+    pub properties_changed: Vec<AnimatableLonghand>,
     /// Vendor prefix type the @keyframes has.
     pub vendor_prefix: Option<VendorPrefix>,
 }
 
 /// Get all the animated properties in a keyframes animation.
 fn get_animated_properties(keyframes: &[Arc<Locked<Keyframe>>], guard: &SharedRwLockReadGuard)
-                           -> Vec<TransitionProperty> {
+                           -> Vec<AnimatableLonghand> {
     let mut ret = vec![];
     let mut seen = LonghandIdSet::new();
     // NB: declarations are already deduplicated, so we don't have to check for
     // it here.
     for keyframe in keyframes {
         let keyframe = keyframe.read_with(&guard);
         let block = keyframe.block.read_with(guard);
         for &(ref declaration, importance) in block.declarations().iter() {
             assert!(!importance.important());
 
-            if let Some(property) = TransitionProperty::from_declaration(declaration) {
-                if !seen.has_transition_property_bit(&property) {
-                    seen.set_transition_property_bit(&property);
+            if let Some(property) = AnimatableLonghand::from_declaration(declaration) {
+                if !seen.has_animatable_longhand_bit(&property) {
+                    seen.set_animatable_longhand_bit(&property);
                     ret.push(property);
                 }
             }
         }
     }
 
     ret
 }
 
 impl KeyframesAnimation {
     /// Create a keyframes animation from a given list of keyframes.
     ///
     /// This will return a keyframe animation with empty steps and
     /// properties_changed if the list of keyframes is empty, or there are no
-    //  animated properties obtained from the keyframes.
+    /// animated properties obtained from the keyframes.
     ///
     /// Otherwise, this will compute and sort the steps used for the animation,
     /// and return the animation object.
     pub fn from_keyframes(keyframes: &[Arc<Locked<Keyframe>>],
                           vendor_prefix: Option<VendorPrefix>,
                           guard: &SharedRwLockReadGuard)
                           -> Self {
         let mut result = KeyframesAnimation {
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -90,17 +90,18 @@ use style::gecko_bindings::sugar::refptr
 use style::gecko_properties::{self, style_structs};
 use style::invalidation::element::restyle_hints::{self, RestyleHint};
 use style::media_queries::{MediaList, parse_media_query_list};
 use style::parallel;
 use style::parser::{PARSING_MODE_DEFAULT, ParserContext};
 use style::properties::{CascadeFlags, ComputedValues, Importance, SourcePropertyDeclaration};
 use style::properties::{LonghandIdSet, PropertyDeclaration, PropertyDeclarationBlock, PropertyId, StyleBuilder};
 use style::properties::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP;
-use style::properties::animated_properties::{Animatable, AnimationValue, TransitionProperty};
+use style::properties::animated_properties::{Animatable, AnimatableLonghand, AnimationValue};
+use style::properties::animated_properties::TransitionProperty;
 use style::properties::parse_one_declaration_into;
 use style::rule_tree::StyleSource;
 use style::selector_parser::PseudoElementCascadeType;
 use style::sequential;
 use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, StylesheetGuards, ToCssWithGuard, Locked};
 use style::string_cache::Atom;
 use style::style_adjuster::StyleAdjuster;
 use style::stylearc::Arc;
@@ -388,17 +389,20 @@ pub extern "C" fn Servo_AnimationCompose
                                          computed_timing: RawGeckoComputedTimingBorrowed,
                                          iteration_composite: IterationCompositeOperation)
 {
     use style::gecko_bindings::bindings::Gecko_AnimationGetBaseStyle;
     use style::gecko_bindings::bindings::Gecko_GetPositionInSegment;
     use style::gecko_bindings::bindings::Gecko_GetProgressFromComputedTiming;
     use style::properties::animated_properties::AnimationValueMap;
 
-    let property: TransitionProperty = css_property.into();
+    let property = match AnimatableLonghand::from_nscsspropertyid(css_property) {
+        Some(longhand) => longhand,
+        None => { return (); }
+    };
     let value_map = AnimationValueMap::from_ffi_mut(raw_value_map);
 
     // We will need an underlying value if either of the endpoints is null...
     let need_underlying_value = segment.mFromValue.mServo.mRawPtr.is_null() ||
                                 segment.mToValue.mServo.mRawPtr.is_null() ||
                                 // ... or if they have a non-replace composite mode ...
                                 segment.mFromComposite != CompositeOperation::Replace ||
                                 segment.mToComposite != CompositeOperation::Replace ||
@@ -2677,20 +2681,20 @@ pub extern "C" fn Servo_GetComputedKeyfr
                 continue;
             }
 
             let declarations = unsafe { &*property.mServoDeclarationBlock.mRawPtr.clone() };
             let declarations = Locked::<PropertyDeclarationBlock>::as_arc(&declarations);
             let guard = declarations.read_with(&guard);
 
             for anim in guard.to_animation_value_iter(&mut context, &default_values) {
-                if !seen.has_transition_property_bit(&anim.0) {
+                if !seen.has_animatable_longhand_bit(&anim.0) {
                     // This is safe since we immediately write to the uninitialized values.
                     unsafe { animation_values.set_len((property_index + 1) as u32) };
-                    seen.set_transition_property_bit(&anim.0);
+                    seen.set_animatable_longhand_bit(&anim.0);
                     animation_values[property_index].mProperty = (&anim.0).into();
                     // We only make sure we have enough space for this variable,
                     // but didn't construct a default value for StyleAnimationValue,
                     // so we should zero it to avoid getting undefined behaviors.
                     animation_values[property_index].mValue.mGecko = unsafe { mem::zeroed() };
                     animation_values[property_index].mValue.mServo.set_arc_leaky(Arc::new(anim.1));
                     property_index += 1;
                 }
@@ -2777,17 +2781,17 @@ pub extern "C" fn Servo_AssertTreeIsClea
         }
     }
 
     assert_subtree_is_clean(root);
 }
 
 fn append_computed_property_value(keyframe: *mut structs::Keyframe,
                                   style: &ComputedValues,
-                                  property: &TransitionProperty,
+                                  property: &AnimatableLonghand,
                                   shared_lock: &SharedRwLock) {
     let block = style.to_declaration_block(property.clone().into());
     unsafe {
         let index = (*keyframe).mPropertyValues.len();
         (*keyframe).mPropertyValues.set_len((index + 1) as u32);
         (*keyframe).mPropertyValues[index].mProperty = property.into();
         // FIXME. Bug 1360398: Do not set computed values once we handles
         // missing keyframes with additive composition.
@@ -2796,25 +2800,25 @@ fn append_computed_property_value(keyfra
     }
 }
 
 enum Offset {
     Zero,
     One
 }
 
-fn fill_in_missing_keyframe_values(all_properties:  &[TransitionProperty],
+fn fill_in_missing_keyframe_values(all_properties:  &[AnimatableLonghand],
                                    timing_function: nsTimingFunctionBorrowed,
                                    style: &ComputedValues,
                                    properties_set_at_offset: &LonghandIdSet,
                                    offset: Offset,
                                    keyframes: RawGeckoKeyframeListBorrowedMut,
                                    shared_lock: &SharedRwLock) {
     let needs_filling = all_properties.iter().any(|ref property| {
-        !properties_set_at_offset.has_transition_property_bit(property)
+        !properties_set_at_offset.has_animatable_longhand_bit(property)
     });
 
     // Return earli if all animated properties are already set.
     if !needs_filling {
         return;
     }
 
     let keyframe = match offset {
@@ -2823,17 +2827,17 @@ fn fill_in_missing_keyframe_values(all_p
         },
         Offset::One => unsafe {
             Gecko_GetOrCreateFinalKeyframe(keyframes, timing_function)
         },
     };
 
     // Append properties that have not been set at this offset.
     for ref property in all_properties.iter() {
-        if !properties_set_at_offset.has_transition_property_bit(property) {
+        if !properties_set_at_offset.has_animatable_longhand_bit(property) {
             append_computed_property_value(keyframe,
                                            style,
                                            property,
                                            shared_lock);
         }
     }
 }
 
@@ -2915,27 +2919,27 @@ pub extern "C" fn Servo_StyleSet_GetKeyf
                     guard.declarations()
                          .iter()
                          .filter(|&&(ref declaration, _)| {
                              declaration.is_animatable()
                          });
 
                 let mut index = unsafe { (*keyframe).mPropertyValues.len() };
                 for &(ref declaration, _) in animatable {
-                    let property = TransitionProperty::from_declaration(declaration).unwrap();
-                    if !properties_set_at_current_offset.has_transition_property_bit(&property) {
-                        properties_set_at_current_offset.set_transition_property_bit(&property);
+                    let property = AnimatableLonghand::from_declaration(declaration).unwrap();
+                    if !properties_set_at_current_offset.has_animatable_longhand_bit(&property) {
+                        properties_set_at_current_offset.set_animatable_longhand_bit(&property);
                         if current_offset == 0.0 {
-                            properties_set_at_start.set_transition_property_bit(&property);
+                            properties_set_at_start.set_animatable_longhand_bit(&property);
                         } else if current_offset == 1.0 {
-                            properties_set_at_end.set_transition_property_bit(&property);
+                            properties_set_at_end.set_animatable_longhand_bit(&property);
                         }
 
                         unsafe {
-                            let property = TransitionProperty::from_declaration(declaration).unwrap();
+                            let property = AnimatableLonghand::from_declaration(declaration).unwrap();
                             (*keyframe).mPropertyValues.set_len((index + 1) as u32);
                             (*keyframe).mPropertyValues[index].mProperty = (&property).into();
                             (*keyframe).mPropertyValues[index].mServoDeclarationBlock.set_arc_leaky(
                                 Arc::new(global_style_data.shared_lock.wrap(
                                   PropertyDeclarationBlock::with_one(
                                       declaration.clone(), Importance::Normal
                                 ))));
                         }
--- a/servo/tests/unit/style/keyframes.rs
+++ b/servo/tests/unit/style/keyframes.rs
@@ -1,14 +1,14 @@
 /* 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/. */
 
 use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, Importance};
-use style::properties::animated_properties::TransitionProperty;
+use style::properties::animated_properties::AnimatableLonghand;
 use style::shared_lock::SharedRwLock;
 use style::stylearc::Arc;
 use style::stylesheets::keyframes_rule::{Keyframe, KeyframesAnimation, KeyframePercentage,  KeyframeSelector};
 use style::stylesheets::keyframes_rule::{KeyframesStep, KeyframesStepValue};
 use style::values::specified::{LengthOrPercentageOrAuto, NoCalcLength};
 
 #[test]
 fn test_empty_keyframe() {
@@ -95,17 +95,17 @@ fn test_missing_property_in_initial_keyf
                 declared_timing_function: false,
             },
             KeyframesStep {
                 start_percentage: KeyframePercentage(1.),
                 value: KeyframesStepValue::Declarations { block: declarations_on_final_keyframe },
                 declared_timing_function: false,
             },
         ],
-        properties_changed: vec![TransitionProperty::Width, TransitionProperty::Height],
+        properties_changed: vec![AnimatableLonghand::Width, AnimatableLonghand::Height],
         vendor_prefix: None,
     };
 
     assert_eq!(format!("{:#?}", animation), format!("{:#?}", expected));
 }
 
 #[test]
 fn test_missing_property_in_final_keyframe() {
@@ -155,17 +155,17 @@ fn test_missing_property_in_final_keyfra
                 declared_timing_function: false,
             },
             KeyframesStep {
                 start_percentage: KeyframePercentage(1.),
                 value: KeyframesStepValue::Declarations { block: declarations_on_final_keyframe },
                 declared_timing_function: false,
             },
         ],
-        properties_changed: vec![TransitionProperty::Width, TransitionProperty::Height],
+        properties_changed: vec![AnimatableLonghand::Width, AnimatableLonghand::Height],
         vendor_prefix: None,
     };
 
     assert_eq!(format!("{:#?}", animation), format!("{:#?}", expected));
 }
 
 #[test]
 fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() {
@@ -217,14 +217,14 @@ fn test_missing_keyframe_in_both_of_init
                 declared_timing_function: false,
             },
             KeyframesStep {
                 start_percentage: KeyframePercentage(1.),
                 value: KeyframesStepValue::ComputedValues,
                 declared_timing_function: false,
             }
         ],
-        properties_changed: vec![TransitionProperty::Width, TransitionProperty::Height],
+        properties_changed: vec![AnimatableLonghand::Width, AnimatableLonghand::Height],
         vendor_prefix: None,
     };
 
     assert_eq!(format!("{:#?}", animation), format!("{:#?}", expected));
 }