diff --git a/components/layout/construct.rs b/components/layout/construct.rs index 35d414283f1..522635543fd 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -43,7 +43,7 @@ use style::LocalName; use crate::block::BlockFlow; use crate::context::{with_thread_local_font_context, LayoutContext}; -use crate::data::{LayoutData, LayoutDataFlags}; +use crate::data::{InnerLayoutData, LayoutDataFlags}; use crate::display_list::items::OpaqueNode; use crate::flex::FlexFlow; use crate::floats::FloatKind; @@ -71,14 +71,15 @@ use crate::table_rowgroup::TableRowGroupFlow; use crate::table_wrapper::TableWrapperFlow; use crate::text::TextRunScanner; use crate::traversal::PostorderNodeMutTraversal; -use crate::wrapper::{LayoutNodeLayoutData, TextContent, ThreadSafeLayoutNodeHelpers}; +use crate::wrapper::{TextContent, ThreadSafeLayoutNodeHelpers}; use crate::{parallel, ServoArc}; /// The results of flow construction for a DOM node. -#[derive(Clone)] +#[derive(Clone, Default)] pub enum ConstructionResult { /// This node contributes nothing at all (`display: none`). Alternately, this is what newly /// created nodes have their `ConstructionResult` set to. + #[default] None, /// This node contributed a flow at the proper position in the tree. @@ -1992,7 +1993,7 @@ trait NodeUtils { /// Returns true if this node doesn't render its kids and false otherwise. fn is_replaced_content(&self) -> bool; - fn construction_result_mut(self, layout_data: &mut LayoutData) -> &mut ConstructionResult; + fn construction_result_mut(self, layout_data: &mut InnerLayoutData) -> &mut ConstructionResult; /// Sets the construction result of a flow. fn set_flow_construction_result(self, result: ConstructionResult); @@ -2029,7 +2030,7 @@ where } } - fn construction_result_mut(self, data: &mut LayoutData) -> &mut ConstructionResult { + fn construction_result_mut(self, data: &mut InnerLayoutData) -> &mut ConstructionResult { match self.get_pseudo_element_type() { PseudoElementType::Before => &mut data.before_flow_construction_result, PseudoElementType::After => &mut data.after_flow_construction_result, diff --git a/components/layout/data.rs b/components/layout/data.rs index 283cea4a724..1170a4414fa 100644 --- a/components/layout/data.rs +++ b/components/layout/data.rs @@ -5,20 +5,12 @@ use atomic_refcell::AtomicRefCell; use bitflags::bitflags; use script_layout_interface::wrapper_traits::LayoutDataTrait; -use script_layout_interface::StyleData; use crate::construct::ConstructionResult; -pub struct StyleAndLayoutData<'dom> { - /// The style data associated with a node. - pub style_data: &'dom StyleData, - /// The layout data associated with a node. - pub layout_data: &'dom AtomicRefCell, -} - /// Data that layout associates with a node. -#[derive(Clone)] -pub struct LayoutData { +#[derive(Clone, Default)] +pub struct InnerLayoutData { /// The current results of flow construction for this node. This is either a /// flow or a `ConstructionItem`. See comments in `construct.rs` for more /// details. @@ -29,31 +21,14 @@ pub struct LayoutData { pub after_flow_construction_result: ConstructionResult, pub details_summary_flow_construction_result: ConstructionResult, - pub details_content_flow_construction_result: ConstructionResult, /// Various flags. pub flags: LayoutDataFlags, } -impl LayoutDataTrait for LayoutData {} - -impl Default for LayoutData { - /// Creates new layout data. - fn default() -> LayoutData { - Self { - flow_construction_result: ConstructionResult::None, - before_flow_construction_result: ConstructionResult::None, - after_flow_construction_result: ConstructionResult::None, - details_summary_flow_construction_result: ConstructionResult::None, - details_content_flow_construction_result: ConstructionResult::None, - flags: LayoutDataFlags::empty(), - } - } -} - bitflags! { - #[derive(Clone, Copy)] + #[derive(Clone, Copy, Default)] pub struct LayoutDataFlags: u8 { /// Whether a flow has been newly constructed. const HAS_NEWLY_CONSTRUCTED_FLOW = 0x01; @@ -61,3 +36,12 @@ bitflags! { const HAS_BEEN_TRAVERSED = 0x02; } } + +/// A wrapper for [`InnerLayoutData`]. This is necessary to give the entire data +/// structure interior mutability, as we will need to mutate the layout data of +/// non-mutable DOM nodes. +#[derive(Clone, Default)] +pub struct LayoutData(pub AtomicRefCell); + +// The implementation of this trait allows the data to be stored in the DOM. +impl LayoutDataTrait for LayoutData {} diff --git a/components/layout/query.rs b/components/layout/query.rs index d8dd633e8c3..a11e4fa8c7a 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -40,7 +40,7 @@ use crate::flow::{Flow, GetBaseFlow}; use crate::fragment::{Fragment, FragmentBorderBoxIterator, FragmentFlags, SpecificFragmentInfo}; use crate::inline::InlineFragmentNodeFlags; use crate::sequential; -use crate::wrapper::LayoutNodeLayoutData; +use crate::wrapper::ThreadSafeLayoutNodeHelpers; // https://drafts.csswg.org/cssom-view/#overflow-directions fn overflow_direction(writing_mode: &WritingMode) -> OverflowDirection { @@ -851,7 +851,7 @@ fn process_resolved_style_request_internal<'dom>( where N: LayoutNode<'dom>, { - let maybe_data = layout_el.borrow_layout_data(); + let maybe_data = layout_el.as_node().borrow_layout_data(); let position = maybe_data.map_or(Point2D::zero(), |data| { match data.flow_construction_result { ConstructionResult::Flow(ref flow_ref, _) => flow_ref @@ -1021,8 +1021,8 @@ fn inner_text_collection_steps<'dom>( _ => child, }; - let element_data = match node.get_style_and_opaque_layout_data() { - Some(data) => &data.style_data.element_data, + let element_data = match node.style_data() { + Some(data) => &data.element_data, None => continue, }; diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 6b49a0be982..47f2cfc55fd 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -18,7 +18,8 @@ use crate::construct::FlowConstructor; use crate::context::LayoutContext; use crate::display_list::DisplayListBuildState; use crate::flow::{Flow, FlowFlags, GetBaseFlow, ImmutableFlowUtils}; -use crate::wrapper::{GetStyleAndLayoutData, LayoutNodeLayoutData, ThreadSafeLayoutNodeHelpers}; +use crate::wrapper::ThreadSafeLayoutNodeHelpers; +use crate::LayoutData; pub struct RecalcStyleAndConstructFlows<'a> { context: LayoutContext<'a>, @@ -58,7 +59,7 @@ where { // FIXME(pcwalton): Stop allocating here. Ideally this should just be // done by the HTML parser. - unsafe { node.initialize_data() }; + unsafe { node.initialize_style_and_layout_data::() }; if !node.is_text_node() { let el = node.as_element().unwrap(); @@ -76,7 +77,7 @@ where // flow construction: // (1) They child doesn't yet have layout data (preorder traversal initializes it). // (2) The parent element has restyle damage (so the text flow also needs fixup). - node.get_style_and_layout_data().is_none() || !parent_data.damage.is_empty() + node.layout_data().is_none() || !parent_data.damage.is_empty() } fn shared_context(&self) -> &SharedStyleContext { diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 49bad6e680c..f4841f2d59a 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -32,59 +32,20 @@ use atomic_refcell::{AtomicRef, AtomicRefMut}; use script_layout_interface::wrapper_traits::{ - GetStyleAndOpaqueLayoutData, ThreadSafeLayoutElement, ThreadSafeLayoutNode, + LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode, }; use style::dom::{NodeInfo, TElement, TNode}; use style::selector_parser::RestyleDamage; use style::values::computed::counters::ContentItem; use style::values::generics::counters::Content; -use crate::data::{LayoutData, LayoutDataFlags, StyleAndLayoutData}; +use crate::data::{InnerLayoutData, LayoutData, LayoutDataFlags}; -pub trait LayoutNodeLayoutData<'dom> { - fn borrow_layout_data(self) -> Option>; - fn mutate_layout_data(self) -> Option>; +pub trait ThreadSafeLayoutNodeHelpers<'dom> { + fn borrow_layout_data(self) -> Option>; + fn mutate_layout_data(self) -> Option>; fn flow_debug_id(self) -> usize; -} -impl<'dom, T> LayoutNodeLayoutData<'dom> for T -where - T: GetStyleAndOpaqueLayoutData<'dom>, -{ - fn borrow_layout_data(self) -> Option> { - self.get_style_and_layout_data() - .map(|d| d.layout_data.borrow()) - } - - fn mutate_layout_data(self) -> Option> { - self.get_style_and_layout_data() - .map(|d| d.layout_data.borrow_mut()) - } - - fn flow_debug_id(self) -> usize { - self.borrow_layout_data() - .map_or(0, |d| d.flow_construction_result.debug_id()) - } -} - -pub trait GetStyleAndLayoutData<'dom> { - fn get_style_and_layout_data(self) -> Option>; -} - -impl<'dom, T> GetStyleAndLayoutData<'dom> for T -where - T: GetStyleAndOpaqueLayoutData<'dom>, -{ - fn get_style_and_layout_data(self) -> Option> { - self.get_style_and_opaque_layout_data() - .map(|data| StyleAndLayoutData { - style_data: &data.style_data, - layout_data: data.generic_data.downcast_ref().unwrap(), - }) - } -} - -pub trait ThreadSafeLayoutNodeHelpers { /// Returns the layout data flags for this node. fn flags(self) -> LayoutDataFlags; @@ -107,10 +68,26 @@ pub trait ThreadSafeLayoutNodeHelpers { fn restyle_damage(self) -> RestyleDamage; } -impl<'dom, T> ThreadSafeLayoutNodeHelpers for T +impl<'dom, T> ThreadSafeLayoutNodeHelpers<'dom> for T where T: ThreadSafeLayoutNode<'dom>, { + fn borrow_layout_data(self) -> Option> { + self.layout_data() + .map(|data| data.downcast_ref::().unwrap().0.borrow()) + } + + fn mutate_layout_data(self) -> Option> { + self.layout_data() + .and_then(|data| data.downcast_ref::()) + .map(|data| data.0.borrow_mut()) + } + + fn flow_debug_id(self) -> usize { + self.borrow_layout_data() + .map_or(0, |d| d.flow_construction_result.debug_id()) + } + fn flags(self) -> LayoutDataFlags { self.borrow_layout_data().as_ref().unwrap().flags } @@ -150,17 +127,16 @@ where } let damage = { - let data = match node.get_style_and_layout_data() { - Some(data) => data, - None => panic!( + let (layout_data, style_data) = match (node.layout_data(), node.style_data()) { + (Some(layout_data), Some(style_data)) => (layout_data, style_data), + _ => panic!( "could not get style and layout data for <{}>", node.as_element().unwrap().local_name() ), }; - if !data - .layout_data - .borrow() + let layout_data = layout_data.downcast_ref::().unwrap().0.borrow(); + if !layout_data .flags .contains(crate::data::LayoutDataFlags::HAS_BEEN_TRAVERSED) { @@ -169,7 +145,7 @@ where // because that's what the code expects. RestyleDamage::rebuild_and_reflow() } else { - data.style_data.element_data.borrow().damage + style_data.element_data.borrow().damage } }; diff --git a/components/layout_2020/dom.rs b/components/layout_2020/dom.rs index 3843e77f217..b065757fefd 100644 --- a/components/layout_2020/dom.rs +++ b/components/layout_2020/dom.rs @@ -5,11 +5,11 @@ use std::marker::PhantomData; use std::sync::{Arc, Mutex}; -use atomic_refcell::{AtomicRefCell, AtomicRefMut}; +use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use msg::constellation_msg::{BrowsingContextId, PipelineId}; use net_traits::image::base::Image as NetImage; use script_layout_interface::wrapper_traits::{LayoutDataTrait, LayoutNode, ThreadSafeLayoutNode}; -use script_layout_interface::{HTMLCanvasDataSource, StyleData}; +use script_layout_interface::HTMLCanvasDataSource; use servo_arc::Arc as ServoArc; use style::properties::ComputedValues; @@ -24,7 +24,7 @@ use crate::replaced::{CanvasInfo, CanvasSource}; /// The data that is stored in each DOM node that is used by layout. #[derive(Default)] -pub struct DOMLayoutData { +pub struct InnerDOMLayoutData { pub(super) self_box: ArcRefCell>, pub(super) pseudo_before_box: ArcRefCell>, pub(super) pseudo_after_box: ArcRefCell>, @@ -38,14 +38,15 @@ pub(super) enum LayoutBox { FlexLevel(ArcRefCell), } +/// A wrapper for [`InnerDOMLayoutData`]. This is necessary to give the entire data +/// structure interior mutability, as we will need to mutate the layout data of +/// non-mutable DOM nodes. +#[derive(Default)] +pub struct DOMLayoutData(AtomicRefCell); + // The implementation of this trait allows the data to be stored in the DOM. impl LayoutDataTrait for DOMLayoutData {} -pub struct StyleAndLayoutData<'dom> { - pub style_data: &'dom StyleData, - pub(super) layout_data: &'dom AtomicRefCell, -} - pub struct BoxSlot<'dom> { pub(crate) slot: Option>>, pub(crate) marker: PhantomData<&'dom ()>, @@ -96,8 +97,8 @@ pub(crate) trait NodeExt<'dom>: 'dom + LayoutNode<'dom> { fn as_video(self) -> Option<(webrender_api::ImageKey, PhysicalSize)>; fn style(self, context: &LayoutContext) -> ServoArc; - fn get_style_and_layout_data(self) -> Option>; - fn layout_data_mut(self) -> AtomicRefMut<'dom, DOMLayoutData>; + fn layout_data_mut(self) -> AtomicRefMut<'dom, InnerDOMLayoutData>; + fn layout_data(self) -> Option>; fn element_box_slot(&self) -> BoxSlot<'dom>; fn pseudo_element_box_slot(&self, which: WhichPseudoElement) -> BoxSlot<'dom>; fn unset_pseudo_element_box(self, which: WhichPseudoElement); @@ -166,10 +167,21 @@ where self.to_threadsafe().style(context.shared_context()) } - fn layout_data_mut(self) -> AtomicRefMut<'dom, DOMLayoutData> { - self.get_style_and_layout_data() - .map(|d| d.layout_data.borrow_mut()) + fn layout_data_mut(self) -> AtomicRefMut<'dom, InnerDOMLayoutData> { + if LayoutNode::layout_data(&self).is_none() { + self.initialize_layout_data::(); + } + LayoutNode::layout_data(&self) .unwrap() + .downcast_ref::() + .unwrap() + .0 + .borrow_mut() + } + + fn layout_data(self) -> Option> { + LayoutNode::layout_data(&self) + .map(|data| data.downcast_ref::().unwrap().0.borrow()) } fn element_box_slot(&self) -> BoxSlot<'dom> { @@ -202,12 +214,4 @@ where // Stylo already takes care of removing all layout data // for DOM descendants of elements with `display: none`. } - - fn get_style_and_layout_data(self) -> Option> { - self.get_style_and_opaque_layout_data() - .map(|data| StyleAndLayoutData { - style_data: &data.style_data, - layout_data: data.generic_data.downcast_ref().unwrap(), - }) - } } diff --git a/components/layout_2020/flow/root.rs b/components/layout_2020/flow/root.rs index 11d7ee5ba27..eecb6d4c2f7 100644 --- a/components/layout_2020/flow/root.rs +++ b/components/layout_2020/flow/root.rs @@ -141,16 +141,13 @@ impl BoxTree { return None; } - // Don't update unstyled nodes. - let data = node.get_style_and_layout_data()?; - - // Don't update nodes that have pseudo-elements. - let element_data = data.style_data.element_data.borrow(); + // Don't update unstyled nodes or nodes that have pseudo-elements. + let element_data = node.style_data()?.element_data.borrow(); if !element_data.styles.pseudos.is_empty() { return None; } - let layout_data = data.layout_data.borrow(); + let layout_data = node.layout_data()?; if layout_data.pseudo_before_box.borrow().is_some() { return None; } diff --git a/components/layout_2020/traversal.rs b/components/layout_2020/traversal.rs index 318967d01f4..d1e4b8b65ef 100644 --- a/components/layout_2020/traversal.rs +++ b/components/layout_2020/traversal.rs @@ -9,7 +9,7 @@ use style::dom::{NodeInfo, TElement, TNode}; use style::traversal::{recalc_style_at, DomTraversal, PerLevelTraversalData}; use crate::context::LayoutContext; -use crate::dom::NodeExt; +use crate::dom::DOMLayoutData; pub struct RecalcStyle<'a> { context: LayoutContext<'a>, @@ -45,7 +45,7 @@ where F: FnMut(E::ConcreteNode), { unsafe { - node.initialize_data(); + node.initialize_style_and_layout_data::(); if !node.is_text_node() { let el = node.as_element().unwrap(); let mut data = el.mutate_data().unwrap(); @@ -65,7 +65,7 @@ where } fn text_node_needs_traversal(node: E::ConcreteNode, parent_data: &ElementData) -> bool { - node.get_style_and_layout_data().is_none() || !parent_data.damage.is_empty() + node.layout_data().is_none() || !parent_data.damage.is_empty() } fn shared_context(&self) -> &SharedStyleContext { diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 05e036d460d..401a45f1f52 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -47,7 +47,7 @@ use layout::traversal::{ construct_flows_at_ancestors, ComputeStackingRelativePositions, PreorderFlowTraversal, RecalcStyleAndConstructFlows, }; -use layout::wrapper::LayoutNodeLayoutData; +use layout::wrapper::ThreadSafeLayoutNodeHelpers; use layout::{layout_debug, layout_debug_scope, parallel, sequential, LayoutData}; use lazy_static::lazy_static; use log::{debug, error, trace, warn}; @@ -761,7 +761,11 @@ impl LayoutThread { } fn try_get_layout_root<'dom>(&self, node: impl LayoutNode<'dom>) -> Option { - let result = node.mutate_layout_data()?.flow_construction_result.get(); + let result = node + .to_threadsafe() + .mutate_layout_data()? + .flow_construction_result + .get(); let mut flow = match result { ConstructionResult::Flow(mut flow, abs_descendants) => { diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 20940788f84..fa5930461bd 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -26,8 +26,8 @@ use msg::constellation_msg::{BrowsingContextId, PipelineId}; use net_traits::image::base::{Image, ImageMetadata}; use script_layout_interface::message::QueryMsg; use script_layout_interface::{ - HTMLCanvasData, HTMLMediaData, LayoutElementType, LayoutNodeType, SVGSVGData, - StyleAndOpaqueLayoutData, TrustedNodeAddress, + GenericLayoutData, HTMLCanvasData, HTMLMediaData, LayoutElementType, LayoutNodeType, + SVGSVGData, StyleData, TrustedNodeAddress, }; use script_traits::{DocumentActivity, UntrustedNodeAddress}; use selectors::matching::{ @@ -155,10 +155,16 @@ pub struct Node { /// are this node. ranges: WeakRangeVec, - /// Style+Layout information. + /// Style data for this node. This is accessed and mutated by style + /// passes and is used to lay out this node and populate layout data. + #[no_trace] + style_data: DomRefCell>>, + + /// Layout data for this node. This is populated during layout and can + /// be used for incremental relayout and script queries. #[ignore_malloc_size_of = "trait object"] #[no_trace] - style_and_layout_data: DomRefCell>>, + layout_data: DomRefCell>>, } /// Flags for node items @@ -289,9 +295,10 @@ impl Node { } } - pub fn clean_up_layout_data(&self) { + pub fn clean_up_style_and_layout_data(&self) { self.owner_doc().cancel_animations_for_node(self); - self.style_and_layout_data.borrow_mut().take(); + self.style_data.borrow_mut().take(); + self.layout_data.borrow_mut().take(); } /// Clean up flags and unbind from tree. @@ -308,7 +315,7 @@ impl Node { ); } for node in root.traverse_preorder(ShadowIncluding::Yes) { - node.clean_up_layout_data(); + node.clean_up_style_and_layout_data(); // This needs to be in its own loop, because unbind_from_tree may // rely on the state of IS_IN_DOC of the context node's descendants, @@ -1261,37 +1268,29 @@ impl Node { } pub fn is_styled(&self) -> bool { - self.style_and_layout_data.borrow().is_some() + self.style_data.borrow().is_some() } pub fn is_display_none(&self) -> bool { - self.style_and_layout_data - .borrow() - .as_ref() - .map_or(true, |data| { - data.style_data - .element_data - .borrow() - .styles - .primary() - .get_box() - .display - .is_none() - }) + self.style_data.borrow().as_ref().map_or(true, |data| { + data.element_data + .borrow() + .styles + .primary() + .get_box() + .display + .is_none() + }) } pub fn style(&self) -> Option> { if !window_from_node(self).layout_reflow(QueryMsg::StyleQuery) { return None; } - self.style_and_layout_data.borrow().as_ref().map(|data| { - data.style_data - .element_data - .borrow() - .styles - .primary() - .clone() - }) + self.style_data + .borrow() + .as_ref() + .map(|data| data.element_data.borrow().styles.primary().clone()) } } @@ -1343,25 +1342,35 @@ pub trait LayoutNodeHelpers<'dom> { fn children_count(self) -> u32; - fn get_style_and_opaque_layout_data(self) -> Option<&'dom StyleAndOpaqueLayoutData>; + fn style_data(self) -> Option<&'dom StyleData>; + fn layout_data(self) -> Option<&'dom GenericLayoutData>; - /// Initialize the style and opaque layout data of this node. + /// Initialize the style data of this node. /// /// # Safety /// /// This method is unsafe because it modifies the given node during /// layout. Callers should ensure that no other layout thread is /// attempting to read or modify the opaque layout data of this node. - unsafe fn init_style_and_opaque_layout_data(self, data: Box); + unsafe fn initialize_style_data(self); - /// Unset and return the style and opaque layout data of this node. + /// Initialize the opaque layout data of this node. /// /// # Safety /// /// This method is unsafe because it modifies the given node during /// layout. Callers should ensure that no other layout thread is /// attempting to read or modify the opaque layout data of this node. - unsafe fn take_style_and_opaque_layout_data(self) -> Box; + unsafe fn initialize_layout_data(self, data: Box); + + /// Clear the style and opaque layout data of this node. + /// + /// # Safety + /// + /// This method is unsafe because it modifies the given node during + /// layout. Callers should ensure that no other layout thread is + /// attempting to read or modify the opaque layout data of this node. + unsafe fn clear_style_and_layout_data(self); fn text_content(self) -> Cow<'dom, str>; fn selection(self) -> Option>; @@ -1482,37 +1491,39 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { // FIXME(nox): How we handle style and layout data needs to be completely // revisited so we can do that more cleanly and safely in layout 2020. - #[inline] #[allow(unsafe_code)] - fn get_style_and_opaque_layout_data(self) -> Option<&'dom StyleAndOpaqueLayoutData> { - unsafe { - self.unsafe_get() - .style_and_layout_data - .borrow_for_layout() - .as_deref() - } + fn style_data(self) -> Option<&'dom StyleData> { + unsafe { self.unsafe_get().style_data.borrow_for_layout().as_deref() } } #[inline] #[allow(unsafe_code)] - unsafe fn init_style_and_opaque_layout_data(self, val: Box) { - let data = self - .unsafe_get() - .style_and_layout_data - .borrow_mut_for_layout(); + fn layout_data(self) -> Option<&'dom GenericLayoutData> { + unsafe { self.unsafe_get().layout_data.borrow_for_layout().as_deref() } + } + + #[inline] + #[allow(unsafe_code)] + unsafe fn initialize_style_data(self) { + let data = self.unsafe_get().style_data.borrow_mut_for_layout(); debug_assert!(data.is_none()); - *data = Some(val); + *data = Some(Box::default()); } #[inline] #[allow(unsafe_code)] - unsafe fn take_style_and_opaque_layout_data(self) -> Box { - self.unsafe_get() - .style_and_layout_data - .borrow_mut_for_layout() - .take() - .unwrap() + unsafe fn initialize_layout_data(self, new_data: Box) { + let data = self.unsafe_get().layout_data.borrow_mut_for_layout(); + debug_assert!(data.is_none()); + *data = Some(new_data); + } + + #[inline] + #[allow(unsafe_code)] + unsafe fn clear_style_and_layout_data(self) { + self.unsafe_get().style_data.borrow_mut_for_layout().take(); + self.unsafe_get().layout_data.borrow_mut_for_layout().take(); } fn text_content(self) -> Cow<'dom, str> { @@ -1833,8 +1844,8 @@ impl Node { flags: Cell::new(flags), inclusive_descendants_version: Cell::new(0), ranges: WeakRangeVec::new(), - - style_and_layout_data: Default::default(), + style_data: Default::default(), + layout_data: Default::default(), } } diff --git a/components/script/layout_dom/element.rs b/components/script/layout_dom/element.rs index 48a0b86bb6a..42b31b69f85 100644 --- a/components/script/layout_dom/element.rs +++ b/components/script/layout_dom/element.rs @@ -10,10 +10,9 @@ use std::sync::atomic::Ordering; use atomic_refcell::{AtomicRef, AtomicRefMut}; use html5ever::{local_name, namespace_url, ns, LocalName, Namespace}; use script_layout_interface::wrapper_traits::{ - GetStyleAndOpaqueLayoutData, LayoutDataTrait, LayoutNode, PseudoElementType, - ThreadSafeLayoutElement, ThreadSafeLayoutNode, + LayoutDataTrait, LayoutNode, PseudoElementType, ThreadSafeLayoutElement, ThreadSafeLayoutNode, }; -use script_layout_interface::{LayoutNodeType, StyleAndOpaqueLayoutData, StyleData}; +use script_layout_interface::{LayoutNodeType, StyleData}; use selectors::attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint}; use selectors::bloom::{BloomFilter, BLOOM_HASH_MASK}; use selectors::matching::{ElementSelectorFlags, MatchingContext, VisitedHandlingMode}; @@ -111,8 +110,7 @@ impl<'dom, LayoutDataType: LayoutDataTrait> ServoLayoutElement<'dom, LayoutDataT } fn get_style_data(&self) -> Option<&StyleData> { - self.get_style_and_opaque_layout_data() - .map(|data| &data.style_data) + self.as_node().style_data() } pub unsafe fn unset_snapshot_flags(&self) { @@ -154,14 +152,6 @@ impl<'dom, LayoutDataType: LayoutDataTrait> ServoLayoutElement<'dom, LayoutDataT } } -impl<'dom, LayoutDataType: LayoutDataTrait> GetStyleAndOpaqueLayoutData<'dom> - for ServoLayoutElement<'dom, LayoutDataType> -{ - fn get_style_and_opaque_layout_data(self) -> Option<&'dom StyleAndOpaqueLayoutData> { - self.as_node().get_style_and_opaque_layout_data() - } -} - impl<'dom, LayoutDataType: LayoutDataTrait> style::dom::TElement for ServoLayoutElement<'dom, LayoutDataType> { @@ -322,17 +312,11 @@ impl<'dom, LayoutDataType: LayoutDataTrait> style::dom::TElement } unsafe fn clear_data(&self) { - if self.get_style_and_opaque_layout_data().is_some() { - drop( - self.as_node() - .get_jsmanaged() - .take_style_and_opaque_layout_data(), - ); - } + self.as_node().get_jsmanaged().clear_style_and_layout_data() } unsafe fn ensure_data(&self) -> AtomicRefMut { - self.as_node().initialize_data(); + self.as_node().get_jsmanaged().initialize_style_data(); self.mutate_data().unwrap() } @@ -968,11 +952,3 @@ impl<'dom, LayoutDataType: LayoutDataTrait> ::selectors::Element true } } - -impl<'dom, LayoutDataType: LayoutDataTrait> GetStyleAndOpaqueLayoutData<'dom> - for ServoThreadSafeLayoutElement<'dom, LayoutDataType> -{ - fn get_style_and_opaque_layout_data(self) -> Option<&'dom StyleAndOpaqueLayoutData> { - self.element.as_node().get_style_and_opaque_layout_data() - } -} diff --git a/components/script/layout_dom/node.rs b/components/script/layout_dom/node.rs index cd5b1780cfa..7e0abf5942d 100644 --- a/components/script/layout_dom/node.rs +++ b/components/script/layout_dom/node.rs @@ -9,18 +9,16 @@ use std::fmt; use std::marker::PhantomData; use std::sync::Arc as StdArc; -use atomic_refcell::AtomicRefCell; use gfx_traits::ByteIndex; use html5ever::{local_name, namespace_url, ns}; use msg::constellation_msg::{BrowsingContextId, PipelineId}; use net_traits::image::base::{Image, ImageMetadata}; use range::Range; use script_layout_interface::wrapper_traits::{ - GetStyleAndOpaqueLayoutData, LayoutDataTrait, LayoutNode, PseudoElementType, - ThreadSafeLayoutNode, + LayoutDataTrait, LayoutNode, PseudoElementType, ThreadSafeLayoutNode, }; use script_layout_interface::{ - HTMLCanvasData, HTMLMediaData, LayoutNodeType, SVGSVGData, StyleAndOpaqueLayoutData, StyleData, + GenericLayoutData, HTMLCanvasData, HTMLMediaData, LayoutNodeType, SVGSVGData, StyleData, TrustedNodeAddress, }; use servo_arc::Arc; @@ -206,30 +204,35 @@ impl<'dom, LayoutDataType: LayoutDataTrait> LayoutNode<'dom> self.script_type_id().into() } - unsafe fn initialize_data(&self) { - if self.get_style_and_opaque_layout_data().is_some() { - return; + unsafe fn initialize_style_and_layout_data(&self) { + let inner = self.get_jsmanaged(); + if inner.style_data().is_none() { + inner.initialize_style_data(); } + if inner.layout_data().is_none() { + inner.initialize_layout_data(Box::::default()); + } + } - let opaque = StyleAndOpaqueLayoutData::new( - StyleData::default(), - AtomicRefCell::new(LayoutDataType::default()), - ); - - self.get_jsmanaged() - .init_style_and_opaque_layout_data(opaque); + fn initialize_layout_data(&self) { + let inner = self.get_jsmanaged(); + if inner.layout_data().is_none() { + unsafe { + inner.initialize_layout_data(Box::::default()); + } + } } fn is_connected(&self) -> bool { unsafe { self.node.get_flag(NodeFlags::IS_CONNECTED) } } -} -impl<'dom, LayoutDataType: LayoutDataTrait> GetStyleAndOpaqueLayoutData<'dom> - for ServoLayoutNode<'dom, LayoutDataType> -{ - fn get_style_and_opaque_layout_data(self) -> Option<&'dom StyleAndOpaqueLayoutData> { - self.get_jsmanaged().get_style_and_opaque_layout_data() + fn style_data(&self) -> Option<&'dom StyleData> { + self.get_jsmanaged().style_data() + } + + fn layout_data(&self) -> Option<&'dom GenericLayoutData> { + self.get_jsmanaged().layout_data() } } @@ -369,8 +372,12 @@ impl<'dom, LayoutDataType: LayoutDataTrait> ThreadSafeLayoutNode<'dom> }) } - fn get_style_and_opaque_layout_data(self) -> Option<&'dom StyleAndOpaqueLayoutData> { - self.node.get_style_and_opaque_layout_data() + fn style_data(&self) -> Option<&'dom StyleData> { + self.node.style_data() + } + + fn layout_data(&self) -> Option<&'dom GenericLayoutData> { + self.node.layout_data() } fn is_ignorable_whitespace(&self, context: &SharedStyleContext) -> bool { @@ -584,11 +591,3 @@ impl<'dom, LayoutDataType: LayoutDataTrait> Iterator } } } - -impl<'dom, LayoutDataType: LayoutDataTrait> GetStyleAndOpaqueLayoutData<'dom> - for ServoThreadSafeLayoutNode<'dom, LayoutDataType> -{ - fn get_style_and_opaque_layout_data(self) -> Option<&'dom StyleAndOpaqueLayoutData> { - self.node.get_style_and_opaque_layout_data() - } -} diff --git a/components/shared/script_layout/lib.rs b/components/shared/script_layout/lib.rs index aca819980d7..a09e0fcf1aa 100644 --- a/components/shared/script_layout/lib.rs +++ b/components/shared/script_layout/lib.rs @@ -47,6 +47,8 @@ use style::stylesheets::Stylesheet; use style_traits::CSSPixel; use webrender_api::{ExternalScrollId, ImageKey}; +pub type GenericLayoutData = dyn Any + Send + Sync; + #[derive(MallocSizeOf)] pub struct StyleData { /// Data that the style system associates with a node. When the @@ -69,33 +71,6 @@ impl Default for StyleData { } } -pub type StyleAndOpaqueLayoutData = StyleAndGenericData; - -#[derive(MallocSizeOf)] -pub struct StyleAndGenericData -where - T: ?Sized, -{ - /// The style data. - pub style_data: StyleData, - /// The opaque layout data. - #[ignore_malloc_size_of = "Trait objects are hard"] - pub generic_data: T, -} - -impl StyleAndOpaqueLayoutData { - #[inline] - pub fn new(style_data: StyleData, layout_data: T) -> Box - where - T: Any + Send + Sync, - { - Box::new(StyleAndGenericData { - style_data, - generic_data: layout_data, - }) - } -} - /// Information that we need stored in each DOM node. #[derive(Default, MallocSizeOf)] pub struct DomParallelInfo { diff --git a/components/shared/script_layout/wrapper_traits.rs b/components/shared/script_layout/wrapper_traits.rs index ad37763c202..c1bd1fc4d95 100644 --- a/components/shared/script_layout/wrapper_traits.rs +++ b/components/shared/script_layout/wrapper_traits.rs @@ -25,7 +25,9 @@ use style::selector_parser::{PseudoElement, PseudoElementCascadeType, SelectorIm use style::stylist::RuleInclusion; use webrender_api::ExternalScrollId; -use crate::{HTMLCanvasData, HTMLMediaData, LayoutNodeType, SVGSVGData, StyleAndOpaqueLayoutData}; +use crate::{ + GenericLayoutData, HTMLCanvasData, HTMLMediaData, LayoutNodeType, SVGSVGData, StyleData, +}; pub trait LayoutDataTrait: Default + Send + Sync + 'static {} @@ -70,19 +72,12 @@ impl PseudoElementType { } } -/// Trait to abstract access to layout data across various data structures. -pub trait GetStyleAndOpaqueLayoutData<'dom> { - fn get_style_and_opaque_layout_data(self) -> Option<&'dom StyleAndOpaqueLayoutData>; -} - /// A wrapper so that layout can access only the methods that it should have access to. Layout must /// only ever see these and must never see instances of `LayoutDom`. /// FIXME(mrobinson): `Send + Sync` is required here for Layout 2020, but eventually it /// should stop sending LayoutNodes to other threads and rely on ThreadSafeLayoutNode /// or some other mechanism to ensure thread safety. -pub trait LayoutNode<'dom>: - Copy + Debug + GetStyleAndOpaqueLayoutData<'dom> + TNode + Send + Sync -{ +pub trait LayoutNode<'dom>: Copy + Debug + TNode + Send + Sync { type ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode<'dom>; fn to_threadsafe(&self) -> Self::ConcreteThreadSafeLayoutNode; @@ -96,7 +91,23 @@ pub trait LayoutNode<'dom>: /// This method is unsafe because it modifies the given node during /// layout. Callers should ensure that no other layout thread is /// attempting to read or modify the opaque layout data of this node. - unsafe fn initialize_data(&self); + unsafe fn initialize_style_and_layout_data(&self); + + /// Initialize this node with empty opaque layout data. + /// + /// # Safety + /// + /// This method is unsafe because it modifies the given node during + /// layout. Callers should ensure that no other layout thread is + /// attempting to read or modify the opaque layout data of this node. + fn initialize_layout_data(&self); + + /// Get the [`StyleData`] for this node. Returns None if the node is unstyled. + fn style_data(&self) -> Option<&'dom StyleData>; + + /// Get the layout data of this node, attempting to downcast it to the desired type. + /// Returns None if there is no layout data or it isn't of the desired type. + fn layout_data(&self) -> Option<&'dom GenericLayoutData>; fn rev_children(self) -> LayoutIterator> { LayoutIterator(ReverseChildrenIterator { @@ -162,9 +173,7 @@ where /// A thread-safe version of `LayoutNode`, used during flow construction. This type of layout /// node does not allow any parents or siblings of nodes to be accessed, to avoid races. -pub trait ThreadSafeLayoutNode<'dom>: - Clone + Copy + Debug + GetStyleAndOpaqueLayoutData<'dom> + NodeInfo + PartialEq + Sized -{ +pub trait ThreadSafeLayoutNode<'dom>: Clone + Copy + Debug + NodeInfo + PartialEq + Sized { type ConcreteNode: LayoutNode<'dom, ConcreteThreadSafeLayoutNode = Self>; type ConcreteElement: TElement; @@ -227,7 +236,12 @@ pub trait ThreadSafeLayoutNode<'dom>: .map_or(PseudoElementType::Normal, |el| el.get_pseudo_element_type()) } - fn get_style_and_opaque_layout_data(self) -> Option<&'dom StyleAndOpaqueLayoutData>; + /// Get the [`StyleData`] for this node. Returns None if the node is unstyled. + fn style_data(&self) -> Option<&'dom StyleData>; + + /// Get the layout data of this node, attempting to downcast it to the desired type. + /// Returns None if there is no layout data or it isn't of the desired type. + fn layout_data(&self) -> Option<&'dom GenericLayoutData>; fn style(&self, context: &SharedStyleContext) -> Arc { if let Some(el) = self.as_element() { @@ -309,12 +323,7 @@ pub trait ThreadSafeLayoutNode<'dom>: } pub trait ThreadSafeLayoutElement<'dom>: - Clone - + Copy - + Sized - + Debug - + ::selectors::Element - + GetStyleAndOpaqueLayoutData<'dom> + Clone + Copy + Sized + Debug + ::selectors::Element { type ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode< 'dom, diff --git a/tests/unit/script/size_of.rs b/tests/unit/script/size_of.rs index c6849956124..d39f6ebacd7 100644 --- a/tests/unit/script/size_of.rs +++ b/tests/unit/script/size_of.rs @@ -30,10 +30,10 @@ macro_rules! sizeof_checker ( // Update the sizes here sizeof_checker!(size_event_target, EventTarget, 48); -sizeof_checker!(size_node, Node, 184); -sizeof_checker!(size_element, Element, 360); -sizeof_checker!(size_htmlelement, HTMLElement, 376); -sizeof_checker!(size_div, HTMLDivElement, 376); -sizeof_checker!(size_span, HTMLSpanElement, 376); -sizeof_checker!(size_text, Text, 216); -sizeof_checker!(size_characterdata, CharacterData, 216); +sizeof_checker!(size_node, Node, 200); +sizeof_checker!(size_element, Element, 376); +sizeof_checker!(size_htmlelement, HTMLElement, 392); +sizeof_checker!(size_div, HTMLDivElement, 392); +sizeof_checker!(size_span, HTMLSpanElement, 392); +sizeof_checker!(size_text, Text, 232); +sizeof_checker!(size_characterdata, CharacterData, 232);