From 23b0dc603c02db990ba793f61dbb6f82066416a8 Mon Sep 17 00:00:00 2001
From: Gregory Terzian <2792687+gterzian@users.noreply.github.com>
Date: Thu, 12 Sep 2024 00:23:20 +0800
Subject: [PATCH] Raf delivery: run rafs for all pipeline if tick received for
 any. (#33395)

* update the rendering: run rafs for all pipeline, if tick received for any

Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com>

* prioritize only one updating of the rendering per event-loop wake-up

Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com>

---------

Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com>
---
 components/script/dom/document.rs             | 11 ++++-
 components/script/script_thread.rs            | 40 +++++++++++++++----
 .../child-document-raf-order.html.ini         |  3 --
 3 files changed, 42 insertions(+), 12 deletions(-)
 delete mode 100644 tests/wpt/meta/html/webappapis/update-rendering/child-document-raf-order.html.ini

diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index d4211868b04..15ade93ec6c 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -3335,10 +3335,17 @@ impl Document {
         self.pending_animation_ticks.borrow_mut().extend(tick_type);
     }
 
+    /// Whether this document has received an animation tick for rafs.
+    pub fn has_received_raf_tick(&self) -> bool {
+        self.pending_animation_ticks
+            .borrow()
+            .contains(AnimationTickType::REQUEST_ANIMATION_FRAME)
+    }
+
     /// As part of a `update_the_rendering` task, tick all pending animations.
-    pub fn tick_all_animations(&self) {
+    pub fn tick_all_animations(&self, should_run_rafs: bool) {
         let tick_type = mem::take(&mut *self.pending_animation_ticks.borrow_mut());
-        if tick_type.contains(AnimationTickType::REQUEST_ANIMATION_FRAME) {
+        if should_run_rafs {
             self.run_the_animation_frame_callbacks();
         }
         if tick_type.contains(AnimationTickType::CSS_ANIMATIONS_AND_TRANSITIONS) {
diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
index 34744d73bb1..f351c153f7f 100644
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@@ -1647,6 +1647,14 @@ impl ScriptThread {
             return;
         }
 
+        // Run rafs for all pipeline, if a raf tick was received for any.
+        // This ensures relative ordering of rafs between parent doc and iframes.
+        let should_run_rafs = self
+            .documents
+            .borrow()
+            .iter()
+            .any(|(_, doc)| doc.has_received_raf_tick());
+
         // TODO: The specification says to filter out non-renderable documents,
         // as well as those for which a rendering update would be unnecessary,
         // but this isn't happening here.
@@ -1708,7 +1716,7 @@ impl ScriptThread {
             // https://html.spec.whatwg.org/multipage/#context-lost-steps.
 
             // Run the animation frame callbacks.
-            document.tick_all_animations();
+            document.tick_all_animations(should_run_rafs);
 
             // Run the resize observer steps.
             let _realm = enter_realm(&*document);
@@ -1813,6 +1821,10 @@ impl ScriptThread {
         };
         debug!("Got event.");
 
+        // Prioritize only a single update of the rendering;
+        // others will run together with the other sequential tasks.
+        let mut rendering_update_already_prioritized = false;
+
         loop {
             let pipeline_id = self.message_to_pipeline(&event);
             let _realm = pipeline_id.map(|id| {
@@ -1886,15 +1898,29 @@ impl ScriptThread {
                     self.handle_event(id, event)
                 },
                 FromScript(MainThreadScriptMsg::Common(CommonScriptMsg::Task(
-                    _,
+                    category,
                     task,
-                    _pipeline_id,
+                    pipeline_id,
                     TaskSourceName::Rendering,
                 ))) => {
-                    // Run the "update the rendering" task.
-                    task.run_box();
-                    // Always perform a microtrask checkpoint after running a task.
-                    self.perform_a_microtask_checkpoint(CanGc::note());
+                    if rendering_update_already_prioritized {
+                        // If we've already updated the rendering,
+                        // run this task along with the other non-prioritized ones.
+                        sequential.push(FromScript(MainThreadScriptMsg::Common(
+                            CommonScriptMsg::Task(
+                                category,
+                                task,
+                                pipeline_id,
+                                TaskSourceName::Rendering,
+                            ),
+                        )));
+                    } else {
+                        // Run the "update the rendering" task.
+                        task.run_box();
+                        // Always perform a microtrask checkpoint after running a task.
+                        self.perform_a_microtask_checkpoint(CanGc::note());
+                        rendering_update_already_prioritized = true;
+                    }
                 },
                 FromScript(MainThreadScriptMsg::Inactive) => {
                     // An event came-in from a document that is not fully-active, it has been stored by the task-queue.
diff --git a/tests/wpt/meta/html/webappapis/update-rendering/child-document-raf-order.html.ini b/tests/wpt/meta/html/webappapis/update-rendering/child-document-raf-order.html.ini
deleted file mode 100644
index 312c6689170..00000000000
--- a/tests/wpt/meta/html/webappapis/update-rendering/child-document-raf-order.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[child-document-raf-order.html]
-  [Ordering of steps in "Update the Rendering" - child document requestAnimationFrame order]
-    expected: FAIL