layout: Fix some particularly bad cases of spurious reflows leading to script thread unresponsiveness.

components/compositing/compositor.rs

@@ -1191,10 +1191,13 @@ impl<Window: WindowMethods> IOCompositor<Window> {
             }
         }
 
-        // We still need to tick layout unfortunately, see things like #12749.
-        let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Layout);
-        if let Err(e) = self.constellation_chan.send(msg) {
-            warn!("Sending tick to constellation failed ({}).", e);
+        // We may need to tick animations in layout. (See #12749.)
+        let animations_running = self.pipeline_details(pipeline_id).animations_running;
+        if animations_running {
+            let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Layout);
+            if let Err(e) = self.constellation_chan.send(msg) {
+                warn!("Sending tick to constellation failed ({}).", e);
+            }
         }
     }
 

components/layout/animation.rs

@@ -88,7 +88,8 @@ pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
 
             if let Animation::Transition(_, unsafe_node, _, ref frame, _) = running_animation {
                 script_chan.send(ConstellationControlMsg::TransitionEnd(unsafe_node,
-                                                                        frame.property_animation.property_name(),
+                                                                        frame.property_animation
+                                                                             .property_name(),
                                                                         frame.duration))
                            .unwrap();
             }
@@ -113,7 +114,7 @@ pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
     for new_running_animation in new_running_animations {
         running_animations.entry(*new_running_animation.node())
                           .or_insert_with(Vec::new)
-                          .push(new_running_animation);
+                          .push(new_running_animation)
     }
 
     let animation_state = if running_animations.is_empty() {
@@ -122,7 +123,8 @@ pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
         AnimationState::AnimationsPresent
     };
 
-    constellation_chan.send(ConstellationMsg::ChangeRunningAnimationsState(pipeline_id, animation_state))
+    constellation_chan.send(ConstellationMsg::ChangeRunningAnimationsState(pipeline_id,
+                                                                           animation_state))
                       .unwrap();
 }
 

components/layout_thread/lib.rs

@@ -1091,7 +1091,7 @@ impl LayoutThread {
         }
 
         let restyles = document.drain_pending_restyles();
-        debug!("Draining restyles: {}", restyles.len());
+        debug!("Draining restyles: {} (needs dirtying? {:?})", restyles.len(), needs_dirtying);
         if !needs_dirtying {
             for (el, restyle) in restyles {
                 // Propagate the descendant bit up the ancestors. Do this before
@@ -1282,6 +1282,10 @@ impl LayoutThread {
     }
 
     fn tick_animations(&mut self, rw_data: &mut LayoutThreadData) {
+        if opts::get().relayout_event {
+            println!("**** pipeline={}\tForDisplay\tSpecial\tAnimationTick", self.id);
+        }
+
         let reflow_info = Reflow {
             goal: ReflowGoal::ForDisplay,
             page_clip_rect: max_rect(),

components/style/animation.rs

@@ -335,6 +335,11 @@ impl PropertyAnimation {
     fn does_animate(&self) -> bool {
         self.property.does_animate() && self.duration != Time(0.0)
     }
+
+    #[inline]
+    pub fn has_the_same_end_value_as(&self, other: &PropertyAnimation) -> bool {
+        self.property.has_the_same_end_value_as(&other.property)
+    }
 }
 
 /// Inserts transitions into the queue of running animations as applicable for
@@ -348,13 +353,26 @@ pub fn start_transitions_if_applicable(new_animations_sender: &Sender<Animation>
                                        unsafe_node: UnsafeNode,
                                        old_style: &ComputedValues,
                                        new_style: &mut Arc<ComputedValues>,
-                                       timer: &Timer)
+                                       timer: &Timer,
+                                       possibly_expired_animations: &[PropertyAnimation])
                                        -> bool {
     let mut had_animations = false;
     for i in 0..new_style.get_box().transition_property_count() {
         // Create any property animations, if applicable.
-        let property_animations = PropertyAnimation::from_transition(i, old_style, Arc::make_mut(new_style));
+        let property_animations = PropertyAnimation::from_transition(i,
+                                                                     old_style,
+                                                                     Arc::make_mut(new_style));
         for property_animation in property_animations {
+            // Per [1], don't trigger a new transition if the end state for that transition is
+            // the same as that of a transition that's already running on the same node.
+            //
+            // [1]: https://drafts.csswg.org/css-transitions/#starting
+            if possibly_expired_animations.iter().any(|animation| {
+                    animation.has_the_same_end_value_as(&property_animation)
+               }) {
+                continue
+            }
+
             // Set the property to the initial value.
             // NB: get_mut is guaranteed to succeed since we called make_mut()
             // above.

components/style/matching.rs

@@ -7,7 +7,7 @@
 #![allow(unsafe_code)]
 
 use {Atom, LocalName};
-use animation;
+use animation::{self, Animation, PropertyAnimation};
 use atomic_refcell::AtomicRefMut;
 use cache::LRUCache;
 use cascade_info::CascadeInfo;
@@ -395,10 +395,10 @@ trait PrivateMatchMethods: TElement {
                                             parent_style: Option<&Arc<ComputedValues>>,
                                             old_style: Option<&Arc<ComputedValues>>,
                                             rule_node: &StrongRuleNode,
+                                            possibly_expired_animations: &[PropertyAnimation],
                                             booleans: CascadeBooleans)
                                             -> Arc<ComputedValues>
-        where Ctx: StyleContext<'a>
-    {
+                                            where Ctx: StyleContext<'a> {
         let shared_context = context.shared_context();
         let mut cascade_info = CascadeInfo::new();
         let mut cascade_flags = CascadeFlags::empty();
@@ -445,7 +445,8 @@ trait PrivateMatchMethods: TElement {
                     self.as_node().to_unsafe(),
                     &**style,
                     &mut this_style,
-                    &shared_context.timer);
+                    &shared_context.timer,
+                    &possibly_expired_animations);
             }
         }
 
@@ -454,11 +455,11 @@ trait PrivateMatchMethods: TElement {
 
     fn update_animations_for_cascade(&self,
                                      context: &SharedStyleContext,
-                                     style: &mut Arc<ComputedValues>) -> bool {
+                                     style: &mut Arc<ComputedValues>,
+                                     possibly_expired_animations: &mut Vec<PropertyAnimation>) {
         // Finish any expired transitions.
         let this_opaque = self.as_node().opaque();
-        let had_animations_to_expire =
-            animation::complete_expired_transitions(this_opaque, style, context);
+        animation::complete_expired_transitions(this_opaque, style, context);
 
         // Merge any running transitions into the current style, and cancel them.
         let had_running_animations = context.running_animations
@@ -467,7 +468,7 @@ trait PrivateMatchMethods: TElement {
                                             .is_some();
         if had_running_animations {
             let mut all_running_animations = context.running_animations.write();
-            for mut running_animation in all_running_animations.get_mut(&this_opaque).unwrap() {
+            for 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
@@ -483,12 +484,12 @@ trait PrivateMatchMethods: TElement {
                     animation::update_style_for_animation(context,
                                                           running_animation,
                                                           style);
-                    running_animation.mark_as_expired();
+                    if let Animation::Transition(_, _, _, ref frame, _) = *running_animation {
+                        possibly_expired_animations.push(frame.property_animation.clone())
+                    }
                 }
             }
         }
-
-        had_animations_to_expire || had_running_animations
     }
 
     fn share_style_with_candidate_if_possible(&self,
@@ -729,6 +730,7 @@ pub trait MatchMethods : TElement {
         let parent_style = parent_data.as_ref().map(|x| &x.current_styles().primary.values);
 
         let mut new_styles;
+        let mut possibly_expired_animations = vec![];
 
         let damage = {
             let (old_primary, old_pseudos) = match data.previous_styles_mut() {
@@ -737,7 +739,8 @@ pub trait MatchMethods : TElement {
                     // Update animations before the cascade. This may modify the
                     // value of the old primary style.
                     self.update_animations_for_cascade(context.shared_context(),
-                                                       &mut previous.primary.values);
+                                                       &mut previous.primary.values,
+                                                       &mut possibly_expired_animations);
                     (Some(&previous.primary.values), Some(&mut previous.pseudos))
                 }
             };
@@ -747,6 +750,7 @@ pub trait MatchMethods : TElement {
                                                  parent_style,
                                                  old_primary,
                                                  &primary_rule_node,
+                                                 &possibly_expired_animations,
                                                  CascadeBooleans {
                                                      shareable: primary_is_shareable,
                                                      animate: true,
@@ -761,7 +765,8 @@ pub trait MatchMethods : TElement {
                                                         &new_styles.primary.values,
                                                         &mut new_styles.pseudos,
                                                         context,
-                                                        pseudo_rule_nodes);
+                                                        pseudo_rule_nodes,
+                                                        &mut possibly_expired_animations);
 
             self.as_node().set_can_be_fragmented(parent.map_or(false, |p| {
                 p.as_node().can_be_fragmented() ||
@@ -774,14 +779,16 @@ pub trait MatchMethods : TElement {
         data.finish_styling(new_styles, damage);
     }
 
-    fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self,
-                                                   old_primary: Option<&Arc<ComputedValues>>,
-                                                   mut old_pseudos: Option<&mut PseudoStyles>,
-                                                   new_primary: &Arc<ComputedValues>,
-                                                   new_pseudos: &mut PseudoStyles,
-                                                   context: &Ctx,
-                                                   mut pseudo_rule_nodes: PseudoRuleNodes)
-                                                   -> RestyleDamage
+    fn compute_damage_and_cascade_pseudos<'a, Ctx>(
+            &self,
+            old_primary: Option<&Arc<ComputedValues>>,
+            mut old_pseudos: Option<&mut PseudoStyles>,
+            new_primary: &Arc<ComputedValues>,
+            new_pseudos: &mut PseudoStyles,
+            context: &Ctx,
+            mut pseudo_rule_nodes: PseudoRuleNodes,
+            possibly_expired_animations: &mut Vec<PropertyAnimation>)
+            -> RestyleDamage
         where Ctx: StyleContext<'a>
     {
         // Here we optimise the case of the style changing but both the
@@ -838,14 +845,18 @@ pub trait MatchMethods : TElement {
                         // Update animations before the cascade. This may modify
                         // the value of old_pseudo_style.
                         self.update_animations_for_cascade(context.shared_context(),
-                                                           &mut old_pseudo_style.values);
+                                                           &mut old_pseudo_style.values,
+                                                           possibly_expired_animations);
                     }
                 }
 
                 let new_pseudo_values =
-                    self.cascade_node_pseudo_element(context, Some(new_primary),
-                                                     maybe_old_pseudo_style.as_ref().map(|s| &s.values),
+                    self.cascade_node_pseudo_element(context,
+                                                     Some(new_primary),
+                                                     maybe_old_pseudo_style.as_ref()
+                                                                           .map(|s| &s.values),
                                                      &new_rule_node,
+                                                     &possibly_expired_animations,
                                                      CascadeBooleans {
                                                          shareable: false,
                                                          animate: animate,

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

@@ -124,6 +124,20 @@ impl AnimatedProperty {
         }
     }
 
+    pub fn has_the_same_end_value_as(&self, other: &AnimatedProperty) -> bool {
+        match (self, other) {
+            % for prop in data.longhands:
+                % if prop.animatable:
+                    (&AnimatedProperty::${prop.camel_case}(_, ref this_end_value),
+                     &AnimatedProperty::${prop.camel_case}(_, ref other_end_value)) => {
+                        this_end_value == other_end_value
+                    }
+                % endif
+            % endfor
+            _ => false,
+        }
+    }
+
     pub fn update(&self, style: &mut ComputedValues, progress: f64) {
         match *self {
             % for prop in data.longhands: