layout: Introduce ReflowPhasesRun (servo#38467)

There were various booleans on `ReflowResults` that represented various
actions that might have been taken during a reflow request. Replace
those with a bitflags that better represents what reflow phases have
actually been run. Update variable names to reflect what they mean.

In addition, run some post-layout tasks unconditionally. They are
already contingent on the results returned from layout.

This simplifies and clarifies the code a good deal.

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

Signed-off-by: Martin Robinson <[email protected]>

Author: mrobinson

components/layout/layout_impl.rs

@@ -27,8 +27,8 @@ use fxhash::FxHashMap;
 use ipc_channel::ipc::IpcSender;
 use layout_api::{
     IFrameSizes, Layout, LayoutConfig, LayoutDamage, LayoutFactory, NodesFromPointQueryType,
-    OffsetParentResponse, QueryMsg, ReflowGoal, ReflowRequest, ReflowRequestRestyle, ReflowResult,
-    TrustedNodeAddress,
+    OffsetParentResponse, QueryMsg, ReflowGoal, ReflowPhasesRun, ReflowRequest,
+    ReflowRequestRestyle, ReflowResult, TrustedNodeAddress,
 };
 use log::{debug, error, warn};
 use malloc_size_of::{MallocConditionalSizeOf, MallocSizeOf, MallocSizeOfOps};
@@ -672,13 +672,13 @@ impl LayoutThread {
         self.maybe_print_reflow_event(&reflow_request);
 
         if self.can_skip_reflow_request_entirely(&reflow_request) {
-            // We could skip the layout, but we might need to update the scroll node.
-            let update_scroll_reflow_target_scrolled =
-                self.handle_update_scroll_node_request(&reflow_request);
-
-            return Some(ReflowResult::new_without_relayout(
-                update_scroll_reflow_target_scrolled,
-            ));
+            // We can skip layout, but we might need to update a scroll node.
+            return self
+                .handle_update_scroll_node_request(&reflow_request)
+                .then(|| ReflowResult {
+                    reflow_phases_run: ReflowPhasesRun::UpdatedScrollNodeOffset,
+                    ..Default::default()
+                });
         }
 
         let document = unsafe { ServoLayoutNode::new(&reflow_request.document) };
@@ -698,30 +698,34 @@ impl LayoutThread {
             animation_timeline_value: reflow_request.animation_timeline_value,
         });
 
-        let (damage, iframe_sizes) = self.restyle_and_build_trees(
+        let (mut reflow_phases_run, damage, iframe_sizes) = self.restyle_and_build_trees(
             &mut reflow_request,
             document,
             root_element,
             &image_resolver,
         );
-        self.calculate_overflow(damage);
-        self.build_stacking_context_tree(&reflow_request, damage);
-        let built_display_list = self.build_display_list(&reflow_request, damage, &image_resolver);
-
-        let update_scroll_reflow_target_scrolled =
-            self.handle_update_scroll_node_request(&reflow_request);
+        if self.calculate_overflow(damage) {
+            reflow_phases_run.insert(ReflowPhasesRun::CalculatedOverflow);
+        }
+        if self.build_stacking_context_tree(&reflow_request, damage) {
+            reflow_phases_run.insert(ReflowPhasesRun::BuiltStackingContextTree);
+        }
+        if self.build_display_list(&reflow_request, damage, &image_resolver) {
+            reflow_phases_run.insert(ReflowPhasesRun::BuiltDisplayList);
+        }
+        if self.handle_update_scroll_node_request(&reflow_request) {
+            reflow_phases_run.insert(ReflowPhasesRun::UpdatedScrollNodeOffset);
+        }
 
         let pending_images = std::mem::take(&mut *image_resolver.pending_images.lock());
         let pending_rasterization_images =
             std::mem::take(&mut *image_resolver.pending_rasterization_images.lock());
 
         Some(ReflowResult {
-            built_display_list,
+            reflow_phases_run,
             pending_images,
             pending_rasterization_images,
             iframe_sizes: Some(iframe_sizes),
-            update_scroll_reflow_target_scrolled,
-            processed_relayout: true,
         })
     }
 
@@ -793,11 +797,11 @@ impl LayoutThread {
         document: ServoLayoutDocument<'_>,
         root_element: ServoLayoutElement<'_>,
         image_resolver: &Arc<ImageResolver>,
-    ) -> (RestyleDamage, IFrameSizes) {
+    ) -> (ReflowPhasesRun, RestyleDamage, IFrameSizes) {
         let mut snapshot_map = SnapshotMap::new();
         let _snapshot_setter = match reflow_request.restyle.as_mut() {
             Some(restyle) => SnapshotSetter::new(restyle, &mut snapshot_map),
-            None => return (RestyleDamage::empty(), IFrameSizes::default()),
+            None => return Default::default(),
         };
 
         let document_shared_lock = document.style_shared_lock();
@@ -877,7 +881,7 @@ impl LayoutThread {
 
             if !token.should_traverse() {
                 layout_context.style_context.stylist.rule_tree().maybe_gc();
-                return (RestyleDamage::empty(), IFrameSizes::default());
+                return Default::default();
             }
 
             dirty_root = driver::traverse_dom(&recalc_style_traversal, token, rayon_pool).as_node();
@@ -895,7 +899,7 @@ impl LayoutThread {
 
         if !damage.contains(RestyleDamage::RELAYOUT) {
             layout_context.style_context.stylist.rule_tree().maybe_gc();
-            return (damage, IFrameSizes::default());
+            return (ReflowPhasesRun::empty(), damage, IFrameSizes::default());
         }
 
         let mut box_tree = self.box_tree.borrow_mut();
@@ -956,13 +960,17 @@ impl LayoutThread {
         layout_context.style_context.stylist.rule_tree().maybe_gc();
 
         let mut iframe_sizes = layout_context.iframe_sizes.lock();
-        (damage, std::mem::take(&mut *iframe_sizes))
+        (
+            ReflowPhasesRun::RanLayout,
+            damage,
+            std::mem::take(&mut *iframe_sizes),
+        )
     }
 
     #[servo_tracing::instrument(name = "Overflow Calculation", skip_all)]
-    fn calculate_overflow(&self, damage: RestyleDamage) {
+    fn calculate_overflow(&self, damage: RestyleDamage) -> bool {
         if !damage.contains(RestyleDamage::RECALCULATE_OVERFLOW) {
-            return;
+            return false;
         }
 
         if let Some(fragment_tree) = &*self.fragment_tree.borrow() {
@@ -976,22 +984,27 @@ impl LayoutThread {
         // display list the next time one is requested.
         self.need_new_display_list.set(true);
         self.need_new_stacking_context_tree.set(true);
+        true
     }
 
     #[servo_tracing::instrument(name = "Stacking Context Tree Construction", skip_all)]
-    fn build_stacking_context_tree(&self, reflow_request: &ReflowRequest, damage: RestyleDamage) {
+    fn build_stacking_context_tree(
+        &self,
+        reflow_request: &ReflowRequest,
+        damage: RestyleDamage,
+    ) -> bool {
         if !ReflowPhases::necessary(&reflow_request.reflow_goal)
             .contains(ReflowPhases::StackingContextTreeConstruction)
         {
-            return;
+            return false;
         }
         let Some(fragment_tree) = &*self.fragment_tree.borrow() else {
-            return;
+            return false;
         };
         if !damage.contains(RestyleDamage::REBUILD_STACKING_CONTEXT) &&
             !self.need_new_stacking_context_tree.get()
         {
-            return;
+            return false;
         }
 
         let mut stacking_context_tree = self.stacking_context_tree.borrow_mut();
@@ -1041,6 +1054,8 @@ impl LayoutThread {
                 );
             }
         }
+
+        true
     }
 
     /// Build the display list for the current layout and send it to the renderer. If no display

components/script/dom/document.rs

@@ -43,7 +43,8 @@ use ipc_channel::ipc;
 use js::rust::{HandleObject, HandleValue, MutableHandleValue};
 use keyboard_types::{Code, Key, KeyState, Modifiers, NamedKey};
 use layout_api::{
-    PendingRestyle, ReflowGoal, RestyleReason, TrustedNodeAddress, node_id_from_scroll_id,
+    PendingRestyle, ReflowGoal, ReflowPhasesRun, RestyleReason, TrustedNodeAddress,
+    node_id_from_scroll_id,
 };
 use metrics::{InteractiveFlag, InteractiveWindow, ProgressiveWebMetrics};
 use net_traits::CookieSource::NonHTTP;
@@ -3694,8 +3695,8 @@ impl Document {
     // > Step 22: For each doc of docs, update the rendering or user interface of
     // > doc and its node navigable to reflect the current state.
     //
-    // Returns true if a reflow occured.
-    pub(crate) fn update_the_rendering(&self) -> bool {
+    // Returns the set of reflow phases run as a [`ReflowPhasesRun`].
+    pub(crate) fn update_the_rendering(&self) -> ReflowPhasesRun {
         self.update_animating_images();
 
         // All dirty canvases are flushed before updating the rendering.
@@ -3730,9 +3731,7 @@ impl Document {
             receiver.recv().unwrap();
         }
 
-        self.window()
-            .reflow(ReflowGoal::UpdateTheRendering)
-            .reflow_issued
+        self.window().reflow(ReflowGoal::UpdateTheRendering)
     }
 
     /// From <https://drafts.csswg.org/css-font-loading/#fontfaceset-pending-on-the-environment>:

components/script/dom/window.rs

@@ -52,8 +52,8 @@ use js::rust::{
 };
 use layout_api::{
     FragmentType, Layout, PendingImage, PendingImageState, PendingRasterizationImage, QueryMsg,
-    ReflowGoal, ReflowRequest, ReflowRequestRestyle, RestyleReason, TrustedNodeAddress,
-    combine_id_with_fragment_type,
+    ReflowGoal, ReflowPhasesRun, ReflowRequest, ReflowRequestRestyle, RestyleReason,
+    TrustedNodeAddress, combine_id_with_fragment_type,
 };
 use malloc_size_of::MallocSizeOf;
 use media::WindowGLContext;
@@ -225,25 +225,6 @@ impl LayoutBlocker {
 
 type PendingImageRasterizationKey = (PendingImageId, DeviceIntSize);
 
-/// Feedbacks of the reflow that is required by the one who is initiating the reflow.
-pub(crate) struct WindowReflowResult {
-    /// Whether the reflow actually happened and it sends a new display list to the embedder.
-    pub reflow_issued: bool,
-    /// Whether the reflow is for [ReflowGoal::UpdateScrollNode] and the target is scrolled.
-    /// Specifically, a node is scrolled whenever the scroll position of it changes. Note
-    /// that reflow that is cancalled would not scroll the target.
-    pub update_scroll_reflow_target_scrolled: bool,
-}
-
-impl WindowReflowResult {
-    fn new_empty() -> Self {
-        WindowReflowResult {
-            reflow_issued: false,
-            update_scroll_reflow_target_scrolled: false,
-        }
-    }
-}
-
 #[dom_struct]
 pub(crate) struct Window {
     globalscope: GlobalScope,
@@ -2165,16 +2146,14 @@ impl Window {
         // TODO Step 1
         // TODO(mrobinson, #18709): Add smooth scrolling support to WebRender so that we can
         // properly process ScrollBehavior here.
-        let WindowReflowResult {
-            update_scroll_reflow_target_scrolled,
-            ..
-        } = self.reflow(ReflowGoal::UpdateScrollNode(scroll_id, Vector2D::new(x, y)));
+        let reflow_phases_run =
+            self.reflow(ReflowGoal::UpdateScrollNode(scroll_id, Vector2D::new(x, y)));
 
         // > If the scroll position did not change as a result of the user interaction or programmatic
         // > invocation, where no translations were applied as a result, then no scrollend event fires
         // > because no scrolling occurred.
         // Even though the note mention the scrollend, it is relevant to the scroll as well.
-        if update_scroll_reflow_target_scrolled {
+        if reflow_phases_run.contains(ReflowPhasesRun::UpdatedScrollNodeOffset) {
             match element {
                 Some(el) => self.Document().handle_element_scroll_event(el),
                 None => self.Document().handle_viewport_scroll_event(),
@@ -2210,25 +2189,25 @@ 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*.
-    pub(crate) fn reflow(&self, reflow_goal: ReflowGoal) -> WindowReflowResult {
+    pub(crate) fn reflow(&self, reflow_goal: ReflowGoal) -> ReflowPhasesRun {
         let document = self.Document();
 
         // Never reflow inactive Documents.
         if !document.is_fully_active() {
-            return WindowReflowResult::new_empty();
+            return ReflowPhasesRun::empty();
         }
 
         self.Document().ensure_safe_to_run_script_or_layout();
 
         // If layouts are blocked, we block all layouts that are for display only. Other
         // layouts (for queries and scrolling) are not blocked, as they do not display
-        // anything and script excpects the layout to be up-to-date after they run.
+        // anything and script expects the layout to be up-to-date after they run.
         let pipeline_id = self.pipeline_id();
         if reflow_goal == ReflowGoal::UpdateTheRendering &&
             self.layout_blocker.get().layout_blocked()
         {
             debug!("Suppressing pre-load-event reflow pipeline {pipeline_id}");
-            return WindowReflowResult::new_empty();
+            return ReflowPhasesRun::empty();
         }
 
         debug!("script: performing reflow for goal {reflow_goal:?}");
@@ -2279,37 +2258,30 @@ impl Window {
             highlighted_dom_node: document.highlighted_dom_node().map(|node| node.to_opaque()),
         };
 
-        let Some(results) = self.layout.borrow_mut().reflow(reflow) else {
-            return WindowReflowResult::new_empty();
+        let Some(reflow_result) = self.layout.borrow_mut().reflow(reflow) else {
+            return ReflowPhasesRun::empty();
         };
 
-        // We are maintaining the previous behavior of layout where we are skipping these behavior if we are not
-        // doing layout calculation.
-        if results.processed_relayout {
-            debug!("script: layout complete");
-            if let Some(marker) = marker {
-                self.emit_timeline_marker(marker.end());
-            }
-
-            self.handle_pending_images_post_reflow(
-                results.pending_images,
-                results.pending_rasterization_images,
-            );
-            if let Some(iframe_sizes) = results.iframe_sizes {
-                document
-                    .iframes_mut()
-                    .handle_new_iframe_sizes_after_layout(self, iframe_sizes);
-            }
-            document.update_animations_post_reflow();
-            self.update_constellation_epoch();
-        } else {
-            debug!("script: layout-side reflow finished without relayout");
+        debug!("script: layout complete");
+        if let Some(marker) = marker {
+            self.emit_timeline_marker(marker.end());
         }
 
-        WindowReflowResult {
-            reflow_issued: results.built_display_list,
-            update_scroll_reflow_target_scrolled: results.update_scroll_reflow_target_scrolled,
+        self.handle_pending_images_post_reflow(
+            reflow_result.pending_images,
+            reflow_result.pending_rasterization_images,
+        );
+
+        if let Some(iframe_sizes) = reflow_result.iframe_sizes {
+            document
+                .iframes_mut()
+                .handle_new_iframe_sizes_after_layout(self, iframe_sizes);
         }
+
+        document.update_animations_post_reflow();
+        self.update_constellation_epoch();
+
+        reflow_result.reflow_phases_run
     }
 
     pub(crate) fn maybe_send_idle_document_state_to_constellation(&self) {

components/script/script_thread.rs

@@ -68,7 +68,9 @@ use js::jsapi::{
 };
 use js::jsval::UndefinedValue;
 use js::rust::ParentRuntime;
-use layout_api::{LayoutConfig, LayoutFactory, RestyleReason, ScriptThreadFactory};
+use layout_api::{
+    LayoutConfig, LayoutFactory, ReflowPhasesRun, RestyleReason, ScriptThreadFactory,
+};
 use media::WindowGLContext;
 use metrics::MAX_TASK_NS;
 use net_traits::image_cache::{ImageCache, ImageCacheResponseMessage};
@@ -1238,6 +1240,8 @@ impl ScriptThread {
     ///
     /// Attempt to update the rendering and then do a microtask checkpoint if rendering was actually
     /// updated.
+    ///
+    /// Returns true if any reflows produced a new display list.
     pub(crate) fn update_the_rendering(&self, can_gc: CanGc) -> bool {
         self.last_render_opportunity_time.set(Some(Instant::now()));
         self.cancel_scheduled_update_the_rendering();
@@ -1275,7 +1279,7 @@ impl ScriptThread {
         // steps per doc in docs. Currently `<iframe>` resizing depends on a parent being able to
         // queue resize events on a child and have those run in the same call to this method, so
         // that needs to be sorted out to fix this.
-        let mut saw_any_reflows = false;
+        let mut built_any_display_lists = false;
         for pipeline_id in documents_in_order.iter() {
             let document = self
                 .documents
@@ -1358,7 +1362,10 @@ impl ScriptThread {
 
             // > Step 22: For each doc of docs, update the rendering or user interface of
             // > doc and its node navigable to reflect the current state.
-            saw_any_reflows = document.update_the_rendering() || saw_any_reflows;
+            built_any_display_lists = document
+                .update_the_rendering()
+                .contains(ReflowPhasesRun::BuiltDisplayList) ||
+                built_any_display_lists;
 
             // TODO: Process top layer removals according to
             // https://drafts.csswg.org/css-position-4/#process-top-layer-removals.
@@ -1367,7 +1374,7 @@ impl ScriptThread {
         // Perform a microtask checkpoint as the specifications says that *update the rendering*
         // should be run in a task and a microtask checkpoint is always done when running tasks.
         self.perform_a_microtask_checkpoint(can_gc);
-        saw_any_reflows
+        built_any_display_lists
     }
 
     /// Schedule a rendering update ("update the rendering"), if necessary. This