Various fixes to getClientBoundingRect()

* Fix queries involving stacking contexts
 * The code was double accumulating stacking context origins.
* Handle queries of inline elements.
 * The node addresses being compared were incorrect (CharacterData vs. Span)
* Handle ScriptQuery reflows correctly.
 * The layout task was skipping the compute absolute positions traversal, so failed before window.onload.

components/gfx/display_list/mod.rs

@@ -64,7 +64,7 @@ pub static BLUR_INFLATION_FACTOR: i32 = 3;
 /// Because the script task's GC does not trace layout, node data cannot be safely stored in layout
 /// data structures. Also, layout code tends to be faster when the DOM is not being accessed, for
 /// locality reasons. Using `OpaqueNode` enforces this invariant.
-#[derive(Clone, PartialEq, Copy)]
+#[derive(Clone, PartialEq, Copy, Debug)]
 pub struct OpaqueNode(pub uintptr_t);
 
 impl OpaqueNode {

components/layout/block.rs

@@ -1914,7 +1914,7 @@ impl Flow for BlockFlow {
                                                             self.base
                                                                 .absolute_position_info
                                                                 .relative_containing_block_mode,
-                                                            CoordinateSystem::Parent)
+                                                            CoordinateSystem::Own)
                               .translate(stacking_context_position));
     }
 

components/layout/construct.rs

@@ -31,7 +31,7 @@ use fragment::TableColumnFragmentInfo;
 use fragment::UnscannedTextFragmentInfo;
 use fragment::{InlineBlockFragmentInfo, SpecificFragmentInfo};
 use incremental::{RECONSTRUCT_FLOW, RestyleDamage};
-use inline::InlineFlow;
+use inline::{InlineFlow, InlineFragmentNodeInfo};
 use list_item::{ListItemFlow, ListStyleTypeContent};
 use multicol::MulticolFlow;
 use opaque_node::OpaqueNodeMethods;
@@ -169,14 +169,14 @@ struct InlineFragmentsAccumulator {
 
     /// Whether we've created a range to enclose all the fragments. This will be Some() if the
     /// outer node is an inline and None otherwise.
-    enclosing_style: Option<Arc<ComputedValues>>,
+    enclosing_node: Option<InlineFragmentNodeInfo>,
 }
 
 impl InlineFragmentsAccumulator {
     fn new() -> InlineFragmentsAccumulator {
         InlineFragmentsAccumulator {
             fragments: LinkedList::new(),
-            enclosing_style: None,
+            enclosing_node: None,
         }
     }
 
@@ -184,7 +184,9 @@ impl InlineFragmentsAccumulator {
         let fragments = LinkedList::new();
         InlineFragmentsAccumulator {
             fragments: fragments,
-            enclosing_style: Some(node.style().clone()),
+            enclosing_node: Some(InlineFragmentNodeInfo {
+                                    address: OpaqueNodeMethods::from_thread_safe_layout_node(node),
+                                    style: node.style().clone() }),
         }
     }
 
@@ -199,9 +201,9 @@ impl InlineFragmentsAccumulator {
     fn to_dlist(self) -> LinkedList<Fragment> {
         let InlineFragmentsAccumulator {
             mut fragments,
-            enclosing_style
+            enclosing_node,
         } = self;
-        if let Some(enclosing_style) = enclosing_style {
+        if let Some(enclosing_node) = enclosing_node {
             let frag_len = fragments.len();
             for (idx, frag) in fragments.iter_mut().enumerate() {
 
@@ -210,7 +212,7 @@ impl InlineFragmentsAccumulator {
                 // frag is the last inline fragment in the inline node
                 let is_last = idx == frag_len - 1;
 
-                frag.add_inline_context_style(enclosing_style.clone(), is_first, is_last);
+                frag.add_inline_context_style(enclosing_node.clone(), is_first, is_last);
             }
         }
         fragments

components/layout/context.rs

@@ -15,7 +15,7 @@ use gfx::font_context::FontContext;
 use msg::constellation_msg::ConstellationChan;
 use net_traits::image::base::Image;
 use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask, ImageState};
-use script::layout_interface::{Animation, LayoutChan};
+use script::layout_interface::{Animation, LayoutChan, ReflowGoal};
 use std::boxed;
 use std::cell::Cell;
 use std::ptr;
@@ -98,6 +98,9 @@ pub struct SharedLayoutContext {
     /// A channel on which new animations that have been triggered by style recalculation can be
     /// sent.
     pub new_animations_sender: Sender<Animation>,
+
+    /// Why is this reflow occurring
+    pub goal: ReflowGoal,
 }
 
 pub struct SharedLayoutContextWrapper(pub *const SharedLayoutContext);

components/layout/display_list_builder.rs

@@ -867,30 +867,30 @@ impl FragmentDisplayListBuilding for Fragment {
 
             // Add shadows, background, borders, and outlines, if applicable.
             if let Some(ref inline_context) = self.inline_context {
-                for style in inline_context.styles.iter().rev() {
+                for node in inline_context.nodes.iter().rev() {
                     self.build_display_list_for_box_shadow_if_applicable(
-                        &**style,
+                        &*node.style,
                         display_list,
                         layout_context,
                         level,
                         &stacking_relative_border_box,
                         &clip);
                     self.build_display_list_for_background_if_applicable(
-                        &**style,
+                        &*node.style,
                         display_list,
                         layout_context,
                         level,
                         &stacking_relative_border_box,
                         &clip);
                     self.build_display_list_for_borders_if_applicable(
-                        &**style,
+                        &*node.style,
                         border_painting_mode,
                         display_list,
                         &stacking_relative_border_box,
                         level,
                         &clip);
                     self.build_display_list_for_outline_if_applicable(
-                        &**style,
+                        &*node.style,
                         display_list,
                         &stacking_relative_border_box,
                         &clip);

components/layout/fragment.rs

@@ -14,7 +14,7 @@ use flow;
 use flow::Flow;
 use flow_ref::FlowRef;
 use incremental::{self, RestyleDamage};
-use inline::{InlineFragmentContext, InlineMetrics};
+use inline::{InlineFragmentContext, InlineFragmentNodeInfo, InlineMetrics};
 use layout_debug;
 use model::{self, IntrinsicISizes, IntrinsicISizesContribution, MaybeAuto, specified};
 use text;
@@ -840,32 +840,37 @@ impl Fragment {
         self.restyle_damage | self.specific.restyle_damage()
     }
 
+    pub fn contains_node(&self, node_address: OpaqueNode) -> bool {
+        node_address == self.node ||
+        self.inline_context.as_ref().map_or(false, |ctx| {
+            ctx.contains_node(node_address)
+        })
+    }
+
     /// Adds a style to the inline context for this fragment. If the inline context doesn't exist
     /// yet, it will be created.
     pub fn add_inline_context_style(&mut self,
-                                    style: Arc<ComputedValues>,
+                                    mut node_info: InlineFragmentNodeInfo,
                                     first_frag: bool,
                                     last_frag: bool) {
         if self.inline_context.is_none() {
             self.inline_context = Some(InlineFragmentContext::new());
         }
-        let frag_style = if first_frag && last_frag {
+        if !first_frag || !last_frag {
-            style.clone()
-        } else {
             // Set the border width to zero and the border style to none on
             // border sides that are not the outermost for a node container.
             // Because with multiple inline fragments they don't have interior
             // borders separating each other.
-            let mut border_width = style.logical_border_width();
+            let mut border_width = node_info.style.logical_border_width();
             if !last_frag {
-                border_width.set_right(style.writing_mode, Zero::zero());
+                border_width.set_right(node_info.style.writing_mode, Zero::zero());
             }
             if !first_frag {
-                border_width.set_left(style.writing_mode, Zero::zero());
+                border_width.set_left(node_info.style.writing_mode, Zero::zero());
             }
-            Arc::new(make_border(&*style, border_width))
+            node_info.style = Arc::new(make_border(&*node_info.style, border_width))
         };
-        self.inline_context.as_mut().unwrap().styles.push(frag_style);
+        self.inline_context.as_mut().unwrap().nodes.push(node_info);
     }
 
     /// Determines which quantities (border/padding/margin/specified) should be included in the
@@ -1000,10 +1005,10 @@ impl Fragment {
         match self.inline_context {
             None => style_border_width,
             Some(ref inline_fragment_context) => {
-                inline_fragment_context.styles
+                inline_fragment_context.nodes
                                        .iter()
                                        .fold(style_border_width,
-                                             |acc, style| acc + style.logical_border_width())
+                                             |acc, node| acc + node.style.logical_border_width())
             }
         }
     }
@@ -1092,10 +1097,10 @@ impl Fragment {
                 match self.inline_context {
                     None => style_padding,
                     Some(ref inline_fragment_context) => {
-                        inline_fragment_context.styles
+                        inline_fragment_context.nodes
                                                .iter()
-                                               .fold(style_padding, |acc, style| {
+                                               .fold(style_padding, |acc, node| {
-                                                   acc + model::padding_from_style(&**style,
+                                                   acc + model::padding_from_style(&*node.style,
                                                                                    Au(0))
                                                })
                     }
@@ -1136,9 +1141,9 @@ impl Fragment {
         };
 
         if let Some(ref inline_fragment_context) = self.inline_context {
-            for style in inline_fragment_context.styles.iter() {
+            for node in inline_fragment_context.nodes.iter() {
-                if style.get_box().position == position::T::relative {
+                if node.style.get_box().position == position::T::relative {
-                    rel_pos = rel_pos + from_style(&**style, containing_block_size);
+                    rel_pos = rel_pos + from_style(&*node.style, containing_block_size);
                 }
             }
         }
@@ -1282,10 +1287,10 @@ impl Fragment {
         // Take borders and padding for parent inline fragments into account, if necessary.
         if self.is_primary_fragment() {
             if let Some(ref context) = self.inline_context {
-                for style in context.styles.iter() {
+                for node in context.nodes.iter() {
-                    let border_width = style.logical_border_width().inline_start_end();
+                    let border_width = node.style.logical_border_width().inline_start_end();
                     let padding_inline_size =
-                        model::padding_from_style(&**style, Au(0)).inline_start_end();
+                        model::padding_from_style(&*node.style, Au(0)).inline_start_end();
                     result.surrounding_size = result.surrounding_size + border_width +
                         padding_inline_size;
                 }
@@ -2097,11 +2102,11 @@ impl<'a> Iterator for InlineStyleIterator<'a> {
             Some(ref inline_context) => inline_context,
         };
         let inline_style_index = self.inline_style_index;
-        if inline_style_index == inline_context.styles.len() {
+        if inline_style_index == inline_context.nodes.len() {
             return None
         }
         self.inline_style_index += 1;
-        Some(&*inline_context.styles[inline_style_index])
+        Some(&*inline_context.nodes[inline_style_index].style)
     }
 }
 

components/layout/inline.rs

@@ -17,6 +17,7 @@ use text;
 
 use collections::VecDeque;
 use geom::{Point2D, Rect};
+use gfx::display_list::OpaqueNode;
 use gfx::font::FontMetrics;
 use gfx::font_context::FontContext;
 use gfx::text::glyph::CharIndex;
@@ -1088,10 +1089,10 @@ impl InlineFlow {
         for frag in self.fragments.fragments.iter() {
             match frag.inline_context {
                 Some(ref inline_context) => {
-                    for style in inline_context.styles.iter() {
+                    for node in inline_context.nodes.iter() {
-                        let font_style = style.get_font_arc();
+                        let font_style = node.style.get_font_arc();
                         let font_metrics = text::font_metrics_for_style(font_context, font_style);
-                        let line_height = text::line_height_from_style(&**style, &font_metrics);
+                        let line_height = text::line_height_from_style(&*node.style, &font_metrics);
                         let inline_metrics = InlineMetrics::from_font_metrics(&font_metrics,
                                                                               line_height);
                         block_size_above_baseline = max(block_size_above_baseline,
@@ -1476,7 +1477,7 @@ impl Flow for InlineFlow {
                              &fragment.stacking_relative_border_box(stacking_relative_position,
                                                                     relative_containing_block_size,
                                                                     relative_containing_block_mode,
-                                                                    CoordinateSystem::Parent)
+                                                                    CoordinateSystem::Own)
                                       .translate(stacking_context_position))
         }
     }
@@ -1494,24 +1495,35 @@ impl fmt::Debug for InlineFlow {
     }
 }
 
+#[derive(Clone)]
+pub struct InlineFragmentNodeInfo {
+    pub address: OpaqueNode,
+    pub style: Arc<ComputedValues>,
+}
+
 #[derive(Clone)]
 pub struct InlineFragmentContext {
-    pub styles: Vec<Arc<ComputedValues>>,
+    pub nodes: Vec<InlineFragmentNodeInfo>,
 }
 
 impl InlineFragmentContext {
     pub fn new() -> InlineFragmentContext {
         InlineFragmentContext {
-            styles: vec!()
+            nodes: vec!(),
         }
     }
 
+    #[inline]
+    pub fn contains_node(&self, node_address: OpaqueNode) -> bool {
+        self.nodes.iter().position(|node| node.address == node_address).is_some()
+    }
+
     fn ptr_eq(&self, other: &InlineFragmentContext) -> bool {
-        if self.styles.len() != other.styles.len() {
+        if self.nodes.len() != other.nodes.len() {
             return false
         }
-        for (this_style, other_style) in self.styles.iter().zip(other.styles.iter()) {
+        for (this_node, other_node) in self.nodes.iter().zip(other.nodes.iter()) {
-            if !util::arc_ptr_eq(this_style, other_style) {
+            if !util::arc_ptr_eq(&this_node.style, &other_node.style) {
                 return false
             }
         }

components/layout/layout_task.rs

@@ -350,7 +350,8 @@ impl LayoutTask {
                                    rw_data: &LayoutTaskData,
                                    screen_size_changed: bool,
                                    reflow_root: Option<&LayoutNode>,
-                                   url: &Url)
+                                   url: &Url,
+                                   goal: ReflowGoal)
                                    -> SharedLayoutContext {
         SharedLayoutContext {
             image_cache_task: rw_data.image_cache_task.clone(),
@@ -366,6 +367,7 @@ impl LayoutTask {
             dirty: Rect::zero(),
             generation: rw_data.generation,
             new_animations_sender: rw_data.new_animations_sender.clone(),
+            goal: goal,
         }
     }
 
@@ -467,7 +469,8 @@ impl LayoutTask {
         let mut layout_context = self.build_shared_layout_context(&*rw_data,
                                                                   false,
                                                                   None,
-                                                                  &self.url);
+                                                                  &self.url,
+                                                                  reflow_info.goal);
 
         self.perform_post_style_recalc_layout_passes(&reflow_info,
                                                      &mut *rw_data,
@@ -751,7 +754,7 @@ impl LayoutTask {
         rw_data.content_boxes_response = iterator.rects;
     }
 
-    fn build_display_list_for_reflow<'a>(&'a self,
+    fn compute_abs_pos_and_build_display_list<'a>(&'a self,
                                                   data: &Reflow,
                                                   layout_root: &mut FlowRef,
                                                   shared_layout_context: &mut SharedLayoutContext,
@@ -773,7 +776,8 @@ impl LayoutTask {
 
             match rw_data.parallel_traversal {
                 None => {
-                    sequential::build_display_list_for_subtree(layout_root, shared_layout_context);
+                    sequential::build_display_list_for_subtree(layout_root,
+                                                               shared_layout_context);
                 }
                 Some(ref mut traversal) => {
                     parallel::build_display_list_for_subtree(layout_root,
@@ -784,6 +788,7 @@ impl LayoutTask {
                 }
             }
 
+            if data.goal == ReflowGoal::ForDisplay {
                 debug!("Done building display list.");
 
                 let root_background_color = get_root_flow_background_color(&mut **layout_root);
@@ -818,6 +823,7 @@ impl LayoutTask {
                 debug!("Layout done!");
 
                 self.paint_chan.send(PaintMsg::PaintInit(stacking_context));
+            }
         });
     }
 
@@ -878,7 +884,8 @@ impl LayoutTask {
         let mut shared_layout_context = self.build_shared_layout_context(&*rw_data,
                                                                          screen_size_changed,
                                                                          Some(&node),
-                                                                         &self.url);
+                                                                         &self.url,
+                                                                         data.reflow_info.goal);
 
         // Recalculate CSS styles and rebuild flows and fragments.
         profile(time::ProfilerCategory::LayoutStyleRecalc,
@@ -946,7 +953,8 @@ impl LayoutTask {
         let mut layout_context = self.build_shared_layout_context(&*rw_data,
                                                                   false,
                                                                   None,
-                                                                  &self.url);
+                                                                  &self.url,
+                                                                  reflow_info.goal);
         let mut root_flow = (*rw_data.root_flow.as_ref().unwrap()).clone();
         profile(time::ProfilerCategory::LayoutStyleRecalc,
                 self.profiler_metadata(),
@@ -1010,16 +1018,10 @@ impl LayoutTask {
         });
 
         // Build the display list if necessary, and send it to the painter.
-        match data.goal {
+        self.compute_abs_pos_and_build_display_list(data,
-            ReflowGoal::ForDisplay => {
-                self.build_display_list_for_reflow(data,
                                                     &mut root_flow,
                                                     &mut *layout_context,
                                                     rw_data);
-            }
-            ReflowGoal::ForScriptQuery => {}
-        }
-
         self.first_reflow.set(false);
 
         if opts::get().trace_layout {
@@ -1182,7 +1184,7 @@ impl FragmentBorderBoxIterator for UnioningFragmentBorderBoxIterator {
     }
 
     fn should_process(&mut self, fragment: &Fragment) -> bool {
-        self.node_address == fragment.node
+        fragment.contains_node(self.node_address)
     }
 }
 
@@ -1206,7 +1208,7 @@ impl FragmentBorderBoxIterator for CollectingFragmentBorderBoxIterator {
     }
 
     fn should_process(&mut self, fragment: &Fragment) -> bool {
-        self.node_address == fragment.node
+        fragment.contains_node(self.node_address)
     }
 }
 

components/layout/list_item.rs

@@ -172,7 +172,7 @@ impl Flow for ListItemFlow {
                                                              .base
                                                              .absolute_position_info
                                                              .relative_containing_block_mode,
-                                                         CoordinateSystem::Parent)
+                                                         CoordinateSystem::Own)
                            .translate(stacking_context_position));
             }
         }

components/layout/traversal.rs

@@ -14,6 +14,7 @@ use flow::{Flow, MutableFlowUtils};
 use flow::{PreorderFlowTraversal, PostorderFlowTraversal};
 use flow;
 use incremental::{self, BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, RestyleDamage};
+use script::layout_interface::ReflowGoal;
 use wrapper::{layout_node_to_unsafe_layout_node, LayoutNode};
 use wrapper::{PostorderNodeMutTraversal, ThreadSafeLayoutNode, UnsafeLayoutNode};
 use wrapper::{PreorderDomTraversal, PostorderDomTraversal};
@@ -378,5 +379,9 @@ impl<'a> PostorderFlowTraversal for BuildDisplayList<'a> {
     fn process(&self, flow: &mut Flow) {
         flow.build_display_list(self.layout_context);
     }
-}
 
+    #[inline]
+    fn should_process(&self, _: &mut Flow) -> bool {
+        self.layout_context.shared.goal == ReflowGoal::ForDisplay
+    }
+}

components/script/layout_interface.rs

@@ -88,7 +88,7 @@ pub struct HitTestResponse(pub UntrustedNodeAddress);
 pub struct MouseOverResponse(pub Vec<UntrustedNodeAddress>);
 
 /// Why we're doing reflow.
-#[derive(PartialEq, Debug)]
+#[derive(PartialEq, Copy, Debug)]
 pub enum ReflowGoal {
     /// We're reflowing in order to send a display list to the screen.
     ForDisplay,

tests/wpt/mozilla/tests/mozilla/getBoundingClientRect.html

@@ -3,18 +3,68 @@
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <style>
-div {
+@font-face {
+    font-family: 'ahem';
+    src: url(../css/fonts/ahem/ahem.ttf);
+}
+#rel-div {
     position: relative;
     top:  100px;
     left: 100px;
     width: 100px;
     height: 100px;
 }
+#abs1 {
+    position: absolute;
+    background-color: red;
+    top: 45px;
+    left: 55px;
+    width: 100px;
+    height: 120px;
+}
+#abs2 {
+    position: absolute;
+    background-color: green;
+    top: 5px;
+    left: 5px;
+    width: 80px;
+    height: 80px;
+}
+#abs3 {
+    position: absolute;
+    background-color: blue;
+    top: 12px;
+    left: 14px;
+    width: 48px;
+    height: 40px;
+}
+#span1 {
+    font-family: 'ahem';
+    font-size: 20px;
+    line-height: 1;
+}
 </style>
 </head>
 <body>
-    <div>my div</div>
+    <div id="rel-div">my div</div>
+    <div id="abs1">
+        <div id="abs2">
+            <div id="abs3">
+                <span id="span1">X</span>
+            </div>
+        </div>
+    </div>
     <script>
+    test_rect = function(name, left, top, bottom, right) {
+      var div = document.getElementById(name);
+      var rect = div.getBoundingClientRect();
+
+      assert_equals(rect.left, left);
+      assert_equals(rect.top, top);
+      assert_equals(rect.bottom, bottom);
+      assert_equals(rect.right, right);
+    }
+
     test(function() {
       assert_equals(String(DOMRect).indexOf("function DOMRect("), 0);
 
@@ -32,6 +82,13 @@ div {
       assert_equals(rect.width,  rect.right  - rect.left);
       assert_equals(rect.height, rect.bottom - rect.top);
     });
+
+    test(function() {
+        test_rect('abs1', 55, 45, 165, 155);
+        test_rect('abs2', 60, 50, 130, 140);
+        test_rect('abs3', 74, 62, 102, 122);
+        test_rect('span1', 74, 62, 82, 94);
+    });
     </script>
 </body>
 </html>