script/layout: Ensure a StackingContextTree before IntersectionObserver geometry queries (#38473)

IntersectionObserver needs to be able to query node geometry without
forcing a layout. A previous layout could have run without needing a
`StackingContextTree`. In that case the layout-less query should finish
building the `StackingContextTree` before doing the query.  Add a new
type of layout API which requests that layout finishes building the
StackingContextTree.

This change also slightly simplifies and corrects the naming of
`Element` APIs around client box queries.

Testing: This should fix intermittent failures in WPT tests.
Fixes: #38380.
Fixes: #38390.
Closes: #38400.

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

components/layout/layout_impl.rs

@@ -261,11 +261,17 @@ impl Layout for LayoutThread {
     ///                      query and possibly, query without consideration of transform.
     #[servo_tracing::instrument(skip_all)]
     fn query_content_box(&self, node: TrustedNodeAddress) -> Option<UntypedRect<Au>> {
+        // If we have not built a fragment tree yet, there is no way we have layout information for
+        // this query, which can be run without forcing a layout (for IntersectionObserver).
+        if self.fragment_tree.borrow().is_none() {
+            return None;
+        }
+
         let node = unsafe { ServoLayoutNode::new(&node) };
         let stacking_context_tree = self.stacking_context_tree.borrow();
         let stacking_context_tree = stacking_context_tree
             .as_ref()
-            .expect("Should always have a StackingContextTree for geometry queries");
+            .expect("Should always have a StackingContextTree for content box queries");
         process_content_box_request(stacking_context_tree, node)
     }
 
@@ -275,11 +281,17 @@ impl Layout for LayoutThread {
     /// See <https://drafts.csswg.org/cssom-view/#dom-element-getclientrects>.
     #[servo_tracing::instrument(skip_all)]
     fn query_content_boxes(&self, node: TrustedNodeAddress) -> Vec<UntypedRect<Au>> {
+        // If we have not built a fragment tree yet, there is no way we have layout information for
+        // this query, which can be run without forcing a layout (for IntersectionObserver).
+        if self.fragment_tree.borrow().is_none() {
+            return vec![];
+        }
+
         let node = unsafe { ServoLayoutNode::new(&node) };
         let stacking_context_tree = self.stacking_context_tree.borrow();
         let stacking_context_tree = stacking_context_tree
             .as_ref()
-            .expect("Should always have a StackingContextTree for geometry queries");
+            .expect("Should always have a StackingContextTree for content box queries");
         process_content_boxes_request(stacking_context_tree, node)
     }
 
@@ -454,6 +466,15 @@ impl Layout for LayoutThread {
         )
     }
 
+    fn ensure_stacking_context_tree(&self, viewport_details: ViewportDetails) {
+        if self.stacking_context_tree.borrow().is_some() &&
+            !self.need_new_stacking_context_tree.get()
+        {
+            return;
+        }
+        self.build_stacking_context_tree(viewport_details);
+    }
+
     fn register_paint_worklet_modules(
         &mut self,
         _name: Atom,
@@ -696,7 +717,7 @@ impl LayoutThread {
         if self.calculate_overflow(damage) {
             reflow_phases_run.insert(ReflowPhasesRun::CalculatedOverflow);
         }
-        if self.build_stacking_context_tree(&reflow_request, damage) {
+        if self.build_stacking_context_tree_for_reflow(&reflow_request, damage) {
             reflow_phases_run.insert(ReflowPhasesRun::BuiltStackingContextTree);
         }
         if self.build_display_list(&reflow_request, damage, &image_resolver) {
@@ -976,8 +997,7 @@ impl LayoutThread {
         true
     }
 
-    #[servo_tracing::instrument(name = "Stacking Context Tree Construction", skip_all)]
-    fn build_stacking_context_tree(
+    fn build_stacking_context_tree_for_reflow(
         &self,
         reflow_request: &ReflowRequest,
         damage: RestyleDamage,
@@ -987,14 +1007,19 @@ impl LayoutThread {
         {
             return false;
         }
-        let Some(fragment_tree) = &*self.fragment_tree.borrow() else {
-            return false;
-        };
         if !damage.contains(RestyleDamage::REBUILD_STACKING_CONTEXT) &&
             !self.need_new_stacking_context_tree.get()
         {
             return false;
         }
+        self.build_stacking_context_tree(reflow_request.viewport_details)
+    }
+
+    #[servo_tracing::instrument(name = "Stacking Context Tree Construction", skip_all)]
+    fn build_stacking_context_tree(&self, viewport_details: ViewportDetails) -> bool {
+        let Some(fragment_tree) = &*self.fragment_tree.borrow() else {
+            return false;
+        };
 
         let mut stacking_context_tree = self.stacking_context_tree.borrow_mut();
         let old_scroll_offsets = stacking_context_tree
@@ -1006,7 +1031,7 @@ impl LayoutThread {
         // applicable spatial and clip nodes.
         let mut new_stacking_context_tree = StackingContextTree::new(
             fragment_tree,
-            reflow_request.viewport_details,
+            viewport_details,
             self.id.into(),
             !self.have_ever_generated_display_list.get(),
             &self.debug,
@@ -1022,6 +1047,13 @@ impl LayoutThread {
                 .set_all_scroll_offsets(&old_scroll_offsets);
         }
 
+        if self.debug.dump_scroll_tree {
+            new_stacking_context_tree
+                .compositor_info
+                .scroll_tree
+                .debug_print();
+        }
+
         *stacking_context_tree = Some(new_stacking_context_tree);
 
         // Force display list generation as layout has changed.
@@ -1030,20 +1062,6 @@ impl LayoutThread {
         // The stacking context tree is up-to-date again.
         self.need_new_stacking_context_tree.set(false);
 
-        if self.debug.dump_scroll_tree {
-            // Print the [ScrollTree], this is done after display list build so we have
-            // the information about webrender id. Whether a scroll tree is initialized
-            // or not depends on the reflow goal.
-            if let Some(tree) = self.stacking_context_tree.borrow().as_ref() {
-                tree.compositor_info.scroll_tree.debug_print();
-            } else {
-                println!(
-                    "Scroll Tree -- reflow {:?}: scroll tree is not initialized yet.",
-                    reflow_request.reflow_goal
-                );
-            }
-        }
-
         true
     }
 

components/script/dom/document.rs

@@ -1056,7 +1056,7 @@ impl Document {
                 // inside other scrollable containers. Ideally this should use an implementation of
                 // `scrollIntoView` when that is available:
                 // See https://github.com/servo/servo/issues/24059.
-                let rect = element.upcast::<Node>().bounding_content_box_or_zero();
+                let rect = element.upcast::<Node>().content_box().unwrap_or_default();
 
                 // In order to align with element edges, we snap to unscaled pixel boundaries, since
                 // the paint thread currently does the same for drawing elements. This is important
@@ -1356,7 +1356,7 @@ impl Document {
 
                 // Notify the embedder to display an input method.
                 if let Some(kind) = elem.input_method_type() {
-                    let rect = elem.upcast::<Node>().bounding_content_box_or_zero();
+                    let rect = elem.upcast::<Node>().content_box().unwrap_or_default();
                     let rect = Rect::new(
                         Point2D::new(rect.origin.x.to_px(), rect.origin.y.to_px()),
                         Size2D::new(rect.size.width.to_px(), rect.size.height.to_px()),

components/script/dom/element.rs

@@ -1005,7 +1005,7 @@ impl Element {
         block: ScrollLogicalPosition,
         inline: ScrollLogicalPosition,
     ) -> ScrollPosition {
-        let target_bounding_box = self.upcast::<Node>().bounding_content_box_or_zero();
+        let target_bounding_box = self.upcast::<Node>().content_box().unwrap_or_default();
 
         let device_pixel_ratio = self
             .upcast::<Node>()
@@ -1071,7 +1071,7 @@ impl Element {
         } else {
             // Handle element-specific scrolling
             // Scrolling box bounds and current scroll position
-            let scrolling_box = scrolling_node.bounding_content_box_or_zero();
+            let scrolling_box = scrolling_node.content_box().unwrap_or_default();
             let scrolling_left = scrolling_box.origin.x.to_nearest_pixel(device_pixel_ratio) as f64;
             let scrolling_top = scrolling_box.origin.y.to_nearest_pixel(device_pixel_ratio) as f64;
             let scrolling_width = scrolling_box
@@ -1125,7 +1125,7 @@ impl Element {
                             if matches!(position, Position::Relative | Position::Absolute) {
                                 // If this element establishes a positioning context,
                                 // Get its bounding box to calculate the offset
-                                let positioning_box = node.bounding_content_box_or_zero();
+                                let positioning_box = node.content_box().unwrap_or_default();
                                 let positioning_left = positioning_box
                                     .origin
                                     .x
@@ -3458,7 +3458,7 @@ impl ElementMethods<crate::DomTypeHolder> for Element {
     // https://drafts.csswg.org/cssom-view/#dom-element-getboundingclientrect
     fn GetBoundingClientRect(&self, can_gc: CanGc) -> DomRoot<DOMRect> {
         let win = self.owner_window();
-        let rect = self.upcast::<Node>().bounding_content_box_or_zero();
+        let rect = self.upcast::<Node>().content_box().unwrap_or_default();
         DOMRect::new(
             win.upcast(),
             rect.origin.x.to_f64_px(),

components/script/dom/htmlanchorelement.rs

@@ -329,7 +329,7 @@ impl Activatable for HTMLAnchorElement {
         if let Some(element) = target.downcast::<Element>() {
             if target.is::<HTMLImageElement>() && element.has_attribute(&local_name!("ismap")) {
                 let target_node = element.upcast::<Node>();
-                let rect = target_node.bounding_content_box_or_zero();
+                let rect = target_node.content_box().unwrap_or_default();
                 ismap_suffix = Some(format!(
                     "?{},{}",
                     mouse_event.ClientX().to_f32().unwrap() - rect.origin.x.to_f32_px(),

components/script/dom/htmlimageelement.rs

@@ -1657,10 +1657,9 @@ impl HTMLImageElementMethods<crate::DomTypeHolder> for HTMLImageElement {
     // https://html.spec.whatwg.org/multipage/#dom-img-width
     fn Width(&self) -> u32 {
         let node = self.upcast::<Node>();
-        match node.bounding_content_box() {
-            Some(rect) => rect.size.width.to_px() as u32,
-            None => self.NaturalWidth(),
-        }
+        node.content_box()
+            .map(|rect| rect.size.width.to_px() as u32)
+            .unwrap_or_else(|| self.NaturalWidth())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-img-width
@@ -1671,10 +1670,9 @@ impl HTMLImageElementMethods<crate::DomTypeHolder> for HTMLImageElement {
     // https://html.spec.whatwg.org/multipage/#dom-img-height
     fn Height(&self) -> u32 {
         let node = self.upcast::<Node>();
-        match node.bounding_content_box() {
-            Some(rect) => rect.size.height.to_px() as u32,
-            None => self.NaturalHeight(),
-        }
+        node.content_box()
+            .map(|rect| rect.size.height.to_px() as u32)
+            .unwrap_or_else(|| self.NaturalHeight())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-img-height

components/script/dom/htmlinputelement.rs

@@ -2736,7 +2736,7 @@ impl HTMLInputElement {
             let (ipc_sender, ipc_receiver) =
                 ipc::channel::<Option<RgbColor>>().expect("Failed to create IPC channel!");
             let document = self.owner_document();
-            let rect = self.upcast::<Node>().bounding_content_box_or_zero();
+            let rect = self.upcast::<Node>().content_box().unwrap_or_default();
             let rect = Rect::new(
                 Point2D::new(rect.origin.x.to_px(), rect.origin.y.to_px()),
                 Size2D::new(rect.size.width.to_px(), rect.size.height.to_px()),

components/script/dom/htmlselectelement.rs

@@ -377,7 +377,7 @@ impl HTMLSelectElement {
             })
             .collect();
 
-        let rect = self.upcast::<Node>().bounding_content_box_or_zero();
+        let rect = self.upcast::<Node>().content_box().unwrap_or_default();
         let rect = Rect::new(
             Point2D::new(rect.origin.x.to_px(), rect.origin.y.to_px()),
             Size2D::new(rect.size.width.to_px(), rect.size.height.to_px()),

components/script/dom/intersectionobserver.rs

@@ -408,6 +408,7 @@ impl IntersectionObserver {
     ///
     /// <https://w3c.github.io/IntersectionObserver/#intersectionobserver-root-intersection-rectangle>
     pub(crate) fn root_intersection_rectangle(&self, document: &Document) -> Option<Rect<Au>> {
+        let window = document.window();
         let intersection_rectangle = match &self.root {
             // Handle if root is an element.
             Some(ElementOrDocument::Element(element)) => {
@@ -419,7 +420,7 @@ impl IntersectionObserver {
 
                 // > Otherwise, it’s the result of getting the bounding box for the intersection root.
                 // TODO: replace this once getBoundingBox() is implemented correctly.
-                DomRoot::upcast::<Node>(element.clone()).bounding_content_box_no_reflow()
+                window.content_box_query_without_reflow(&DomRoot::upcast::<Node>(element.clone()))
             },
             // Handle if root is a Document, which includes implicit root and explicit Document root.
             _ => {
@@ -503,7 +504,9 @@ impl IntersectionObserver {
         // This is what we are currently using for getBoundingBox(). However, it is not correct,
         // mainly because it is not considering transform and scroll offset.
         // TODO: replace this once getBoundingBox() is implemented correctly.
-        let maybe_target_rect = target.upcast::<Node>().bounding_content_box_no_reflow();
+        let maybe_target_rect = document
+            .window()
+            .content_box_query_without_reflow(target.upcast::<Node>());
 
         // Following the implementation of Gecko, we will skip further processing if these
         // information not available. This would also handle display none element.

components/script/dom/node.rs

@@ -942,20 +942,10 @@ impl Node {
         TrustedNodeAddress(self as *const Node as *const libc::c_void)
     }
 
-    /// Returns the rendered bounding content box if the element is rendered,
-    /// and none otherwise.
-    pub(crate) fn bounding_content_box(&self) -> Option<Rect<Au>> {
+    pub(crate) fn content_box(&self) -> Option<Rect<Au>> {
         self.owner_window().content_box_query(self)
     }
 
-    pub(crate) fn bounding_content_box_or_zero(&self) -> Rect<Au> {
-        self.bounding_content_box().unwrap_or_else(Rect::zero)
-    }
-
-    pub(crate) fn bounding_content_box_no_reflow(&self) -> Option<Rect<Au>> {
-        self.owner_window().content_box_query_unchecked(self)
-    }
-
     pub(crate) fn content_boxes(&self) -> Vec<Rect<Au>> {
         self.owner_window().content_boxes_query(self)
     }

components/script/dom/window.rs

@@ -2434,16 +2434,19 @@ impl Window {
         )
     }
 
-    // Query content box without considering any reflow
-    pub(crate) fn content_box_query_unchecked(&self, node: &Node) -> Option<UntypedRect<Au>> {
-        self.layout
-            .borrow()
-            .query_content_box(node.to_trusted_node_address())
+    /// Do the same kind of query as `Self::content_box_query`, but do not force a reflow.
+    /// This is used for things like `IntersectionObserver` which should observe the value
+    /// from the most recent reflow, but do not need it to reflect the current state of
+    /// the DOM / style.
+    pub(crate) fn content_box_query_without_reflow(&self, node: &Node) -> Option<UntypedRect<Au>> {
+        let layout = self.layout.borrow();
+        layout.ensure_stacking_context_tree(self.viewport_details.get());
+        layout.query_content_box(node.to_trusted_node_address())
     }
 
     pub(crate) fn content_box_query(&self, node: &Node) -> Option<UntypedRect<Au>> {
         self.layout_reflow(QueryMsg::ContentBox);
-        self.content_box_query_unchecked(node)
+        self.content_box_query_without_reflow(node)
     }
 
     pub(crate) fn content_boxes_query(&self, node: &Node) -> Vec<UntypedRect<Au>> {

components/shared/layout/lib.rs

@@ -241,6 +241,10 @@ pub trait Layout {
     /// Requests a reflow.
     fn reflow(&mut self, reflow_request: ReflowRequest) -> Option<ReflowResult>;
 
+    /// Do not request a reflow, but ensure that any previous reflow completes building a stacking
+    /// context tree so that it is ready to query the final size of any elements in script.
+    fn ensure_stacking_context_tree(&self, viewport_details: ViewportDetails);
+
     /// Tells layout that script has added some paint worklet modules.
     fn register_paint_worklet_modules(
         &mut self,

tests/wpt/meta/intersection-observer/fixed-position-iframe-scroll.html.ini

@@ -0,0 +1,3 @@
+[fixed-position-iframe-scroll.html]
+  [scrollTo(0, 1000)]
+    expected: FAIL