Add support for canceling CSS transitions

This change adds support for canceling CSS transitions when a property
is no longer transitionable or when an element becomes styled with
display:none. Support for canceling and replacing CSS transitions when
the end value changes is still pending. This change also takes advantage
of updating the constellation message to fix a bug where transition
events could be sent for closed pipelines.

Fixes #15079.

components/atoms/static_atoms.txt

@@ -129,6 +129,7 @@ time
 timeupdate
 toggle
 track
+transitioncancel
 transitionend
 unhandledrejection
 unload

components/layout/animation.rs

@@ -13,7 +13,9 @@ use fxhash::{FxHashMap, FxHashSet};
 use ipc_channel::ipc::IpcSender;
 use msg::constellation_msg::PipelineId;
 use script_traits::UntrustedNodeAddress;
-use script_traits::{AnimationState, ConstellationControlMsg, LayoutMsg as ConstellationMsg};
+use script_traits::{
+    AnimationState, ConstellationControlMsg, LayoutMsg as ConstellationMsg, TransitionEventType,
+};
 use style::animation::{update_style_for_animation, Animation};
 use style::dom::TElement;
 use style::font_metrics::ServoMetricsProvider;
@@ -28,6 +30,7 @@ pub fn update_animation_state<E>(
     script_chan: &IpcSender<ConstellationControlMsg>,
     running_animations: &mut FxHashMap<OpaqueNode, Vec<Animation>>,
     expired_animations: &mut FxHashMap<OpaqueNode, Vec<Animation>>,
+    cancelled_animations: &mut FxHashMap<OpaqueNode, Vec<Animation>>,
     mut keys_to_remove: FxHashSet<OpaqueNode>,
     mut newly_transitioning_nodes: Option<&mut Vec<UntrustedNodeAddress>>,
     new_animations_receiver: &Receiver<Animation>,
@@ -36,6 +39,8 @@ pub fn update_animation_state<E>(
 ) where
     E: TElement,
 {
+    send_events_for_cancelled_animations(script_chan, cancelled_animations, pipeline_id);
+
     let mut new_running_animations = vec![];
     while let Ok(animation) = new_animations_receiver.try_recv() {
         let mut should_push = true;
@@ -102,11 +107,13 @@ pub fn update_animation_state<E>(
 
             if let Animation::Transition(node, _, ref property_animation) = running_animation {
                 script_chan
-                    .send(ConstellationControlMsg::TransitionEnd(
+                    .send(ConstellationControlMsg::TransitionEvent {
-                        node.to_untrusted_node_address(),
+                        pipeline_id,
-                        property_animation.property_name().into(),
+                        event_type: TransitionEventType::TransitionEnd,
-                        property_animation.duration,
+                        node: node.to_untrusted_node_address(),
-                    ))
+                        property_name: property_animation.property_name().into(),
+                        elapsed_time: property_animation.duration,
+                    })
                     .unwrap();
             }
 
@@ -161,6 +168,37 @@ pub fn update_animation_state<E>(
         .unwrap();
 }
 
+/// Send events for cancelled animations. Currently this only handles cancelled
+/// transitions, but eventually this should handle cancelled CSS animations as
+/// well.
+pub fn send_events_for_cancelled_animations(
+    script_channel: &IpcSender<ConstellationControlMsg>,
+    cancelled_animations: &mut FxHashMap<OpaqueNode, Vec<Animation>>,
+    pipeline_id: PipelineId,
+) {
+    for (node, animations) in cancelled_animations.drain() {
+        for animation in animations {
+            match animation {
+                Animation::Transition(transition_node, _, ref property_animation) => {
+                    debug_assert!(transition_node == node);
+                    script_channel
+                        .send(ConstellationControlMsg::TransitionEvent {
+                            pipeline_id,
+                            event_type: TransitionEventType::TransitionCancel,
+                            node: node.to_untrusted_node_address(),
+                            property_name: property_animation.property_name().into(),
+                            elapsed_time: property_animation.duration,
+                        })
+                        .unwrap();
+                },
+                Animation::Keyframes(..) => {
+                    warn!("Got unexpected animation in expired transitions list.")
+                },
+            }
+        }
+    }
+}
+
 /// Recalculates style for a set of animations. This does *not* run with the DOM
 /// lock held. Returns a set of nodes associated with animations that are no longer
 /// valid.

components/layout_thread/lib.rs

@@ -207,6 +207,9 @@ pub struct LayoutThread {
     /// The list of animations that have expired since the last style recalculation.
     expired_animations: ServoArc<RwLock<FxHashMap<OpaqueNode, Vec<Animation>>>>,
 
+    /// The list of animations that have been cancelled during the last style recalculation.
+    cancelled_animations: ServoArc<RwLock<FxHashMap<OpaqueNode, Vec<Animation>>>>,
+
     /// A counter for epoch messages
     epoch: Cell<Epoch>,
 
@@ -576,6 +579,7 @@ impl LayoutThread {
             document_shared_lock: None,
             running_animations: ServoArc::new(RwLock::new(Default::default())),
             expired_animations: ServoArc::new(RwLock::new(Default::default())),
+            cancelled_animations: ServoArc::new(RwLock::new(Default::default())),
             // Epoch starts at 1 because of the initial display list for epoch 0 that we send to WR
             epoch: Cell::new(Epoch(1)),
             viewport_size: Size2D::new(Au(0), Au(0)),
@@ -655,6 +659,7 @@ impl LayoutThread {
                 visited_styles_enabled: false,
                 running_animations: self.running_animations.clone(),
                 expired_animations: self.expired_animations.clone(),
+                cancelled_animations: self.cancelled_animations.clone(),
                 registered_speculative_painters: &self.registered_painters,
                 local_context_creation_data: Mutex::new(thread_local_style_context_creation_data),
                 timer: self.timer.clone(),
@@ -1785,6 +1790,7 @@ impl LayoutThread {
                 &self.script_chan,
                 &mut *self.running_animations.write(),
                 &mut *self.expired_animations.write(),
+                &mut *self.cancelled_animations.write(),
                 invalid_nodes,
                 newly_transitioning_nodes,
                 &self.new_animations_receiver,

components/layout_thread_2020/lib.rs

@@ -609,6 +609,7 @@ impl LayoutThread {
                 visited_styles_enabled: false,
                 running_animations: Default::default(),
                 expired_animations: Default::default(),
+                cancelled_animations: Default::default(),
                 registered_speculative_painters: &self.registered_painters,
                 local_context_creation_data: Mutex::new(thread_local_style_context_creation_data),
                 timer: self.timer.clone(),

components/script/dom/macros.rs

@@ -496,6 +496,7 @@ macro_rules! global_event_handlers(
         event_handler!(suspend, GetOnsuspend, SetOnsuspend);
         event_handler!(timeupdate, GetOntimeupdate, SetOntimeupdate);
         event_handler!(toggle, GetOntoggle, SetOntoggle);
+        event_handler!(transitioncancel, GetOntransitioncancel, SetOntransitioncancel);
         event_handler!(transitionend, GetOntransitionend, SetOntransitionend);
         event_handler!(volumechange, GetOnvolumechange, SetOnvolumechange);
         event_handler!(waiting, GetOnwaiting, SetOnwaiting);

components/script/dom/webidls/EventHandler.webidl

@@ -93,6 +93,7 @@ interface mixin GlobalEventHandlers {
 // https://drafts.csswg.org/css-transitions/#interface-globaleventhandlers-idl
 partial interface mixin GlobalEventHandlers {
            attribute EventHandler ontransitionend;
+           attribute EventHandler ontransitioncancel;
 };
 
 // https://w3c.github.io/selection-api/#extensions-to-globaleventhandlers-interface

components/script/script_thread.rs

@@ -133,17 +133,15 @@ use script_traits::CompositorEvent::{
     CompositionEvent, KeyboardEvent, MouseButtonEvent, MouseMoveEvent, ResizeEvent, TouchEvent,
     WheelEvent,
 };
-use script_traits::StructuredSerializedData;
-use script_traits::{CompositorEvent, ConstellationControlMsg};
 use script_traits::{
-    DiscardBrowsingContext, DocumentActivity, EventResult, HistoryEntryReplacement,
+    CompositorEvent, ConstellationControlMsg, DiscardBrowsingContext, DocumentActivity,
+    EventResult, HistoryEntryReplacement, InitialScriptState, JsEvalResult, LayoutMsg, LoadData,
+    LoadOrigin, MediaSessionActionType, MouseButton, MouseEventType, NewLayoutInfo, Painter,
+    ProgressiveWebMetricType, ScriptMsg, ScriptThreadFactory, ScriptToConstellationChan,
+    StructuredSerializedData, TimerSchedulerMsg, TouchEventType, TouchId, TransitionEventType,
+    UntrustedNodeAddress, UpdatePipelineIdReason, WebrenderIpcSender, WheelDelta, WindowSizeData,
+    WindowSizeType,
 };
-use script_traits::{InitialScriptState, JsEvalResult, LayoutMsg, LoadData, LoadOrigin};
-use script_traits::{MediaSessionActionType, MouseButton, MouseEventType, NewLayoutInfo};
-use script_traits::{Painter, ProgressiveWebMetricType, ScriptMsg, ScriptThreadFactory};
-use script_traits::{ScriptToConstellationChan, TimerSchedulerMsg};
-use script_traits::{TouchEventType, TouchId, UntrustedNodeAddress, WheelDelta};
-use script_traits::{UpdatePipelineIdReason, WebrenderIpcSender, WindowSizeData, WindowSizeType};
 use servo_atoms::Atom;
 use servo_config::opts;
 use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl};
@@ -1668,46 +1666,43 @@ impl ScriptThread {
     fn message_to_pipeline(&self, msg: &MixedMessage) -> Option<PipelineId> {
         use script_traits::ConstellationControlMsg::*;
         match *msg {
-            MixedMessage::FromConstellation(ref inner_msg) => {
+            MixedMessage::FromConstellation(ref inner_msg) => match *inner_msg {
-                match *inner_msg {
+                StopDelayingLoadEventsMode(id) => Some(id),
-                    StopDelayingLoadEventsMode(id) => Some(id),
+                NavigationResponse(id, _) => Some(id),
-                    NavigationResponse(id, _) => Some(id),
+                AttachLayout(ref new_layout_info) => Some(new_layout_info.new_pipeline_id),
-                    AttachLayout(ref new_layout_info) => Some(new_layout_info.new_pipeline_id),
+                Resize(id, ..) => Some(id),
-                    Resize(id, ..) => Some(id),
+                ResizeInactive(id, ..) => Some(id),
-                    ResizeInactive(id, ..) => Some(id),
+                UnloadDocument(id) => Some(id),
-                    UnloadDocument(id) => Some(id),
+                ExitPipeline(id, ..) => Some(id),
-                    ExitPipeline(id, ..) => Some(id),
+                ExitScriptThread => None,
-                    ExitScriptThread => None,
+                SendEvent(id, ..) => Some(id),
-                    SendEvent(id, ..) => Some(id),
+                Viewport(id, ..) => Some(id),
-                    Viewport(id, ..) => Some(id),
+                SetScrollState(id, ..) => Some(id),
-                    SetScrollState(id, ..) => Some(id),
+                GetTitle(id) => Some(id),
-                    GetTitle(id) => Some(id),
+                SetDocumentActivity(id, ..) => Some(id),
-                    SetDocumentActivity(id, ..) => Some(id),
+                ChangeFrameVisibilityStatus(id, ..) => Some(id),
-                    ChangeFrameVisibilityStatus(id, ..) => Some(id),
+                NotifyVisibilityChange(id, ..) => Some(id),
-                    NotifyVisibilityChange(id, ..) => Some(id),
+                NavigateIframe(id, ..) => Some(id),
-                    NavigateIframe(id, ..) => Some(id),
+                PostMessage { target: id, .. } => Some(id),
-                    PostMessage { target: id, .. } => Some(id),
+                UpdatePipelineId(_, _, _, id, _) => Some(id),
-                    UpdatePipelineId(_, _, _, id, _) => Some(id),
+                UpdateHistoryState(id, ..) => Some(id),
-                    UpdateHistoryState(id, ..) => Some(id),
+                RemoveHistoryStates(id, ..) => Some(id),
-                    RemoveHistoryStates(id, ..) => Some(id),
+                FocusIFrame(id, ..) => Some(id),
-                    FocusIFrame(id, ..) => Some(id),
+                WebDriverScriptCommand(id, ..) => Some(id),
-                    WebDriverScriptCommand(id, ..) => Some(id),
+                TickAllAnimations(id) => Some(id),
-                    TickAllAnimations(id) => Some(id),
+                TransitionEvent { .. } => None,
-                    // FIXME https://github.com/servo/servo/issues/15079
+                WebFontLoaded(id) => Some(id),
-                    TransitionEnd(..) => None,
+                DispatchIFrameLoadEvent {
-                    WebFontLoaded(id) => Some(id),
+                    target: _,
-                    DispatchIFrameLoadEvent {
+                    parent: id,
-                        target: _,
+                    child: _,
-                        parent: id,
+                } => Some(id),
-                        child: _,
+                DispatchStorageEvent(id, ..) => Some(id),
-                    } => Some(id),
+                ReportCSSError(id, ..) => Some(id),
-                    DispatchStorageEvent(id, ..) => Some(id),
+                Reload(id, ..) => Some(id),
-                    ReportCSSError(id, ..) => Some(id),
+                PaintMetric(..) => None,
-                    Reload(id, ..) => Some(id),
+                ExitFullScreen(id, ..) => Some(id),
-                    PaintMetric(..) => None,
+                MediaSessionAction(..) => None,
-                    ExitFullScreen(id, ..) => Some(id),
-                    MediaSessionAction(..) => None,
-                }
             },
             MixedMessage::FromDevtools(_) => None,
             MixedMessage::FromScript(ref inner_msg) => match *inner_msg {
@@ -1896,8 +1891,20 @@ impl ScriptThread {
             ConstellationControlMsg::TickAllAnimations(pipeline_id) => {
                 self.handle_tick_all_animations(pipeline_id)
             },
-            ConstellationControlMsg::TransitionEnd(unsafe_node, name, duration) => {
+            ConstellationControlMsg::TransitionEvent {
-                self.handle_transition_event(unsafe_node, name, duration)
+                pipeline_id,
+                event_type,
+                node,
+                property_name,
+                elapsed_time,
+            } => {
+                self.handle_transition_event(
+                    pipeline_id,
+                    event_type,
+                    node,
+                    property_name,
+                    elapsed_time,
+                );
             },
             ConstellationControlMsg::WebFontLoaded(pipeline_id) => {
                 self.handle_web_font_loaded(pipeline_id)
@@ -2899,12 +2906,16 @@ impl ScriptThread {
         document.run_the_animation_frame_callbacks();
     }
 
-    /// Handles firing of transition events.
+    /// Handles firing of transition-related events.
+    ///
+    /// TODO(mrobinson): Add support for more events.
     fn handle_transition_event(
         &self,
+        pipeline_id: PipelineId,
+        event_type: TransitionEventType,
         unsafe_node: UntrustedNodeAddress,
-        name: String,
+        property_name: String,
-        duration: f64,
+        elapsed_time: f64,
     ) {
         let js_runtime = self.js_runtime.rt();
         let node = unsafe { from_untrusted_node_address(js_runtime, unsafe_node) };
@@ -2926,29 +2937,35 @@ impl ScriptThread {
             },
         }
 
-        let window = window_from_node(&*node);
+        if self.closed_pipelines.borrow().contains(&pipeline_id) {
-
+            warn!("Ignoring transition event for closed pipeline.");
-        // Not quite the right thing - see #13865.
+            return;
-        node.dirty(NodeDamage::NodeStyleDamaged);
-
-        if let Some(el) = node.downcast::<Element>() {
-            if !el.has_css_layout_box() {
-                return;
-            }
         }
 
-        let init = TransitionEventInit {
+        let event_atom = match event_type {
+            TransitionEventType::TransitionEnd => {
+                // Not quite the right thing - see #13865.
+                node.dirty(NodeDamage::NodeStyleDamaged);
+                atom!("transitionend")
+            },
+            TransitionEventType::TransitionCancel => atom!("transitioncancel"),
+        };
+
+        let event_init = TransitionEventInit {
             parent: EventInit {
                 bubbles: true,
                 cancelable: false,
             },
-            propertyName: DOMString::from(name),
+            propertyName: DOMString::from(property_name),
-            elapsedTime: Finite::new(duration as f32).unwrap(),
+            elapsedTime: Finite::new(elapsed_time as f32).unwrap(),
-            // FIXME: Handle pseudo-elements properly
+            // TODO: Handle pseudo-elements properly
             pseudoElement: DOMString::new(),
         };
-        let transition_event = TransitionEvent::new(&window, atom!("transitionend"), &init);
+
-        transition_event.upcast::<Event>().fire(node.upcast());
+        let window = window_from_node(&*node);
+        TransitionEvent::new(&window, event_atom, &event_init)
+            .upcast::<Event>()
+            .fire(node.upcast());
     }
 
     /// Handles a Web font being loaded. Does nothing if the page no longer exists.

components/script_traits/lib.rs

@@ -282,6 +282,16 @@ pub enum UpdatePipelineIdReason {
     Traversal,
 }
 
+/// The type of transition event to trigger.
+#[derive(Clone, Copy, Debug, Deserialize, Serialize)]
+pub enum TransitionEventType {
+    /// The transition has ended by reaching the end of its animation.
+    TransitionEnd,
+    /// The transition ended early for some reason, such as the property
+    /// no longer being transitionable or being replaced by another transition.
+    TransitionCancel,
+}
+
 /// Messages sent from the constellation or layout to the script thread.
 #[derive(Deserialize, Serialize)]
 pub enum ConstellationControlMsg {
@@ -368,8 +378,19 @@ pub enum ConstellationControlMsg {
     WebDriverScriptCommand(PipelineId, WebDriverScriptCommand),
     /// Notifies script thread that all animations are done
     TickAllAnimations(PipelineId),
-    /// Notifies the script thread of a transition end
+    /// Notifies the script thread that a transition related event should be sent.
-    TransitionEnd(UntrustedNodeAddress, String, f64),
+    TransitionEvent {
+        /// The pipeline id of the layout task that sent this message.
+        pipeline_id: PipelineId,
+        /// The type of transition event this should trigger.
+        event_type: TransitionEventType,
+        /// The address of the node which owns this transition.
+        node: UntrustedNodeAddress,
+        /// The property name of the property that is transitioning.
+        property_name: String,
+        /// The elapsed time property to send with this transition event.
+        elapsed_time: f64,
+    },
     /// Notifies the script thread that a new Web font has been loaded, and thus the page should be
     /// reflowed.
     WebFontLoaded(PipelineId),
@@ -429,7 +450,7 @@ impl fmt::Debug for ConstellationControlMsg {
             FocusIFrame(..) => "FocusIFrame",
             WebDriverScriptCommand(..) => "WebDriverScriptCommand",
             TickAllAnimations(..) => "TickAllAnimations",
-            TransitionEnd(..) => "TransitionEnd",
+            TransitionEvent { .. } => "TransitionEvent",
             WebFontLoaded(..) => "WebFontLoaded",
             DispatchIFrameLoadEvent { .. } => "DispatchIFrameLoadEvent",
             DispatchStorageEvent(..) => "DispatchStorageEvent",

components/style/animation.rs

@@ -15,7 +15,7 @@ use crate::font_metrics::FontMetricsProvider;
 use crate::properties::animated_properties::{AnimatedProperty, TransitionPropertyIteration};
 use crate::properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection;
 use crate::properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState;
-use crate::properties::{self, CascadeMode, ComputedValues, LonghandId};
+use crate::properties::{self, CascadeMode, ComputedValues, LonghandId, LonghandIdSet};
 use crate::stylesheets::keyframes_rule::{KeyframesAnimation, KeyframesStep, KeyframesStepValue};
 use crate::stylesheets::Origin;
 use crate::timer::Timer;
@@ -245,6 +245,17 @@ impl Animation {
             Animation::Keyframes(..) => false,
         }
     }
+
+    /// Whether this animation has the same end value as another one.
+    #[inline]
+    pub fn is_transition_with_same_end_value(&self, other_animation: &PropertyAnimation) -> bool {
+        match *self {
+            Animation::Transition(_, _, ref animation) => {
+                animation.has_the_same_end_value_as(other_animation)
+            },
+            Animation::Keyframes(..) => false,
+        }
+    }
 }
 
 /// Represents an animation for a given property.
@@ -261,6 +272,11 @@ pub struct PropertyAnimation {
 }
 
 impl PropertyAnimation {
+    /// Returns the given property longhand id.
+    pub fn property_id(&self) -> LonghandId {
+        self.property.id()
+    }
+
     /// Returns the given property name.
     pub fn property_name(&self) -> &'static str {
         self.property.name()
@@ -351,20 +367,86 @@ impl PropertyAnimation {
     }
 }
 
-/// Inserts transitions into the queue of running animations as applicable for
+/// Start any new transitions for this node and ensure that any existing transitions
-/// the given style difference. This is called from the layout worker threads.
+/// that are cancelled are marked as cancelled in the SharedStyleContext. This is
-/// Returns true if any animations were kicked off and false otherwise.
+/// at the end of calculating style for a single node.
-pub fn start_transitions_if_applicable(
+#[cfg(feature = "servo")]
+pub fn update_transitions(
+    context: &SharedStyleContext,
     new_animations_sender: &Sender<Animation>,
     opaque_node: OpaqueNode,
     old_style: &ComputedValues,
     new_style: &mut Arc<ComputedValues>,
-    timer: &Timer,
+    expired_transitions: &[PropertyAnimation],
-    running_and_expired_transitions: &[PropertyAnimation],
+) {
-) -> bool {
+    let mut all_running_animations = context.running_animations.write();
-    let mut had_animations = false;
+    let previously_running_animations = all_running_animations
+        .remove(&opaque_node)
+        .unwrap_or_else(Vec::new);
+
+    let properties_that_transition = start_transitions_if_applicable(
+        context,
+        new_animations_sender,
+        opaque_node,
+        old_style,
+        new_style,
+        expired_transitions,
+        &previously_running_animations,
+    );
+
+    let mut all_cancelled_animations = context.cancelled_animations.write();
+    let mut cancelled_animations = all_cancelled_animations
+        .remove(&opaque_node)
+        .unwrap_or_else(Vec::new);
+    let mut running_animations = vec![];
+
+    // For every animation that was running before this style change, we cancel it
+    // if the property no longer transitions.
+    for running_animation in previously_running_animations.into_iter() {
+        if let Animation::Transition(_, _, ref property_animation) = running_animation {
+            if !properties_that_transition.contains(property_animation.property_id()) {
+                cancelled_animations.push(running_animation);
+                continue;
+            }
+        }
+        running_animations.push(running_animation);
+    }
+
+    if !cancelled_animations.is_empty() {
+        all_cancelled_animations.insert(opaque_node, cancelled_animations);
+    }
+    if !running_animations.is_empty() {
+        all_running_animations.insert(opaque_node, running_animations);
+    }
+}
+
+/// Kick off any new transitions for this node and return all of the properties that are
+/// transitioning. This is at the end of calculating style for a single node.
+#[cfg(feature = "servo")]
+pub fn start_transitions_if_applicable(
+    context: &SharedStyleContext,
+    new_animations_sender: &Sender<Animation>,
+    opaque_node: OpaqueNode,
+    old_style: &ComputedValues,
+    new_style: &mut Arc<ComputedValues>,
+    expired_transitions: &[PropertyAnimation],
+    running_animations: &[Animation],
+) -> LonghandIdSet {
+    // If the style of this element is display:none, then we don't start any transitions
+    // and we cancel any currently running transitions by returning an empty LonghandIdSet.
+    if new_style.get_box().clone_display().is_none() {
+        return LonghandIdSet::new();
+    }
+
+    let mut properties_that_transition = LonghandIdSet::new();
     let transitions: Vec<TransitionPropertyIteration> = new_style.transition_properties().collect();
     for transition in &transitions {
+        if properties_that_transition.contains(transition.longhand_id) {
+            continue;
+        } else {
+            properties_that_transition.insert(transition.longhand_id);
+        }
+
         let property_animation = match PropertyAnimation::from_longhand(
             transition.longhand_id,
             new_style
@@ -387,20 +469,27 @@ pub fn start_transitions_if_applicable(
         property_animation.update(Arc::get_mut(new_style).unwrap(), 0.0);
 
         // 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
+        // transition is the same as that of a transition that's expired.
-        // running on the same node.
-        //
         // [1]: https://drafts.csswg.org/css-transitions/#starting
-        debug!(
+        debug!("checking {:?} for matching end value", expired_transitions);
-            "checking {:?} for matching end value",
+        if expired_transitions
-            running_and_expired_transitions
-        );
-        if running_and_expired_transitions
             .iter()
             .any(|animation| animation.has_the_same_end_value_as(&property_animation))
         {
             debug!(
-                "Not initiating transition for {}, other transition \
+                "Not initiating transition for {}, expired transition \
+                 found with the same end value",
+                property_animation.property_name()
+            );
+            continue;
+        }
+
+        if running_animations
+            .iter()
+            .any(|animation| animation.is_transition_with_same_end_value(&property_animation))
+        {
+            debug!(
+                "Not initiating transition for {}, running transition \
                  found with the same end value",
                 property_animation.property_name()
             );
@@ -410,7 +499,7 @@ pub fn start_transitions_if_applicable(
         // Kick off the animation.
         debug!("Kicking off transition of {:?}", property_animation);
         let box_style = new_style.get_box();
-        let now = timer.seconds();
+        let now = context.timer.seconds();
         let start_time = now + (box_style.transition_delay_mod(transition.index).seconds() as f64);
         new_animations_sender
             .send(Animation::Transition(
@@ -419,11 +508,9 @@ pub fn start_transitions_if_applicable(
                 property_animation,
             ))
             .unwrap();
-
-        had_animations = true;
     }
 
-    had_animations
+    properties_that_transition
 }
 
 fn compute_style_for_animation_step<E>(
@@ -608,12 +695,6 @@ where
             if progress >= 0.0 {
                 property_animation.update(Arc::make_mut(style), progress);
             }
-
-            // FIXME(emilio): Should check before updating the style that the
-            // transition_property still transitions this, or bail out if not.
-            //
-            // Or doing it in process_animations, only if transition_property
-            // changed somehow (even better).
             AnimationUpdate::Regular
         },
         Animation::Keyframes(_, ref animation, ref name, ref state) => {

components/style/context.rs

@@ -194,6 +194,10 @@ pub struct SharedStyleContext<'a> {
     #[cfg(feature = "servo")]
     pub expired_animations: Arc<RwLock<FxHashMap<OpaqueNode, Vec<Animation>>>>,
 
+    /// The list of animations that have expired since the last style recalculation.
+    #[cfg(feature = "servo")]
+    pub cancelled_animations: Arc<RwLock<FxHashMap<OpaqueNode, Vec<Animation>>>>,
+
     /// Paint worklets
     #[cfg(feature = "servo")]
     pub registered_speculative_painters: &'a dyn RegisteredSpeculativePainters,

components/style/matching.rs

@@ -440,7 +440,7 @@ trait PrivateMatchMethods: TElement {
         use crate::animation;
 
         let this_opaque = self.as_node().opaque();
-        let mut running_and_expired_transitions = vec![];
+        let mut expired_transitions = vec![];
         let shared_context = context.shared;
         if let Some(ref mut old_values) = *old_values {
             // We apply the expired transitions and animations to the old style
@@ -454,14 +454,13 @@ trait PrivateMatchMethods: TElement {
                 shared_context,
                 this_opaque,
                 old_values,
-                &mut running_and_expired_transitions,
+                &mut expired_transitions,
             );
 
-            Self::update_style_for_animations_and_collect_running_transitions(
+            Self::update_style_for_animations(
                 shared_context,
                 this_opaque,
                 old_values,
-                &mut running_and_expired_transitions,
                 &context.thread_local.font_metrics_provider,
             );
         }
@@ -479,13 +478,13 @@ trait PrivateMatchMethods: TElement {
         // Trigger transitions if necessary. This will set `new_values` to
         // the starting value of the transition if it did trigger a transition.
         if let Some(ref values) = old_values {
-            animation::start_transitions_if_applicable(
+            animation::update_transitions(
+                &shared_context,
                 new_animations_sender,
                 this_opaque,
                 &values,
                 new_values,
-                &shared_context.timer,
+                &expired_transitions,
-                &running_and_expired_transitions,
             );
         }
     }
@@ -627,33 +626,30 @@ trait PrivateMatchMethods: TElement {
     }
 
     #[cfg(feature = "servo")]
-    fn update_style_for_animations_and_collect_running_transitions(
+    fn update_style_for_animations(
         context: &SharedStyleContext,
         node: OpaqueNode,
         style: &mut Arc<ComputedValues>,
-        running_transitions: &mut Vec<crate::animation::PropertyAnimation>,
         font_metrics: &dyn crate::font_metrics::FontMetricsProvider,
     ) {
         use crate::animation::{self, Animation, AnimationUpdate};
 
-        let had_running_animations = context.running_animations.read().get(&node).is_some();
-        if !had_running_animations {
-            return;
-        }
-
         let mut all_running_animations = context.running_animations.write();
-        for mut running_animation in all_running_animations.get_mut(&node).unwrap() {
+        let running_animations = match all_running_animations.get_mut(&node) {
-            if let Animation::Transition(_, _, ref property_animation) = *running_animation {
+            Some(running_animations) => running_animations,
-                running_transitions.push(property_animation.clone());
+            None => return,
-                continue;
+        };
-            }
 
-            let update = animation::update_style_for_animation::<Self>(
+        for running_animation in running_animations.iter_mut() {
-                context,
+            let update = match *running_animation {
-                &mut running_animation,
+                Animation::Transition(..) => continue,
-                style,
+                Animation::Keyframes(..) => animation::update_style_for_animation::<Self>(
-                font_metrics,
+                    context,
-            );
+                    running_animation,
+                    style,
+                    font_metrics,
+                ),
+            };
 
             match *running_animation {
                 Animation::Transition(..) => unreachable!(),

tests/wpt/metadata/MANIFEST.json

@@ -398363,6 +398363,13 @@
        }
       ]
      ],
+     "transitioncancel-002.html": [
+      "e62b17b5dc60ef762e0a0780c967b6e014da5bc9",
+      [
+       null,
+       {}
+      ]
+     ],
      "transitionevent-interface.html": [
       "a40ba4537518361c13aab1d9b0648387f7c88aaa",
       [

tests/wpt/metadata/css/css-transitions/idlharness.html.ini

@@ -5,42 +5,24 @@
   [HTMLElement interface: attribute ontransitionstart]
     expected: FAIL
 
-  [Document interface: attribute ontransitioncancel]
-    expected: FAIL
-
   [Document interface: document must inherit property "ontransitionstart" with the proper type]
     expected: FAIL
 
-  [Document interface: document must inherit property "ontransitioncancel" with the proper type]
-    expected: FAIL
-
   [HTMLElement interface: attribute ontransitionrun]
     expected: FAIL
 
-  [HTMLElement interface: document must inherit property "ontransitioncancel" with the proper type]
-    expected: FAIL
-
   [Window interface: attribute ontransitionrun]
     expected: FAIL
 
-  [Window interface: attribute ontransitioncancel]
-    expected: FAIL
-
   [HTMLElement interface: document must inherit property "ontransitionstart" with the proper type]
     expected: FAIL
 
-  [HTMLElement interface: attribute ontransitioncancel]
-    expected: FAIL
-
   [HTMLElement interface: document must inherit property "ontransitionrun" with the proper type]
     expected: FAIL
 
   [Window interface: window must inherit property "ontransitionrun" with the proper type]
     expected: FAIL
 
-  [Window interface: window must inherit property "ontransitioncancel" with the proper type]
-    expected: FAIL
-
   [Document interface: attribute ontransitionstart]
     expected: FAIL
 

tests/wpt/metadata/css/css-transitions/transitioncancel-001.html.ini

@@ -1,4 +0,0 @@
-[transitioncancel-001.html]
-  [transitioncancel should be fired if the element is made display:none during the transition]
-    expected: FAIL
-

tests/wpt/web-platform-tests/css/css-transitions/transitioncancel-002.html

@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>CSS Transitions Test: Removing transitioning property from transition-property triggers transitioncancel</title>
+<link rel="author" title="Martin Robinson" href="mailto:mrobinson@igalia.com">
+<meta name="assert" content="Removing transitioning property from transition-property
+causes transitioncancel.">
+<link rel="help" href="https://drafts.csswg.org/css-transitions-2/#event-dispatch">
+
+<script src="/resources/testharness.js" type="text/javascript"></script>
+<script src="/resources/testharnessreport.js" type="text/javascript"></script>
+<script src="./support/helper.js" type="text/javascript"></script>
+
+</head>
+<body>
+<div id="log"></div>
+
+<script>
+promise_test(async t => {
+  // Create element and prepare to trigger a transition on it.
+  const div = addDiv(t, {
+    style: 'transition: background-color 0.25s; background-color: red;',
+  });
+
+  // Attach event listeners
+  const eventWatcher = new EventWatcher(t, div, ['transitioncancel']);
+  div.addEventListener('transitionend', t.step_func((event) => {
+    assert_unreached('transitionend event should not be fired');
+  }));
+
+  // Trigger transition
+  getComputedStyle(div).backgroundColor;
+  div.style.backgroundColor = 'green';
+  getComputedStyle(div).backgroundColor;
+
+  // Remove the transitioning property from transition-property asynchronously.
+  await waitForFrame();
+  div.style.transitionProperty = 'none';
+
+  await eventWatcher.wait_for('transitioncancel');
+}, 'Removing a transitioning property from transition-property should trigger transitioncancel');
+</script>
+
+</body>
+</html>