compositor: Tick animations for an entire WebView at once (#36662)

Previously, when processing animations, the compositor would sent a tick
message to each pipeline. This is an issue because now the
`ScriptThread` always processes rendering updates for all `Document`s in
order to ensure properly ordering. This change makes it so that tick
messages are sent for an entire WebView. This means that each
`ScriptThread` will always receive a single tick for every time that
animations are processed, no matter how many frames are animating. This
is the first step toward a refresh driver.

In addition, we discard the idea of ticking animation only for
animations and or only for request animation frame callbacks. The
`ScriptThread` can no longer make this distinction due to the
specification and the compositor shouldn't either.

This should not really change observable behavior, but should make Servo
more efficient when more than a single frame in a `ScriptThread` is
animting at once.

Testing: This is covered by existing WPT tests as it mainly just improve
animation efficiency in a particular case.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

2 files changed, 75 insertions, 88 deletions

diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs
index 4550188a7fa..41286a2760a 100644
--- a/components/compositing/compositor.rs
+++ b/components/compositing/compositor.rs
@@ -21,12 +21,12 @@ use compositing_traits::rendering_context::RenderingContext;
 use compositing_traits::{
     CompositionPipeline, CompositorMsg, ImageUpdate, SendableFrameTree, WebViewTrait,
 };
-use constellation_traits::{AnimationTickType, EmbedderToConstellationMessage, PaintMetricEvent};
+use constellation_traits::{EmbedderToConstellationMessage, PaintMetricEvent};
 use crossbeam_channel::{Receiver, Sender};
 use dpi::PhysicalSize;
 use embedder_traits::{
-    AnimationState, CompositorHitTestResult, Cursor, InputEvent, MouseButtonEvent, MouseMoveEvent,
-    ShutdownState, TouchEventType, UntrustedNodeAddress, ViewportDetails,
+    CompositorHitTestResult, Cursor, InputEvent, MouseButtonEvent, MouseMoveEvent, ShutdownState,
+    TouchEventType, UntrustedNodeAddress, ViewportDetails,
 };
 use euclid::{Point2D, Rect, Scale, Size2D, Transform3D};
 use fnv::FnvHashMap;
@@ -197,9 +197,6 @@ pub(crate) struct PipelineDetails {
     /// The pipeline associated with this PipelineDetails object.
     pub pipeline: Option<CompositionPipeline>,
 
-    /// The [`PipelineId`] of this pipeline.
-    pub id: PipelineId,
-
     /// The id of the parent pipeline, if any.
     pub parent_pipeline_id: Option<PipelineId>,
 
@@ -243,32 +240,12 @@ impl PipelineDetails {
     pub(crate) fn animating(&self) -> bool {
         !self.throttled && (self.animation_callbacks_running || self.animations_running)
     }
-
-    pub(crate) fn tick_animations(&self, compositor: &IOCompositor) {
-        if !self.animating() {
-            return;
-        }
-
-        let mut tick_type = AnimationTickType::empty();
-        if self.animations_running {
-            tick_type.insert(AnimationTickType::CSS_ANIMATIONS_AND_TRANSITIONS);
-        }
-        if self.animation_callbacks_running {
-            tick_type.insert(AnimationTickType::REQUEST_ANIMATION_FRAME);
-        }
-
-        let msg = EmbedderToConstellationMessage::TickAnimation(self.id, tick_type);
-        if let Err(e) = compositor.global.borrow().constellation_sender.send(msg) {
-            warn!("Sending tick to constellation failed ({:?}).", e);
-        }
-    }
 }
 
 impl PipelineDetails {
-    pub(crate) fn new(id: PipelineId) -> PipelineDetails {
+    pub(crate) fn new() -> PipelineDetails {
         PipelineDetails {
             pipeline: None,
-            id,
             parent_pipeline_id: None,
             most_recent_display_list_epoch: None,
             animations_running: false,
@@ -543,22 +520,14 @@ impl IOCompositor {
                 pipeline_id,
                 animation_state,
             ) => {
-                let mut throttled = true;
                 if let Some(webview_renderer) = self.webview_renderers.get_mut(webview_id) {
-                    throttled = webview_renderer
-                        .change_running_animations_state(pipeline_id, animation_state);
-                }
-
-                // These operations should eventually happen per-WebView, but they are global now as rendering
-                // is still global to all WebViews.
-                if !throttled && animation_state == AnimationState::AnimationsPresent {
-                    self.set_needs_repaint(RepaintReason::ChangedAnimationState);
-                }
-
-                if !throttled && animation_state == AnimationState::AnimationCallbacksPresent {
-                    // We need to fetch the WebView again in order to avoid a double borrow.
-                    if let Some(webview_renderer) = self.webview_renderers.get(webview_id) {
-                        webview_renderer.tick_animations_for_pipeline(pipeline_id, self);
+                    if webview_renderer
+                        .change_pipeline_running_animations_state(pipeline_id, animation_state) &&
+                        webview_renderer.animating()
+                    {
+                        // These operations should eventually happen per-WebView, but they are
+                        // global now as rendering is still global to all WebViews.
+                        self.process_animations(true);
                     }
                 }
             },
@@ -605,8 +574,13 @@ impl IOCompositor {
 
             CompositorMsg::SetThrottled(webview_id, pipeline_id, throttled) => {
                 if let Some(webview_renderer) = self.webview_renderers.get_mut(webview_id) {
-                    webview_renderer.set_throttled(pipeline_id, throttled);
-                    self.process_animations(true);
+                    if webview_renderer.set_throttled(pipeline_id, throttled) &&
+                        webview_renderer.animating()
+                    {
+                        // These operations should eventually happen per-WebView, but they are
+                        // global now as rendering is still global to all WebViews.
+                        self.process_animations(true);
+                    }
                 }
             },
 
@@ -1283,8 +1257,23 @@ impl IOCompositor {
         }
         self.last_animation_tick = Instant::now();
 
-        for webview_renderer in self.webview_renderers.iter() {
-            webview_renderer.tick_all_animations(self);
+        let animating_webviews: Vec<_> = self
+            .webview_renderers
+            .iter()
+            .filter_map(|webview_renderer| {
+                if webview_renderer.animating() {
+                    Some(webview_renderer.id)
+                } else {
+                    None
+                }
+            })
+            .collect();
+        if !animating_webviews.is_empty() {
+            if let Err(error) = self.global.borrow().constellation_sender.send(
+                EmbedderToConstellationMessage::TickAnimation(animating_webviews),
+            ) {
+                warn!("Sending tick to constellation failed ({error:?}).");
+            }
         }
     }
 
diff --git a/components/compositing/webview_renderer.rs b/components/compositing/webview_renderer.rs
index 6ad77d46043..614ef0ff4c3 100644
--- a/components/compositing/webview_renderer.rs
+++ b/components/compositing/webview_renderer.rs
@@ -86,6 +86,9 @@ pub(crate) struct WebViewRenderer {
     /// The HiDPI scale factor for the `WebView` associated with this renderer. This is controlled
     /// by the embedding layer.
     hidpi_scale_factor: Scale<f32, DeviceIndependentPixel, DevicePixel>,
+    /// Whether or not this [`WebViewRenderer`] isn't throttled and has a pipeline with
+    /// active animations or animation frame callbacks.
+    animating: bool,
 }
 
 impl Drop for WebViewRenderer {
@@ -119,6 +122,7 @@ impl WebViewRenderer {
             min_viewport_zoom: Some(PinchZoomFactor::new(1.0)),
             max_viewport_zoom: None,
             hidpi_scale_factor: Scale::new(hidpi_scale_factor.0),
+            animating: false,
         }
     }
 
@@ -138,6 +142,10 @@ impl WebViewRenderer {
         self.pipelines.keys()
     }
 
+    pub(crate) fn animating(&self) -> bool {
+        self.animating
+    }
+
     /// Returns the [`PipelineDetails`] for the given [`PipelineId`], creating it if needed.
     pub(crate) fn ensure_pipeline_details(
         &mut self,
@@ -148,14 +156,10 @@ impl WebViewRenderer {
                 .borrow_mut()
                 .pipeline_to_webview_map
                 .insert(pipeline_id, self.id);
-            PipelineDetails::new(pipeline_id)
+            PipelineDetails::new()
         })
     }
 
-    pub(crate) fn set_throttled(&mut self, pipeline_id: PipelineId, throttled: bool) {
-        self.ensure_pipeline_details(pipeline_id).throttled = throttled;
-    }
-
     pub(crate) fn remove_pipeline(&mut self, pipeline_id: PipelineId) {
         self.global
             .borrow_mut()
@@ -245,51 +249,45 @@ impl WebViewRenderer {
             })
     }
 
-    /// Sets or unsets the animations-running flag for the given pipeline. Returns true if
-    /// the pipeline is throttled.
-    pub(crate) fn change_running_animations_state(
+    /// Sets or unsets the animations-running flag for the given pipeline. Returns
+    /// true if the [`WebViewRenderer`]'s overall animating state changed.
+    pub(crate) fn change_pipeline_running_animations_state(
         &mut self,
         pipeline_id: PipelineId,
         animation_state: AnimationState,
     ) -> bool {
-        let throttled = {
-            let pipeline_details = self.ensure_pipeline_details(pipeline_id);
-            match animation_state {
-                AnimationState::AnimationsPresent => {
-                    pipeline_details.animations_running = true;
-                },
-                AnimationState::AnimationCallbacksPresent => {
-                    pipeline_details.animation_callbacks_running = true;
-                },
-                AnimationState::NoAnimationsPresent => {
-                    pipeline_details.animations_running = false;
-                },
-                AnimationState::NoAnimationCallbacksPresent => {
-                    pipeline_details.animation_callbacks_running = false;
-                },
-            }
-            pipeline_details.throttled
-        };
-
-        let animating = self.pipelines.values().any(PipelineDetails::animating);
-        self.webview.set_animating(animating);
-        throttled
+        let pipeline_details = self.ensure_pipeline_details(pipeline_id);
+        match animation_state {
+            AnimationState::AnimationsPresent => {
+                pipeline_details.animations_running = true;
+            },
+            AnimationState::AnimationCallbacksPresent => {
+                pipeline_details.animation_callbacks_running = true;
+            },
+            AnimationState::NoAnimationsPresent => {
+                pipeline_details.animations_running = false;
+            },
+            AnimationState::NoAnimationCallbacksPresent => {
+                pipeline_details.animation_callbacks_running = false;
+            },
+        }
+        self.update_animation_state()
     }
 
-    pub(crate) fn tick_all_animations(&self, compositor: &IOCompositor) {
-        for pipeline_details in self.pipelines.values() {
-            pipeline_details.tick_animations(compositor)
-        }
+    /// Sets or unsets the throttled flag for the given pipeline. Returns
+    /// true if the [`WebViewRenderer`]'s overall animating state changed.
+    pub(crate) fn set_throttled(&mut self, pipeline_id: PipelineId, throttled: bool) -> bool {
+        self.ensure_pipeline_details(pipeline_id).throttled = throttled;
+
+        // Throttling a pipeline can cause it to be taken into the "not-animating" state.
+        self.update_animation_state()
     }
 
-    pub(crate) fn tick_animations_for_pipeline(
-        &self,
-        pipeline_id: PipelineId,
-        compositor: &IOCompositor,
-    ) {
-        if let Some(pipeline_details) = self.pipelines.get(&pipeline_id) {
-            pipeline_details.tick_animations(compositor);
-        }
+    pub(crate) fn update_animation_state(&mut self) -> bool {
+        let animating = self.pipelines.values().any(PipelineDetails::animating);
+        let old_animating = std::mem::replace(&mut self.animating, animating);
+        self.webview.set_animating(self.animating);
+        old_animating != self.animating
     }
 
     /// On a Window refresh tick (e.g. vsync)