Auto merge of #16295 - jdm:transition-safety, r=nox

Root nodes for the duration of their CSS transitions

This ensures that we can pass a node address as part of the asynchronous
transition end notification, making it safe to fire the corresponding
DOM event on the node from the script thread. Without explicitly rooting
this node when the transition starts, we risk the node being GCed before
the transition is complete.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #14972
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16295)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-05-15 15:00:19 -05:00 committed by GitHub
commit fa251ec96b
15 changed files with 244 additions and 90 deletions

View file

@ -8,7 +8,7 @@
use Atom;
use bezier::Bezier;
use context::SharedStyleContext;
use dom::{OpaqueNode, UnsafeNode};
use dom::OpaqueNode;
use euclid::point::Point2D;
use font_metrics::FontMetricsProvider;
use keyframes::{KeyframesStep, KeyframesStepValue};
@ -188,7 +188,7 @@ pub enum Animation {
/// 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, UnsafeNode, f64, AnimationFrame, bool),
Transition(OpaqueNode, f64, AnimationFrame, bool),
/// A keyframes animation is identified by a name, and can have a
/// node-dependent state (i.e. iteration count, etc.).
Keyframes(OpaqueNode, Atom, KeyframesAnimationState),
@ -200,7 +200,7 @@ impl Animation {
pub fn mark_as_expired(&mut self) {
debug_assert!(!self.is_expired());
match *self {
Animation::Transition(_, _, _, _, ref mut expired) => *expired = true,
Animation::Transition(_, _, _, ref mut expired) => *expired = true,
Animation::Keyframes(_, _, ref mut state) => state.expired = true,
}
}
@ -209,7 +209,7 @@ impl Animation {
#[inline]
pub fn is_expired(&self) -> bool {
match *self {
Animation::Transition(_, _, _, _, expired) => expired,
Animation::Transition(_, _, _, expired) => expired,
Animation::Keyframes(_, _, ref state) => state.expired,
}
}
@ -218,7 +218,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,
}
}
@ -231,6 +231,15 @@ impl Animation {
Animation::Keyframes(_, _, ref state) => state.is_paused(),
}
}
/// Whether this animation is a transition.
#[inline]
pub fn is_transition(&self) -> bool {
match *self {
Animation::Transition(..) => true,
Animation::Keyframes(..) => false,
}
}
}
@ -402,7 +411,6 @@ impl PropertyAnimation {
#[cfg(feature = "servo")]
pub fn start_transitions_if_applicable(new_animations_sender: &Sender<Animation>,
opaque_node: OpaqueNode,
unsafe_node: UnsafeNode,
old_style: &ComputedValues,
new_style: &mut Arc<ComputedValues>,
timer: &Timer,
@ -436,7 +444,7 @@ pub fn start_transitions_if_applicable(new_animations_sender: &Sender<Animation>
let start_time =
now + (box_style.transition_delay_mod(i).seconds() as f64);
new_animations_sender
.send(Animation::Transition(opaque_node, unsafe_node, start_time, AnimationFrame {
.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();
@ -589,7 +597,7 @@ pub fn update_style_for_animation(context: &SharedStyleContext,
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();
@ -767,7 +775,7 @@ pub fn complete_expired_transitions(node: OpaqueNode, style: &mut Arc<ComputedVa
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);
}
}

View file

@ -745,7 +745,6 @@ trait PrivateMatchMethods: TElement {
animation::start_transitions_if_applicable(
new_animations_sender,
this_opaque,
self.as_node().to_unsafe(),
&**values,
new_values,
&shared_context.timer,
@ -843,7 +842,7 @@ trait PrivateMatchMethods: TElement {
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())
}
}