From a1d3649f7c282aec9220ab67b203297fe33e5a07 Mon Sep 17 00:00:00 2001 From: Taym Haddadi Date: Fri, 9 Aug 2024 09:41:20 +0200 Subject: [PATCH] Fix ordering of documents (#32574) * Fix ordering of documents Signed-off-by: Bentaimia Haddadi Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com> * order pipeline ids, get document later, avoid use of document_from_node on iframe because it returns the owner doc Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com> * Fix build issue Signed-off-by: Bentaimia Haddadi * Use iter::once to avoid allocation Signed-off-by: Bentaimia Haddadi * scope batches of rendering opportunities by pipeline Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com> --------- Signed-off-by: Bentaimia Haddadi Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com> Co-authored-by: gterzian <2792687+gterzian@users.noreply.github.com> --- components/script/script_thread.rs | 33 ++++++++++++++----- .../child-document-raf-order.html.ini | 4 --- 2 files changed, 25 insertions(+), 12 deletions(-) delete mode 100644 tests/wpt/meta/html/webappapis/update-rendering/child-document-raf-order.html.ini diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 0f9e9be6419..568c8ba4bdd 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -517,7 +517,8 @@ pub struct ScriptThread { /// last_render_opportunity_time: DomRefCell>, /// Used to batch rendering opportunities - has_queued_update_the_rendering_task: DomRefCell, + #[no_trace] + update_the_rendering_task_queued_for_pipeline: DomRefCell>, /// The documents for pipelines managed by this thread documents: DomRefCell, /// The window proxies known by this thread @@ -1331,7 +1332,7 @@ impl ScriptThread { ScriptThread { documents: DomRefCell::new(Documents::default()), last_render_opportunity_time: Default::default(), - has_queued_update_the_rendering_task: Default::default(), + update_the_rendering_task_queued_for_pipeline: Default::default(), window_proxies: DomRefCell::new(HashMapTracedValues::new()), incomplete_loads: DomRefCell::new(vec![]), incomplete_parser_contexts: IncompleteParserContexts(RefCell::new(vec![])), @@ -1640,7 +1641,9 @@ impl ScriptThread { /// fn update_the_rendering(&self) { - *self.has_queued_update_the_rendering_task.borrow_mut() = false; + self.update_the_rendering_task_queued_for_pipeline + .borrow_mut() + .clear(); if !self.can_continue_running_inner() { return; @@ -1649,15 +1652,26 @@ impl ScriptThread { // 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. - let pipeline_and_docs: Vec<(PipelineId, DomRoot)> = self + let pipelines_to_update: Vec = self .documents .borrow() .iter() - .map(|(id, document)| (id, DomRoot::from_ref(&*document))) + .filter(|(_, document)| document.window().is_top_level()) + .flat_map(|(id, document)| { + std::iter::once(id.clone()).chain( + document + .iter_iframes() + .filter_map(|iframe| iframe.pipeline_id()), + ) + }) .collect(); // Note: the spec reads: "for doc in docs" at each step // whereas this runs all steps per doc in docs. - for (pipeline_id, document) in pipeline_and_docs { + for pipeline_id in pipelines_to_update { + let Some(document) = self.documents.borrow().find_document(pipeline_id) else { + warn!("Updating the rendering for closed pipeline {pipeline_id}."); + continue; + }; // TODO(#32004): The rendering should be updated according parent and shadow root order // in the specification, but this isn't happening yet. @@ -1744,10 +1758,13 @@ impl ScriptThread { let rendering_task_source = task_manager.rendering_task_source(); let canceller = task_manager.task_canceller(TaskSourceName::Rendering); - if *self.has_queued_update_the_rendering_task.borrow() { + if !self + .update_the_rendering_task_queued_for_pipeline + .borrow_mut() + .insert(pipeline_id) + { return; } - *self.has_queued_update_the_rendering_task.borrow_mut() = true; // Queues a task to update the rendering. // 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 d2269e3918b..00000000000 --- a/tests/wpt/meta/html/webappapis/update-rendering/child-document-raf-order.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[child-document-raf-order.html] - [Ordering of steps in "Update the Rendering" - child document requestAnimationFrame order] - expected: FAIL -