fix(script): request animation ticks if `Animations::pending_events` is not empty

Fixes the test case `/_mozilla/css/css-transition-cancel-event
.html`, which was failing under a specific circumstance.

The observed sequence of events during the failing test run looks like
this:

1. Transitions start in `div1` and `div2`.

2. `div1` generates a `transitionend` event.

3. The `transitionend` event handler removes `div2` from DOM, cancelling
   its ongoing transition.

4. `div2` is supposed to generate a `transitioncancel` event in a timely
   manner, which it does not. The test fails as a result.

What is going on here? Here's a possible explaination:

1. During one invocation of `ScriptThread::handle_msgs`...

2. In step 2, `ScriptThread::update_animations_send_events` -> `Document
   ::update_for_new_timeline_value` detects the completion of the
   transition, and in response, pends the `transitionend` event.

3. In step 3, `ScriptThread::update_animations_send_events` ->
   `Animations::send_pending_events` calls the `transitionend` handler.

4. The `transitionend` event handler removes `div2`, thereby cancelling
   its ongoing transition and triggering a reflow.

5. Reflow takes place. During this, `Animations::do_post_reflow_update`
   -> `Animations::handle_canceled_animations` pends the
   `transitioncancel` event (precursor to step 4).

6. Having discovering that there was no running animation, `Animations::
   do_post_reflow_update` calls `self.update_running_animation_presence
   (_, false)`, which sends `AnimationState::NoAnimationsPresent`.

7. The invocation of `ScriptThread::handle_msgs` ends, and another
   starts. It blocks waiting for events.

8. Meanwhile, the compositor receives `AnimationState::
   NoAnimationsPresent` and stops further generation of animation ticks.

9. With no events to wake it up, the script thread is stuck waiting
   despite having the pending `transitioncancel` event (step 4).

The HTML specification [says][1] that "an event loop must continually
run [...] as long as it exists" and does not say it can block if there
is nothing to do. Blocking is merely optimization in a user agent
implementation. Pending animation-related events must be processed every
time a "rendering opportunity" arises unless the user agent has a reason
to believe that it "would have no visible effect".

Skipping the processing of animation-related events would have visible
effect if such events are indeed present. The correct implementation in
Servo, therefore, would be to request more animation ticks so that such
events are processed in a subsequent tick.

[1]: https://html.spec.whatwg.org/multipage/#event-loop-processing-model

1 files changed, 24 insertions, 8 deletions

diff --git a/components/script/animations.rs b/components/script/animations.rs
index cd581ed3cb3..73131763430 100644
--- a/components/script/animations.rs
+++ b/components/script/animations.rs
@@ -40,7 +40,7 @@ pub(crate) struct Animations {
     pub sets: DocumentAnimationSet,
 
     /// Whether or not we have animations that are running.
-    have_running_animations: Cell<bool>,
+    has_running_animations: Cell<bool>,
 
     /// A list of nodes with in-progress CSS transitions or pending events.
     rooted_nodes: DomRefCell<FxHashMap<OpaqueNode, Dom<Node>>>,
@@ -53,7 +53,7 @@ impl Animations {
     pub(crate) fn new() -> Self {
         Animations {
             sets: Default::default(),
-            have_running_animations: Cell::new(false),
+            has_running_animations: Cell::new(false),
             rooted_nodes: Default::default(),
             pending_events: Default::default(),
         }
@@ -97,6 +97,7 @@ impl Animations {
         }
 
         self.unroot_unused_nodes(&sets);
+        //self.update_running_animations_presence(window);
     }
 
     /// Cancel animations for the given node, if any exist.
@@ -142,17 +143,25 @@ impl Animations {
     }
 
     fn update_running_animations_presence(&self, window: &Window, new_value: bool) {
-        let have_running_animations = self.have_running_animations.get();
-        if new_value == have_running_animations {
+        let had_running_animations = self.has_running_animations.get();
+        if new_value == had_running_animations {
             return;
         }
 
-        self.have_running_animations.set(new_value);
-        let state = match new_value {
+        self.has_running_animations.set(new_value);
+        self.handle_animation_presence_or_pending_events_change(window);
+    }
+
+    fn handle_animation_presence_or_pending_events_change(&self, window: &Window) {
+        let has_running_animations = self.has_running_animations.get();
+        let has_pending_events = !self.pending_events.borrow().is_empty();
+
+        // Do not send the NoAnimationCallbacksPresent state until all pending
+        // animation events are delivered.
+        let state = match has_running_animations || has_pending_events {
             true => AnimationsPresentState::AnimationsPresent,
             false => AnimationsPresentState::NoAnimationsPresent,
         };
-
         window.send_to_constellation(ScriptMsg::ChangeRunningAnimationsState(state));
     }
 
@@ -425,10 +434,13 @@ impl Animations {
             });
     }
 
-    pub(crate) fn send_pending_events(&self) {
+    pub(crate) fn send_pending_events(&self, window: &Window) {
         // Take all of the events here, in case sending one of these events
         // triggers adding new events by forcing a layout.
         let events = std::mem::replace(&mut *self.pending_events.borrow_mut(), Vec::new());
+        if events.is_empty() {
+            return;
+        }
 
         for event in events.into_iter() {
             // We root the node here to ensure that sending this event doesn't
@@ -488,6 +500,10 @@ impl Animations {
                     .fire(node.upcast());
             }
         }
+
+        if self.pending_events.borrow().is_empty() {
+            self.handle_animation_presence_or_pending_events_change(window);
+        }
     }
 }