From c10e8399246e5254a080c2dc809d1c180f06cc93 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 7 Apr 2020 13:13:33 +0200 Subject: [PATCH] Don't go through the layout thread to retrieve a node's primary style --- components/layout/query.rs | 18 +----------------- components/layout_2020/query.rs | 15 +-------------- components/layout_thread/lib.rs | 16 ++++------------ components/layout_thread_2020/lib.rs | 15 ++++----------- components/script/devtools.rs | 7 +++---- components/script/dom/element.rs | 2 +- components/script/dom/node.rs | 19 +++++++++++++++++++ components/script/dom/window.rs | 11 ++--------- components/script_layout_interface/message.rs | 6 +++--- components/script_layout_interface/rpc.rs | 8 -------- 10 files changed, 38 insertions(+), 79 deletions(-) diff --git a/components/layout/query.rs b/components/layout/query.rs index 9cff8fa61c47..a5f66cb9dcd9 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -22,7 +22,7 @@ use msg::constellation_msg::PipelineId; use script_layout_interface::rpc::TextIndexResponse; use script_layout_interface::rpc::{ContentBoxResponse, ContentBoxesResponse, LayoutRPC}; use script_layout_interface::rpc::{NodeGeometryResponse, NodeScrollIdResponse}; -use script_layout_interface::rpc::{OffsetParentResponse, ResolvedStyleResponse, StyleResponse}; +use script_layout_interface::rpc::{OffsetParentResponse, ResolvedStyleResponse}; use script_layout_interface::wrapper_traits::{ LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode, }; @@ -76,9 +76,6 @@ pub struct LayoutThreadData { /// A queued response for the offset parent/rect of a node. pub offset_parent_response: OffsetParentResponse, - /// A queued response for the style of a node. - pub style_response: StyleResponse, - /// Scroll offsets of scrolling regions. pub scroll_offsets: ScrollOffsetMap, @@ -179,12 +176,6 @@ impl LayoutRPC for LayoutRPCImpl { rw_data.offset_parent_response.clone() } - fn style(&self) -> StyleResponse { - let &LayoutRPCImpl(ref rw_data) = self; - let rw_data = rw_data.lock().unwrap(); - rw_data.style_response.clone() - } - fn text_index(&self) -> TextIndexResponse { let &LayoutRPCImpl(ref rw_data) = self; let rw_data = rw_data.lock().unwrap(); @@ -965,13 +956,6 @@ pub fn process_offset_parent_query( } } -pub fn process_style_query<'dom>(requested_node: impl LayoutNode<'dom>) -> StyleResponse { - let element = requested_node.as_element().unwrap(); - let data = element.borrow_data(); - - StyleResponse(data.map(|d| d.styles.primary().clone())) -} - enum InnerTextItem { Text(String), RequiredLineBreakCount(u32), diff --git a/components/layout_2020/query.rs b/components/layout_2020/query.rs index fc4ac9852530..b863d58cdc3e 100644 --- a/components/layout_2020/query.rs +++ b/components/layout_2020/query.rs @@ -15,7 +15,7 @@ use msg::constellation_msg::PipelineId; use script_layout_interface::rpc::TextIndexResponse; use script_layout_interface::rpc::{ContentBoxResponse, ContentBoxesResponse, LayoutRPC}; use script_layout_interface::rpc::{NodeGeometryResponse, NodeScrollIdResponse}; -use script_layout_interface::rpc::{OffsetParentResponse, ResolvedStyleResponse, StyleResponse}; +use script_layout_interface::rpc::{OffsetParentResponse, ResolvedStyleResponse}; use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutNode}; use script_traits::LayoutMsg as ConstellationMsg; use script_traits::UntrustedNodeAddress; @@ -59,9 +59,6 @@ pub struct LayoutThreadData { /// A queued response for the offset parent/rect of a node. pub offset_parent_response: OffsetParentResponse, - /// A queued response for the style of a node. - pub style_response: StyleResponse, - /// Scroll offsets of scrolling regions. pub scroll_offsets: HashMap>, @@ -139,12 +136,6 @@ impl LayoutRPC for LayoutRPCImpl { rw_data.offset_parent_response.clone() } - fn style(&self) -> StyleResponse { - let &LayoutRPCImpl(ref rw_data) = self; - let rw_data = rw_data.lock().unwrap(); - rw_data.style_response.clone() - } - fn text_index(&self) -> TextIndexResponse { let &LayoutRPCImpl(ref rw_data) = self; let rw_data = rw_data.lock().unwrap(); @@ -220,10 +211,6 @@ pub fn process_offset_parent_query(_requested_node: OpaqueNode) -> OffsetParentR OffsetParentResponse::empty() } -pub fn process_style_query<'dom>(_requested_node: impl LayoutNode<'dom>) -> StyleResponse { - StyleResponse(None) -} - // https://html.spec.whatwg.org/multipage/#the-innertext-idl-attribute pub fn process_element_inner_text_query<'dom>(_node: impl LayoutNode<'dom>) -> String { "".to_owned() diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 453067de4cdf..98cdef1492b9 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -55,9 +55,7 @@ use layout::query::{ process_content_box_request, process_content_boxes_request, LayoutRPCImpl, LayoutThreadData, }; use layout::query::{process_node_scroll_area_request, process_node_scroll_id_request}; -use layout::query::{ - process_offset_parent_query, process_resolved_style_request, process_style_query, -}; +use layout::query::{process_offset_parent_query, process_resolved_style_request}; use layout::sequential; use layout::traversal::{ ComputeStackingRelativePositions, PreorderFlowTraversal, RecalcStyleAndConstructFlows, @@ -80,7 +78,7 @@ use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType}; use script_layout_interface::message::{LayoutThreadInit, Msg, NodesFromPointQueryType, Reflow}; use script_layout_interface::message::{QueryMsg, ReflowComplete, ReflowGoal, ScriptReflow}; use script_layout_interface::rpc::TextIndexResponse; -use script_layout_interface::rpc::{LayoutRPC, OffsetParentResponse, StyleResponse}; +use script_layout_interface::rpc::{LayoutRPC, OffsetParentResponse}; use script_layout_interface::wrapper_traits::LayoutNode; use script_traits::{ConstellationControlMsg, LayoutControlMsg, LayoutMsg as ConstellationMsg}; use script_traits::{DrawAPaintImageResult, IFrameSizeMsg, PaintWorkletError, WindowSizeType}; @@ -595,7 +593,6 @@ impl LayoutThread { scroll_area_response: Rect::zero(), resolved_style_response: String::new(), offset_parent_response: OffsetParentResponse::empty(), - style_response: StyleResponse(None), scroll_offsets: HashMap::new(), text_index_response: TextIndexResponse(None), nodes_from_point_response: vec![], @@ -1308,9 +1305,7 @@ impl LayoutThread { &QueryMsg::OffsetParentQuery(_) => { rw_data.offset_parent_response = OffsetParentResponse::empty(); }, - &QueryMsg::StyleQuery(_) => { - rw_data.style_response = StyleResponse(None); - }, + &QueryMsg::StyleQuery => {}, &QueryMsg::TextIndexQuery(..) => { rw_data.text_index_response = TextIndexResponse(None); }, @@ -1634,10 +1629,7 @@ impl LayoutThread { &QueryMsg::OffsetParentQuery(node) => { rw_data.offset_parent_response = process_offset_parent_query(node, root_flow); }, - &QueryMsg::StyleQuery(node) => { - let node = unsafe { ServoLayoutNode::new(&node) }; - rw_data.style_response = process_style_query(node); - }, + &QueryMsg::StyleQuery => {}, &QueryMsg::NodesFromPointQuery(client_point, ref reflow_goal) => { let mut flags = match reflow_goal { &NodesFromPointQueryType::Topmost => webrender_api::HitTestFlags::empty(), diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs index 0dbc6b44cdfc..3a808d287ae9 100644 --- a/components/layout_thread_2020/lib.rs +++ b/components/layout_thread_2020/lib.rs @@ -42,8 +42,7 @@ use layout::query::{ use layout::query::{process_element_inner_text_query, process_node_geometry_request}; use layout::query::{process_node_scroll_area_request, process_node_scroll_id_request}; use layout::query::{ - process_offset_parent_query, process_resolved_style_request, process_style_query, - process_text_index_request, + process_offset_parent_query, process_resolved_style_request, process_text_index_request, }; use layout::traversal::RecalcStyle; use layout::{BoxTreeRoot, FragmentTreeRoot}; @@ -64,7 +63,7 @@ use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType}; use script_layout_interface::message::{LayoutThreadInit, Msg, NodesFromPointQueryType}; use script_layout_interface::message::{QueryMsg, ReflowComplete, ReflowGoal, ScriptReflow}; use script_layout_interface::rpc::TextIndexResponse; -use script_layout_interface::rpc::{LayoutRPC, OffsetParentResponse, StyleResponse}; +use script_layout_interface::rpc::{LayoutRPC, OffsetParentResponse}; use script_traits::{ConstellationControlMsg, LayoutControlMsg, LayoutMsg as ConstellationMsg}; use script_traits::{DrawAPaintImageResult, PaintWorkletError}; use script_traits::{Painter, WebrenderIpcSender}; @@ -551,7 +550,6 @@ impl LayoutThread { scroll_area_response: Rect::zero(), resolved_style_response: String::new(), offset_parent_response: OffsetParentResponse::empty(), - style_response: StyleResponse(None), scroll_offsets: HashMap::new(), text_index_response: TextIndexResponse(None), nodes_from_point_response: vec![], @@ -978,9 +976,7 @@ impl LayoutThread { &QueryMsg::OffsetParentQuery(_) => { rw_data.offset_parent_response = OffsetParentResponse::empty(); }, - &QueryMsg::StyleQuery(_) => { - rw_data.style_response = StyleResponse(None); - }, + &QueryMsg::StyleQuery => {}, &QueryMsg::TextIndexQuery(..) => { rw_data.text_index_response = TextIndexResponse(None); }, @@ -1261,10 +1257,7 @@ impl LayoutThread { &QueryMsg::OffsetParentQuery(node) => { rw_data.offset_parent_response = process_offset_parent_query(node); }, - &QueryMsg::StyleQuery(node) => { - let node = unsafe { ServoLayoutNode::new(&node) }; - rw_data.style_response = process_style_query(node); - }, + &QueryMsg::StyleQuery => {}, &QueryMsg::NodesFromPointQuery(client_point, ref reflow_goal) => { let mut flags = match reflow_goal { &NodesFromPointQueryType::Topmost => webrender_api::HitTestFlags::empty(), diff --git a/components/script/devtools.rs b/components/script/devtools.rs index 2298883c555e..49d855ff20fc 100644 --- a/components/script/devtools.rs +++ b/components/script/devtools.rs @@ -15,7 +15,6 @@ use crate::dom::document::AnimationFrameCallback; use crate::dom::element::Element; use crate::dom::globalscope::GlobalScope; use crate::dom::node::{window_from_node, Node, ShadowIncluding}; -use crate::dom::window::Window; use crate::realms::enter_realm; use crate::script_thread::Documents; use devtools_traits::{AutoMargins, ComputedNodeLayout, TimelineMarkerType}; @@ -150,7 +149,7 @@ pub fn handle_get_layout( position: String::from(computed_style.Position()), zIndex: String::from(computed_style.ZIndex()), boxSizing: String::from(computed_style.BoxSizing()), - autoMargins: determine_auto_margins(&window, &*node), + autoMargins: determine_auto_margins(&node), marginTop: String::from(computed_style.MarginTop()), marginRight: String::from(computed_style.MarginRight()), marginBottom: String::from(computed_style.MarginBottom()), @@ -169,8 +168,8 @@ pub fn handle_get_layout( .unwrap(); } -fn determine_auto_margins(window: &Window, node: &Node) -> AutoMargins { - let style = window.style_query(node.to_trusted_node_address()).unwrap(); +fn determine_auto_margins(node: &Node) -> AutoMargins { + let style = node.style().unwrap(); let margin = style.get_margin(); AutoMargins { top: margin.margin_top.is_auto(), diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 0c991998f0ee..2fd6780cab01 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -396,7 +396,7 @@ impl Element { /// 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`. pub fn style(&self) -> Option> { - window_from_node(self).style_query(self.upcast::().to_trusted_node_address()) + self.upcast::().style() } // https://drafts.csswg.org/cssom-view/#css-layout-box diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index be40fc6d5568..4579890a03ea 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -75,6 +75,7 @@ use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use msg::constellation_msg::{BrowsingContextId, PipelineId}; use net_traits::image::base::{Image, ImageMetadata}; use ref_slice::ref_slice; +use script_layout_interface::message::QueryMsg; use script_layout_interface::{HTMLCanvasData, HTMLMediaData, LayoutElementType, LayoutNodeType}; use script_layout_interface::{SVGSVGData, StyleAndOpaqueLayoutData, TrustedNodeAddress}; use script_traits::DocumentActivity; @@ -95,6 +96,7 @@ use std::ops::Range; use std::sync::Arc as StdArc; use style::context::QuirksMode; use style::dom::OpaqueNode; +use style::properties::ComputedValues; use style::selector_parser::{SelectorImpl, SelectorParser}; use style::stylesheets::Stylesheet; use uuid::Uuid; @@ -1229,6 +1231,23 @@ impl Node { _ => false, } } + + #[allow(unsafe_code)] + pub fn style(&self) -> Option> { + if !window_from_node(self).layout_reflow(QueryMsg::StyleQuery) { + return None; + } + unsafe { + (*self.style_and_layout_data.get()).as_ref().map(|data| { + data.style_data + .element_data + .borrow() + .styles + .primary() + .clone() + }) + } + } } /// Iterate through `nodes` until we find a `Node` that is not in `not_in` diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 6782047c5a22..ee7d38b57e16 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -131,7 +131,7 @@ use style::dom::OpaqueNode; use style::error_reporting::{ContextualParseError, ParseErrorReporter}; use style::media_queries; use style::parser::ParserContext as CssParserContext; -use style::properties::{ComputedValues, PropertyId}; +use style::properties::PropertyId; use style::selector_parser::PseudoElement; use style::str::HTML_SPACE_CHARACTERS; use style::stylesheets::CssRuleType; @@ -1904,13 +1904,6 @@ impl Window { (element, response.rect) } - pub fn style_query(&self, node: TrustedNodeAddress) -> Option> { - if !self.layout_reflow(QueryMsg::StyleQuery(node)) { - return None; - } - self.layout_rpc.style().0 - } - pub fn text_index_query( &self, node: &Node, @@ -2461,7 +2454,7 @@ fn debug_reflow_events(id: PipelineId, reflow_goal: &ReflowGoal, reason: &Reflow &QueryMsg::NodeScrollIdQuery(_n) => "\tNodeScrollIdQuery", &QueryMsg::ResolvedStyleQuery(_, _, _) => "\tResolvedStyleQuery", &QueryMsg::OffsetParentQuery(_n) => "\tOffsetParentQuery", - &QueryMsg::StyleQuery(_n) => "\tStyleQuery", + &QueryMsg::StyleQuery => "\tStyleQuery", &QueryMsg::TextIndexQuery(..) => "\tTextIndexQuery", &QueryMsg::ElementInnerTextQuery(_) => "\tElementInnerTextQuery", &QueryMsg::InnerWindowDimensionsQuery(_) => "\tInnerWindowDimensionsQuery", diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index 48ec72232506..63854134cac9 100644 --- a/components/script_layout_interface/message.rs +++ b/components/script_layout_interface/message.rs @@ -122,7 +122,7 @@ pub enum QueryMsg { // garbage values such as `0xdeadbeef as *const _`, this is unsound. NodeScrollIdQuery(TrustedNodeAddress), ResolvedStyleQuery(TrustedNodeAddress, Option, PropertyId), - StyleQuery(TrustedNodeAddress), + StyleQuery, ElementInnerTextQuery(TrustedNodeAddress), InnerWindowDimensionsQuery(BrowsingContextId), } @@ -153,7 +153,7 @@ impl ReflowGoal { QueryMsg::NodeScrollIdQuery(_) | QueryMsg::ResolvedStyleQuery(..) | QueryMsg::OffsetParentQuery(_) | - QueryMsg::StyleQuery(_) => false, + QueryMsg::StyleQuery => false, }, } } @@ -175,7 +175,7 @@ impl ReflowGoal { QueryMsg::ResolvedStyleQuery(..) | QueryMsg::OffsetParentQuery(_) | QueryMsg::InnerWindowDimensionsQuery(_) | - QueryMsg::StyleQuery(_) => false, + QueryMsg::StyleQuery => false, }, } } diff --git a/components/script_layout_interface/rpc.rs b/components/script_layout_interface/rpc.rs index 19da8fa6afc0..991437553cd9 100644 --- a/components/script_layout_interface/rpc.rs +++ b/components/script_layout_interface/rpc.rs @@ -6,8 +6,6 @@ use app_units::Au; use euclid::default::Rect; use euclid::Size2D; use script_traits::UntrustedNodeAddress; -use servo_arc::Arc; -use style::properties::ComputedValues; use style_traits::CSSPixel; use webrender_api::ExternalScrollId; @@ -33,9 +31,6 @@ pub trait LayoutRPC { /// Query layout for the resolved value of a given CSS property fn resolved_style(&self) -> ResolvedStyleResponse; fn offset_parent(&self) -> OffsetParentResponse; - /// Requests the styles for an element. Contains a `None` value if the element is in a `display: - /// none` subtree. - fn style(&self) -> StyleResponse; fn text_index(&self) -> TextIndexResponse; /// Requests the list of nodes from the given point. fn nodes_from_point_response(&self) -> Vec; @@ -72,8 +67,5 @@ impl OffsetParentResponse { } } -#[derive(Clone)] -pub struct StyleResponse(pub Option>); - #[derive(Clone)] pub struct TextIndexResponse(pub Option);