Skip to content

Commit

Permalink
Don't go through the layout thread to retrieve a node's primary style
Browse files Browse the repository at this point in the history
  • Loading branch information
nox committed Apr 7, 2020
1 parent 030a1cf commit c10e839
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 79 deletions.
18 changes: 1 addition & 17 deletions components/layout/query.rs
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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,

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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),
Expand Down
15 changes: 1 addition & 14 deletions components/layout_2020/query.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ExternalScrollId, Vector2D<f32, LayoutPixel>>,

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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()
Expand Down
16 changes: 4 additions & 12 deletions components/layout_thread/lib.rs
Expand Up @@ -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,
Expand All @@ -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};
Expand Down Expand Up @@ -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![],
Expand Down Expand Up @@ -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);
},
Expand Down Expand Up @@ -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(),
Expand Down
15 changes: 4 additions & 11 deletions components/layout_thread_2020/lib.rs
Expand Up @@ -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};
Expand All @@ -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};
Expand Down Expand Up @@ -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![],
Expand Down Expand Up @@ -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);
},
Expand Down Expand Up @@ -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(),
Expand Down
7 changes: 3 additions & 4 deletions components/script/devtools.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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()),
Expand All @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion components/script/dom/element.rs
Expand Up @@ -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<Arc<ComputedValues>> {
window_from_node(self).style_query(self.upcast::<Node>().to_trusted_node_address())
self.upcast::<Node>().style()
}

// https://drafts.csswg.org/cssom-view/#css-layout-box
Expand Down
19 changes: 19 additions & 0 deletions components/script/dom/node.rs
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -1229,6 +1231,23 @@ impl Node {
_ => false,
}
}

#[allow(unsafe_code)]
pub fn style(&self) -> Option<Arc<ComputedValues>> {
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`
Expand Down
11 changes: 2 additions & 9 deletions components/script/dom/window.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -1904,13 +1904,6 @@ impl Window {
(element, response.rect)
}

pub fn style_query(&self, node: TrustedNodeAddress) -> Option<servo_arc::Arc<ComputedValues>> {
if !self.layout_reflow(QueryMsg::StyleQuery(node)) {
return None;
}
self.layout_rpc.style().0
}

pub fn text_index_query(
&self,
node: &Node,
Expand Down Expand Up @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions components/script_layout_interface/message.rs
Expand Up @@ -122,7 +122,7 @@ pub enum QueryMsg {
// garbage values such as `0xdeadbeef as *const _`, this is unsound.
NodeScrollIdQuery(TrustedNodeAddress),
ResolvedStyleQuery(TrustedNodeAddress, Option<PseudoElement>, PropertyId),
StyleQuery(TrustedNodeAddress),
StyleQuery,
ElementInnerTextQuery(TrustedNodeAddress),
InnerWindowDimensionsQuery(BrowsingContextId),
}
Expand Down Expand Up @@ -153,7 +153,7 @@ impl ReflowGoal {
QueryMsg::NodeScrollIdQuery(_) |
QueryMsg::ResolvedStyleQuery(..) |
QueryMsg::OffsetParentQuery(_) |
QueryMsg::StyleQuery(_) => false,
QueryMsg::StyleQuery => false,
},
}
}
Expand All @@ -175,7 +175,7 @@ impl ReflowGoal {
QueryMsg::ResolvedStyleQuery(..) |
QueryMsg::OffsetParentQuery(_) |
QueryMsg::InnerWindowDimensionsQuery(_) |
QueryMsg::StyleQuery(_) => false,
QueryMsg::StyleQuery => false,
},
}
}
Expand Down
8 changes: 0 additions & 8 deletions components/script_layout_interface/rpc.rs
Expand Up @@ -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;

Expand All @@ -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<UntrustedNodeAddress>;
Expand Down Expand Up @@ -72,8 +67,5 @@ impl OffsetParentResponse {
}
}

#[derive(Clone)]
pub struct StyleResponse(pub Option<Arc<ComputedValues>>);

#[derive(Clone)]
pub struct TextIndexResponse(pub Option<usize>);

0 comments on commit c10e839

Please sign in to comment.