Cancel animations that affect nodes that do not participate in layout.

components/layout/animation.rs

@@ -9,7 +9,7 @@ use crate::display_list::items::OpaqueNode;
 use crate::flow::{Flow, GetBaseFlow};
 use crate::opaque_node::OpaqueNodeMethods;
 use crossbeam_channel::Receiver;
-use fxhash::FxHashMap;
+use fxhash::{FxHashMap, FxHashSet};
 use ipc_channel::ipc::IpcSender;
 use msg::constellation_msg::PipelineId;
 use script_traits::UntrustedNodeAddress;
@@ -28,6 +28,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>>,
+    mut keys_to_remove: FxHashSet<OpaqueNode>,
     mut newly_transitioning_nodes: Option<&mut Vec<UntrustedNodeAddress>>,
     new_animations_receiver: &Receiver<Animation>,
     pipeline_id: PipelineId,
@@ -74,7 +75,6 @@ pub fn update_animation_state<E>(
     // TODO: Do not expunge Keyframes animations, since we need that state if
     // the animation gets re-triggered. Probably worth splitting in two
     // different maps, or at least using a linked list?
-    let mut keys_to_remove = vec![];
     for (key, running_animations) in running_animations.iter_mut() {
         let mut animations_still_running = vec![];
         for mut running_animation in running_animations.drain(..) {
@@ -116,7 +116,7 @@ pub fn update_animation_state<E>(
         }
 
         if animations_still_running.is_empty() {
-            keys_to_remove.push(*key);
+            keys_to_remove.insert(*key);
         } else {
             *running_animations = animations_still_running
         }
@@ -160,17 +160,33 @@ pub fn update_animation_state<E>(
 }
 
 /// Recalculates style for a set of animations. This does *not* run with the DOM
-/// lock held.
+/// lock held. Returns a set of nodes associated with animations that are no longer
+/// valid.
 pub fn recalc_style_for_animations<E>(
     context: &LayoutContext,
     flow: &mut dyn Flow,
     animations: &FxHashMap<OpaqueNode, Vec<Animation>>,
+) -> FxHashSet<OpaqueNode>
+where
+    E: TElement,
+{
+    let mut invalid_nodes = animations.keys().cloned().collect();
+    do_recalc_style_for_animations::<E>(context, flow, animations, &mut invalid_nodes);
+    invalid_nodes
+}
+
+fn do_recalc_style_for_animations<E>(
+    context: &LayoutContext,
+    flow: &mut dyn Flow,
+    animations: &FxHashMap<OpaqueNode, Vec<Animation>>,
+    invalid_nodes: &mut FxHashSet<OpaqueNode>,
 ) where
     E: TElement,
 {
     let mut damage = RestyleDamage::empty();
     flow.mutate_fragments(&mut |fragment| {
         if let Some(ref animations) = animations.get(&fragment.node) {
+            invalid_nodes.remove(&fragment.node);
             for animation in animations.iter() {
                 let old_style = fragment.style.clone();
                 update_style_for_animation::<E>(
@@ -189,6 +205,6 @@ pub fn recalc_style_for_animations<E>(
     let base = flow.mut_base();
     base.restyle_damage.insert(damage);
     for kid in base.children.iter_mut() {
-        recalc_style_for_animations::<E>(context, kid, animations)
+        do_recalc_style_for_animations::<E>(context, kid, animations, invalid_nodes)
     }
 }

components/layout_thread/lib.rs

@@ -27,7 +27,7 @@ use crossbeam_channel::{unbounded, Receiver, Sender};
 use embedder_traits::resources::{self, Resource};
 use euclid::{Point2D, Rect, Size2D, TypedScale, TypedSize2D};
 use fnv::FnvHashMap;
-use fxhash::FxHashMap;
+use fxhash::{FxHashMap, FxHashSet};
 use gfx::font;
 use gfx::font_cache_thread::FontCacheThread;
 use gfx::font_context;
@@ -647,6 +647,7 @@ impl LayoutThread {
             },
             Msg::RegisterPaint(..) => LayoutHangAnnotation::RegisterPaint,
             Msg::SetNavigationStart(..) => LayoutHangAnnotation::SetNavigationStart,
+            Msg::GetRunningAnimations(..) => LayoutHangAnnotation::GetRunningAnimations,
         };
         self.background_hang_monitor
             .notify_activity(HangAnnotation::Layout(hang_annotation));
@@ -818,6 +819,9 @@ impl LayoutThread {
             Msg::SetNavigationStart(time) => {
                 self.paint_time_metrics.set_navigation_start(time);
             },
+            Msg::GetRunningAnimations(sender) => {
+                let _ = sender.send(self.running_animations.read().len());
+            },
         }
 
         true
@@ -1447,6 +1451,7 @@ impl LayoutThread {
                 Some(&document),
                 &mut rw_data,
                 &mut layout_context,
+                FxHashSet::default(),
             );
         }
 
@@ -1624,7 +1629,7 @@ impl LayoutThread {
             let snapshots = SnapshotMap::new();
             let mut layout_context = self.build_layout_context(guards, false, &snapshots);
 
-            {
+            let invalid_nodes = {
                 // Perform an abbreviated style recalc that operates without access to the DOM.
                 let animations = self.running_animations.read();
                 profile(
@@ -1638,15 +1643,16 @@ impl LayoutThread {
                             &animations,
                         )
                     },
-                );
-            }
+                )
+            };
             self.perform_post_style_recalc_layout_passes(
                 &mut root_flow,
                 &reflow_info,
                 &ReflowGoal::TickAnimations,
                 None,
                 &mut *rw_data,
                 &mut layout_context,
+                invalid_nodes,
             );
             assert!(layout_context.pending_images.is_none());
             assert!(layout_context.newly_transitioning_nodes.is_none());
@@ -1661,6 +1667,7 @@ impl LayoutThread {
         document: Option<&ServoLayoutDocument>,
         rw_data: &mut LayoutThreadData,
         context: &mut LayoutContext,
+        invalid_nodes: FxHashSet<OpaqueNode>,
     ) {
         {
             let mut newly_transitioning_nodes = context
@@ -1675,6 +1682,7 @@ impl LayoutThread {
                 &self.script_chan,
                 &mut *self.running_animations.write(),
                 &mut *self.expired_animations.write(),
+                invalid_nodes,
                 newly_transitioning_nodes,
                 &self.new_animations_receiver,
                 self.id,

components/msg/constellation_msg.rs

@@ -310,6 +310,7 @@ pub enum LayoutHangAnnotation {
     UpdateScrollStateFromScript,
     RegisterPaint,
     SetNavigationStart,
+    GetRunningAnimations,
 }
 
 #[derive(Clone, Copy, Debug, Deserialize, Serialize)]

components/script/dom/webidls/Window.webidl

@@ -173,3 +173,8 @@ partial interface Window {
    readonly attribute TestRunner testRunner;
    //readonly attribute EventSender eventSender;
 };
+
+partial interface Window {
+   [Pref="css.animations.testing.enabled"]
+   readonly attribute unsigned long runningAnimationCount;
+};

components/script/dom/window.rs

@@ -74,7 +74,7 @@ use devtools_traits::{ScriptToDevtoolsControlMsg, TimelineMarker, TimelineMarker
 use dom_struct::dom_struct;
 use embedder_traits::EmbedderMsg;
 use euclid::{Point2D, Rect, Size2D, TypedPoint2D, TypedScale, TypedSize2D, Vector2D};
-use ipc_channel::ipc::IpcSender;
+use ipc_channel::ipc::{channel, IpcSender};
 use ipc_channel::router::ROUTER;
 use js::jsapi::JSAutoCompartment;
 use js::jsapi::JSContext;
@@ -1144,6 +1144,12 @@ impl WindowMethods for Window {
         self.test_runner.or_init(|| TestRunner::new(self.upcast()))
     }
 
+    fn RunningAnimationCount(&self) -> u32 {
+        let (sender, receiver) = channel().unwrap();
+        let _ = self.layout_chan.send(Msg::GetRunningAnimations(sender));
+        receiver.recv().unwrap_or(0) as u32
+    }
+
     // https://html.spec.whatwg.org/multipage/#dom-name
     fn SetName(&self, name: DOMString) {
         self.window_proxy().set_name(name);

components/script_layout_interface/message.rs

@@ -98,6 +98,9 @@ pub enum Msg {
 
     /// Send to layout the precise time when the navigation started.
     SetNavigationStart(u64),
+
+    /// Request the current number of animations that are running.
+    GetRunningAnimations(IpcSender<usize>),
 }
 
 #[derive(Debug, PartialEq)]

tests/wpt/mozilla/meta/MANIFEST.json

@@ -13458,6 +13458,12 @@
      {}
     ]
    ], 
+   "mozilla/animation-removed-node.html": [
+    [
+     "/_mozilla/mozilla/animation-removed-node.html", 
+     {}
+    ]
+   ], 
    "mozilla/binding_keyword.html": [
     [
      "/_mozilla/mozilla/binding_keyword.html", 
@@ -26613,6 +26619,10 @@
    "81de5b389c922067c61effe03208ea740ba8e067", 
    "testharness"
   ], 
+  "mozilla/animation-removed-node.html": [
+   "6ba0318ea1e07b42ef444f838753adbefe9633d6", 
+   "testharness"
+  ], 
   "mozilla/binding_keyword.html": [
    "818d2aa29471026c1b4215dfcd1b9939a052b1ea", 
    "testharness"

tests/wpt/mozilla/meta/mozilla/animation-removed-node.html.ini

@@ -0,0 +1,2 @@
+[animation-removed-node.html]
+  prefs: [css.animations.testing.enabled:true]

tests/wpt/mozilla/tests/mozilla/animation-removed-node.html

@@ -0,0 +1,28 @@
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+@keyframes boo {
+  0%   { opacity: 0; }
+  100% { opacity: 1; }
+}
+div.test { animation: boo 1s infinite; }
+</style>
+<div class="test" id="first">hi there!</div>
+<div class="test" id="second">hi again!</div>
+<script>
+  async_test(function(t) {
+    window.onload = t.step_func(function() {
+      // Verify that there are the expected animations active.
+      assert_equals(window.runningAnimationCount, 2);
+      // Cause the animating nodes to become uninvolved with layout.
+      document.getElementById('first').remove();
+      document.getElementById('second').style.display = 'none';
+      // Ensure that we wait until the next layout is complete.
+      requestAnimationFrame(t.step_func(function() {
+        // Verify that the previous animations are no longer considered active.
+        assert_equals(window.runningAnimationCount, 0);
+        t.done();
+      }));
+    });
+  }, "Animations are no longer active when a node can't be animated.");
+</script>