diff options
21 files changed, 184 insertions, 247 deletions
diff --git a/components/layout/data.rs b/components/layout/data.rs index 5f6672d1488..b965bc44ebd 100644 --- a/components/layout/data.rs +++ b/components/layout/data.rs @@ -8,8 +8,7 @@ use script_layout_interface::StyleData; #[repr(C)] pub struct StyleAndLayoutData { - /// Data accessed by script_layout_interface. This must be first to allow - /// casting between StyleAndLayoutData and StyleData. + /// The style data associated with a node. pub style_data: StyleData, /// The layout data associated with a node. pub layout_data: AtomicRefCell<LayoutData>, diff --git a/components/layout/query.rs b/components/layout/query.rs index cd3e75bf28c..b1e33a1dedf 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -6,6 +6,7 @@ use crate::construct::ConstructionResult; use crate::context::LayoutContext; +use crate::data::StyleAndLayoutData; use crate::display_list::items::{DisplayList, OpaqueNode, ScrollOffsetMap}; use crate::display_list::IndexableText; use crate::flow::{Flow, GetBaseFlow}; @@ -26,7 +27,6 @@ use script_layout_interface::rpc::{OffsetParentResponse, ResolvedStyleResponse, use script_layout_interface::wrapper_traits::{ LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode, }; -use script_layout_interface::StyleData; use script_layout_interface::{LayoutElementType, LayoutNodeType}; use script_traits::LayoutMsg as ConstellationMsg; use script_traits::UntrustedNodeAddress; @@ -761,7 +761,7 @@ pub fn process_resolved_style_request<'dom>( // We call process_resolved_style_request after performing a whole-document // traversal, so in the common case, the element is styled. - if element.get_data().is_some() { + if element.has_data() { return process_resolved_style_request_internal(node, pseudo, property, layout_root); } @@ -1036,9 +1036,14 @@ fn inner_text_collection_steps<'dom>( _ => child, }; - let element_data = unsafe { - node.get_style_and_layout_data() - .map(|d| &(*(d.ptr.as_ptr() as *mut StyleData)).element_data) + let element_data = { + &node.get_style_and_layout_data().as_ref().map(|opaque| { + &opaque + .downcast_ref::<StyleAndLayoutData>() + .unwrap() + .style_data + .element_data + }) }; if element_data.is_none() { diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 60eace1f16a..04648ad3abf 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -39,23 +39,21 @@ use style::selector_parser::RestyleDamage; use style::values::computed::counters::ContentItem; use style::values::generics::counters::Content; -pub trait LayoutNodeLayoutData { - /// Similar to borrow_data*, but returns the full PersistentLayoutData rather - /// than only the style::data::ElementData. - fn borrow_layout_data(&self) -> Option<AtomicRef<LayoutData>>; - fn mutate_layout_data(&self) -> Option<AtomicRefMut<LayoutData>>; +pub trait LayoutNodeLayoutData<'dom> { + fn borrow_layout_data(self) -> Option<AtomicRef<'dom, LayoutData>>; + fn mutate_layout_data(self) -> Option<AtomicRefMut<'dom, LayoutData>>; fn flow_debug_id(self) -> usize; } -impl<'dom, T> LayoutNodeLayoutData for T +impl<'dom, T> LayoutNodeLayoutData<'dom> for T where T: GetLayoutData<'dom>, { - fn borrow_layout_data(&self) -> Option<AtomicRef<LayoutData>> { + fn borrow_layout_data(self) -> Option<AtomicRef<'dom, LayoutData>> { self.get_raw_data().map(|d| d.layout_data.borrow()) } - fn mutate_layout_data(&self) -> Option<AtomicRefMut<LayoutData>> { + fn mutate_layout_data(self) -> Option<AtomicRefMut<'dom, LayoutData>> { self.get_raw_data().map(|d| d.layout_data.borrow_mut()) } @@ -65,19 +63,17 @@ where } } -pub trait GetRawData { - fn get_raw_data(&self) -> Option<&StyleAndLayoutData>; +pub trait GetRawData<'dom> { + fn get_raw_data(self) -> Option<&'dom StyleAndLayoutData>; } -impl<'dom, T> GetRawData for T +impl<'dom, T> GetRawData<'dom> for T where T: GetLayoutData<'dom>, { - fn get_raw_data(&self) -> Option<&StyleAndLayoutData> { - self.get_style_and_layout_data().map(|opaque| { - let container = opaque.ptr.as_ptr() as *mut StyleAndLayoutData; - unsafe { &*container } - }) + fn get_raw_data(self) -> Option<&'dom StyleAndLayoutData> { + self.get_style_and_layout_data() + .map(|opaque| opaque.downcast_ref().unwrap()) } } diff --git a/components/layout_2020/dom_traversal.rs b/components/layout_2020/dom_traversal.rs index ef4570a85bb..8ea6bb01de7 100644 --- a/components/layout_2020/dom_traversal.rs +++ b/components/layout_2020/dom_traversal.rs @@ -369,7 +369,7 @@ pub(crate) trait NodeExt<'dom>: 'dom + Copy + LayoutNode<'dom> + Send + Sync { fn style(self, context: &LayoutContext) -> ServoArc<ComputedValues>; fn as_opaque(self) -> OpaqueNode; - fn layout_data_mut(&self) -> AtomicRefMut<LayoutDataForElement>; + fn layout_data_mut(self) -> AtomicRefMut<'dom, LayoutDataForElement>; 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); @@ -446,7 +446,8 @@ where self.opaque() } - fn layout_data_mut(&self) -> AtomicRefMut<LayoutDataForElement> { + #[allow(unsafe_code)] + fn layout_data_mut(self) -> AtomicRefMut<'dom, LayoutDataForElement> { self.get_raw_data() .map(|d| d.layout_data.borrow_mut()) .unwrap() diff --git a/components/layout_2020/wrapper.rs b/components/layout_2020/wrapper.rs index 504ab9af2cc..e7c5ab5a7b1 100644 --- a/components/layout_2020/wrapper.rs +++ b/components/layout_2020/wrapper.rs @@ -7,18 +7,16 @@ use crate::data::StyleAndLayoutData; use script_layout_interface::wrapper_traits::GetLayoutData; -pub trait GetRawData { - fn get_raw_data(&self) -> Option<&StyleAndLayoutData>; +pub trait GetRawData<'dom> { + fn get_raw_data(self) -> Option<&'dom StyleAndLayoutData>; } -impl<'dom, T> GetRawData for T +impl<'dom, T> GetRawData<'dom> for T where T: GetLayoutData<'dom>, { - fn get_raw_data(&self) -> Option<&StyleAndLayoutData> { - self.get_style_and_layout_data().map(|opaque| { - let container = opaque.ptr.as_ptr() as *mut StyleAndLayoutData; - unsafe { &*container } - }) + fn get_raw_data(self) -> Option<&'dom StyleAndLayoutData> { + self.get_style_and_layout_data() + .map(|opaque| opaque.downcast_ref().unwrap()) } } diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 9237e9d47e7..cbd0df1f9a4 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -30,7 +30,7 @@ #![allow(unsafe_code)] -use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; +use atomic_refcell::{AtomicRef, AtomicRefMut}; use gfx_traits::ByteIndex; use html5ever::{LocalName, Namespace}; use layout::data::StyleAndLayoutData; @@ -70,7 +70,6 @@ use std::borrow::Cow; use std::fmt; use std::fmt::Debug; use std::hash::{Hash, Hasher}; -use std::ptr::NonNull; use std::sync::atomic::Ordering; use std::sync::Arc as StdArc; use style::applicable_declarations::ApplicableDeclarationBlock; @@ -92,12 +91,6 @@ use style::str::is_whitespace; use style::stylist::CascadeData; use style::CaseSensitivityExt; -pub unsafe fn drop_style_and_layout_data(data: OpaqueStyleAndLayoutData) { - let ptr = data.ptr.as_ptr() as *mut StyleData; - let non_opaque: *mut StyleAndLayoutData = ptr as *mut _; - let _ = Box::from_raw(non_opaque); -} - #[derive(Clone, Copy)] pub struct ServoLayoutNode<'dom> { /// The wrapped node. @@ -237,7 +230,7 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { } fn opaque(&self) -> OpaqueNode { - unsafe { self.get_jsmanaged().opaque() } + self.get_jsmanaged().opaque() } fn debug_id(self) -> usize { @@ -276,10 +269,7 @@ impl<'ln> LayoutNode<'ln> for ServoLayoutNode<'ln> { unsafe fn initialize_data(&self) { if self.get_raw_data().is_none() { - let ptr: *mut StyleAndLayoutData = Box::into_raw(Box::new(StyleAndLayoutData::new())); - let opaque = OpaqueStyleAndLayoutData { - ptr: NonNull::new_unchecked(ptr as *mut StyleData), - }; + let opaque = OpaqueStyleAndLayoutData::new(StyleAndLayoutData::new()); self.init_style_and_layout_data(opaque); }; } @@ -297,34 +287,33 @@ impl<'ln> LayoutNode<'ln> for ServoLayoutNode<'ln> { } } -impl<'ln> GetLayoutData<'ln> for ServoLayoutNode<'ln> { - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> { - unsafe { self.get_jsmanaged().get_style_and_layout_data() } +impl<'dom> GetLayoutData<'dom> for ServoLayoutNode<'dom> { + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> { + self.get_jsmanaged().get_style_and_layout_data() } } -impl<'le> GetLayoutData<'le> for ServoLayoutElement<'le> { - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> { +impl<'dom> GetLayoutData<'dom> for ServoLayoutElement<'dom> { + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> { self.as_node().get_style_and_layout_data() } } -impl<'ln> GetLayoutData<'ln> for ServoThreadSafeLayoutNode<'ln> { - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> { +impl<'dom> GetLayoutData<'dom> for ServoThreadSafeLayoutNode<'dom> { + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> { self.node.get_style_and_layout_data() } } -impl<'le> GetLayoutData<'le> for ServoThreadSafeLayoutElement<'le> { - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> { +impl<'dom> GetLayoutData<'dom> for ServoThreadSafeLayoutElement<'dom> { + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> { self.element.as_node().get_style_and_layout_data() } } -impl<'ln> ServoLayoutNode<'ln> { - /// Returns the interior of this node as a `LayoutDom`. This is highly unsafe for layout to - /// call and as such is marked `unsafe`. - pub unsafe fn get_jsmanaged(&self) -> LayoutDom<'ln, Node> { +impl<'dom> ServoLayoutNode<'dom> { + /// Returns the interior of this node as a `LayoutDom`. + pub fn get_jsmanaged(self) -> LayoutDom<'dom, Node> { self.node } } @@ -548,7 +537,7 @@ impl<'le> TElement for ServoLayoutElement<'le> { unsafe fn clear_data(&self) { if self.get_raw_data().is_some() { - drop_style_and_layout_data(self.as_node().take_style_and_layout_data()); + drop(self.as_node().take_style_and_layout_data()); } } @@ -557,11 +546,20 @@ impl<'le> TElement for ServoLayoutElement<'le> { self.mutate_data().unwrap() } - fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> { - unsafe { - self.get_style_and_layout_data() - .map(|d| &(*(d.ptr.as_ptr() as *mut StyleData)).element_data) - } + /// Whether there is an ElementData container. + fn has_data(&self) -> bool { + self.get_style_data().is_some() + } + + /// Immutably borrows the ElementData. + fn borrow_data(&self) -> Option<AtomicRef<ElementData>> { + self.get_style_data().map(|data| data.element_data.borrow()) + } + + /// Mutably borrows the ElementData. + fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> { + self.get_style_data() + .map(|data| data.element_data.borrow_mut()) } fn skip_item_display_fixup(&self) -> bool { @@ -697,10 +695,12 @@ impl<'le> ServoLayoutElement<'le> { } fn get_style_data(&self) -> Option<&StyleData> { - unsafe { - self.get_style_and_layout_data() - .map(|d| &*(d.ptr.as_ptr() as *mut StyleData)) - } + self.get_style_and_layout_data().map(|opaque| { + &opaque + .downcast_ref::<StyleAndLayoutData>() + .unwrap() + .style_data + }) } pub unsafe fn unset_snapshot_flags(&self) { @@ -1034,7 +1034,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> for ServoThreadSafeLayoutNode<'ln> { fn parent_style(&self) -> Arc<ComputedValues> { let parent = self.node.parent_node().unwrap().as_element().unwrap(); - let parent_data = parent.get_data().unwrap().borrow(); + let parent_data = parent.borrow_data().unwrap(); parent_data.styles.primary().clone() } @@ -1060,7 +1060,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> for ServoThreadSafeLayoutNode<'ln> { }) } - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> { + fn get_style_and_layout_data(self) -> Option<&'ln OpaqueStyleAndLayoutData> { self.node.get_style_and_layout_data() } @@ -1319,10 +1319,7 @@ impl<'le> ThreadSafeLayoutElement<'le> for ServoThreadSafeLayoutElement<'le> { } fn style_data(&self) -> AtomicRef<ElementData> { - self.element - .get_data() - .expect("Unstyled layout node?") - .borrow() + self.element.borrow_data().expect("Unstyled layout node?") } fn is_shadow_host(&self) -> bool { diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 54f1ec8d656..4c458065986 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -23,7 +23,6 @@ extern crate profile_traits; mod dom_wrapper; -use crate::dom_wrapper::drop_style_and_layout_data; use crate::dom_wrapper::{ServoLayoutDocument, ServoLayoutElement, ServoLayoutNode}; use app_units::Au; use crossbeam_channel::{unbounded, Receiver, Sender}; @@ -692,7 +691,6 @@ impl LayoutThread { Msg::GetRPC(..) => LayoutHangAnnotation::GetRPC, Msg::TickAnimations(..) => LayoutHangAnnotation::TickAnimations, Msg::AdvanceClockMs(..) => LayoutHangAnnotation::AdvanceClockMs, - Msg::ReapStyleAndLayoutData(..) => LayoutHangAnnotation::ReapStyleAndLayoutData, Msg::CollectReports(..) => LayoutHangAnnotation::CollectReports, Msg::PrepareToExit(..) => LayoutHangAnnotation::PrepareToExit, Msg::ExitNow => LayoutHangAnnotation::ExitNow, @@ -833,9 +831,6 @@ impl LayoutThread { webrender_api::ScrollClamping::ToContentBounds, ); }, - Msg::ReapStyleAndLayoutData(dead_data) => unsafe { - drop_style_and_layout_data(dead_data) - }, Msg::CollectReports(reports_chan) => { self.collect_reports(reports_chan, possibly_locked_rw_data); }, @@ -967,9 +962,6 @@ impl LayoutThread { response_chan.send(()).unwrap(); loop { match self.port.recv().unwrap() { - Msg::ReapStyleAndLayoutData(dead_data) => unsafe { - drop_style_and_layout_data(dead_data) - }, Msg::ExitNow => { debug!("layout thread is exiting..."); self.exit_now(); @@ -1472,7 +1464,7 @@ impl LayoutThread { // If we haven't styled this node yet, we don't need to track a // restyle. - let style_data = match el.get_data() { + let mut style_data = match el.mutate_data() { Some(d) => d, None => { unsafe { el.unset_snapshot_flags() }; @@ -1485,8 +1477,6 @@ impl LayoutThread { map.insert(el.as_node().opaque(), s); } - let mut style_data = style_data.borrow_mut(); - // Stash the data on the element for processing by the style system. style_data.hint.insert(restyle.hint.into()); style_data.damage = restyle.damage; diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index 5a722cd82b8..b396eeee5cc 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -30,7 +30,7 @@ #![allow(unsafe_code)] -use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; +use atomic_refcell::{AtomicRef, AtomicRefMut}; use gfx_traits::ByteIndex; use html5ever::{LocalName, Namespace}; use layout::data::StyleAndLayoutData; @@ -70,7 +70,6 @@ use std::borrow::Cow; use std::fmt; use std::fmt::Debug; use std::hash::{Hash, Hasher}; -use std::ptr::NonNull; use std::sync::atomic::Ordering; use std::sync::Arc as StdArc; use style::applicable_declarations::ApplicableDeclarationBlock; @@ -92,12 +91,6 @@ use style::str::is_whitespace; use style::stylist::CascadeData; use style::CaseSensitivityExt; -pub unsafe fn drop_style_and_layout_data(data: OpaqueStyleAndLayoutData) { - let ptr = data.ptr.as_ptr() as *mut StyleData; - let non_opaque: *mut StyleAndLayoutData = ptr as *mut _; - let _ = Box::from_raw(non_opaque); -} - #[derive(Clone, Copy)] pub struct ServoLayoutNode<'dom> { /// The wrapped node. @@ -283,10 +276,7 @@ impl<'ln> LayoutNode<'ln> for ServoLayoutNode<'ln> { unsafe fn initialize_data(&self) { if self.get_raw_data().is_none() { - let ptr: *mut StyleAndLayoutData = Box::into_raw(Box::new(StyleAndLayoutData::new())); - let opaque = OpaqueStyleAndLayoutData { - ptr: NonNull::new_unchecked(ptr as *mut StyleData), - }; + let opaque = OpaqueStyleAndLayoutData::new(StyleAndLayoutData::new()); self.init_style_and_layout_data(opaque); }; } @@ -304,26 +294,26 @@ impl<'ln> LayoutNode<'ln> for ServoLayoutNode<'ln> { } } -impl<'ln> GetLayoutData<'ln> for ServoLayoutNode<'ln> { - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> { +impl<'dom> GetLayoutData<'dom> for ServoLayoutNode<'dom> { + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> { unsafe { self.get_jsmanaged().get_style_and_layout_data() } } } -impl<'le> GetLayoutData<'le> for ServoLayoutElement<'le> { - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> { +impl<'dom> GetLayoutData<'dom> for ServoLayoutElement<'dom> { + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> { self.as_node().get_style_and_layout_data() } } -impl<'ln> GetLayoutData<'ln> for ServoThreadSafeLayoutNode<'ln> { - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> { +impl<'dom> GetLayoutData<'dom> for ServoThreadSafeLayoutNode<'dom> { + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> { self.node.get_style_and_layout_data() } } -impl<'le> GetLayoutData<'le> for ServoThreadSafeLayoutElement<'le> { - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> { +impl<'dom> GetLayoutData<'dom> for ServoThreadSafeLayoutElement<'dom> { + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> { self.element.as_node().get_style_and_layout_data() } } @@ -555,7 +545,7 @@ impl<'le> TElement for ServoLayoutElement<'le> { unsafe fn clear_data(&self) { if self.get_raw_data().is_some() { - drop_style_and_layout_data(self.as_node().take_style_and_layout_data()); + drop(self.as_node().take_style_and_layout_data()); } } @@ -564,11 +554,20 @@ impl<'le> TElement for ServoLayoutElement<'le> { self.mutate_data().unwrap() } - fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> { - unsafe { - self.get_style_and_layout_data() - .map(|d| &(*(d.ptr.as_ptr() as *mut StyleData)).element_data) - } + /// Whether there is an ElementData container. + fn has_data(&self) -> bool { + self.get_style_data().is_some() + } + + /// Immutably borrows the ElementData. + fn borrow_data(&self) -> Option<AtomicRef<ElementData>> { + self.get_style_data().map(|data| data.element_data.borrow()) + } + + /// Mutably borrows the ElementData. + fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> { + self.get_style_data() + .map(|data| data.element_data.borrow_mut()) } fn skip_item_display_fixup(&self) -> bool { @@ -704,10 +703,12 @@ impl<'le> ServoLayoutElement<'le> { } fn get_style_data(&self) -> Option<&StyleData> { - unsafe { - self.get_style_and_layout_data() - .map(|d| &*(d.ptr.as_ptr() as *mut StyleData)) - } + self.get_style_and_layout_data().map(|opaque| { + &opaque + .downcast_ref::<StyleAndLayoutData>() + .unwrap() + .style_data + }) } pub unsafe fn unset_snapshot_flags(&self) { @@ -1041,7 +1042,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> for ServoThreadSafeLayoutNode<'ln> { fn parent_style(&self) -> Arc<ComputedValues> { let parent = self.node.parent_node().unwrap().as_element().unwrap(); - let parent_data = parent.get_data().unwrap().borrow(); + let parent_data = parent.borrow_data().unwrap(); parent_data.styles.primary().clone() } @@ -1067,7 +1068,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> for ServoThreadSafeLayoutNode<'ln> { }) } - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> { + fn get_style_and_layout_data(self) -> Option<&'ln OpaqueStyleAndLayoutData> { self.node.get_style_and_layout_data() } @@ -1326,10 +1327,7 @@ impl<'le> ThreadSafeLayoutElement<'le> for ServoThreadSafeLayoutElement<'le> { } fn style_data(&self) -> AtomicRef<ElementData> { - self.element - .get_data() - .expect("Unstyled layout node?") - .borrow() + self.element.borrow_data().expect("Unstyled layout node?") } fn is_shadow_host(&self) -> bool { diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs index 05663f7d68b..812a8823f38 100644 --- a/components/layout_thread_2020/lib.rs +++ b/components/layout_thread_2020/lib.rs @@ -21,7 +21,6 @@ extern crate profile_traits; mod dom_wrapper; -use crate::dom_wrapper::drop_style_and_layout_data; use crate::dom_wrapper::{ServoLayoutDocument, ServoLayoutElement, ServoLayoutNode}; use app_units::Au; use crossbeam_channel::{unbounded, Receiver, Sender}; @@ -640,7 +639,6 @@ impl LayoutThread { Msg::GetRPC(..) => LayoutHangAnnotation::GetRPC, Msg::TickAnimations(..) => LayoutHangAnnotation::TickAnimations, Msg::AdvanceClockMs(..) => LayoutHangAnnotation::AdvanceClockMs, - Msg::ReapStyleAndLayoutData(..) => LayoutHangAnnotation::ReapStyleAndLayoutData, Msg::CollectReports(..) => LayoutHangAnnotation::CollectReports, Msg::PrepareToExit(..) => LayoutHangAnnotation::PrepareToExit, Msg::ExitNow => LayoutHangAnnotation::ExitNow, @@ -779,9 +777,6 @@ impl LayoutThread { webrender_api::ScrollClamping::ToContentBounds, ); }, - Msg::ReapStyleAndLayoutData(dead_data) => unsafe { - drop_style_and_layout_data(dead_data) - }, Msg::CollectReports(reports_chan) => { self.collect_reports(reports_chan, possibly_locked_rw_data); }, @@ -890,9 +885,6 @@ impl LayoutThread { response_chan.send(()).unwrap(); loop { match self.port.recv().unwrap() { - Msg::ReapStyleAndLayoutData(dead_data) => unsafe { - drop_style_and_layout_data(dead_data) - }, Msg::ExitNow => { debug!("layout thread is exiting..."); self.exit_now(); @@ -1118,7 +1110,7 @@ impl LayoutThread { // If we haven't styled this node yet, we don't need to track a // restyle. - let style_data = match el.get_data() { + let mut style_data = match el.mutate_data() { Some(d) => d, None => { unsafe { el.unset_snapshot_flags() }; @@ -1131,8 +1123,6 @@ impl LayoutThread { map.insert(el.as_node().opaque(), s); } - let mut style_data = style_data.borrow_mut(); - // Stash the data on the element for processing by the style system. style_data.hint.insert(restyle.hint.into()); style_data.damage = restyle.damage; diff --git a/components/msg/constellation_msg.rs b/components/msg/constellation_msg.rs index a878bc43d3d..ae0ceb39b18 100644 --- a/components/msg/constellation_msg.rs +++ b/components/msg/constellation_msg.rs @@ -532,7 +532,6 @@ pub enum LayoutHangAnnotation { GetRPC, TickAnimations, AdvanceClockMs, - ReapStyleAndLayoutData, CollectReports, PrepareToExit, ExitNow, diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index f9007bcf9c6..8530078c919 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -65,7 +65,6 @@ use crate::dom::virtualmethods::{vtable_for, VirtualMethods}; use crate::dom::window::Window; use crate::script_thread::ScriptThread; use app_units::Au; -use crossbeam_channel::Sender; use devtools_traits::NodeInfo; use dom_struct::dom_struct; use euclid::default::{Point2D, Rect, Size2D, Vector2D}; @@ -76,7 +75,6 @@ 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::Msg; use script_layout_interface::{HTMLCanvasData, HTMLMediaData, LayoutElementType, LayoutNodeType}; use script_layout_interface::{OpaqueStyleAndLayoutData, SVGSVGData, TrustedNodeAddress}; use script_traits::DocumentActivity; @@ -99,7 +97,6 @@ use style::context::QuirksMode; use style::dom::OpaqueNode; use style::selector_parser::{SelectorImpl, SelectorParser}; use style::stylesheets::Stylesheet; -use style::thread_state; use uuid::Uuid; // @@ -155,7 +152,8 @@ pub struct Node { /// /// Must be sent back to the layout thread to be destroyed when this /// node is finalized. - style_and_layout_data: Cell<Option<OpaqueStyleAndLayoutData>>, + #[ignore_malloc_size_of = "shrug"] + style_and_layout_data: UnsafeCell<Option<OpaqueStyleAndLayoutData>>, } bitflags! { @@ -205,15 +203,6 @@ impl NodeFlags { } } -impl Drop for Node { - #[allow(unsafe_code)] - fn drop(&mut self) { - if let Some(data) = self.style_and_layout_data.get() { - self.dispose(data, ScriptThread::get_any_layout_chan().as_ref()); - } - } -} - /// suppress observers flag /// <https://dom.spec.whatwg.org/#concept-node-insert> /// <https://dom.spec.whatwg.org/#concept-node-remove> @@ -224,21 +213,6 @@ enum SuppressObserver { } impl Node { - /// Sends the style and layout data, if any, back to the layout thread to be destroyed. - pub(crate) fn dispose( - &self, - data: OpaqueStyleAndLayoutData, - layout_chan: Option<&Sender<Msg>>, - ) { - debug_assert!(thread_state::get().is_script()); - self.style_and_layout_data.set(None); - if layout_chan.map_or(false, |chan| { - chan.send(Msg::ReapStyleAndLayoutData(data)).is_err() - }) { - warn!("layout thread unreachable - leaking layout data"); - } - } - /// Adds a new child to the end of this node's list of children. /// /// Fails unless `new_child` is disconnected from the tree. @@ -317,16 +291,11 @@ impl Node { false, ); } - let window = window_from_node(root); - let layout_chan = window.layout_chan(); for node in root.traverse_preorder(ShadowIncluding::Yes) { // 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, // e.g. when removing a <form>. vtable_for(&&*node).unbind_from_tree(&context); - if let Some(data) = node.style_and_layout_data.get() { - node.dispose(data, Some(layout_chan)); - } // https://dom.spec.whatwg.org/#concept-node-remove step 14 if let Some(element) = node.as_custom_element() { ScriptThread::enqueue_callback_reaction( @@ -507,15 +476,6 @@ impl<'a> Iterator for QuerySelectorIterator { impl Node { impl_rare_data!(NodeRareData); - pub(crate) fn teardown(&self, layout_chan: &Sender<Msg>) { - if let Some(data) = self.style_and_layout_data.get() { - self.dispose(data, Some(layout_chan)); - } - for kid in self.children() { - kid.teardown(layout_chan); - } - } - /// Returns true if this node is before `other` in the same connected DOM /// tree. pub fn is_before(&self, other: &Node) -> bool { @@ -1322,8 +1282,8 @@ pub trait LayoutNodeHelpers<'dom> { fn children_count(self) -> u32; - unsafe fn get_style_and_layout_data(self) -> Option<OpaqueStyleAndLayoutData>; - unsafe fn init_style_and_layout_data(self, _: OpaqueStyleAndLayoutData); + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData>; + unsafe fn init_style_and_layout_data(self, data: OpaqueStyleAndLayoutData); unsafe fn take_style_and_layout_data(self) -> OpaqueStyleAndLayoutData; fn text_content(self) -> Cow<'dom, str>; @@ -1450,23 +1410,24 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { #[inline] #[allow(unsafe_code)] - unsafe fn get_style_and_layout_data(self) -> Option<OpaqueStyleAndLayoutData> { - (*self.unsafe_get()).style_and_layout_data.get() + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> { + unsafe { (*self.unsafe_get().style_and_layout_data.get()).as_ref() } } #[inline] #[allow(unsafe_code)] unsafe fn init_style_and_layout_data(self, val: OpaqueStyleAndLayoutData) { - debug_assert!((*self.unsafe_get()).style_and_layout_data.get().is_none()); - (*self.unsafe_get()).style_and_layout_data.set(Some(val)); + let data = &mut *self.unsafe_get().style_and_layout_data.get(); + debug_assert!(data.is_none()); + *data = Some(val); } #[inline] #[allow(unsafe_code)] unsafe fn take_style_and_layout_data(self) -> OpaqueStyleAndLayoutData { - let val = (*self.unsafe_get()).style_and_layout_data.get().unwrap(); - (*self.unsafe_get()).style_and_layout_data.set(None); - val + (*self.unsafe_get().style_and_layout_data.get()) + .take() + .unwrap() } fn text_content(self) -> Cow<'dom, str> { @@ -1781,7 +1742,7 @@ impl Node { inclusive_descendants_version: Cell::new(0), ranges: WeakRangeVec::new(), - style_and_layout_data: Cell::new(None), + style_and_layout_data: UnsafeCell::new(None), } } diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 36916f3ef51..6782047c5a2 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1399,13 +1399,6 @@ impl Window { } pub fn clear_js_runtime(&self) { - // We tear down the active document, which causes all the attached - // nodes to dispose of their layout data. This messages the layout - // thread, informing it that it can safely free the memory. - self.Document() - .upcast::<Node>() - .teardown(self.layout_chan()); - // Remove the infra for managing messageports and broadcast channels. self.upcast::<GlobalScope>().remove_web_messaging_infra(); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 714c6857d9c..d6a8481afb3 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -822,23 +822,6 @@ impl ScriptThreadFactory for ScriptThread { } impl ScriptThread { - pub(crate) fn get_any_layout_chan() -> Option<Sender<Msg>> { - SCRIPT_THREAD_ROOT.with(|root| { - let script_thread = match root.get() { - Some(s) => unsafe { &*s }, - None => return None, - }; - script_thread - .documents - .borrow() - .map - .values() - .next() - .map(|d| d.window().layout_chan()) - .cloned() - }) - } - pub fn runtime_handle() -> ParentRuntime { SCRIPT_THREAD_ROOT.with(|root| { let script_thread = unsafe { &*root.get().unwrap() }; diff --git a/components/script_layout_interface/lib.rs b/components/script_layout_interface/lib.rs index ef9382378cb..6fb859aa112 100644 --- a/components/script_layout_interface/lib.rs +++ b/components/script_layout_interface/lib.rs @@ -7,6 +7,7 @@ //! to depend on script. #![deny(unsafe_code)] +#![feature(box_into_raw_non_null)] #[macro_use] extern crate html5ever; @@ -24,7 +25,7 @@ use libc::c_void; use net_traits::image_cache::PendingImageId; use script_traits::UntrustedNodeAddress; use servo_url::{ImmutableOrigin, ServoUrl}; -use std::ptr::NonNull; +use std::any::Any; use std::sync::atomic::AtomicIsize; use style::data::ElementData; @@ -49,12 +50,32 @@ impl StyleData { } } -#[derive(Clone, Copy, MallocSizeOf)] +#[derive(MallocSizeOf)] pub struct OpaqueStyleAndLayoutData { // NB: We really store a `StyleAndLayoutData` here, so be careful! - #[ignore_malloc_size_of = "TODO(#6910) Box value that should be counted but \ - the type lives in layout"] - pub ptr: NonNull<StyleData>, + #[ignore_malloc_size_of = "Trait objects are hard"] + ptr: Box<dyn Any + Send + Sync>, +} + +impl OpaqueStyleAndLayoutData { + #[inline] + pub fn new<T>(value: T) -> Self + where + T: Any + Send + Sync, + { + Self { + ptr: Box::new(value) as Box<dyn Any + Send + Sync>, + } + } + + /// Extremely cursed. + #[inline] + pub fn downcast_ref<T>(&self) -> Option<&T> + where + T: Any + Send + Sync, + { + self.ptr.downcast_ref() + } } #[allow(unsafe_code)] diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index 96c4a132368..48ec7223250 100644 --- a/components/script_layout_interface/message.rs +++ b/components/script_layout_interface/message.rs @@ -3,7 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use crate::rpc::LayoutRPC; -use crate::{OpaqueStyleAndLayoutData, PendingImage, TrustedNodeAddress}; +use crate::{PendingImage, TrustedNodeAddress}; use app_units::Au; use crossbeam_channel::{Receiver, Sender}; use euclid::default::{Point2D, Rect}; @@ -56,11 +56,6 @@ pub enum Msg { /// field is whether animations should be force-ticked. AdvanceClockMs(i32, bool, ImmutableOrigin), - /// Destroys layout data associated with a DOM node. - /// - /// TODO(pcwalton): Maybe think about batching to avoid message traffic. - ReapStyleAndLayoutData(OpaqueStyleAndLayoutData), - /// Requests that the layout thread measure its memory usage. The resulting reports are sent back /// via the supplied channel. CollectReports(ReportsChan), diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 462d11f2c7e..671dd1c8230 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -80,7 +80,7 @@ impl PseudoElementType { /// Trait to abstract access to layout data across various data structures. pub trait GetLayoutData<'dom> { - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData>; + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData>; } /// A wrapper so that layout can access only the methods that it should have access to. Layout must @@ -224,7 +224,7 @@ pub trait ThreadSafeLayoutNode<'dom>: .map_or(PseudoElementType::Normal, |el| el.get_pseudo_element_type()) } - fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData>; + fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData>; fn style(&self, context: &SharedStyleContext) -> Arc<ComputedValues> { if let Some(el) = self.as_element() { diff --git a/components/style/dom.rs b/components/style/dom.rs index 7629bb11e18..d07cac998ae 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -22,7 +22,7 @@ use crate::shared_lock::Locked; use crate::stylist::CascadeData; use crate::traversal_flags::TraversalFlags; use crate::{Atom, LocalName, Namespace, WeakAtom}; -use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; +use atomic_refcell::{AtomicRef, AtomicRefMut}; use selectors::matching::{ElementSelectorFlags, QuirksMode, VisitedHandlingMode}; use selectors::sink::Push; use selectors::Element as SelectorsElement; @@ -689,18 +689,14 @@ pub trait TElement: /// Unsafe following the same reasoning as ensure_data. unsafe fn clear_data(&self); - /// Gets a reference to the ElementData container. - fn get_data(&self) -> Option<&AtomicRefCell<ElementData>>; + /// Whether there is an ElementData container. + fn has_data(&self) -> bool; /// Immutably borrows the ElementData. - fn borrow_data(&self) -> Option<AtomicRef<ElementData>> { - self.get_data().map(|x| x.borrow()) - } + fn borrow_data(&self) -> Option<AtomicRef<ElementData>>; /// Mutably borrows the ElementData. - fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> { - self.get_data().map(|x| x.borrow_mut()) - } + fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>>; /// Whether we should skip any root- or item-based display property /// blockification on this element. (This function exists so that Gecko diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index ba1c724d58f..2b842e72d9b 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -558,6 +558,11 @@ impl<'le> fmt::Debug for GeckoElement<'le> { impl<'le> GeckoElement<'le> { #[inline(always)] + fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> { + unsafe { self.0.mServoData.get().as_ref() } + } + + #[inline(always)] fn attrs(&self) -> &[structs::AttrArray_InternalAttr] { unsafe { let attrs = match self.0.mAttrs.mImpl.mPtr.as_ref() { @@ -1299,7 +1304,7 @@ impl<'le> TElement for GeckoElement<'le> { } unsafe fn set_handled_snapshot(&self) { - debug_assert!(self.get_data().is_some()); + debug_assert!(self.has_data()); self.set_flags(ELEMENT_HANDLED_SNAPSHOT as u32) } @@ -1309,7 +1314,7 @@ impl<'le> TElement for GeckoElement<'le> { } unsafe fn set_dirty_descendants(&self) { - debug_assert!(self.get_data().is_some()); + debug_assert!(self.has_data()); debug!("Setting dirty descendants: {:?}", self); self.set_flags(ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32) } @@ -1378,13 +1383,23 @@ impl<'le> TElement for GeckoElement<'le> { panic!("Atomic child count not implemented in Gecko"); } - #[inline(always)] - fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> { - unsafe { self.0.mServoData.get().as_ref() } + /// Whether there is an ElementData container. + fn has_data(&self) -> bool { + self.get_data().is_some() + } + + /// Immutably borrows the ElementData. + fn borrow_data(&self) -> Option<AtomicRef<ElementData>> { + self.get_data().map(|x| x.borrow()) + } + + /// Mutably borrows the ElementData. + fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> { + self.get_data().map(|x| x.borrow_mut()) } unsafe fn ensure_data(&self) -> AtomicRefMut<ElementData> { - if self.get_data().is_none() { + if !self.has_data() { debug!("Creating ElementData for {:?}", self); let ptr = Box::into_raw(Box::new(AtomicRefCell::new(ElementData::default()))); self.0.mServoData.set(ptr); diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index 628f174a3d7..e906f8d8405 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/components/style/invalidation/element/state_and_attributes.rs @@ -90,7 +90,7 @@ pub fn invalidated_descendants<E>(element: E, child: E) where E: TElement, { - if child.get_data().is_none() { + if !child.has_data() { return; } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 254d40f0393..a6aa220ccfa 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -845,7 +845,7 @@ where // // By consequence, any element without data has no descendants with // data. - if kid.get_data().is_some() { + if kid.has_data() { kid.clear_data(); parents.push(kid); } diff --git a/tests/unit/script/size_of.rs b/tests/unit/script/size_of.rs index e3107c13290..4b18decc66a 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, 56); -sizeof_checker!(size_node, Node, 176); -sizeof_checker!(size_element, Element, 352); -sizeof_checker!(size_htmlelement, HTMLElement, 368); -sizeof_checker!(size_div, HTMLDivElement, 368); -sizeof_checker!(size_span, HTMLSpanElement, 368); -sizeof_checker!(size_text, Text, 208); -sizeof_checker!(size_characterdata, CharacterData, 208); +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); |