servo: Merge #17331 - Support animation of the display property (from birtles:animate-display); r=hiro

These are the Servo-side changes of [Mozilla bug 1371518](https://bugzilla.mozilla.org/show_bug.cgi?id=1371518).

These changes have been reviewed by @hiikezoe.

Source-Repo: https://github.com/servo/servo
Source-Revision: 09b8f124f59f98d83e4fe72a3fe4d506e912cdb6

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : c1d1941bcbf78d38a7ec6c1201d1df4061dbc18a

servo/components/style/animation.rs

@@ -12,7 +12,7 @@ use dom::OpaqueNode;
 use euclid::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;
@@ -330,7 +330,27 @@ impl PropertyAnimation {
                                 -> 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);
 
@@ -743,22 +763,22 @@ pub fn update_style_for_animation(context: &SharedStyleContext,
 
             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);
                     }
                 }
             }

servo/components/style/gecko/generated/bindings.rs

@@ -2238,13 +2238,6 @@ extern "C" {
                                            result:
                                                RawGeckoComputedKeyframeValuesListBorrowedMut);
 }
-extern "C" {
-    pub fn Servo_AnimationValueMap_Push(arg1:
-                                            RawServoAnimationValueMapBorrowedMut,
-                                        property: nsCSSPropertyID,
-                                        value:
-                                            RawServoAnimationValueBorrowed);
-}
 extern "C" {
     pub fn Servo_ComputedValues_ExtractAnimationValue(computed_values:
                                                           ServoComputedValuesBorrowed,
@@ -2255,6 +2248,9 @@ extern "C" {
 extern "C" {
     pub fn Servo_Property_IsAnimatable(property: nsCSSPropertyID) -> bool;
 }
+extern "C" {
+    pub fn Servo_Property_IsTransitionable(property: nsCSSPropertyID) -> bool;
+}
 extern "C" {
     pub fn Servo_Property_IsDiscreteAnimatable(property: nsCSSPropertyID)
      -> bool;

servo/components/style/gecko/wrapper.rs

@@ -68,7 +68,8 @@ 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};
@@ -1039,7 +1040,6 @@ impl<'le> TElement for GeckoElement<'le> {
                                 after_change_style: &ComputedValues)
                                 -> bool {
         use gecko_bindings::structs::nsCSSPropertyID;
-        use properties::{PropertyId, animated_properties};
         use std::collections::HashSet;
 
         debug_assert!(self.might_need_transitions_update(Some(before_change_style),
@@ -1074,6 +1074,8 @@ impl<'le> TElement for GeckoElement<'le> {
                 continue;
             }
 
+            let transition_property: TransitionProperty = property.into();
+
             let mut property_check_helper = |property: &TransitionProperty| -> bool {
                 if self.needs_transitions_update_per_property(property,
                                                               combined_duration,
@@ -1090,26 +1092,25 @@ impl<'le> TElement for GeckoElement<'le> {
                 }
                 false
             };
-            if property == nsCSSPropertyID::eCSSPropertyExtra_all_properties {
-                if TransitionProperty::any(property_check_helper) {
-                    return true;
-                }
-            } else {
-                let is_shorthand = PropertyId::from_nscsspropertyid(property).ok().map_or(false, |p| {
-                        p.as_shorthand().is_ok()
-                });
-                if is_shorthand {
-                    let shorthand: TransitionProperty = property.into();
+
+            match transition_property {
+                TransitionProperty::All => {
+                    if TransitionProperty::any(property_check_helper) {
+                        return true;
+                    }
+                },
+                TransitionProperty::Unsupported(_) => { },
+                ref shorthand if shorthand.is_shorthand() => {
                     if shorthand.longhands().iter().any(|p| property_check_helper(p)) {
                         return true;
                     }
-                } else {
-                    if animated_properties::nscsspropertyid_is_animatable(property) &&
-                       property_check_helper(&property.into()) {
+                },
+                ref longhand => {
+                    if property_check_helper(longhand) {
                         return true;
                     }
-                }
-            }
+                },
+            };
         }
 
         // Check if we have to cancel the running transition because this is not a matching
@@ -1129,22 +1130,21 @@ impl<'le> TElement for GeckoElement<'le> {
                                              -> bool {
         use properties::animated_properties::AnimatedProperty;
 
-        // We don't allow transitions on properties that are not interpolable.
-        if property.is_discrete() {
-            return false;
-        }
+        // |property| should be an animatable longhand
+        let animatable_longhand = AnimatableLonghand::from_transition_property(property).unwrap();
 
         if existing_transitions.contains_key(property) {
             // 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));
+                Arc::new(AnimationValue::from_computed_values(&animatable_longhand,
+                                                              after_change_style));
             return existing_transitions.get(property).unwrap() != &after_value;
         }
 
         combined_duration > 0.0f32 &&
-        AnimatedProperty::from_transition_property(property,
+        AnimatedProperty::from_animatable_longhand(&animatable_longhand,
                                                    before_change_style,
                                                    after_change_style).does_animate()
     }

servo/components/style/properties/data.py

@@ -194,12 +194,15 @@ class Longhand(object):
         self.animation_value_type = animation_value_type
 
         self.animatable = animation_value_type != "none"
+        self.transitionable = animation_value_type != "none" \
+            and animation_value_type != "discrete"
         self.is_animatable_with_computed_value = animation_value_type == "ComputedValue" \
             or animation_value_type == "discrete"
         if self.logical:
             # Logical properties will be animatable (i.e. the animation type is
             # discrete). For now, it is still non-animatable.
             self.animatable = False
+            self.transitionable = False
             self.animation_type = None
         # NB: Animatable implies clone because a property animation requires a
         # copy of the computed value.
@@ -234,6 +237,25 @@ class Shorthand(object):
         self.allowed_in_keyframe_block = allowed_in_keyframe_block \
             and allowed_in_keyframe_block != "False"
 
+    def get_animatable(self):
+        animatable = False
+        for sub in self.sub_properties:
+            if sub.animatable:
+                animatable = True
+                break
+        return animatable
+
+    def get_transitionable(self):
+        transitionable = False
+        for sub in self.sub_properties:
+            if sub.transitionable:
+                transitionable = True
+                break
+        return transitionable
+
+    animatable = property(get_animatable)
+    transitionable = property(get_transitionable)
+
 
 class Method(object):
     def __init__(self, name, return_type=None, arg_types=None, is_mut=False):

servo/components/style/properties/declaration_block.rs

@@ -126,7 +126,7 @@ impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> {
 }
 
 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;
@@ -136,11 +136,11 @@ impl<'a, 'cx, 'cx_a:'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> {
             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) {
@@ -201,7 +201,7 @@ impl PropertyDeclarationBlock {
         }
     }
 
-    /// 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>)
@@ -554,7 +554,7 @@ impl PropertyDeclarationBlock {
         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));
         }
 

servo/components/style/properties/helpers/animated_properties.mako.rs

@@ -46,28 +46,152 @@ use values::generics::border::BorderCornerRadius as GenericBorderCornerRadius;
 use values::generics::position as generic_position;
 
 
-/// A given transition property, that is either `All`, or an animatable
-/// property.
-// NB: This needs to be here because it needs all the longhands generated
-// beforehand.
+/// 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 TransitionProperty {
-    /// All, any animatable property changing should generate a transition.
-    All,
+pub enum AnimatableLonghand {
     % for prop in data.longhands:
         % if prop.animatable:
             /// ${prop.name}
             ${prop.camel_case},
         % endif
     % endfor
-    // Shorthand properties may or may not contain any animatable property. Either should still be
-    // parsed properly.
-    % for prop in data.shorthands_except_all():
-        /// ${prop.name}
-        ${prop.camel_case},
+}
+
+impl AnimatableLonghand {
+    /// Returns true if this AnimatableLonghand is one of the discretely animatable properties.
+    pub fn is_discrete(&self) -> bool {
+        match *self {
+            % for prop in data.longhands:
+                % if prop.animation_value_type == "discrete":
+                    AnimatableLonghand::${prop.camel_case} => true,
+                % endif
+            % endfor
+            _ => false
+        }
+    }
+
+    /// 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:
+                % if prop.transitionable and 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
+        }
+    }
+}
+
+/// Returns true if this nsCSSPropertyID is one of the animatable properties.
+#[cfg(feature = "gecko")]
+pub fn nscsspropertyid_is_animatable(property: nsCSSPropertyID) -> bool {
+    match property {
+        % for prop in data.longhands + data.shorthands_except_all():
+            % if prop.animatable:
+                ${helpers.to_nscsspropertyid(prop.ident)} => true,
+            % endif
+        % endfor
+        _ => false
+    }
+}
+
+/// A given transition property, that is either `All`, a transitionable longhand property,
+/// 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))]
+pub enum TransitionProperty {
+    /// All, any transitionable property changing should generate a transition.
+    All,
+    % for prop in data.longhands + data.shorthands_except_all():
+        % if prop.transitionable:
+            /// ${prop.name}
+            ${prop.camel_case},
+        % endif
     % endfor
-    /// Unrecognized property which could be any non-animatable, custom property, or
+    /// Unrecognized property which could be any non-transitionable, custom property, or
     /// unknown property.
     Unsupported(Atom)
 }
@@ -80,17 +204,17 @@ impl TransitionProperty {
     /// Iterates over each longhand property.
     pub fn each<F: FnMut(&TransitionProperty) -> ()>(mut cb: F) {
         % for prop in data.longhands:
-            % if prop.animatable:
+            % if prop.transitionable:
                 cb(&TransitionProperty::${prop.camel_case});
             % endif
         % endfor
     }
 
-    /// Iterates over every property that is not TransitionProperty::All, stopping and returning
-    /// true when the provided callback returns true for the first time.
+    /// Iterates over every longhand property that is not TransitionProperty::All, stopping and
+    /// returning true when the provided callback returns true for the first time.
     pub fn any<F: FnMut(&TransitionProperty) -> bool>(mut cb: F) -> bool {
         % for prop in data.longhands:
-            % if prop.animatable:
+            % if prop.transitionable:
                 if cb(&TransitionProperty::${prop.camel_case}) {
                     return true;
                 }
@@ -104,14 +228,11 @@ impl TransitionProperty {
         let ident = try!(input.expect_ident());
         (match_ignore_ascii_case! { &ident,
             "all" => Ok(TransitionProperty::All),
-            % for prop in data.longhands:
-                % if prop.animatable:
+            % for prop in data.longhands + data.shorthands_except_all():
+                % if prop.transitionable:
                     "${prop.name}" => Ok(TransitionProperty::${prop.camel_case}),
                 % endif
             % endfor
-            % for prop in data.shorthands_except_all():
-                "${prop.name}" => Ok(TransitionProperty::${prop.camel_case}),
-            % endfor
             "none" => Err(()),
             _ => {
                 match CSSWideKeyword::from_ident(&ident) {
@@ -122,59 +243,24 @@ impl TransitionProperty {
         }).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
-            % endfor
-            _ => false
-        }
-    }
-
-    /// Return animatable longhands of this shorthand TransitionProperty, except for "all".
+    /// Return transitionable longhands of this shorthand TransitionProperty, except for "all".
     pub fn longhands(&self) -> &'static [TransitionProperty] {
         % for prop in data.shorthands_except_all():
-            static ${prop.ident.upper()}: &'static [TransitionProperty] = &[
-                % for sub in prop.sub_properties:
-                    % if sub.animatable:
-                        TransitionProperty::${sub.camel_case},
-                    % endif
-                % endfor
-            ];
+            % if prop.transitionable:
+                static ${prop.ident.upper()}: &'static [TransitionProperty] = &[
+                    % for sub in prop.sub_properties:
+                        % if sub.transitionable:
+                            TransitionProperty::${sub.camel_case},
+                        % endif
+                    % endfor
+                ];
+            % endif
         % endfor
         match *self {
             % for prop in data.shorthands_except_all():
-                TransitionProperty::${prop.camel_case} => ${prop.ident.upper()},
+                % if prop.transitionable:
+                    TransitionProperty::${prop.camel_case} => ${prop.ident.upper()},
+                % endif
             % endfor
             _ => panic!("Not allowed to call longhands() for this TransitionProperty")
         }
@@ -184,50 +270,26 @@ impl TransitionProperty {
     pub fn is_shorthand(&self) -> bool {
         match *self {
             % for prop in data.shorthands_except_all():
-                TransitionProperty::${prop.camel_case} => true,
+                % if prop.transitionable:
+                    TransitionProperty::${prop.camel_case} => true,
+                % endif
             % endfor
             _ => false
         }
     }
 }
 
-/// Returns true if this nsCSSPropertyID is one of the animatable properties.
-#[cfg(feature = "gecko")]
-pub fn nscsspropertyid_is_animatable(property: nsCSSPropertyID) -> bool {
-    match property {
-        % for prop in data.longhands:
-            % if prop.animatable:
-                ${helpers.to_nscsspropertyid(prop.ident)} => true,
-            % endif
-        % endfor
-        % for prop in data.shorthands_except_all():
-            <%
-                animatable = "false"
-                for sub in prop.sub_properties:
-                    if sub.animatable:
-                        animatable = "true"
-                        break
-            %>
-            ${helpers.to_nscsspropertyid(prop.ident)} => ${animatable},
-        % endfor
-        _ => false
-    }
-}
-
 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:
-                % if prop.animatable:
+            % for prop in data.longhands + data.shorthands_except_all():
+                % if prop.transitionable:
                     TransitionProperty::${prop.camel_case} => dest.write_str("${prop.name}"),
                 % endif
             % endfor
-            % for prop in data.shorthands_except_all():
-                TransitionProperty::${prop.camel_case} => dest.write_str("${prop.name}"),
-            % endfor
             #[cfg(feature = "gecko")]
             TransitionProperty::Unsupported(ref atom) => serialize_identifier(&atom.to_string(),
                                                                               dest),
@@ -243,16 +305,12 @@ impl ToCss for TransitionProperty {
 impl<'a> From< &'a TransitionProperty> for nsCSSPropertyID {
     fn from(transition_property: &'a TransitionProperty) -> nsCSSPropertyID {
         match *transition_property {
-            % for prop in data.longhands:
-                % if prop.animatable:
+            % for prop in data.longhands + data.shorthands_except_all():
+                % if prop.transitionable:
                     TransitionProperty::${prop.camel_case}
                         => ${helpers.to_nscsspropertyid(prop.ident)},
                 % endif
             % endfor
-            % for prop in data.shorthands_except_all():
-                TransitionProperty::${prop.camel_case}
-                    => ${helpers.to_nscsspropertyid(prop.ident)},
-            % endfor
             TransitionProperty::All => nsCSSPropertyID::eCSSPropertyExtra_all_properties,
             _ => panic!("Unconvertable Servo transition property: {:?}", transition_property),
         }
@@ -265,8 +323,8 @@ impl<'a> From< &'a TransitionProperty> for nsCSSPropertyID {
 impl From<nsCSSPropertyID> for TransitionProperty {
     fn from(property: nsCSSPropertyID) -> TransitionProperty {
         match property {
-            % for prop in data.longhands:
-                % if prop.animatable:
+            % for prop in data.longhands + data.shorthands_except_all():
+                % if prop.transitionable:
                     ${helpers.to_nscsspropertyid(prop.ident)}
                         => TransitionProperty::${prop.camel_case},
                 % else:
@@ -274,30 +332,22 @@ impl From<nsCSSPropertyID> for TransitionProperty {
                         => TransitionProperty::Unsupported(Atom::from("${prop.ident}")),
                 % endif
             % endfor
-            % for prop in data.shorthands_except_all():
-                ${helpers.to_nscsspropertyid(prop.ident)}
-                    => TransitionProperty::${prop.camel_case},
-            % endfor
             nsCSSPropertyID::eCSSPropertyExtra_all_properties => TransitionProperty::All,
             _ => panic!("Unconvertable nsCSSPropertyID: {:?}", property),
         }
     }
 }
 
-/// Convert to PropertyDeclarationId.
+/// Returns true if this nsCSSPropertyID is one of the transitionable properties.
 #[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!(),
-        }
+pub fn nscsspropertyid_is_transitionable(property: nsCSSPropertyID) -> bool {
+    match property {
+        % for prop in data.longhands + data.shorthands_except_all():
+            % if prop.transitionable:
+                ${helpers.to_nscsspropertyid(prop.ident)} => true,
+            % endif
+        % endfor
+        _ => false
     }
 }
 
@@ -387,22 +437,20 @@ impl AnimatedProperty {
 
     /// 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),
         }
     }
 }
@@ -411,7 +459,7 @@ impl AnimatedProperty {
 /// 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;
@@ -579,15 +627,14 @@ impl AnimationValue {
         }
     }
 
-    /// Get an AnimationValue for a TransitionProperty from a given computed values.
-    pub fn from_computed_values(transition_property: &TransitionProperty,
+    /// Get an AnimationValue for an AnimatableLonghand from a given computed values.
+    pub fn from_computed_values(property: &AnimatableLonghand,
                                 computed_values: &ComputedValues)
                                 -> Self {
-        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} => {
                         AnimationValue::${prop.camel_case}(
                         % if prop.is_animatable_with_computed_value:
                             computed_values.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}())
@@ -598,7 +645,6 @@ impl AnimationValue {
                     }
                 % endif
             % endfor
-            ref other => panic!("Can't use TransitionProperty::{:?} here.", other),
         }
     }
 }

servo/components/style/properties/longhand/box.mako.rs

@@ -12,7 +12,7 @@
 // TODO(SimonSapin): don't parse `inline-table`, since we don't support it
 <%helpers:longhand name="display"
                    need_clone="True"
-                   animation_value_type="none"
+                   animation_value_type="discrete"
                    custom_cascade="${product == 'servo'}"
                    spec="https://drafts.csswg.org/css-display/#propdef-display">
     <%

servo/components/style/properties/properties.mako.rs

@@ -32,7 +32,7 @@ use font_metrics::FontMetricsProvider;
 use logical_geometry::WritingMode;
 use media_queries::Device;
 use parser::{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;
@@ -300,29 +300,25 @@ impl LonghandIdSet {
         }
     }
 
-    /// 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),
         }
     }
 }

servo/components/style/stylesheets/keyframes_rule.rs

@@ -11,7 +11,7 @@ use parser::{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};
@@ -337,14 +337,14 @@ 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
@@ -355,9 +355,12 @@ fn get_animated_properties(keyframes: &[Arc<Locked<Keyframe>>], guard: &SharedRw
         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) {
+                // Skip the 'display' property because although it is animatable from SMIL,
+                // it should not be animatable from CSS Animations or Web Animations.
+                if property != AnimatableLonghand::Display &&
+                   !seen.has_animatable_longhand_bit(&property) {
+                    seen.set_animatable_longhand_bit(&property);
                     ret.push(property);
                 }
             }
@@ -372,7 +375,7 @@ impl KeyframesAnimation {
     ///
     /// 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.

servo/ports/geckolib/glue.rs

@@ -95,7 +95,7 @@ use style::parser::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::parse_one_declaration_into;
 use style::rule_tree::StyleSource;
 use style::selector_parser::PseudoElementCascadeType;
@@ -379,18 +379,6 @@ pub extern "C" fn Servo_AnimationValues_ComputeDistance(from: RawServoAnimationV
     from_value.compute_distance(to_value).unwrap_or(0.0)
 }
 
-#[no_mangle]
-pub extern "C" fn Servo_AnimationValueMap_Push(value_map: RawServoAnimationValueMapBorrowedMut,
-                                               property: nsCSSPropertyID,
-                                               value: RawServoAnimationValueBorrowed)
-{
-    use style::properties::animated_properties::AnimationValueMap;
-
-    let value_map = AnimationValueMap::from_ffi_mut(value_map);
-    let value = AnimationValue::as_arc(&value).as_ref();
-    value_map.insert(property.into(), value.clone());
-}
-
 #[no_mangle]
 pub extern "C" fn Servo_AnimationCompose(raw_value_map: RawServoAnimationValueMapBorrowedMut,
                                          base_values: *mut ::std::os::raw::c_void,
@@ -405,7 +393,10 @@ pub extern "C" fn Servo_AnimationCompose(raw_value_map: RawServoAnimationValueMa
     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...
@@ -696,8 +687,13 @@ pub extern "C" fn Servo_ComputedValues_ExtractAnimationValue(computed_values: Se
                                                              property_id: nsCSSPropertyID)
                                                              -> RawServoAnimationValueStrong
 {
+    let property = match AnimatableLonghand::from_nscsspropertyid(property_id) {
+        Some(longhand) => longhand,
+        None => { return Strong::null(); }
+    };
+
     let computed_values = ComputedValues::as_arc(&computed_values);
-    Arc::new(AnimationValue::from_computed_values(&property_id.into(), computed_values)).into_strong()
+    Arc::new(AnimationValue::from_computed_values(&property, computed_values)).into_strong()
 }
 
 #[no_mangle]
@@ -706,10 +702,18 @@ pub extern "C" fn Servo_Property_IsAnimatable(property: nsCSSPropertyID) -> bool
     animated_properties::nscsspropertyid_is_animatable(property)
 }
 
+#[no_mangle]
+pub extern "C" fn Servo_Property_IsTransitionable(property: nsCSSPropertyID) -> bool {
+    use style::properties::animated_properties;
+    animated_properties::nscsspropertyid_is_transitionable(property)
+}
+
 #[no_mangle]
 pub extern "C" fn Servo_Property_IsDiscreteAnimatable(property: nsCSSPropertyID) -> bool {
-    let property: TransitionProperty = property.into();
-    property.is_discrete()
+    match AnimatableLonghand::from_nscsspropertyid(property) {
+        Some(longhand) => longhand.is_discrete(),
+        None => false
+    }
 }
 
 #[no_mangle]
@@ -2695,10 +2699,10 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeLis
             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,
@@ -2795,7 +2799,7 @@ pub extern "C" fn Servo_AssertTreeIsClean(root: RawGeckoElementBorrowed) {
 
 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 {
@@ -2814,7 +2818,7 @@ enum Offset {
     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,
@@ -2822,7 +2826,7 @@ fn fill_in_missing_keyframe_values(all_properties:  &[TransitionProperty],
                                    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.
@@ -2841,7 +2845,7 @@ fn fill_in_missing_keyframe_values(all_properties:  &[TransitionProperty],
 
     // 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,
@@ -2933,17 +2937,20 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName(raw_data: RawServoStyleSetB
 
                 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();
+                    // Skip the 'display' property because although it is animatable from SMIL,
+                    // it should not be animatable from CSS Animations.
+                    if property != AnimatableLonghand::Display &&
+                        !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(

servo/tests/unit/style/keyframes.rs

@@ -3,7 +3,7 @@
  * 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};
@@ -100,7 +100,7 @@ fn test_missing_property_in_initial_keyframe() {
                 declared_timing_function: false,
             },
         ],
-        properties_changed: vec![TransitionProperty::Width, TransitionProperty::Height],
+        properties_changed: vec![AnimatableLonghand::Width, AnimatableLonghand::Height],
         vendor_prefix: None,
     };
 
@@ -160,7 +160,7 @@ fn test_missing_property_in_final_keyframe() {
                 declared_timing_function: false,
             },
         ],
-        properties_changed: vec![TransitionProperty::Width, TransitionProperty::Height],
+        properties_changed: vec![AnimatableLonghand::Width, AnimatableLonghand::Height],
         vendor_prefix: None,
     };
 
@@ -222,7 +222,7 @@ fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() {
                 declared_timing_function: false,
             }
         ],
-        properties_changed: vec![TransitionProperty::Width, TransitionProperty::Height],
+        properties_changed: vec![AnimatableLonghand::Width, AnimatableLonghand::Height],
         vendor_prefix: None,
     };