Add layout RPC query for getting an element's style

This enables us to implement Element::has_css_layout_box() in a more direct way, and also enables us to remove some of the existing more specific queries. Fixes #19811.
servo · Jan 28, 2018 · fe583fc · fe583fc
1 parent c9ba16f
commit fe583fc
Show file tree

Hide file tree

Showing 10 changed files with 88 additions and 136 deletions.
diff --git a/components/layout/query.rs b/components/layout/query.rs
@@ -16,9 +16,9 @@ use ipc_channel::ipc::IpcSender;
 use msg::constellation_msg::PipelineId;
 use opaque_node::OpaqueNodeMethods;
 use script_layout_interface::rpc::{ContentBoxResponse, ContentBoxesResponse, LayoutRPC};
-use script_layout_interface::rpc::{MarginStyleResponse, NodeGeometryResponse};
-use script_layout_interface::rpc::{NodeOverflowResponse, NodeScrollRootIdResponse};
-use script_layout_interface::rpc::{OffsetParentResponse, ResolvedStyleResponse, TextIndexResponse};
+use script_layout_interface::rpc::{NodeGeometryResponse, NodeScrollRootIdResponse};
+use script_layout_interface::rpc::{OffsetParentResponse, ResolvedStyleResponse, StyleResponse};
+use script_layout_interface::rpc::TextIndexResponse;
 use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode};
 use script_traits::LayoutMsg as ConstellationMsg;
 use script_traits::UntrustedNodeAddress;
@@ -59,9 +59,6 @@ pub struct LayoutThreadData {
     /// A queued response for the scroll root id for a given node.
     pub scroll_root_id_response: Option<ClipId>,
 
-    /// A pair of overflow property in x and y
-    pub overflow_response: NodeOverflowResponse,
-
     /// A queued response for the scroll {top, left, width, height} of a node in pixels.
     pub scroll_area_response: Rect<i32>,
 
@@ -71,8 +68,8 @@ pub struct LayoutThreadData {
     /// A queued response for the offset parent/rect of a node.
     pub offset_parent_response: OffsetParentResponse,
 
-    /// A queued response for the offset parent/rect of a node.
-    pub margin_style_response: MarginStyleResponse,
+    /// A queued response for the style of a node.
+    pub style_response: StyleResponse,
 
     /// Scroll offsets of scrolling regions.
     pub scroll_offsets: ScrollOffsetMap,
@@ -128,10 +125,6 @@ impl LayoutRPC for LayoutRPCImpl {
         }
     }
 
-    fn node_overflow(&self) -> NodeOverflowResponse {
-        NodeOverflowResponse(self.0.lock().unwrap().overflow_response.0)
-    }
-
     fn node_scroll_area(&self) -> NodeGeometryResponse {
         NodeGeometryResponse {
             client_rect: self.0.lock().unwrap().scroll_area_response
@@ -157,10 +150,10 @@ impl LayoutRPC for LayoutRPCImpl {
         rw_data.offset_parent_response.clone()
     }
 
-    fn margin_style(&self) -> MarginStyleResponse {
+    fn style(&self) -> StyleResponse {
         let &LayoutRPCImpl(ref rw_data) = self;
         let rw_data = rw_data.lock().unwrap();
-        rw_data.margin_style_response.clone()
+        rw_data.style_response.clone()
     }
 
     fn text_index(&self) -> TextIndexResponse {
@@ -863,24 +856,10 @@ pub fn process_offset_parent_query<N: LayoutNode>(requested_node: N, layout_root
     }
 }
 
-pub fn process_node_overflow_request<N: LayoutNode>(requested_node: N) -> NodeOverflowResponse {
-    let layout_node = requested_node.to_threadsafe();
-    let style = &*layout_node.as_element().unwrap().resolved_style();
-    let style_box = style.get_box();
-
-    NodeOverflowResponse(Some(Point2D::new(style_box.overflow_x, style_box.overflow_y)))
-}
-
-pub fn process_margin_style_query<N: LayoutNode>(requested_node: N)
-        -> MarginStyleResponse {
-    let layout_node = requested_node.to_threadsafe();
-    let style = &*layout_node.as_element().unwrap().resolved_style();
-    let margin = style.get_margin();
+pub fn process_style_query<N: LayoutNode>(requested_node: N)
+        -> StyleResponse {
+    let element = requested_node.as_element().unwrap();
+    let data = element.borrow_data();
 
-    MarginStyleResponse {
-        top: margin.margin_top,
-        right: margin.margin_right,
-        bottom: margin.margin_bottom,
-        left: margin.margin_left,
-    }
+    StyleResponse(data.map(|d| d.styles.primary().clone()))
 }
diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
@@ -75,9 +75,9 @@ use layout::incremental::{LayoutDamageComputation, RelayoutMode, SpecialRestyleD
 use layout::layout_debug;
 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::query::{process_node_scroll_root_id_request, process_offset_parent_query, process_resolved_style_request};
+use layout::query::process_style_query;
 use layout::sequential;
 use layout::traversal::{ComputeStackingRelativePositions, PreorderFlowTraversal, RecalcStyleAndConstructFlows};
 use layout::wrapper::LayoutNodeLayoutData;
@@ -94,7 +94,7 @@ use profile_traits::time::{self, TimerMetadata, profile};
 use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType};
 use script_layout_interface::message::{Msg, NewLayoutThreadInfo, NodesFromPointQueryType, Reflow};
 use script_layout_interface::message::{ReflowComplete, ReflowGoal, ScriptReflow};
-use script_layout_interface::rpc::{LayoutRPC, MarginStyleResponse, NodeOverflowResponse, OffsetParentResponse};
+use script_layout_interface::rpc::{LayoutRPC, StyleResponse, OffsetParentResponse};
 use script_layout_interface::rpc::TextIndexResponse;
 use script_layout_interface::wrapper_traits::LayoutNode;
 use script_traits::{ConstellationControlMsg, LayoutControlMsg, LayoutMsg as ConstellationMsg};
@@ -519,10 +519,9 @@ impl LayoutThread {
                     client_rect_response: Rect::zero(),
                     scroll_root_id_response: None,
                     scroll_area_response: Rect::zero(),
-                    overflow_response: NodeOverflowResponse(None),
                     resolved_style_response: String::new(),
                     offset_parent_response: OffsetParentResponse::empty(),
-                    margin_style_response: MarginStyleResponse::empty(),
+                    style_response: StyleResponse(None),
                     scroll_offsets: HashMap::new(),
                     text_index_response: TextIndexResponse(None),
                     nodes_from_point_response: vec![],
@@ -1092,9 +1091,6 @@ impl LayoutThread {
                     ReflowGoal::NodeScrollGeometryQuery(_) => {
                         rw_data.scroll_area_response = Rect::zero();
                     },
-                    ReflowGoal::NodeOverflowQuery(_) => {
-                        rw_data.overflow_response = NodeOverflowResponse(None);
-                    },
                     ReflowGoal::NodeScrollRootIdQuery(_) => {
                         rw_data.scroll_root_id_response = None;
                     },
@@ -1104,8 +1100,8 @@ impl LayoutThread {
                     ReflowGoal::OffsetParentQuery(_) => {
                         rw_data.offset_parent_response = OffsetParentResponse::empty();
                     },
-                    ReflowGoal::MarginStyleQuery(_) => {
-                        rw_data.margin_style_response = MarginStyleResponse::empty();
+                    ReflowGoal::StyleQuery(_) => {
+                        rw_data.style_response = StyleResponse(None);
                     },
                     ReflowGoal::TextIndexQuery(..) => {
                         rw_data.text_index_response = TextIndexResponse(None);
@@ -1379,10 +1375,6 @@ impl LayoutThread {
                 let node = unsafe { ServoLayoutNode::new(&node) };
                 rw_data.scroll_area_response = process_node_scroll_area_request(node, root_flow);
             },
-            ReflowGoal::NodeOverflowQuery(node) => {
-                let node = unsafe { ServoLayoutNode::new(&node) };
-                rw_data.overflow_response = process_node_overflow_request(node);
-            },
             ReflowGoal::NodeScrollRootIdQuery(node) => {
                 let node = unsafe { ServoLayoutNode::new(&node) };
                 rw_data.scroll_root_id_response = Some(process_node_scroll_root_id_request(self.id,
@@ -1401,9 +1393,9 @@ impl LayoutThread {
                 let node = unsafe { ServoLayoutNode::new(&node) };
                 rw_data.offset_parent_response = process_offset_parent_query(node, root_flow);
             },
-            ReflowGoal::MarginStyleQuery(node) => {
+            ReflowGoal::StyleQuery(node) => {
                 let node = unsafe { ServoLayoutNode::new(&node) };
-                rw_data.margin_style_response = process_margin_style_query(node);
+                rw_data.style_response = process_style_query(node);
             },
             ReflowGoal::NodesFromPointQuery(client_point, ref reflow_goal) => {
                 let mut flags = match reflow_goal {

diff --git a/components/script/devtools.rs b/components/script/devtools.rs
@@ -154,12 +154,13 @@ pub fn handle_get_layout(documents: &Documents,
 }
 
 fn determine_auto_margins(window: &Window, node: &Node) -> AutoMargins {
-    let margin = window.margin_style_query(node.to_trusted_node_address());
+    let style = window.style_query(node.to_trusted_node_address()).unwrap();
+    let margin = style.get_margin();
     AutoMargins {
-        top: margin.top == margin_top::computed_value::T::Auto,
-        right: margin.right == margin_right::computed_value::T::Auto,
-        bottom: margin.bottom == margin_bottom::computed_value::T::Auto,
-        left: margin.left == margin_left::computed_value::T::Auto,
+        top: margin.margin_top == margin_top::computed_value::T::Auto,
+        right: margin.margin_right == margin_right::computed_value::T::Auto,
+        bottom: margin.margin_bottom == margin_bottom::computed_value::T::Auto,
+        left: margin.margin_left == margin_left::computed_value::T::Auto,
     }
 }
 

diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
@@ -348,60 +348,58 @@ impl Element {
     }
 
     // https://drafts.csswg.org/cssom-view/#css-layout-box
-    //
-    // We'll have no content box if there's no fragment for the node, and we use
-    // bounding_content_box, for simplicity, to detect this (rather than making a more specific
-    // query to the layout thread).
     fn has_css_layout_box(&self) -> bool {
-        self.upcast::<Node>().bounding_content_box().is_some()
+        let style = self.upcast::<Node>().style();
+
+        // style will be None for elements in a display: none subtree. otherwise, the element has a
+        // layout box iff it doesn't have display: none.
+        style.map_or(false, |s| !s.get_box().clone_display().is_none())
     }
 
     // https://drafts.csswg.org/cssom-view/#potentially-scrollable
     fn potentially_scrollable(&self) -> bool {
-        self.has_css_layout_box() &&
-        !self.overflow_x_is_visible() &&
-        !self.overflow_y_is_visible()
+        self.has_css_layout_box() && !self.has_any_visible_overflow()
     }
 
     // https://drafts.csswg.org/cssom-view/#scrolling-box
     fn has_scrolling_box(&self) -> bool {
         // TODO: scrolling mechanism, such as scrollbar (We don't have scrollbar yet)
         //       self.has_scrolling_mechanism()
-        self.overflow_x_is_hidden() ||
-        self.overflow_y_is_hidden()
+        self.has_any_hidden_overflow()
     }
 
     fn has_overflow(&self) -> bool {
         self.ScrollHeight() > self.ClientHeight() ||
         self.ScrollWidth() > self.ClientWidth()
     }
 
-    // used value of overflow-x is "visible"
-    fn overflow_x_is_visible(&self) -> bool {
-        let window = window_from_node(self);
-        let overflow_pair = window.overflow_query(self.upcast::<Node>().to_trusted_node_address());
-        overflow_pair.x == overflow_x::computed_value::T::Visible
-    }
+    // TODO: Once #19183 is closed (overflow-x/y types moved out of mako), then we could implement
+    //       a more generic `fn has_some_overflow(&self, overflow: Overflow)` rather than have
+    //       these two `has_any_{visible,hidden}_overflow` methods which are very structurally
+    //       similar.
 
-    // used value of overflow-y is "visible"
-    fn overflow_y_is_visible(&self) -> bool {
-        let window = window_from_node(self);
-        let overflow_pair = window.overflow_query(self.upcast::<Node>().to_trusted_node_address());
-        overflow_pair.y == overflow_y::computed_value::T::Visible
-    }
+    /// Computed value of overflow-x or overflow-y is "visible"
+    fn has_any_visible_overflow(&self) -> bool {
+        let style = self.upcast::<Node>().style();
 
-    // used value of overflow-x is "hidden"
-    fn overflow_x_is_hidden(&self) -> bool {
-        let window = window_from_node(self);
-        let overflow_pair = window.overflow_query(self.upcast::<Node>().to_trusted_node_address());
-        overflow_pair.x == overflow_x::computed_value::T::Hidden
+        style.map_or(false, |s| {
+            let box_ = s.get_box();
+
+            box_.clone_overflow_x() == overflow_x::computed_value::T::Visible ||
+                box_.clone_overflow_y() == overflow_y::computed_value::T::Visible
+        })
     }
 
-    // used value of overflow-y is "hidden"
-    fn overflow_y_is_hidden(&self) -> bool {
-        let window = window_from_node(self);
-        let overflow_pair = window.overflow_query(self.upcast::<Node>().to_trusted_node_address());
-        overflow_pair.y == overflow_y::computed_value::T::Hidden
+    /// Computed value of overflow-x or overflow-y is "hidden"
+    fn has_any_hidden_overflow(&self) -> bool {
+        let style = self.upcast::<Node>().style();
+
+        style.map_or(false, |s| {
+            let box_ = s.get_box();
+
+            box_.clone_overflow_x() == overflow_x::computed_value::T::Hidden ||
+                box_.clone_overflow_y() == overflow_y::computed_value::T::Hidden
+        })
     }
 }
 

diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs
@@ -82,6 +82,7 @@ use std::mem;
 use std::ops::Range;
 use style::context::QuirksMode;
 use style::dom::OpaqueNode;
+use style::properties::ComputedValues;
 use style::selector_parser::{SelectorImpl, SelectorParser};
 use style::stylesheets::Stylesheet;
 use style::thread_state;
@@ -619,6 +620,10 @@ impl Node {
         window_from_node(self).client_rect_query(self.to_trusted_node_address())
     }
 
+    pub fn style(&self) -> Option<Arc<ComputedValues>> {
+        window_from_node(self).style_query(self.to_trusted_node_address())
+    }
+
     // https://drafts.csswg.org/cssom-view/#dom-element-scrollwidth
     // https://drafts.csswg.org/cssom-view/#dom-element-scrollheight
     // https://drafts.csswg.org/cssom-view/#dom-element-scrolltop

diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
@@ -72,8 +72,7 @@ use script_layout_interface::{TrustedNodeAddress, PendingImageState};
 use script_layout_interface::message::{Msg, Reflow, ReflowGoal, ScriptReflow};
 use script_layout_interface::reporter::CSSErrorReporter;
 use script_layout_interface::rpc::{ContentBoxResponse, ContentBoxesResponse, LayoutRPC};
-use script_layout_interface::rpc::{MarginStyleResponse, NodeScrollRootIdResponse};
-use script_layout_interface::rpc::{ResolvedStyleResponse, TextIndexResponse};
+use script_layout_interface::rpc::{NodeScrollRootIdResponse, ResolvedStyleResponse, TextIndexResponse};
 use script_runtime::{CommonScriptMsg, ScriptChan, ScriptPort, ScriptThreadEventCategory, Runtime};
 use script_thread::{ImageCacheMsg, MainThreadScriptChan, MainThreadScriptMsg};
 use script_thread::{ScriptThread, SendableMainThreadScriptChan};
@@ -82,6 +81,7 @@ use script_traits::{ScriptToConstellationChan, ScriptMsg, ScrollState, TimerEven
 use script_traits::{TimerSchedulerMsg, UntrustedNodeAddress, WindowSizeData, WindowSizeType};
 use script_traits::webdriver_msg::{WebDriverJSError, WebDriverJSResult};
 use selectors::attr::CaseSensitivity;
+use servo_arc;
 use servo_config::opts;
 use servo_config::prefs::PREFS;
 use servo_geometry::{f32_rect_to_au_rect, MaxRect};
@@ -102,8 +102,7 @@ use std::sync::mpsc::{Sender, channel};
 use std::sync::mpsc::TryRecvError::{Disconnected, Empty};
 use style::media_queries;
 use style::parser::ParserContext as CssParserContext;
-use style::properties::PropertyId;
-use style::properties::longhands::overflow_x;
+use style::properties::{ComputedValues, PropertyId};
 use style::selector_parser::PseudoElement;
 use style::str::HTML_SPACE_CHARACTERS;
 use style::stylesheets::CssRuleType;
@@ -1403,16 +1402,6 @@ impl Window {
         self.layout_rpc.node_scroll_area().client_rect
     }
 
-    pub fn overflow_query(&self,
-                          node: TrustedNodeAddress) -> Point2D<overflow_x::computed_value::T> {
-        // NB: This is only called if the document is fully active, and the only
-        // reason to bail out from a query is if there's no viewport, so this
-        // *must* issue a reflow.
-        assert!(self.reflow(ReflowGoal::NodeOverflowQuery(node), ReflowReason::Query));
-
-        self.layout_rpc.node_overflow().0.unwrap()
-    }
-
     pub fn scroll_offset_query(&self, node: &Node) -> Vector2D<f32> {
         if let Some(scroll_offset) = self.scroll_offsets
                                          .borrow()
@@ -1477,11 +1466,11 @@ impl Window {
         (element, response.rect)
     }
 
-    pub fn margin_style_query(&self, node: TrustedNodeAddress) -> MarginStyleResponse {
-        if !self.reflow(ReflowGoal::MarginStyleQuery(node), ReflowReason::Query) {
-            return MarginStyleResponse::empty();
+    pub fn style_query(&self, node: TrustedNodeAddress) -> Option<servo_arc::Arc<ComputedValues>> {
+        if !self.reflow(ReflowGoal::StyleQuery(node), ReflowReason::Query) {
+            return None
         }
-        self.layout_rpc.margin_style()
+        self.layout_rpc.style().0
     }
 
     pub fn text_index_query(
@@ -1898,12 +1887,11 @@ fn debug_reflow_events(id: PipelineId, reflow_goal: &ReflowGoal, reason: &Reflow
         ReflowGoal::ContentBoxesQuery(_n) => "\tContentBoxesQuery",
         ReflowGoal::NodesFromPointQuery(..) => "\tNodesFromPointQuery",
         ReflowGoal::NodeGeometryQuery(_n) => "\tNodeGeometryQuery",
-        ReflowGoal::NodeOverflowQuery(_n) => "\tNodeOverFlowQuery",
         ReflowGoal::NodeScrollGeometryQuery(_n) => "\tNodeScrollGeometryQuery",
         ReflowGoal::NodeScrollRootIdQuery(_n) => "\tNodeScrollRootIdQuery",
         ReflowGoal::ResolvedStyleQuery(_, _, _) => "\tResolvedStyleQuery",
         ReflowGoal::OffsetParentQuery(_n) => "\tOffsetParentQuery",
-        ReflowGoal::MarginStyleQuery(_n) => "\tMarginStyleQuery",
+        ReflowGoal::StyleQuery(_n) => "\tStyleQuery",
         ReflowGoal::TextIndexQuery(..) => "\tTextIndexQuery",
         ReflowGoal::TickAnimations => "\tTickAnimations",
     });