From 1ba57cbaad863c0befa11fc0cb1e68c280f5642b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sat, 5 May 2018 18:36:22 +0200
Subject: [PATCH] style: Remove unused expired boolean in
 Animation::Transition.

The last caller who used was #14418, which did fix a problem but introduced
multiple. In particular, now transitions don't get expired ever, until they
finish running of course.

That is not ok, given you can have something that the user can trigger to change
the style (hi, :hover, for example), and right now that triggers new
transitions, getting this into a really funny state.

I should give fixing this a shot, but it's non-trivial at all.
---
 components/layout/animation.rs | 15 +++++++--------
 components/style/animation.rs  | 23 +++++------------------
 components/style/matching.rs   |  2 +-
 3 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/components/layout/animation.rs b/components/layout/animation.rs
index b42933bb400..4c4865cc11a 100644
--- a/components/layout/animation.rs
+++ b/components/layout/animation.rs
@@ -79,7 +79,7 @@ pub fn update_animation_state<E>(
         let mut animations_still_running = vec![];
         for mut running_animation in running_animations.drain(..) {
             let still_running = !running_animation.is_expired() && match running_animation {
-                Animation::Transition(_, started_at, ref frame, _expired) => {
+                Animation::Transition(_, started_at, ref frame) => {
                     now < started_at + frame.duration
                 },
                 Animation::Keyframes(_, _, _, ref mut state) => {
@@ -94,13 +94,12 @@ pub fn update_animation_state<E>(
                 continue;
             }
 
-            if let Animation::Transition(node, _, ref frame, _) = running_animation {
-                script_chan
-                    .send(ConstellationControlMsg::TransitionEnd(
-                        node.to_untrusted_node_address(),
-                        frame.property_animation.property_name().into(),
-                        frame.duration,
-                    )).unwrap();
+            if let Animation::Transition(node, _, ref frame) = running_animation {
+                script_chan.send(ConstellationControlMsg::TransitionEnd(
+                    node.to_untrusted_node_address(),
+                    frame.property_animation.property_name().into(),
+                    frame.duration,
+                )).unwrap();
             }
 
             expired_animations
diff --git a/components/style/animation.rs b/components/style/animation.rs
index aa831fe7ee1..b5b4d806dd8 100644
--- a/components/style/animation.rs
+++ b/components/style/animation.rs
@@ -214,9 +214,7 @@ pub enum Animation {
     /// A transition is just a single frame triggered at a time, with a reflow.
     ///
     /// the f64 field is the start time as returned by `time::precise_time_s()`.
-    ///
-    /// The `bool` field is werther this animation should no longer run.
-    Transition(OpaqueNode, f64, AnimationFrame, bool),
+    Transition(OpaqueNode, f64, AnimationFrame),
     /// A keyframes animation is identified by a name, and can have a
     /// node-dependent state (i.e. iteration count, etc.).
     ///
@@ -230,21 +228,11 @@ pub enum Animation {
 }
 
 impl Animation {
-    /// Mark this animation as expired.
-    #[inline]
-    pub fn mark_as_expired(&mut self) {
-        debug_assert!(!self.is_expired());
-        match *self {
-            Animation::Transition(_, _, _, ref mut expired) => *expired = true,
-            Animation::Keyframes(_, _, _, ref mut state) => state.expired = true,
-        }
-    }
-
     /// Whether this animation is expired.
     #[inline]
     pub fn is_expired(&self) -> bool {
         match *self {
-            Animation::Transition(_, _, _, expired) => expired,
+            Animation::Transition(..) => false,
             Animation::Keyframes(_, _, _, ref state) => state.expired,
         }
     }
@@ -253,7 +241,7 @@ impl Animation {
     #[inline]
     pub fn node(&self) -> &OpaqueNode {
         match *self {
-            Animation::Transition(ref node, _, _, _) => node,
+            Animation::Transition(ref node, _, _) => node,
             Animation::Keyframes(ref node, _, _, _) => node,
         }
     }
@@ -465,7 +453,6 @@ pub fn start_transitions_if_applicable(
                         duration: box_style.transition_duration_mod(i).seconds() as f64,
                         property_animation: property_animation,
                     },
-                    /* is_expired = */ false,
                 )).unwrap();
 
             had_animations = true;
@@ -656,7 +643,7 @@ pub fn update_style_for_animation<E>(
     debug_assert!(!animation.is_expired());
 
     match *animation {
-        Animation::Transition(_, start_time, ref frame, _) => {
+        Animation::Transition(_, start_time, ref frame) => {
             debug!("update_style_for_animation: transition found");
             let now = context.timer.seconds();
             let mut new_style = (*style).clone();
@@ -868,7 +855,7 @@ pub fn complete_expired_transitions(
         if let Some(ref animations) = animations_to_expire {
             for animation in *animations {
                 // TODO: support animation-fill-mode
-                if let Animation::Transition(_, _, ref frame, _) = *animation {
+                if let Animation::Transition(_, _, ref frame) = *animation {
                     frame.property_animation.update(Arc::make_mut(style), 1.0);
                 }
             }
diff --git a/components/style/matching.rs b/components/style/matching.rs
index bc8cf13659c..005f74fce92 100644
--- a/components/style/matching.rs
+++ b/components/style/matching.rs
@@ -623,7 +623,7 @@ trait PrivateMatchMethods: TElement {
                 font_metrics,
             );
 
-            if let Animation::Transition(_, _, ref frame, _) = *running_animation {
+            if let Animation::Transition(_, _, ref frame) = *running_animation {
                 possibly_expired_animations.push(frame.property_animation.clone())
             }
         }