servo: Merge #15768 - Trigger reflow on document.elementsFromPoint (from ferjm:issue-15592-document-elementsFromPoint); r=emilio
authorFernando Jiménez Moreno <ferjmoreno@gmail.com>
Thu, 02 Mar 2017 11:43:17 -0800
changeset 394633 9fbb6524544f306147501fb9e6f6379db3bd69df
parent 394632 077b849d2beb99a92dc9ec8caa8d2023b170dcbc
child 394634 f7fe1818b01efa2430c2edcaedd427fa820d97b8
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #15768 - Trigger reflow on document.elementsFromPoint (from ferjm:issue-15592-document-elementsFromPoint); r=emilio As [suggested](https://github.com/servo/servo/issues/15592#issuecomment-280379805) by @jdm `Document::nodes_from_point` now triggers a reflow. I added a new reftest that panics with `ERROR:servo: Tried to hit test without a DisplayList` if this patch is not applied. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #15592. - [X] There are tests for these changes Source-Repo: https://github.com/servo/servo Source-Revision: fa32d50c7a2fc9cb29c7245dc45a46ed68551601
servo/components/layout/lib.rs
servo/components/layout/query.rs
servo/components/layout_thread/lib.rs
servo/components/script/dom/document.rs
servo/components/script/dom/window.rs
servo/components/script_layout_interface/message.rs
servo/components/script_layout_interface/rpc.rs
--- a/servo/components/layout/lib.rs
+++ b/servo/components/layout/lib.rs
@@ -68,17 +68,17 @@ pub mod flow_ref;
 mod fragment;
 mod generated_content;
 pub mod incremental;
 mod inline;
 mod linked_list;
 mod list_item;
 mod model;
 mod multicol;
-mod opaque_node;
+pub mod opaque_node;
 pub mod parallel;
 mod persistent_list;
 pub mod query;
 pub mod sequential;
 mod table;
 mod table_caption;
 mod table_cell;
 mod table_colgroup;
--- a/servo/components/layout/query.rs
+++ b/servo/components/layout/query.rs
@@ -89,16 +89,19 @@ pub struct LayoutThreadData {
     /// Scroll offsets of stacking contexts. This will only be populated if WebRender is in use.
     pub stacking_context_scroll_offsets: ScrollOffsetMap,
 
     /// Index in a text fragment. We need this do determine the insertion point.
     pub text_index_response: TextIndexResponse,
 
     /// A list of images requests that need to be initiated.
     pub pending_images: Vec<PendingImage>,
+
+    /// A queued response for the list of nodes at a given point.
+    pub nodes_from_point_response: Vec<UntrustedNodeAddress>,
 }
 
 pub struct LayoutRPCImpl(pub Arc<Mutex<LayoutThreadData>>);
 
 // https://drafts.csswg.org/cssom-view/#overflow-directions
 fn overflow_direction(writing_mode: &WritingMode) -> OverflowDirection {
     match (writing_mode.block_flow_direction(), writing_mode.inline_base_direction()) {
         (BlockFlowDirection::TopToBottom, InlineBaseDirection::LeftToRight) |
@@ -139,43 +142,20 @@ impl LayoutRPC for LayoutRPCImpl {
             };
             rw_data.constellation_chan.send(ConstellationMsg::SetCursor(cursor)).unwrap();
         }
         HitTestResponse {
             node_address: result.map(|dim| dim.node.to_untrusted_node_address()),
         }
     }
 
-    fn nodes_from_point(&self,
-                        page_point: Point2D<f32>,
-                        client_point: Point2D<f32>) -> Vec<UntrustedNodeAddress> {
-        let page_point = Point2D::new(Au::from_f32_px(page_point.x),
-                                      Au::from_f32_px(page_point.y));
-        let client_point = Point2D::new(Au::from_f32_px(client_point.x),
-                                        Au::from_f32_px(client_point.y));
-
-        let nodes_from_point_list = {
-            let &LayoutRPCImpl(ref rw_data) = self;
-            let rw_data = rw_data.lock().unwrap();
-            let result = match rw_data.display_list {
-                None => panic!("Tried to hit test without a DisplayList"),
-                Some(ref display_list) => {
-                    display_list.hit_test(&page_point,
-                                          &client_point,
-                                          &rw_data.stacking_context_scroll_offsets)
-                }
-            };
-
-            result
-        };
-
-        nodes_from_point_list.iter()
-           .rev()
-           .map(|metadata| metadata.node.to_untrusted_node_address())
-           .collect()
+    fn nodes_from_point_response(&self) -> Vec<UntrustedNodeAddress> {
+        let &LayoutRPCImpl(ref rw_data) = self;
+        let rw_data = rw_data.lock().unwrap();
+        rw_data.nodes_from_point_response.clone()
     }
 
     fn node_geometry(&self) -> NodeGeometryResponse {
         let &LayoutRPCImpl(ref rw_data) = self;
         let rw_data = rw_data.lock().unwrap();
         NodeGeometryResponse {
             client_rect: rw_data.client_rect_response
         }
@@ -188,18 +168,18 @@ impl LayoutRPC for LayoutRPCImpl {
     fn node_scroll_area(&self) -> NodeGeometryResponse {
         NodeGeometryResponse {
             client_rect: self.0.lock().unwrap().scroll_area_response
         }
     }
 
     fn node_scroll_root_id(&self) -> NodeScrollRootIdResponse {
         NodeScrollRootIdResponse(self.0.lock()
-                                        .unwrap().scroll_root_id_response
-                                        .expect("scroll_root_id is not correctly fetched"))
+                                       .unwrap().scroll_root_id_response
+                                       .expect("scroll_root_id is not correctly fetched"))
     }
 
     /// Retrieves the resolved value for a CSS style property.
     fn resolved_style(&self) -> ResolvedStyleResponse {
         let &LayoutRPCImpl(ref rw_data) = self;
         let rw_data = rw_data.lock().unwrap();
         ResolvedStyleResponse(rw_data.resolved_style_response.clone())
     }
--- a/servo/components/layout_thread/lib.rs
+++ b/servo/components/layout_thread/lib.rs
@@ -58,16 +58,17 @@ use layout::animation;
 use layout::construct::ConstructionResult;
 use layout::context::LayoutContext;
 use layout::context::heap_size_of_persistent_local_context;
 use layout::display_list_builder::ToGfxColor;
 use layout::flow::{self, Flow, ImmutableFlowUtils, MutableFlowUtils, MutableOwnedFlowUtils};
 use layout::flow_ref::FlowRef;
 use layout::incremental::{LayoutDamageComputation, REFLOW_ENTIRE_DOCUMENT};
 use layout::layout_debug;
+use layout::opaque_node::OpaqueNodeMethods;
 use layout::parallel;
 use layout::query::{LayoutRPCImpl, LayoutThreadData, process_content_box_request, process_content_boxes_request};
 use layout::query::{process_margin_style_query, process_node_overflow_request, process_resolved_style_request};
 use layout::query::{process_node_geometry_request, process_node_scroll_area_request};
 use layout::query::{process_node_scroll_root_id_request, process_offset_parent_query};
 use layout::sequential;
 use layout::traversal::{ComputeAbsolutePositions, RecalcStyleAndConstructFlows};
 use layout::webrender_helpers::WebRenderDisplayListConverter;
@@ -454,16 +455,17 @@ impl LayoutThread {
                     scroll_area_response: Rect::zero(),
                     overflow_response: NodeOverflowResponse(None),
                     resolved_style_response: String::new(),
                     offset_parent_response: OffsetParentResponse::empty(),
                     margin_style_response: MarginStyleResponse::empty(),
                     stacking_context_scroll_offsets: HashMap::new(),
                     text_index_response: TextIndexResponse(None),
                     pending_images: vec![],
+                    nodes_from_point_response: vec![],
                 })),
             error_reporter: CSSErrorReporter {
                 pipelineid: id,
                 script_chan: Arc::new(Mutex::new(script_chan)),
             },
             webrender_image_cache:
                 Arc::new(RwLock::new(HashMap::with_hasher(Default::default()))),
             timer:
@@ -972,16 +974,19 @@ impl LayoutThread {
                         rw_data.content_box_response = None;
                     },
                     ReflowQueryType::ContentBoxesQuery(_) => {
                         rw_data.content_boxes_response = Vec::new();
                     },
                     ReflowQueryType::HitTestQuery(..) => {
                         rw_data.hit_test_response = (None, false);
                     },
+                    ReflowQueryType::NodesFromPoint(..) => {
+                        rw_data.nodes_from_point_response = Vec::new();
+                    },
                     ReflowQueryType::NodeGeometryQuery(_) => {
                         rw_data.client_rect_response = Rect::zero();
                     },
                     ReflowQueryType::NodeScrollGeometryQuery(_) => {
                         rw_data.scroll_area_response = Rect::zero();
                     },
                     ReflowQueryType::NodeOverflowQuery(_) => {
                         rw_data.overflow_response = NodeOverflowResponse(None);
@@ -1274,16 +1279,39 @@ impl LayoutThread {
             ReflowQueryType::OffsetParentQuery(node) => {
                 let node = unsafe { ServoLayoutNode::new(&node) };
                 rw_data.offset_parent_response = process_offset_parent_query(node, root_flow);
             },
             ReflowQueryType::MarginStyleQuery(node) => {
                 let node = unsafe { ServoLayoutNode::new(&node) };
                 rw_data.margin_style_response = process_margin_style_query(node);
             },
+            ReflowQueryType::NodesFromPoint(page_point, client_point) => {
+                let page_point = Point2D::new(Au::from_f32_px(page_point.x),
+                                              Au::from_f32_px(page_point.y));
+                let client_point = Point2D::new(Au::from_f32_px(client_point.x),
+                                                Au::from_f32_px(client_point.y));
+                let nodes_from_point_list = {
+                    let result = match rw_data.display_list {
+                        None => panic!("Tried to hit test without a DisplayList"),
+                        Some(ref display_list) => {
+                            display_list.hit_test(&page_point,
+                                                  &client_point,
+                                                  &rw_data.stacking_context_scroll_offsets)
+                        }
+                    };
+
+                    result
+                };
+
+                rw_data.nodes_from_point_response = nodes_from_point_list.iter()
+                   .rev()
+                   .map(|metadata| metadata.node.to_untrusted_node_address())
+                   .collect()
+            },
             ReflowQueryType::NoQuery => {}
         }
     }
 
     fn set_stacking_context_scroll_states<'a, 'b>(
             &mut self,
             new_scroll_states: Vec<StackingContextScrollState>,
             possibly_locked_rw_data: &mut RwData<'a, 'b>) {
@@ -1580,17 +1608,18 @@ fn get_ua_stylesheets() -> Result<UserAg
         quirks_mode_stylesheet: quirks_mode_stylesheet,
     })
 }
 
 /// Returns true if the given reflow query type needs a full, up-to-date display list to be present
 /// or false if it only needs stacking-relative positions.
 fn reflow_query_type_needs_display_list(query_type: &ReflowQueryType) -> bool {
     match *query_type {
-        ReflowQueryType::HitTestQuery(..) | ReflowQueryType::TextIndexQuery(..) => true,
+        ReflowQueryType::HitTestQuery(..) | ReflowQueryType::TextIndexQuery(..) |
+        ReflowQueryType::NodesFromPoint(..) => true,
         ReflowQueryType::ContentBoxQuery(_) | ReflowQueryType::ContentBoxesQuery(_) |
         ReflowQueryType::NodeGeometryQuery(_) | ReflowQueryType::NodeScrollGeometryQuery(_) |
         ReflowQueryType::NodeOverflowQuery(_) | ReflowQueryType::NodeScrollRootIdQuery(_) |
         ReflowQueryType::ResolvedStyleQuery(..) | ReflowQueryType::OffsetParentQuery(_) |
         ReflowQueryType::MarginStyleQuery(_) | ReflowQueryType::NoQuery => false,
     }
 }
 
--- a/servo/components/script/dom/document.rs
+++ b/servo/components/script/dom/document.rs
@@ -1872,17 +1872,23 @@ impl Document {
         !self.has_browsing_context || !url_has_network_scheme(&self.url())
     }
 
     pub fn nodes_from_point(&self, client_point: &Point2D<f32>) -> Vec<UntrustedNodeAddress> {
         let page_point =
             Point2D::new(client_point.x + self.window.PageXOffset() as f32,
                          client_point.y + self.window.PageYOffset() as f32);
 
-        self.window.layout().nodes_from_point(page_point, *client_point)
+        if !self.window.reflow(ReflowGoal::ForScriptQuery,
+                               ReflowQueryType::NodesFromPoint(page_point, *client_point),
+                               ReflowReason::Query) {
+            return vec!();
+        };
+
+        self.window.layout().nodes_from_point_response()
     }
 }
 
 #[derive(PartialEq, HeapSizeOf)]
 pub enum DocumentSource {
     FromParser,
     NotFromParser,
 }
--- a/servo/components/script/dom/window.rs
+++ b/servo/components/script/dom/window.rs
@@ -1806,16 +1806,17 @@ fn debug_reflow_events(id: PipelineId, g
         ReflowGoal::ForScriptQuery => "\tForScriptQuery",
     });
 
     debug_msg.push_str(match *query_type {
         ReflowQueryType::NoQuery => "\tNoQuery",
         ReflowQueryType::ContentBoxQuery(_n) => "\tContentBoxQuery",
         ReflowQueryType::ContentBoxesQuery(_n) => "\tContentBoxesQuery",
         ReflowQueryType::HitTestQuery(..) => "\tHitTestQuery",
+        ReflowQueryType::NodesFromPoint(..) => "\tNodesFromPoint",
         ReflowQueryType::NodeGeometryQuery(_n) => "\tNodeGeometryQuery",
         ReflowQueryType::NodeOverflowQuery(_n) => "\tNodeOverFlowQuery",
         ReflowQueryType::NodeScrollGeometryQuery(_n) => "\tNodeScrollGeometryQuery",
         ReflowQueryType::NodeScrollRootIdQuery(_n) => "\tNodeScrollRootIdQuery",
         ReflowQueryType::ResolvedStyleQuery(_, _, _) => "\tResolvedStyleQuery",
         ReflowQueryType::OffsetParentQuery(_n) => "\tOffsetParentQuery",
         ReflowQueryType::MarginStyleQuery(_n) => "\tMarginStyleQuery",
         ReflowQueryType::TextIndexQuery(..) => "\tTextIndexQuery",
--- a/servo/components/script_layout_interface/message.rs
+++ b/servo/components/script_layout_interface/message.rs
@@ -96,16 +96,17 @@ pub enum ReflowQueryType {
     HitTestQuery(Point2D<f32>, Point2D<f32>, bool),
     NodeScrollRootIdQuery(TrustedNodeAddress),
     NodeGeometryQuery(TrustedNodeAddress),
     NodeScrollGeometryQuery(TrustedNodeAddress),
     ResolvedStyleQuery(TrustedNodeAddress, Option<PseudoElement>, PropertyId),
     OffsetParentQuery(TrustedNodeAddress),
     MarginStyleQuery(TrustedNodeAddress),
     TextIndexQuery(TrustedNodeAddress, i32, i32),
+    NodesFromPoint(Point2D<f32>, Point2D<f32>),
 }
 
 /// Information needed for a reflow.
 pub struct Reflow {
     /// The goal of reflow: either to render to the screen or to flush layout info for script.
     pub goal: ReflowGoal,
     ///  A clipping rectangle for the page, an enlarged rectangle containing the viewport.
     pub page_clip_rect: Rect<Au>,
--- a/servo/components/script_layout_interface/rpc.rs
+++ b/servo/components/script_layout_interface/rpc.rs
@@ -35,17 +35,18 @@ pub trait LayoutRPC {
     fn hit_test(&self) -> HitTestResponse;
     /// Query layout for the resolved value of a given CSS property
     fn resolved_style(&self) -> ResolvedStyleResponse;
     fn offset_parent(&self) -> OffsetParentResponse;
     /// Query layout for the resolve values of the margin properties for an element.
     fn margin_style(&self) -> MarginStyleResponse;
     /// Requests the list of not-yet-loaded images that were encountered in the last reflow.
     fn pending_images(&self) -> Vec<PendingImage>;
-    fn nodes_from_point(&self, page_point: Point2D<f32>, client_point: Point2D<f32>) -> Vec<UntrustedNodeAddress>;
+    /// Requests the list of nodes from the given point.
+    fn nodes_from_point_response(&self) -> Vec<UntrustedNodeAddress>;
 
     fn text_index(&self) -> TextIndexResponse;
 }
 
 pub struct ContentBoxResponse(pub Option<Rect<Au>>);
 
 pub struct ContentBoxesResponse(pub Vec<Rect<Au>>);