Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
style: Do not re-expire animations.
If we're restyling a page with animations and layout takes too long, we
might still have the expired animations from the last restyle, without these
being cleared out by layout on `tick_animations`.

Unfortunately it's hard (near to impossible?) to make a reduced test case for
this, since it heavily depends on the speed of the build and conditions that
only happen under heavy loads.

Mainly, it depends on how accurately the `TickAllAnimations` message is sent
from the constellation.

Thus, we can't just re-expire an animation, and we should re-check for it as a
previous animation.

Fixes #12171
  • Loading branch information
emilio committed Jul 3, 2016
1 parent 194fb3e commit 62345ae
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
4 changes: 2 additions & 2 deletions components/style/animation.rs
Expand Up @@ -83,6 +83,7 @@ impl<Impl: SelectorImplExt> KeyframesAnimationState<Impl> {
/// Returns true if the animation should keep running.
pub fn tick(&mut self) -> bool {
debug!("KeyframesAnimationState::tick");
debug_assert!(!self.expired);

self.started_at += self.duration + self.delay;
match self.running_state {
Expand Down Expand Up @@ -495,9 +496,9 @@ pub fn update_style_for_animation<Damage, Impl>(context: &SharedStyleContext<Imp
where Impl: SelectorImplExt,
Damage: TRestyleDamage<ConcreteComputedValues = Impl::ComputedValues> {
debug!("update_style_for_animation: entering");
debug_assert!(!animation.is_expired());
match *animation {
Animation::Transition(_, start_time, ref frame, expired) => {
debug_assert!(!expired);
debug!("update_style_for_animation: transition found");
let now = time::precise_time_s();
let mut new_style = (*style).clone();
Expand All @@ -513,7 +514,6 @@ where Impl: SelectorImplExt,
}
}
Animation::Keyframes(_, ref name, ref state) => {
debug_assert!(!state.expired);
debug!("update_style_for_animation: animation found: \"{}\", {:?}", name, state);
let duration = state.duration;
let started_at = state.started_at;
Expand Down
19 changes: 16 additions & 3 deletions components/style/matching.rs
Expand Up @@ -498,9 +498,22 @@ trait PrivateMatchMethods: TNode
if had_running_animations {
let mut all_running_animations = context.running_animations.write().unwrap();
for mut running_animation in all_running_animations.get_mut(&this_opaque).unwrap() {
animation::update_style_for_animation::<Self::ConcreteRestyleDamage,
<Self::ConcreteElement as Element>::Impl>(context, running_animation, style, None);
running_animation.mark_as_expired();
// 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.
//
// Thus, we can't assume all the animations have been already
// updated by layout, because other restyle due to script might
// be triggered by layout before the animation tick.
//
// See #12171 and the associated PR for an example where this
// happened while debugging other release panic.
if !running_animation.is_expired() {
animation::update_style_for_animation::<Self::ConcreteRestyleDamage,
<Self::ConcreteElement as Element>::Impl>(context, running_animation, style, None);
running_animation.mark_as_expired();
}
}
}

Expand Down

0 comments on commit 62345ae

Please sign in to comment.