script|layout: Do not force restyle when doing script queries (#37677)

Instead of doing a restyle whenever layout is requested, only do one if
script believes that the `Document` has changed in a way that needs a
restyle. In addition, track the different reasons this might be the
case. This will be used later to provide better debugging output.

In layout, when a restyle isn't requested, provide:
 - an early return if layout is up-to-date enough for the reflow goal.
 - skipping restyle and reflow if it isn't necessary.

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

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

Cargo.lock

@@ -4722,6 +4722,7 @@ dependencies = [
  "app_units",
  "atomic_refcell",
  "base",
+ "bitflags 2.9.1",
  "compositing_traits",
  "constellation_traits",
  "embedder_traits",

components/layout/layout_impl.rs

@@ -580,9 +580,44 @@ impl LayoutThread {
         );
     }
 
+    /// In some cases, if a restyle isn't necessary we can skip doing any work for layout
+    /// entirely. This check allows us to return early from layout without doing any work
+    /// at all.
+    fn can_skip_reflow_request_entirely(&self, reflow_request: &ReflowRequest) -> bool {
+        // If a restyle is necessary, restyle and reflow is a necessity.
+        if reflow_request.restyle_reason.needs_restyle() {
+            return false;
+        }
+
+        // If only the fragment tree is required, and it's up-to-date, layout is unnecessary.
+        if !reflow_request.reflow_goal.needs_display() && self.fragment_tree.borrow().is_some() {
+            return true;
+        }
+
+        // If only the stacking context tree is required, and it's up-to-date, layout is unnecessary.
+        if !reflow_request.reflow_goal.needs_display_list() &&
+            self.stacking_context_tree.borrow().is_some() &&
+            !self.need_new_stacking_context_tree.get()
+        {
+            return true;
+        }
+
+        // Otherwise, the only interesting thing is whether the current display list is up-to-date.
+        !self.need_new_display_list.get()
+    }
+
     /// The high-level routine that performs layout.
     #[servo_tracing::instrument(skip_all)]
     fn handle_reflow(&mut self, mut reflow_request: ReflowRequest) -> Option<ReflowResult> {
+        if self.can_skip_reflow_request_entirely(&reflow_request) {
+            if let ReflowGoal::UpdateScrollNode(external_scroll_id, offset) =
+                reflow_request.reflow_goal
+            {
+                self.set_scroll_offset_from_script(external_scroll_id, offset);
+            }
+            return None;
+        }
+
         let document = unsafe { ServoLayoutNode::new(&reflow_request.document) };
         let document = document.as_document().unwrap();
         let Some(root_element) = document.root_element() else {
@@ -602,15 +637,17 @@ impl LayoutThread {
             ua_or_user: &ua_or_user_guard,
         };
 
-        let viewport_changed = self.viewport_did_change(reflow_request.viewport_details);
+        let mut snapshot_map = SnapshotMap::new();
+        let _snapshot_setter = SnapshotSetter::new(&mut reflow_request, &mut snapshot_map);
+        let mut viewport_changed = false;
+        if reflow_request.restyle_reason.needs_restyle() {
+            viewport_changed = self.viewport_did_change(reflow_request.viewport_details);
             if self.update_device_if_necessary(&reflow_request, viewport_changed, &guards) {
                 if let Some(mut data) = root_element.mutate_data() {
                     data.hint.insert(RestyleHint::recascade_subtree());
                 }
             }
 
-        let mut snapshot_map = SnapshotMap::new();
-        let _snapshot_setter = SnapshotSetter::new(&mut reflow_request, &mut snapshot_map);
             self.prepare_stylist_for_reflow(
                 &reflow_request,
                 document,
@@ -625,6 +662,7 @@ impl LayoutThread {
                 // changed or not.
                 self.need_new_display_list.set(true);
             }
+        }
 
         let mut layout_context = LayoutContext {
             id: self.id,
@@ -650,7 +688,9 @@ impl LayoutThread {
             highlighted_dom_node: reflow_request.highlighted_dom_node,
         };
 
-        let damage = self.restyle_and_build_trees(
+        let mut damage = RestyleDamage::empty();
+        if reflow_request.restyle_reason.needs_restyle() {
+            damage = self.restyle_and_build_trees(
                 &reflow_request,
                 root_element,
                 rayon_pool,
@@ -658,6 +698,7 @@ impl LayoutThread {
                 viewport_changed,
             );
             self.calculate_overflow(damage);
+        };
         self.build_stacking_context_tree(&reflow_request, damage);
         let built_display_list =
             self.build_display_list(&reflow_request, damage, &mut layout_context);

components/script/dom/document.rs

@@ -42,7 +42,9 @@ use hyper_serde::Serde;
 use ipc_channel::ipc;
 use js::rust::{HandleObject, HandleValue};
 use keyboard_types::{Code, Key, KeyState, Modifiers};
-use layout_api::{PendingRestyle, ReflowGoal, TrustedNodeAddress, node_id_from_scroll_id};
+use layout_api::{
+    PendingRestyle, ReflowGoal, RestyleReason, TrustedNodeAddress, node_id_from_scroll_id,
+};
 use metrics::{InteractiveFlag, InteractiveWindow, ProgressiveWebMetrics};
 use net_traits::CookieSource::NonHTTP;
 use net_traits::CoreResourceMsg::{GetCookiesForUrl, SetCookiesForUrl};
@@ -383,11 +385,11 @@ pub(crate) struct Document {
     /// Information on elements needing restyle to ship over to layout when the
     /// time comes.
     pending_restyles: DomRefCell<FnvHashMap<Dom<Element>, NoTrace<PendingRestyle>>>,
-    /// This flag will be true if the `Document` needs to be painted again
+    /// A collection of reasons that the [`Document`] needs to be restyled at the next
-    /// during the next full layout attempt due to some external change such as
+    /// opportunity for a reflow. If this is empty, then the [`Document`] does not need to
-    /// the web view changing size, or because the previous layout was only for
+    /// be restyled.
-    /// layout queries (which do not trigger display).
+    #[no_trace]
-    needs_paint: Cell<bool>,
+    needs_restyle: Cell<RestyleReason>,
     /// <http://w3c.github.io/touch-events/#dfn-active-touch-point>
     active_touch_points: DomRefCell<Vec<Dom<Touch>>>,
     /// Navigation Timing properties:
@@ -841,32 +843,34 @@ impl Document {
         }
     }
 
-    pub(crate) fn set_needs_paint(&self, value: bool) {
+    pub(crate) fn add_restyle_reason(&self, reason: RestyleReason) {
-        self.needs_paint.set(value)
+        self.needs_restyle.set(self.needs_restyle.get() | reason)
+    }
+
+    pub(crate) fn clear_restyle_reasons(&self) {
+        self.needs_restyle.set(RestyleReason::empty());
+    }
+
+    pub(crate) fn restyle_reason(&self) -> RestyleReason {
+        let mut condition = self.needs_restyle.get();
+        if self.stylesheets.borrow().has_changed() {
+            condition.insert(RestyleReason::StylesheetsChanged);
         }
 
-    pub(crate) fn needs_reflow(&self) -> Option<ReflowTriggerCondition> {
         // FIXME: This should check the dirty bit on the document,
         // not the document element. Needs some layout changes to make
         // that workable.
-        if self.stylesheets.borrow().has_changed() {
+        if let Some(root) = self.GetDocumentElement() {
-            return Some(ReflowTriggerCondition::StylesheetsChanged);
-        }
-
-        let root = self.GetDocumentElement()?;
             if root.upcast::<Node>().has_dirty_descendants() {
-            return Some(ReflowTriggerCondition::DirtyDescendants);
+                condition.insert(RestyleReason::DOMChanged);
+            }
         }
 
         if !self.pending_restyles.borrow().is_empty() {
-            return Some(ReflowTriggerCondition::PendingRestyles);
+            condition.insert(RestyleReason::PendingRestyles);
         }
 
-        if self.needs_paint.get() {
+        condition
-            return Some(ReflowTriggerCondition::PaintPostponed);
-        }
-
-        None
     }
 
     /// Returns the first `base` element in the DOM that has an `href` attribute.
@@ -4141,7 +4145,7 @@ impl Document {
             base_element: Default::default(),
             appropriate_template_contents_owner_document: Default::default(),
             pending_restyles: DomRefCell::new(FnvHashMap::default()),
-            needs_paint: Cell::new(false),
+            needs_restyle: Cell::new(RestyleReason::DOMChanged),
             active_touch_points: DomRefCell::new(Vec::new()),
             dom_interactive: Cell::new(Default::default()),
             dom_content_loaded_event_start: Cell::new(Default::default()),
@@ -5194,9 +5198,10 @@ impl Document {
         self.has_trustworthy_ancestor_origin.get() ||
             self.origin().immutable().is_potentially_trustworthy()
     }
+
     pub(crate) fn highlight_dom_node(&self, node: Option<&Node>) {
         self.highlighted_dom_node.set(node);
-        self.set_needs_paint(true);
+        self.add_restyle_reason(RestyleReason::HighlightedDOMNodeChanged);
     }
 
     pub(crate) fn highlighted_dom_node(&self) -> Option<DomRoot<Node>> {
@@ -6827,14 +6832,6 @@ impl PendingScript {
     }
 }
 
-#[derive(Clone, Copy, Debug, PartialEq)]
-pub(crate) enum ReflowTriggerCondition {
-    StylesheetsChanged,
-    DirtyDescendants,
-    PendingRestyles,
-    PaintPostponed,
-}
-
 fn is_named_element_with_name_attribute(elem: &Element) -> bool {
     let type_ = match elem.upcast::<Node>().type_id() {
         NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,

components/script/dom/element.rs

@@ -103,9 +103,7 @@ use crate::dom::customelementregistry::{
     CallbackReaction, CustomElementDefinition, CustomElementReaction, CustomElementState,
     is_valid_custom_element_name,
 };
-use crate::dom::document::{
+use crate::dom::document::{Document, LayoutDocumentHelpers, determine_policy_for_token};
-    Document, LayoutDocumentHelpers, ReflowTriggerCondition, determine_policy_for_token,
-};
 use crate::dom::documentfragment::DocumentFragment;
 use crate::dom::domrect::DOMRect;
 use crate::dom::domrectlist::DOMRectList;
@@ -4693,10 +4691,7 @@ impl Element {
             .and_then(|data| data.client_rect.as_ref())
             .and_then(|rect| rect.get().ok())
         {
-            if matches!(
+            if doc.restyle_reason().is_empty() {
-                doc.needs_reflow(),
-                None | Some(ReflowTriggerCondition::PaintPostponed)
-            ) {
                 return rect;
             }
         }

components/script/dom/window.rs

@@ -52,7 +52,7 @@ use js::rust::{
     MutableHandleValue,
 };
 use layout_api::{
-    FragmentType, Layout, PendingImageState, QueryMsg, ReflowGoal, ReflowRequest,
+    FragmentType, Layout, PendingImageState, QueryMsg, ReflowGoal, ReflowRequest, RestyleReason,
     TrustedNodeAddress, combine_id_with_fragment_type,
 };
 use malloc_size_of::MallocSizeOf;
@@ -124,7 +124,7 @@ use crate::dom::bluetooth::BluetoothExtraPermissionData;
 use crate::dom::crypto::Crypto;
 use crate::dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration, CSSStyleOwner};
 use crate::dom::customelementregistry::CustomElementRegistry;
-use crate::dom::document::{AnimationFrameCallback, Document, ReflowTriggerCondition};
+use crate::dom::document::{AnimationFrameCallback, Document};
 use crate::dom::element::Element;
 use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus};
 use crate::dom::eventtarget::EventTarget;
@@ -2128,34 +2128,30 @@ 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(
+    fn force_reflow(&self, reflow_goal: ReflowGoal) -> bool {
-        &self,
+        let document = self.Document();
-        reflow_goal: ReflowGoal,
+        document.ensure_safe_to_run_script_or_layout();
-        condition: Option<ReflowTriggerCondition>,
-    ) -> bool {
-        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.
-        let layout_blocked = self.layout_blocker.get().layout_blocked();
         let pipeline_id = self.pipeline_id();
-        if reflow_goal == ReflowGoal::UpdateTheRendering && layout_blocked {
+        if reflow_goal == ReflowGoal::UpdateTheRendering &&
+            self.layout_blocker.get().layout_blocked()
+        {
             debug!("Suppressing pre-load-event reflow pipeline {pipeline_id}");
             return false;
         }
 
-        if condition != Some(ReflowTriggerCondition::PaintPostponed) {
+        let restyle_reason = document.restyle_reason();
-            debug!(
+        document.clear_restyle_reasons();
-                "Invalidating layout cache due to reflow condition {:?}",
+
-                condition
+        if restyle_reason.needs_restyle() {
-            );
+            debug!("Invalidating layout cache due to reflow condition {restyle_reason:?}",);
             // Invalidate any existing cached layout values.
             self.layout_marker.borrow().set(false);
             // Create a new layout caching token.
             *self.layout_marker.borrow_mut() = Rc::new(Cell::new(true));
-        } else {
-            debug!("Not invalidating cached layout values for paint-only reflow.");
         }
 
         debug!("script: performing reflow for goal {reflow_goal:?}");
@@ -2170,10 +2166,7 @@ impl Window {
             debug_reflow_events(pipeline_id, &reflow_goal);
         }
 
-        let document = self.Document();
-
         let stylesheets_changed = document.flush_stylesheets_for_reflow();
-        let for_display = reflow_goal.needs_display();
         let pending_restyles = document.drain_pending_restyles();
         let dirty_root = document
             .take_dirty_root()
@@ -2185,6 +2178,7 @@ impl Window {
 
         // Send new document and relevant styles to layout.
         let reflow = ReflowRequest {
+            restyle_reason,
             document: document.upcast::<Node>().to_trusted_node_address(),
             dirty_root,
             stylesheets_changed,
@@ -2209,12 +2203,6 @@ impl Window {
             self.emit_timeline_marker(marker.end());
         }
 
-        // Either this reflow caused new contents to be displayed or on the next
-        // full layout attempt a reflow should be forced in order to update the
-        // visual contents of the page. A case where full display might be delayed
-        // is when reflowing just for the purpose of doing a layout query.
-        document.set_needs_paint(!for_display);
-
         for image in results.pending_images {
             let id = image.id;
             let node = unsafe { from_untrusted_node_address(image.node) };
@@ -2295,31 +2283,8 @@ impl Window {
 
         self.Document().ensure_safe_to_run_script_or_layout();
 
-        let mut issued_reflow = false;
-        let condition = self.Document().needs_reflow();
         let updating_the_rendering = reflow_goal == ReflowGoal::UpdateTheRendering;
-        let for_display = reflow_goal.needs_display();
+        let issued_reflow = self.force_reflow(reflow_goal);
-        if !updating_the_rendering || condition.is_some() {
-            debug!("Reflowing document ({:?})", self.pipeline_id());
-            issued_reflow = self.force_reflow(reflow_goal, condition);
-
-            // We shouldn't need a reflow immediately after a completed reflow, unless the reflow didn't
-            // display anything and it wasn't for display. Queries can cause this to happen.
-            if issued_reflow {
-                let condition = self.Document().needs_reflow();
-                let display_is_pending = condition == Some(ReflowTriggerCondition::PaintPostponed);
-                assert!(
-                    condition.is_none() || (display_is_pending && !for_display),
-                    "Needed reflow after reflow: {:?}",
-                    condition
-                );
-            }
-        } else {
-            debug!(
-                "Document ({:?}) doesn't need reflow - skipping it (goal {reflow_goal:?})",
-                self.pipeline_id()
-            );
-        }
 
         let document = self.Document();
         let font_face_set = document.Fonts(can_gc);
@@ -2420,7 +2385,6 @@ impl Window {
 
         self.layout_blocker
             .set(LayoutBlocker::FiredLoadEventOrParsingTimerExpired);
-        self.Document().set_needs_paint(true);
 
         // We do this immediately instead of scheduling a future task, because this can
         // happen if parsing is taking a very long time, which means that the
@@ -2777,7 +2741,8 @@ impl Window {
             return;
         }
         self.theme.set(new_theme);
-        self.Document().set_needs_paint(true);
+        self.Document()
+            .add_restyle_reason(RestyleReason::ThemeChanged);
     }
 
     pub(crate) fn get_url(&self) -> ServoUrl {
@@ -2811,7 +2776,14 @@ impl Window {
 
         // The document needs to be repainted, because the initial containing block
         // is now a different size.
-        self.Document().set_needs_paint(true);
+        self.Document()
+            .add_restyle_reason(RestyleReason::ViewportSizeChanged);
+
+        // If viewport units were used, all nodes need to be restyled, because
+        // we currently do not track which ones rely on viewport units.
+        if self.layout().device().used_viewport_units() {
+            self.Document().dirty_all_nodes();
+        }
     }
 
     pub(crate) fn suspend(&self, can_gc: CanGc) {
@@ -2906,6 +2878,18 @@ impl Window {
         );
         self.set_viewport_details(new_size);
 
+        // The document needs to be repainted, because the initial containing
+        // block is now a different size. This should be triggered before the
+        // event is fired below so that any script queries trigger a restyle.
+        self.Document()
+            .add_restyle_reason(RestyleReason::ViewportSizeChanged);
+
+        // If viewport units were used, all nodes need to be restyled, because
+        // we currently do not track which ones rely on viewport units.
+        if self.layout().device().used_viewport_units() {
+            self.Document().dirty_all_nodes();
+        }
+
         // http://dev.w3.org/csswg/cssom-view/#resizing-viewports
         if size_type == WindowSizeType::Resize {
             let uievent = UIEvent::new(
@@ -2920,10 +2904,6 @@ impl Window {
             uievent.upcast::<Event>().fire(self.upcast(), can_gc);
         }
 
-        // The document needs to be repainted, because the initial containing block
-        // is now a different size.
-        self.Document().set_needs_paint(true);
-
         true
     }
 

components/script/script_thread.rs

@@ -68,7 +68,7 @@ use js::jsapi::{
 };
 use js::jsval::UndefinedValue;
 use js::rust::ParentRuntime;
-use layout_api::{LayoutConfig, LayoutFactory, ScriptThreadFactory};
+use layout_api::{LayoutConfig, LayoutFactory, RestyleReason, ScriptThreadFactory};
 use media::WindowGLContext;
 use metrics::MAX_TASK_NS;
 use net_traits::image_cache::{ImageCache, ImageCacheResponseMessage};
@@ -1374,7 +1374,7 @@ impl ScriptThread {
         let Some((_, document)) = self.documents.borrow().iter().find(|(_, document)| {
             document.is_fully_active() &&
                 !document.window().layout_blocked() &&
-                document.needs_reflow().is_some()
+                !document.restyle_reason().is_empty()
         }) else {
             return;
         };
@@ -3132,7 +3132,7 @@ impl ScriptThread {
     /// page no longer exists.
     fn handle_worklet_loaded(&self, pipeline_id: PipelineId) {
         if let Some(document) = self.documents.borrow().find_document(pipeline_id) {
-            document.set_needs_paint(true)
+            document.add_restyle_reason(RestyleReason::PaintWorkletLoaded);
         }
     }
 

components/shared/layout/Cargo.toml

@@ -13,6 +13,7 @@ path = "lib.rs"
 
 [dependencies]
 base = { workspace = true }
+bitflags = { workspace = true }
 app_units = { workspace = true }
 atomic_refcell = { workspace = true }
 compositing_traits = { workspace = true }

components/shared/layout/lib.rs

@@ -19,6 +19,7 @@ use app_units::Au;
 use atomic_refcell::AtomicRefCell;
 use base::Epoch;
 use base::id::{BrowsingContextId, PipelineId, WebViewId};
+use bitflags::bitflags;
 use compositing_traits::CrossProcessCompositorApi;
 use constellation_traits::LoadData;
 use embedder_traits::{Theme, UntrustedNodeAddress, ViewportDetails};
@@ -28,7 +29,7 @@ use fonts::{FontContext, SystemFontServiceProxy};
 use fxhash::FxHashMap;
 use ipc_channel::ipc::IpcSender;
 use libc::c_void;
-use malloc_size_of::{MallocSizeOf as MallocSizeOfTrait, MallocSizeOfOps};
+use malloc_size_of::{MallocSizeOf as MallocSizeOfTrait, MallocSizeOfOps, malloc_size_of_is_0};
 use malloc_size_of_derive::MallocSizeOf;
 use net_traits::image_cache::{ImageCache, PendingImageId};
 use parking_lot::RwLock;
@@ -400,6 +401,29 @@ pub struct IFrameSize {
 
 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)]
+    pub struct RestyleReason: u16 {
+        const StylesheetsChanged = 1 << 0;
+        const DOMChanged = 1 << 1;
+        const PendingRestyles = 1 << 2;
+        const HighlightedDOMNodeChanged = 1 << 3;
+        const ThemeChanged = 1 << 4;
+        const ViewportSizeChanged = 1 << 5;
+        const PaintWorkletLoaded = 1 << 6;
+    }
+}
+
+malloc_size_of_is_0!(RestyleReason);
+
+impl RestyleReason {
+    pub fn needs_restyle(&self) -> bool {
+        !self.is_empty()
+    }
+}
+
 /// Information derived from a layout pass that needs to be returned to the script thread.
 #[derive(Debug, Default)]
 pub struct ReflowResult {
@@ -418,6 +442,8 @@ pub struct ReflowResult {
 /// Information needed for a script-initiated reflow.
 #[derive(Debug)]
 pub struct ReflowRequest {
+    /// Whether or not (and for what reasons) restyle needs to happen.
+    pub restyle_reason: RestyleReason,
     /// The document node.
     pub document: TrustedNodeAddress,
     /// The dirty root from which to restyle.