Auto merge of #15299 - hiikezoe:empty-keyframe, r=emilio,pcwalton

Create KeyframeAnimation for empty keyframe rules

<!-- Please describe your changes on the following line: -->
We should create KeyframeAnimation even if css keyframe rule is empty because we should fire css animation events for such CSS animations.
Also we should ignore properties that are annotated with '!important'.  From the spec[1].
 using !important on them is invalid and will cause the property to be ignored

[1] https://drafts.csswg.org/css-animations-1/#keyframes
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #15257

<!-- Either: -->
- [X] There are tests for these changes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15299)
<!-- Reviewable:end -->

components/style/keyframes.rs

@@ -254,7 +254,9 @@ fn get_animated_properties(keyframe: &Keyframe) -> Vec<TransitionProperty> {
     let mut ret = vec![];
     // NB: declarations are already deduplicated, so we don't have to check for
     // it here.
-    for &(ref declaration, _) in keyframe.block.read().declarations.iter() {
+    for &(ref declaration, importance) in keyframe.block.read().declarations.iter() {
+        assert!(!importance.important());
+
         if let Some(property) = TransitionProperty::from_declaration(declaration) {
             ret.push(property);
         }
@@ -266,50 +268,51 @@ fn get_animated_properties(keyframe: &Keyframe) -> Vec<TransitionProperty> {
 impl KeyframesAnimation {
     /// Create a keyframes animation from a given list of keyframes.
     ///
-    /// This will return `None` if the list of keyframes is empty, or there are
-    /// no animated properties obtained from the 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.
     ///
     /// Otherwise, this will compute and sort the steps used for the animation,
     /// and return the animation object.
-    pub fn from_keyframes(keyframes: &[Arc<RwLock<Keyframe>>]) -> Option<Self> {
+    pub fn from_keyframes(keyframes: &[Arc<RwLock<Keyframe>>]) -> Self {
+        let mut result = KeyframesAnimation {
+            steps: vec![],
+            properties_changed: vec![],
+        };
+
         if keyframes.is_empty() {
-            return None;
+            return result;
         }
 
-        let animated_properties = get_animated_properties(&keyframes[0].read());
-        if animated_properties.is_empty() {
-            return None;
+        result.properties_changed = get_animated_properties(&keyframes[0].read());
+        if result.properties_changed.is_empty() {
+            return result;
         }
 
-        let mut steps = vec![];
-
         for keyframe in keyframes {
             let keyframe = keyframe.read();
             for percentage in keyframe.selector.0.iter() {
-                steps.push(KeyframesStep::new(*percentage, KeyframesStepValue::Declarations {
+                result.steps.push(KeyframesStep::new(*percentage, KeyframesStepValue::Declarations {
                     block: keyframe.block.clone(),
                 }));
             }
         }
 
         // Sort by the start percentage, so we can easily find a frame.
-        steps.sort_by_key(|step| step.start_percentage);
+        result.steps.sort_by_key(|step| step.start_percentage);
 
         // Prepend autogenerated keyframes if appropriate.
-        if steps[0].start_percentage.0 != 0. {
-            steps.insert(0, KeyframesStep::new(KeyframePercentage::new(0.),
-                                               KeyframesStepValue::ComputedValues));
+        if result.steps[0].start_percentage.0 != 0. {
+            result.steps.insert(0, KeyframesStep::new(KeyframePercentage::new(0.),
+                                                      KeyframesStepValue::ComputedValues));
         }
 
-        if steps.last().unwrap().start_percentage.0 != 1. {
-            steps.push(KeyframesStep::new(KeyframePercentage::new(1.),
-                                          KeyframesStepValue::ComputedValues));
+        if result.steps.last().unwrap().start_percentage.0 != 1. {
+            result.steps.push(KeyframesStep::new(KeyframePercentage::new(1.),
+                                                 KeyframesStepValue::ComputedValues));
         }
 
-        Some(KeyframesAnimation {
-            steps: steps,
-            properties_changed: animated_properties,
-        })
+        result
     }
 }
 

components/style/stylist.rs

@@ -182,6 +182,7 @@ impl Stylist {
         self.precomputed_pseudo_element_decls = Default::default();
         self.rules_source_order = 0;
         self.state_deps.clear();
+        self.animations.clear();
 
         self.sibling_affecting_selectors.clear();
         self.non_common_style_affecting_attributes_selectors.clear();
@@ -274,16 +275,9 @@ impl Stylist {
                 CssRule::Keyframes(ref keyframes_rule) => {
                     let keyframes_rule = keyframes_rule.read();
                     debug!("Found valid keyframes rule: {:?}", *keyframes_rule);
-                    if let Some(animation) = KeyframesAnimation::from_keyframes(&keyframes_rule.keyframes) {
-                        debug!("Found valid keyframe animation: {:?}", animation);
-                        self.animations.insert(keyframes_rule.name.clone(),
-                                               animation);
-                    } else {
-                        // If there's a valid keyframes rule, even if it doesn't
-                        // produce an animation, should shadow other animations
-                        // with the same name.
-                        self.animations.remove(&keyframes_rule.name);
-                    }
+                    let animation = KeyframesAnimation::from_keyframes(&keyframes_rule.keyframes);
+                    debug!("Found valid keyframe animation: {:?}", animation);
+                    self.animations.insert(keyframes_rule.name.clone(), animation);
                 }
                 // We don't care about any other rule.
                 _ => {}

tests/unit/style/keyframes.rs

@@ -0,0 +1,40 @@
+/* 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 parking_lot::RwLock;
+use std::sync::Arc;
+use style::keyframes::{Keyframe, KeyframesAnimation, KeyframePercentage,  KeyframeSelector};
+use style::properties::PropertyDeclarationBlock;
+
+#[test]
+fn test_empty_keyframe() {
+    let keyframes = vec![];
+    let animation = KeyframesAnimation::from_keyframes(&keyframes);
+    let expected = KeyframesAnimation {
+        steps: vec![],
+        properties_changed: vec![],
+    };
+
+    assert_eq!(format!("{:#?}", animation), format!("{:#?}", expected));
+}
+
+#[test]
+fn test_no_property_in_keyframe() {
+    let keyframes = vec![
+        Arc::new(RwLock::new(Keyframe {
+            selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]),
+            block: Arc::new(RwLock::new(PropertyDeclarationBlock {
+                declarations: vec![],
+                important_count: 0,
+            }))
+        })),
+    ];
+    let animation = KeyframesAnimation::from_keyframes(&keyframes);
+    let expected = KeyframesAnimation {
+        steps: vec![],
+        properties_changed: vec![],
+    };
+
+    assert_eq!(format!("{:#?}", animation), format!("{:#?}", expected));
+}

tests/unit/style/lib.rs

@@ -25,6 +25,7 @@ extern crate test;
 mod animated_properties;
 mod attr;
 mod cache;
+mod keyframes;
 mod logical_geometry;
 mod media_queries;
 mod owning_handle;