Bug 1500260 - Remove unused expired boolean in Animation::Transition. r=emilio
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 05 May 2018 18:36:22 +0200
changeset 500547 6d0c84abdfa1d1b5e64e78a1826762e110c7ea57
parent 500546 bfac8c708a3f50ed6cb301548edec21b097c0a7f
child 500548 7037075929099a8d88ef702257e88d15cbe3ab59
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1500260, 14418, 20757
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1500260 - Remove unused expired boolean in Animation::Transition. r=emilio 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. This cherry-picks part of servo/servo#20757.
servo/components/style/animation.rs
servo/components/style/matching.rs
--- a/servo/components/style/animation.rs
+++ b/servo/components/style/animation.rs
@@ -209,56 +209,44 @@ impl fmt::Debug for KeyframesAnimationSt
 }
 
 /// State relating to an animation.
 #[derive(Clone, Debug)]
 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.).
     ///
     /// TODO(emilio): The animation object could be refcounted.
     Keyframes(
         OpaqueNode,
         KeyframesAnimation,
         Atom,
         KeyframesAnimationState,
     ),
 }
 
 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,
         }
     }
 
     /// The opaque node that owns the animation.
     #[inline]
     pub fn node(&self) -> &OpaqueNode {
         match *self {
-            Animation::Transition(ref node, _, _, _) => node,
+            Animation::Transition(ref node, _, _) => node,
             Animation::Keyframes(ref node, _, _, _) => node,
         }
     }
 
     /// Whether this animation is paused. A transition can never be paused.
     #[inline]
     pub fn is_paused(&self) -> bool {
         match *self {
@@ -460,17 +448,16 @@ pub fn start_transitions_if_applicable(
             new_animations_sender
                 .send(Animation::Transition(
                     opaque_node,
                     start_time,
                     AnimationFrame {
                         duration: box_style.transition_duration_mod(i).seconds() as f64,
                         property_animation: property_animation,
                     },
-                    /* is_expired = */ false,
                 )).unwrap();
 
             had_animations = true;
         }
     }
 
     had_animations
 }
@@ -651,17 +638,17 @@ pub fn update_style_for_animation<E>(
     font_metrics_provider: &FontMetricsProvider,
 ) where
     E: TElement,
 {
     debug!("update_style_for_animation: entering");
     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();
             let updated_style =
                 update_style_for_animation_frame(&mut new_style, now, start_time, frame);
             if updated_style {
                 *style = new_style
             }
@@ -863,17 +850,17 @@ pub fn complete_expired_transitions(
     let had_animations_to_expire;
     {
         let all_expired_animations = context.expired_animations.read();
         let animations_to_expire = all_expired_animations.get(&node);
         had_animations_to_expire = animations_to_expire.is_some();
         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);
                 }
             }
         }
     }
 
     if had_animations_to_expire {
         context.expired_animations.write().remove(&node);
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -620,17 +620,17 @@ trait PrivateMatchMethods: TElement {
 
             animation::update_style_for_animation::<Self>(
                 context,
                 running_animation,
                 style,
                 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())
             }
         }
     }
 }
 
 impl<E: TElement> PrivateMatchMethods for E {}