style: Expire keyframes animations when no longer referenced by the style.

It's a long way to make this sound in general...

Fixes #20731

components/style/animation.rs

@@ -630,13 +630,31 @@ pub fn update_style_for_animation_frame(
     true
 }
 
+/// Returns the kind of animation update that happened.
+pub enum AnimationUpdate {
+    /// The style was successfully updated, the animation is still running.
+    Regular,
+    /// A style change canceled this animation.
+    AnimationCanceled,
+}
+
 /// Updates a single animation and associated style based on the current time.
+///
+/// FIXME(emilio): This doesn't handle any kind of dynamic change to the
+/// animation or transition properties in any reasonable way.
+///
+/// This should probably be split in two, one from updating animations and
+/// transitions in response to a style change (that is,
+/// consider_starting_transitions + maybe_start_animations, but handling
+/// canceled animations, duration changes, etc, there instead of here), and this
+/// function should be only about the style update in response of a transition.
 pub fn update_style_for_animation<E>(
     context: &SharedStyleContext,
     animation: &Animation,
     style: &mut Arc<ComputedValues>,
     font_metrics_provider: &FontMetricsProvider,
-) where
+) -> AnimationUpdate
+where
     E: TElement,
 {
     debug!("update_style_for_animation: entering");
@@ -652,6 +670,12 @@ pub fn update_style_for_animation<E>(
             if updated_style {
                 *style = new_style
             }
+            // FIXME(emilio): Should check before updating the style that the
+            // transition_property still transitions this, or bail out if not.
+            //
+            // Or doing it in process_animations, only if transition_property
+            // changed somehow (even better).
+            AnimationUpdate::Regular
         },
         Animation::Keyframes(_, ref animation, ref name, ref state) => {
             debug!(
@@ -675,28 +699,18 @@ pub fn update_style_for_animation<E>(
 
             let index = match maybe_index {
                 Some(index) => index,
-                None => {
+                None => return AnimationUpdate::AnimationCanceled,
-                    warn!(
-                        "update_style_for_animation: Animation {:?} not found in style",
-                        name
-                    );
-                    return;
-                },
             };
 
             let total_duration = style.get_box().animation_duration_mod(index).seconds() as f64;
             if total_duration == 0. {
-                debug!(
+                return AnimationUpdate::AnimationCanceled;
-                    "update_style_for_animation: zero duration for animation {:?}",
-                    name
-                );
-                return;
             }
 
             let mut total_progress = (now - started_at) / total_duration;
             if total_progress < 0. {
                 warn!("Negative progress found for animation {:?}", name);
-                return;
+                return AnimationUpdate::Regular;
             }
             if total_progress > 1. {
                 total_progress = 1.;
@@ -748,11 +762,7 @@ pub fn update_style_for_animation<E>(
 
             let target_keyframe = match target_keyframe_position {
                 Some(target) => &animation.steps[target],
-                None => {
+                None => return AnimationUpdate::Regular,
-                    warn!("update_style_for_animation: No current keyframe found for animation \"{}\" at progress {}",
-                          name, total_progress);
-                    return;
-                },
             };
 
             let last_keyframe = &animation.steps[last_keyframe_position];
@@ -836,6 +846,7 @@ pub fn update_style_for_animation<E>(
                 name
             );
             *style = new_style;
+            AnimationUpdate::Regular
         },
     }
 }

components/style/matching.rs

@@ -583,7 +583,7 @@ trait PrivateMatchMethods: TElement {
         possibly_expired_animations: &mut Vec<::animation::PropertyAnimation>,
         font_metrics: &::font_metrics::FontMetricsProvider,
     ) {
-        use animation::{self, Animation};
+        use animation::{self, Animation, AnimationUpdate};
         use dom::TNode;
 
         // Finish any expired transitions.
@@ -601,7 +601,7 @@ trait PrivateMatchMethods: TElement {
         }
 
         let mut all_running_animations = context.running_animations.write();
-        for running_animation in all_running_animations.get_mut(&this_opaque).unwrap() {
+        for mut running_animation in all_running_animations.get_mut(&this_opaque).unwrap() {
             // This shouldn't happen frequently, but under some circumstances
             // mainly huge load or debug builds, the constellation might be
             // delayed in sending the `TickAllAnimations` message to layout.
@@ -616,16 +616,26 @@ trait PrivateMatchMethods: TElement {
                 continue;
             }
 
-            animation::update_style_for_animation::<Self>(
+            let update = animation::update_style_for_animation::<Self>(
                 context,
-                running_animation,
+                &mut running_animation,
                 style,
                 font_metrics,
             );
 
-            if let Animation::Transition(_, _, ref frame) = *running_animation {
+            match *running_animation {
+                Animation::Transition(_, _, ref frame) => {
                     possibly_expired_animations.push(frame.property_animation.clone())
                 }
+                Animation::Keyframes(_, _, _, ref mut state) => {
+                    match update {
+                        AnimationUpdate::Regular => {},
+                        AnimationUpdate::AnimationCanceled => {
+                            state.expired = true;
+                        }
+                    }
+                }
+            }
         }
     }
 }