layout: Send back layout results directly and remove channels (#34609)

This eliminates the way that crossbeam channels are used to send layout
results back to script, which should increase the efficiency of layout.
If asynchronous layout is re-established it can be written as a layer on
top of the layout interface, that way layout doesn't have to know so
many details of how the asynchronocity works.

Renames:
 - `ScriptReflow` to `ReflowRequest`: Script is the only thing that
   requests reflow.
 - `ReflowComplete` to `ReflowResult`

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

components/layout_thread/lib.rs

@@ -10,7 +10,6 @@
 
 use std::borrow::ToOwned;
 use std::cell::{Cell, RefCell};
-use std::ops::{Deref, DerefMut};
 use std::process;
 use std::sync::{Arc, LazyLock, Mutex};
 
@@ -63,7 +62,7 @@ use script::layout_dom::{ServoLayoutElement, ServoLayoutNode};
 use script_layout_interface::wrapper_traits::LayoutNode;
 use script_layout_interface::{
     Layout, LayoutConfig, LayoutFactory, NodesFromPointQueryType, OffsetParentResponse, Reflow,
-    ReflowComplete, ReflowGoal, ScriptReflow, TrustedNodeAddress,
+    ReflowGoal, ReflowRequest, ReflowResult, TrustedNodeAddress,
 };
 use script_traits::{
     ConstellationControlMsg, DrawAPaintImageResult, IFrameSizeMsg, LayoutMsg as ConstellationMsg,
@@ -73,7 +72,7 @@ use servo_arc::Arc as ServoArc;
 use servo_atoms::Atom;
 use servo_config::opts::{self, DebugOptions};
 use servo_config::pref;
-use servo_url::{ImmutableOrigin, ServoUrl};
+use servo_url::ServoUrl;
 use style::animation::{AnimationSetKey, DocumentAnimationSet, ElementAnimationSet};
 use style::context::{
     QuirksMode, RegisteredSpeculativePainter, RegisteredSpeculativePainters, SharedStyleContext,
@@ -218,42 +217,6 @@ impl LayoutFactory for LayoutFactoryImpl {
     }
 }
 
-struct ScriptReflowResult {
-    script_reflow: ScriptReflow,
-    result: RefCell<Option<ReflowComplete>>,
-}
-
-impl Deref for ScriptReflowResult {
-    type Target = ScriptReflow;
-    fn deref(&self) -> &ScriptReflow {
-        &self.script_reflow
-    }
-}
-
-impl DerefMut for ScriptReflowResult {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.script_reflow
-    }
-}
-
-impl ScriptReflowResult {
-    fn new(script_reflow: ScriptReflow) -> ScriptReflowResult {
-        ScriptReflowResult {
-            script_reflow,
-            result: RefCell::new(Some(Default::default())),
-        }
-    }
-}
-
-impl Drop for ScriptReflowResult {
-    fn drop(&mut self) {
-        self.script_reflow
-            .script_join_chan
-            .send(self.result.borrow_mut().take().unwrap())
-            .unwrap();
-    }
-}
-
 impl Drop for LayoutThread {
     fn drop(&mut self) {
         let (keys, instance_keys) = self
@@ -536,14 +499,13 @@ impl Layout for LayoutThread {
         });
     }
 
-    fn reflow(&mut self, script_reflow: script_layout_interface::ScriptReflow) {
+    fn reflow(&mut self, reflow_request: ReflowRequest) -> Option<ReflowResult> {
-        let mut result = ScriptReflowResult::new(script_reflow);
         time_profile!(
             profile_time::ProfilerCategory::LayoutPerform,
             self.profiler_metadata(),
             self.time_profiler_chan.clone(),
-            || self.handle_reflow(&mut result),
+            || self.handle_reflow(reflow_request),
-        );
+        )
     }
 
     fn set_scroll_states(&mut self, scroll_states: &[ScrollState]) {
@@ -663,24 +625,21 @@ impl LayoutThread {
         &'a self,
         guards: StylesheetGuards<'a>,
         snapshot_map: &'a SnapshotMap,
-        origin: ImmutableOrigin,
+        reflow_request: &ReflowRequest,
-        animation_timeline_value: f64,
-        animations: &DocumentAnimationSet,
-        stylesheets_changed: bool,
     ) -> LayoutContext<'a> {
-        let traversal_flags = match stylesheets_changed {
+        let traversal_flags = match reflow_request.stylesheets_changed {
             true => TraversalFlags::ForCSSRuleChanges,
             false => TraversalFlags::empty(),
         };
 
         LayoutContext {
             id: self.id,
-            origin,
+            origin: reflow_request.origin.clone(),
             style_context: self.build_shared_style_context(
                 guards,
                 snapshot_map,
-                animation_timeline_value,
+                reflow_request.animation_timeline_value,
-                animations,
+                &reflow_request.animations,
                 traversal_flags,
             ),
             image_cache: self.image_cache.clone(),
@@ -951,25 +910,25 @@ impl LayoutThread {
     }
 
     /// The high-level routine that performs layout.
-    fn handle_reflow(&mut self, data: &mut ScriptReflowResult) {
+    fn handle_reflow(&mut self, mut reflow_request: ReflowRequest) -> Option<ReflowResult> {
-        let document = unsafe { ServoLayoutNode::new(&data.document) };
+        let document = unsafe { ServoLayoutNode::new(&reflow_request.document) };
         let document = document.as_document().unwrap();
 
         // Parallelize if there's more than 750 objects based on rzambre's suggestion
         // https://github.com/servo/servo/issues/10110
-        self.parallel_flag = data.dom_count > 750;
+        self.parallel_flag = reflow_request.dom_count > 750;
         debug!("layout: received layout request for: {}", self.url);
-        debug!("Number of objects in DOM: {}", data.dom_count);
+        debug!("Number of objects in DOM: {}", reflow_request.dom_count);
         debug!("layout: parallel? {}", self.parallel_flag);
 
         let Some(root_element) = document.root_element() else {
             debug!("layout: No root node: bailing");
-            return;
+            return None;
         };
 
         debug!(
             "layout: processing reflow request for: {:?} ({}) (query={:?})",
-            root_element, self.url, data.reflow_goal
+            root_element, self.url, reflow_request.reflow_goal
         );
         trace!("{:?}", ShowSubtree(root_element.as_node()));
 
@@ -986,7 +945,8 @@ impl LayoutThread {
         };
 
         let had_used_viewport_units = self.stylist.device().used_viewport_units();
-        let viewport_size_changed = self.handle_viewport_change(data.window_size, &guards);
+        let viewport_size_changed =
+            self.handle_viewport_change(reflow_request.window_size, &guards);
         if viewport_size_changed && had_used_viewport_units {
             if let Some(mut data) = root_element.mutate_data() {
                 data.hint.insert(RestyleHint::recascade_subtree());
@@ -1013,7 +973,7 @@ impl LayoutThread {
             }
         }
 
-        if data.stylesheets_changed {
+        if reflow_request.stylesheets_changed {
             debug!("Doc sheets changed, flushing author sheets too");
             self.stylist
                 .force_stylesheet_origins_dirty(Origin::Author.into());
@@ -1033,7 +993,7 @@ impl LayoutThread {
         // Flush shadow roots stylesheets if dirty.
         document.flush_shadow_roots_stylesheets(&mut self.stylist, guards.author);
 
-        let restyles = std::mem::take(&mut data.pending_restyles);
+        let restyles = std::mem::take(&mut reflow_request.pending_restyles);
         debug!("Draining restyles: {}", restyles.len());
 
         let mut map = SnapshotMap::new();
@@ -1070,14 +1030,7 @@ impl LayoutThread {
         self.stylist.flush(&guards, Some(root_element), Some(&map));
 
         // Create a layout context for use throughout the following passes.
-        let mut layout_context = self.build_layout_context(
+        let mut layout_context = self.build_layout_context(guards.clone(), &map, &reflow_request);
-            guards.clone(),
-            &map,
-            data.origin.clone(),
-            data.animation_timeline_value,
-            &data.animations,
-            data.stylesheets_changed,
-        );
 
         let pool = STYLE_THREAD_POOL.lock().unwrap();
         let thread_pool = pool.pool();
@@ -1088,7 +1041,7 @@ impl LayoutThread {
         };
 
         let dirty_root = unsafe {
-            ServoLayoutNode::new(&data.dirty_root.unwrap())
+            ServoLayoutNode::new(&reflow_request.dirty_root.unwrap())
                 .as_element()
                 .unwrap()
         };
@@ -1163,8 +1116,8 @@ impl LayoutThread {
         if let Some(mut root_flow) = self.root_flow.borrow().clone() {
             self.perform_post_style_recalc_layout_passes(
                 &mut root_flow,
-                &data.reflow_info,
+                &reflow_request.reflow_info,
-                &data.reflow_goal,
+                &reflow_request.reflow_goal,
                 &mut layout_context,
                 thread_pool,
             );
@@ -1172,12 +1125,12 @@ impl LayoutThread {
 
         self.first_reflow.set(false);
 
-        data.result.borrow_mut().as_mut().unwrap().pending_images =
+        if let ReflowGoal::UpdateScrollNode(scroll_state) = reflow_request.reflow_goal {
-            std::mem::take(&mut *layout_context.pending_images.lock().unwrap());
-
-        if let ReflowGoal::UpdateScrollNode(scroll_state) = data.reflow_goal {
             self.update_scroll_node_state(&scroll_state);
         }
+
+        let pending_images = std::mem::take(&mut *layout_context.pending_images.lock().unwrap());
+        Some(ReflowResult { pending_images })
     }
 
     fn update_scroll_node_state(&self, state: &ScrollState) {

components/layout_thread_2020/lib.rs

@@ -11,7 +11,6 @@
 use std::cell::{Cell, RefCell};
 use std::collections::HashMap;
 use std::fmt::Debug;
-use std::ops::{Deref, DerefMut};
 use std::process;
 use std::sync::{Arc, LazyLock};
 
@@ -49,8 +48,8 @@ use profile_traits::time::{
 use profile_traits::{path, time_profile};
 use script::layout_dom::{ServoLayoutElement, ServoLayoutNode};
 use script_layout_interface::{
-    Layout, LayoutConfig, LayoutFactory, NodesFromPointQueryType, OffsetParentResponse,
+    Layout, LayoutConfig, LayoutFactory, NodesFromPointQueryType, OffsetParentResponse, ReflowGoal,
-    ReflowComplete, ReflowGoal, ScriptReflow, TrustedNodeAddress,
+    ReflowRequest, ReflowResult, TrustedNodeAddress,
 };
 use script_traits::{
     ConstellationControlMsg, DrawAPaintImageResult, IFrameSizeMsg, LayoutMsg as ConstellationMsg,
@@ -60,7 +59,7 @@ use servo_arc::Arc as ServoArc;
 use servo_atoms::Atom;
 use servo_config::opts::{self, DebugOptions};
 use servo_config::pref;
-use servo_url::{ImmutableOrigin, ServoUrl};
+use servo_url::ServoUrl;
 use style::animation::DocumentAnimationSet;
 use style::context::{
     QuirksMode, RegisteredSpeculativePainter, RegisteredSpeculativePainters, SharedStyleContext,
@@ -194,43 +193,6 @@ impl LayoutFactory for LayoutFactoryImpl {
     }
 }
 
-#[derive(Debug)]
-struct ScriptReflowResult {
-    script_reflow: ScriptReflow,
-    result: RefCell<Option<ReflowComplete>>,
-}
-
-impl Deref for ScriptReflowResult {
-    type Target = ScriptReflow;
-    fn deref(&self) -> &ScriptReflow {
-        &self.script_reflow
-    }
-}
-
-impl DerefMut for ScriptReflowResult {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.script_reflow
-    }
-}
-
-impl ScriptReflowResult {
-    fn new(script_reflow: ScriptReflow) -> ScriptReflowResult {
-        ScriptReflowResult {
-            script_reflow,
-            result: RefCell::new(Some(Default::default())),
-        }
-    }
-}
-
-impl Drop for ScriptReflowResult {
-    fn drop(&mut self) {
-        self.script_reflow
-            .script_join_chan
-            .send(self.result.borrow_mut().take().unwrap())
-            .unwrap();
-    }
-}
-
 impl Drop for LayoutThread {
     fn drop(&mut self) {
         let (keys, instance_keys) = self
@@ -507,14 +469,13 @@ impl Layout for LayoutThread {
         self.stylist.set_quirks_mode(quirks_mode);
     }
 
-    fn reflow(&mut self, script_reflow: ScriptReflow) {
+    fn reflow(&mut self, reflow_request: ReflowRequest) -> Option<ReflowResult> {
-        let mut result = ScriptReflowResult::new(script_reflow);
         time_profile!(
             profile_time::ProfilerCategory::LayoutPerform,
             self.profiler_metadata(),
             self.time_profiler_chan.clone(),
-            || self.handle_reflow(&mut result),
+            || self.handle_reflow(reflow_request),
-        );
+        )
     }
 
     fn register_paint_worklet_modules(
@@ -641,25 +602,22 @@ impl LayoutThread {
         &'a self,
         guards: StylesheetGuards<'a>,
         snapshot_map: &'a SnapshotMap,
-        origin: ImmutableOrigin,
+        reflow_request: &ReflowRequest,
-        animation_timeline_value: f64,
-        animations: &DocumentAnimationSet,
-        stylesheets_changed: bool,
         use_rayon: bool,
     ) -> LayoutContext<'a> {
-        let traversal_flags = match stylesheets_changed {
+        let traversal_flags = match reflow_request.stylesheets_changed {
             true => TraversalFlags::ForCSSRuleChanges,
             false => TraversalFlags::empty(),
         };
 
         LayoutContext {
             id: self.id,
-            origin,
+            origin: reflow_request.origin.clone(),
             style_context: self.build_shared_style_context(
                 guards,
                 snapshot_map,
-                animation_timeline_value,
+                reflow_request.animation_timeline_value,
-                animations,
+                &reflow_request.animations,
                 traversal_flags,
             ),
             image_cache: self.image_cache.clone(),
@@ -713,12 +671,12 @@ impl LayoutThread {
         feature = "tracing",
         tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace")
     )]
-    fn handle_reflow(&mut self, data: &mut ScriptReflowResult) {
+    fn handle_reflow(&mut self, mut reflow_request: ReflowRequest) -> Option<ReflowResult> {
-        let document = unsafe { ServoLayoutNode::new(&data.document) };
+        let document = unsafe { ServoLayoutNode::new(&reflow_request.document) };
         let document = document.as_document().unwrap();
         let Some(root_element) = document.root_element() else {
             debug!("layout: No root node: bailing");
-            return;
+            return None;
         };
 
         // Calculate the actual viewport as per DEVICE-ADAPT § 6
@@ -734,11 +692,11 @@ impl LayoutThread {
         };
 
         let had_used_viewport_units = self.stylist.device().used_viewport_units();
-        let viewport_size_changed = self.viewport_did_change(data.window_size);
+        let viewport_size_changed = self.viewport_did_change(reflow_request.window_size);
-        let theme_changed = self.theme_did_change(data.theme);
+        let theme_changed = self.theme_did_change(reflow_request.theme);
 
         if viewport_size_changed || theme_changed {
-            self.update_device(data.window_size, data.theme, &guards);
+            self.update_device(reflow_request.window_size, reflow_request.theme, &guards);
         }
 
         if viewport_size_changed && had_used_viewport_units {
@@ -766,7 +724,7 @@ impl LayoutThread {
             }
         }
 
-        if data.stylesheets_changed {
+        if reflow_request.stylesheets_changed {
             self.stylist
                 .force_stylesheet_origins_dirty(Origin::Author.into());
         }
@@ -774,7 +732,7 @@ impl LayoutThread {
         // Flush shadow roots stylesheets if dirty.
         document.flush_shadow_roots_stylesheets(&mut self.stylist, guards.author);
 
-        let restyles = std::mem::take(&mut data.pending_restyles);
+        let restyles = std::mem::take(&mut reflow_request.pending_restyles);
         debug!("Draining restyles: {}", restyles.len());
 
         let mut map = SnapshotMap::new();
@@ -815,18 +773,11 @@ impl LayoutThread {
         let rayon_pool = rayon_pool.as_ref();
 
         // Create a layout context for use throughout the following passes.
-        let mut layout_context = self.build_layout_context(
+        let mut layout_context =
-            guards.clone(),
+            self.build_layout_context(guards.clone(), &map, &reflow_request, rayon_pool.is_some());
-            &map,
-            data.origin.clone(),
-            data.animation_timeline_value,
-            &data.animations,
-            data.stylesheets_changed,
-            rayon_pool.is_some(),
-        );
 
         let dirty_root = unsafe {
-            ServoLayoutNode::new(&data.dirty_root.unwrap())
+            ServoLayoutNode::new(&reflow_request.dirty_root.unwrap())
                 .as_element()
                 .unwrap()
         };
@@ -904,18 +855,19 @@ impl LayoutThread {
         if let Some(root) = &*self.fragment_tree.borrow() {
             self.perform_post_style_recalc_layout_passes(
                 root.clone(),
-                &data.reflow_goal,
+                &reflow_request.reflow_goal,
                 &mut layout_context,
             );
         }
 
         self.first_reflow.set(false);
 
-        data.result.borrow_mut().as_mut().unwrap().pending_images =
+        if let ReflowGoal::UpdateScrollNode(scroll_state) = reflow_request.reflow_goal {
-            std::mem::take(&mut *layout_context.pending_images.lock());
-        if let ReflowGoal::UpdateScrollNode(scroll_state) = data.reflow_goal {
             self.update_scroll_node_state(&scroll_state);
         }
+
+        let pending_images = std::mem::take(&mut *layout_context.pending_images.lock());
+        Some(ReflowResult { pending_images })
     }
 
     fn update_scroll_node_state(&self, state: &ScrollState) {

components/script/dom/window.rs

@@ -22,7 +22,7 @@ use base::id::{BrowsingContextId, PipelineId};
 use base64::Engine;
 use bluetooth_traits::BluetoothRequest;
 use canvas_traits::webgl::WebGLChan;
-use crossbeam_channel::{unbounded, Sender, TryRecvError};
+use crossbeam_channel::{unbounded, Sender};
 use cssparser::{Parser, ParserInput, SourceLocation};
 use devtools_traits::{ScriptToDevtoolsControlMsg, TimelineMarker, TimelineMarkerType};
 use dom_struct::dom_struct;
@@ -52,7 +52,7 @@ use profile_traits::mem::ProfilerChan as MemProfilerChan;
 use profile_traits::time::ProfilerChan as TimeProfilerChan;
 use script_layout_interface::{
     combine_id_with_fragment_type, FragmentType, Layout, PendingImageState, QueryMsg, Reflow,
-    ReflowGoal, ScriptReflow, TrustedNodeAddress,
+    ReflowGoal, ReflowRequest, TrustedNodeAddress,
 };
 use script_traits::webdriver_msg::{WebDriverJSError, WebDriverJSResult};
 use script_traits::{
@@ -1873,9 +1873,6 @@ impl Window {
             None
         };
 
-        // Layout will let us know when it's done.
-        let (join_chan, join_port) = unbounded();
-
         // On debug mode, print the reflow event information.
         if self.relayout_event {
             debug_reflow_events(pipeline_id, &reflow_goal);
@@ -1903,7 +1900,7 @@ impl Window {
             .map(|root| root.upcast::<Node>().to_trusted_node_address());
 
         // Send new document and relevant styles to layout.
-        let reflow = ScriptReflow {
+        let reflow = ReflowRequest {
             reflow_info: Reflow {
                 page_clip_rect: self.page_clip_rect.get(),
             },
@@ -1913,7 +1910,6 @@ impl Window {
             window_size: self.window_size.get(),
             origin: self.origin().immutable().clone(),
             reflow_goal,
-            script_join_chan: join_chan,
             dom_count: document.dom_count(),
             pending_restyles,
             animation_timeline_value: document.current_animation_timeline_value(),
@@ -1921,21 +1917,11 @@ impl Window {
             theme: self.theme.get(),
         };
 
-        self.layout.borrow_mut().reflow(reflow);
+        let Some(results) = self.layout.borrow_mut().reflow(reflow) else {
-
+            return false;
-        let complete = match join_port.try_recv() {
-            Err(TryRecvError::Empty) => {
-                debug!("script: waiting on layout");
-                join_port.recv().unwrap()
-            },
-            Ok(reflow_complete) => reflow_complete,
-            Err(TryRecvError::Disconnected) => {
-                panic!("Layout failed while script was waiting for a result.");
-            },
         };
 
         debug!("script: layout complete");
-
         if let Some(marker) = marker {
             self.emit_timeline_marker(marker.end());
         }
@@ -1946,7 +1932,7 @@ impl Window {
         // is when reflowing just for the purpose of doing a layout query.
         document.set_needs_paint(!for_display);
 
-        for image in complete.pending_images {
+        for image in results.pending_images {
             let id = image.id;
             let node = unsafe { from_untrusted_node_address(image.node) };
 

components/shared/script_layout/lib.rs

@@ -21,7 +21,6 @@ use base::cross_process_instant::CrossProcessInstant;
 use base::id::{BrowsingContextId, PipelineId};
 use base::Epoch;
 use canvas_traits::canvas::{CanvasId, CanvasMsg};
-use crossbeam_channel::Sender;
 use euclid::default::{Point2D, Rect};
 use euclid::Size2D;
 use fonts::SystemFontServiceProxy;
@@ -238,7 +237,7 @@ pub trait Layout {
     fn remove_stylesheet(&mut self, stylesheet: ServoArc<Stylesheet>);
 
     /// Requests a reflow.
-    fn reflow(&mut self, script_reflow: ScriptReflow);
+    fn reflow(&mut self, reflow_request: ReflowRequest) -> Option<ReflowResult>;
 
     /// Tells layout that script has added some paint worklet modules.
     fn register_paint_worklet_modules(
@@ -403,14 +402,14 @@ pub struct Reflow {
 
 /// Information derived from a layout pass that needs to be returned to the script thread.
 #[derive(Debug, Default)]
-pub struct ReflowComplete {
+pub struct ReflowResult {
     /// The list of images that were encountered that are in progress.
     pub pending_images: Vec<PendingImage>,
 }
 
 /// Information needed for a script-initiated reflow.
 #[derive(Debug)]
-pub struct ScriptReflow {
+pub struct ReflowRequest {
     /// General reflow data.
     pub reflow_info: Reflow,
     /// The document node.
@@ -421,8 +420,6 @@ pub struct ScriptReflow {
     pub stylesheets_changed: bool,
     /// The current window size.
     pub window_size: WindowSizeData,
-    /// The channel that we send a notification to.
-    pub script_join_chan: Sender<ReflowComplete>,
     /// The goal of this reflow.
     pub reflow_goal: ReflowGoal,
     /// The number of objects in the dom #10110