script: Clean up Window::force_reflow a little (#37725)

- Move some of the image handling code to a separate function.
 - Move reflow event debugging into layout itself and use the `Debug`
   implementation to print the event.
 - A few other small cleanups

Testing: This should not change behavior and is thus covered by existing
WPT
tests.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Oriol Brufau <obrufau@igalia.com>

components/layout/layout_impl.rs

@@ -617,9 +617,28 @@ impl LayoutThread {
         !self.need_new_display_list.get()
     }
 
+    fn maybe_print_reflow_event(&self, reflow_request: &ReflowRequest) {
+        if !self.debug.relayout_event {
+            return;
+        }
+
+        println!(
+            "**** Reflow({}) => {:?}, {:?}",
+            self.id,
+            reflow_request.reflow_goal,
+            reflow_request
+                .restyle
+                .as_ref()
+                .map(|restyle| restyle.reason)
+                .unwrap_or_default()
+        );
+    }
+
     /// The high-level routine that performs layout.
     #[servo_tracing::instrument(skip_all)]
     fn handle_reflow(&mut self, mut reflow_request: ReflowRequest) -> Option<ReflowResult> {
+        self.maybe_print_reflow_event(&reflow_request);
+
         if self.can_skip_reflow_request_entirely(&reflow_request) {
             if let ReflowGoal::UpdateScrollNode(external_scroll_id, offset) =
                 reflow_request.reflow_goal

components/script/dom/window.rs

@@ -52,8 +52,9 @@ use js::rust::{
     MutableHandleValue,
 };
 use layout_api::{
-    FragmentType, Layout, PendingImageState, QueryMsg, ReflowGoal, ReflowRequest,
-    ReflowRequestRestyle, RestyleReason, TrustedNodeAddress, combine_id_with_fragment_type,
+    FragmentType, Layout, PendingImage, PendingImageState, PendingRasterizationImage, QueryMsg,
+    ReflowGoal, ReflowRequest, ReflowRequestRestyle, RestyleReason, TrustedNodeAddress,
+    combine_id_with_fragment_type,
 };
 use malloc_size_of::MallocSizeOf;
 use media::WindowGLContext;
@@ -375,9 +376,6 @@ pub(crate) struct Window {
     /// It is used to avoid sending idle message more than once, which is unneccessary.
     has_sent_idle_message: Cell<bool>,
 
-    /// Emits notifications when there is a relayout.
-    relayout_event: bool,
-
     /// Unminify Css.
     unminify_css: bool,
 
@@ -2127,7 +2125,6 @@ impl Window {
     ///
     /// NOTE: This method should almost never be called directly! Layout and rendering updates should
     /// happen as part of the HTML event loop via *update the rendering*.
-    #[allow(unsafe_code)]
     fn force_reflow(&self, reflow_goal: ReflowGoal) -> bool {
         let document = self.Document();
         document.ensure_safe_to_run_script_or_layout();
@@ -2150,11 +2147,6 @@ impl Window {
             None
         };
 
-        // On debug mode, print the reflow event information.
-        if self.relayout_event {
-            debug_reflow_events(pipeline_id, &reflow_goal);
-        }
-
         let restyle_reason = document.restyle_reason();
         document.clear_restyle_reasons();
         let restyle = if restyle_reason.needs_restyle() {
@@ -2182,9 +2174,6 @@ impl Window {
             None
         };
 
-        let highlighted_dom_node = document.highlighted_dom_node().map(|node| node.to_opaque());
-
-        // Send new document and relevant styles to layout.
         let reflow = ReflowRequest {
             document: document.upcast::<Node>().to_trusted_node_address(),
             restyle,
@@ -2196,7 +2185,7 @@ impl Window {
             animations: document.animations().sets.clone(),
             node_to_animating_image_map: document.image_animation_manager().node_to_image_map(),
             theme: self.theme.get(),
-            highlighted_dom_node,
+            highlighted_dom_node: document.highlighted_dom_node().map(|node| node.to_opaque()),
         };
 
         let Some(results) = self.layout.borrow_mut().reflow(reflow) else {
@@ -2208,63 +2197,13 @@ impl Window {
             self.emit_timeline_marker(marker.end());
         }
 
-        for image in results.pending_images {
-            let id = image.id;
-            let node = unsafe { from_untrusted_node_address(image.node) };
-
-            if let PendingImageState::Unrequested(ref url) = image.state {
-                fetch_image_for_layout(url.clone(), &node, id, self.image_cache.clone());
-            }
-
-            let mut images = self.pending_layout_images.borrow_mut();
-            if !images.contains_key(&id) {
-                let trusted_node = Trusted::new(&*node);
-                let sender = self.register_image_cache_listener(id, move |response| {
-                    trusted_node
-                        .root()
-                        .owner_window()
-                        .pending_layout_image_notification(response);
-                });
-
-                self.image_cache.add_listener(ImageLoadListener::new(
-                    sender,
-                    self.pipeline_id(),
-                    id,
-                ));
-            }
-
-            let nodes = images.entry(id).or_default();
-            if !nodes.iter().any(|n| std::ptr::eq(&**n, &*node)) {
-                nodes.push(Dom::from_ref(&*node));
-            }
-        }
-
-        for image in results.pending_rasterization_images {
-            let node = unsafe { from_untrusted_node_address(image.node) };
-
-            let mut images = self.pending_images_for_rasterization.borrow_mut();
-            if !images.contains_key(&(image.id, image.size)) {
-                self.image_cache.add_rasterization_complete_listener(
-                    pipeline_id,
-                    image.id,
-                    image.size,
-                    self.image_cache_sender.clone(),
-                );
-            }
-
-            let nodes = images.entry((image.id, image.size)).or_default();
-            if !nodes.iter().any(|n| std::ptr::eq(&**n, &*node)) {
-                nodes.push(Dom::from_ref(&*node));
-            }
-        }
-
-        let size_messages = self
-            .Document()
+        self.handle_pending_images_post_reflow(
+            results.pending_images,
+            results.pending_rasterization_images,
+        );
+        document
             .iframes_mut()
-            .handle_new_iframe_sizes_after_layout(results.iframe_sizes);
-        if !size_messages.is_empty() {
-            self.send_to_constellation(ScriptToConstellationMessage::IFrameSizes(size_messages));
-        }
+            .handle_new_iframe_sizes_after_layout(self, results.iframe_sizes);
         document.update_animations_post_reflow();
         self.update_constellation_epoch();
 
@@ -2991,6 +2930,61 @@ impl Window {
     pub(crate) fn in_immersive_xr_session(&self) -> bool {
         false
     }
+
+    #[allow(unsafe_code)]
+    fn handle_pending_images_post_reflow(
+        &self,
+        pending_images: Vec<PendingImage>,
+        pending_rasterization_images: Vec<PendingRasterizationImage>,
+    ) {
+        let pipeline_id = self.pipeline_id();
+        for image in pending_images {
+            let id = image.id;
+            let node = unsafe { from_untrusted_node_address(image.node) };
+
+            if let PendingImageState::Unrequested(ref url) = image.state {
+                fetch_image_for_layout(url.clone(), &node, id, self.image_cache.clone());
+            }
+
+            let mut images = self.pending_layout_images.borrow_mut();
+            if !images.contains_key(&id) {
+                let trusted_node = Trusted::new(&*node);
+                let sender = self.register_image_cache_listener(id, move |response| {
+                    trusted_node
+                        .root()
+                        .owner_window()
+                        .pending_layout_image_notification(response);
+                });
+
+                self.image_cache
+                    .add_listener(ImageLoadListener::new(sender, pipeline_id, id));
+            }
+
+            let nodes = images.entry(id).or_default();
+            if !nodes.iter().any(|n| std::ptr::eq(&**n, &*node)) {
+                nodes.push(Dom::from_ref(&*node));
+            }
+        }
+
+        for image in pending_rasterization_images {
+            let node = unsafe { from_untrusted_node_address(image.node) };
+
+            let mut images = self.pending_images_for_rasterization.borrow_mut();
+            if !images.contains_key(&(image.id, image.size)) {
+                self.image_cache.add_rasterization_complete_listener(
+                    pipeline_id,
+                    image.id,
+                    image.size,
+                    self.image_cache_sender.clone(),
+                );
+            }
+
+            let nodes = images.entry((image.id, image.size)).or_default();
+            if !nodes.iter().any(|n| std::ptr::eq(&**n, &*node)) {
+                nodes.push(Dom::from_ref(&*node));
+            }
+        }
+    }
 }
 
 impl Window {
@@ -3022,7 +3016,6 @@ impl Window {
         #[cfg(feature = "webxr")] webxr_registry: Option<webxr_api::Registry>,
         microtask_queue: Rc<MicrotaskQueue>,
         compositor_api: CrossProcessCompositorApi,
-        relayout_event: bool,
         unminify_js: bool,
         unminify_css: bool,
         local_script_source: Option<String>,
@@ -3110,7 +3103,6 @@ impl Window {
             exists_mut_observer: Cell::new(false),
             compositor_api,
             has_sent_idle_message: Cell::new(false),
-            relayout_event,
             unminify_css,
             user_content_manager,
             player_context,
@@ -3198,29 +3190,6 @@ fn should_move_clip_rect(clip_rect: UntypedRect<Au>, new_viewport: UntypedRect<f
         (clip_rect.max_y() - new_viewport.max_y()).abs() <= viewport_scroll_margin.height
 }
 
-fn debug_reflow_events(id: PipelineId, reflow_goal: &ReflowGoal) {
-    let goal_string = match *reflow_goal {
-        ReflowGoal::UpdateTheRendering => "\tFull",
-        ReflowGoal::UpdateScrollNode(..) => "\tUpdateScrollNode",
-        ReflowGoal::LayoutQuery(ref query_msg) => match *query_msg {
-            QueryMsg::ContentBox => "\tContentBoxQuery",
-            QueryMsg::ContentBoxes => "\tContentBoxesQuery",
-            QueryMsg::NodesFromPointQuery => "\tNodesFromPointQuery",
-            QueryMsg::ClientRectQuery => "\tClientRectQuery",
-            QueryMsg::ScrollingAreaOrOffsetQuery => "\tNodeScrollGeometryQuery",
-            QueryMsg::ResolvedStyleQuery => "\tResolvedStyleQuery",
-            QueryMsg::ResolvedFontStyleQuery => "\nResolvedFontStyleQuery",
-            QueryMsg::OffsetParentQuery => "\tOffsetParentQuery",
-            QueryMsg::StyleQuery => "\tStyleQuery",
-            QueryMsg::TextIndexQuery => "\tTextIndexQuery",
-            QueryMsg::ElementInnerOuterTextQuery => "\tElementInnerOuterTextQuery",
-            QueryMsg::InnerWindowDimensionsQuery => "\tInnerWindowDimensionsQuery",
-        },
-    };
-
-    println!("**** pipeline={id}\t{goal_string}");
-}
-
 impl Window {
     // https://html.spec.whatwg.org/multipage/#dom-window-postmessage step 7.
     pub(crate) fn post_message(

components/script/iframe_collection.rs

@@ -6,7 +6,7 @@ use std::cell::Cell;
 use std::default::Default;
 
 use base::id::BrowsingContextId;
-use constellation_traits::{IFrameSizeMsg, WindowSizeType};
+use constellation_traits::{IFrameSizeMsg, ScriptToConstellationMessage, WindowSizeType};
 use embedder_traits::ViewportDetails;
 use fnv::FnvHashMap;
 use layout_api::IFrameSizes;
@@ -15,7 +15,7 @@ use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::root::{Dom, DomRoot};
 use crate::dom::htmliframeelement::HTMLIFrameElement;
 use crate::dom::node::{Node, ShadowIncluding};
-use crate::dom::types::Document;
+use crate::dom::types::{Document, Window};
 use crate::script_thread::with_script_thread;
 
 #[derive(JSTraceable, MallocSizeOf)]
@@ -111,13 +111,14 @@ impl IFrameCollection {
     /// message is only sent when the size actually changes.
     pub(crate) fn handle_new_iframe_sizes_after_layout(
         &mut self,
+        window: &Window,
         new_iframe_sizes: IFrameSizes,
-    ) -> Vec<IFrameSizeMsg> {
+    ) {
         if new_iframe_sizes.is_empty() {
-            return vec![];
+            return;
         }
 
-        new_iframe_sizes
+        let size_messages: Vec<_> = new_iframe_sizes
             .into_iter()
             .filter_map(|(browsing_context_id, iframe_size)| {
                 // Batch resize message to any local `Pipeline`s now, rather than waiting for them
@@ -151,7 +152,11 @@ impl IFrameCollection {
                     type_: size_type,
                 })
             })
-            .collect()
+            .collect();
+
+        if !size_messages.is_empty() {
+            window.send_to_constellation(ScriptToConstellationMessage::IFrameSizes(size_messages));
+        }
     }
 
     pub(crate) fn iter(&self) -> impl Iterator<Item = DomRoot<HTMLIFrameElement>> + use<'_> {

components/script/script_thread.rs

@@ -295,9 +295,6 @@ pub struct ScriptThread {
     /// Print Progressive Web Metrics to console.
     print_pwm: bool,
 
-    /// Emits notifications when there is a relayout.
-    relayout_event: bool,
-
     /// Unminify Javascript.
     unminify_js: bool,
 
@@ -964,7 +961,6 @@ impl ScriptThread {
             compositor_api: state.compositor_api,
             profile_script_events: opts.debug.profile_script_events,
             print_pwm: opts.print_pwm,
-            relayout_event: opts.debug.relayout_event,
             unminify_js: opts.unminify_js,
             local_script_source: opts.local_script_source.clone(),
             unminify_css: opts.unminify_css,
@@ -3341,7 +3337,6 @@ impl ScriptThread {
             self.webxr_registry.clone(),
             self.microtask_queue.clone(),
             self.compositor_api.clone(),
-            self.relayout_event,
             self.unminify_js,
             self.unminify_css,
             self.local_script_source.clone(),

components/shared/layout/lib.rs

@@ -360,7 +360,7 @@ pub type IFrameSizes = FnvHashMap<BrowsingContextId, IFrameSize>;
 bitflags! {
     /// Conditions which cause a [`Document`] to need to be restyled during reflow, which
     /// might cause the rest of layout to happen as well.
-    #[derive(Clone, Copy, Debug, Eq, PartialEq)]
+    #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
     pub struct RestyleReason: u16 {
         const StylesheetsChanged = 1 << 0;
         const DOMChanged = 1 << 1;