From fb70ee2c0c907fc079350baa116f7315cf8869e3 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 28 Oct 2016 15:37:22 -0700 Subject: [PATCH] Drop style data from descendants on display:none. MozReview-Commit-ID: 8ls43oAGWRg --- components/layout/traversal.rs | 6 +++- components/layout/wrapper.rs | 13 ++++++++ components/layout_thread/lib.rs | 17 +++-------- components/script/dom/node.rs | 9 ++++++ components/script/layout_wrapper.rs | 4 +++ .../script_layout_interface/wrapper_traits.rs | 1 + components/style/gecko/traversal.rs | 4 +++ components/style/traversal.rs | 30 ++++++++++++++++--- 8 files changed, 66 insertions(+), 18 deletions(-) diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 124793203c9..c897838875a 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -28,6 +28,7 @@ pub struct RecalcStyleAndConstructFlows<'lc> { root: OpaqueNode, } +#[allow(unsafe_code)] impl<'lc, N> DomTraversalContext for RecalcStyleAndConstructFlows<'lc> where N: LayoutNode + TNode, N::ConcreteElement: LayoutElement @@ -133,12 +134,15 @@ impl<'lc, N> DomTraversalContext for RecalcStyleAndConstructFlows<'lc> } } - #[allow(unsafe_code)] unsafe fn ensure_element_data(element: &N::ConcreteElement) -> &AtomicRefCell { element.as_node().initialize_data(); element.get_data().unwrap() } + unsafe fn clear_element_data(element: &N::ConcreteElement) { + element.as_node().clear_data(); + } + fn local_context(&self) -> &LocalStyleContext { self.context.local_context() } diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index bc524395be2..9c1645c5f5e 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -42,6 +42,12 @@ use style::traversal::prepare_for_styling; pub type NonOpaqueStyleAndLayoutData = AtomicRefCell; +pub unsafe fn drop_style_and_layout_data(data: OpaqueStyleAndLayoutData) { + let ptr: *mut AtomicRefCell = *data.ptr; + let non_opaque: *mut NonOpaqueStyleAndLayoutData = ptr as *mut _; + let _ = Box::from_raw(non_opaque); +} + pub trait LayoutNodeLayoutData { /// Similar to borrow_data*, but returns the full PersistentLayoutData rather /// than only the style::data::ElementData. @@ -79,6 +85,7 @@ impl GetRawData for T { pub trait LayoutNodeHelpers { fn initialize_data(&self); + fn clear_data(&self); } impl LayoutNodeHelpers for T { @@ -95,6 +102,12 @@ impl LayoutNodeHelpers for T { } }; } + + fn clear_data(&self) { + if self.get_raw_data().is_some() { + unsafe { drop_style_and_layout_data(self.take_style_and_layout_data()) }; + } + } } pub trait ThreadSafeLayoutNodeHelpers { diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index b531e035c68..8016bb68b44 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -77,7 +77,8 @@ use layout::query::process_offset_parent_query; use layout::sequential; use layout::traversal::{ComputeAbsolutePositions, RecalcStyleAndConstructFlows}; use layout::webrender_helpers::{WebRenderDisplayListConverter, WebRenderFrameBuilder}; -use layout::wrapper::{LayoutNodeLayoutData, NonOpaqueStyleAndLayoutData}; +use layout::wrapper::LayoutNodeLayoutData; +use layout::wrapper::drop_style_and_layout_data; use layout_traits::LayoutThreadFactory; use msg::constellation_msg::PipelineId; use net_traits::image_cache_thread::{ImageCacheChan, ImageCacheResult, ImageCacheThread}; @@ -86,7 +87,6 @@ use profile_traits::mem::{self, Report, ReportKind, ReportsChan}; use profile_traits::time::{self, TimerMetadata, profile}; use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType}; use script::layout_wrapper::{ServoLayoutDocument, ServoLayoutNode}; -use script_layout_interface::{OpaqueStyleAndLayoutData, PartialPersistentLayoutData}; use script_layout_interface::message::{Msg, NewLayoutThreadInfo, Reflow, ReflowQueryType, ScriptReflow}; use script_layout_interface::reporter::CSSErrorReporter; use script_layout_interface::restyle_damage::{REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, REPOSITION}; @@ -105,7 +105,6 @@ use std::sync::{Arc, Mutex, MutexGuard, RwLock}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::mpsc::{Receiver, Sender, channel}; use style::animation::Animation; -use style::atomic_refcell::AtomicRefCell; use style::computed_values::{filter, mix_blend_mode}; use style::context::{LocalStyleContextCreationInfo, ReflowGoal, SharedStyleContext}; use style::dom::{TDocument, TElement, TNode}; @@ -648,7 +647,7 @@ impl LayoutThread { } Msg::ReapStyleAndLayoutData(dead_data) => { unsafe { - self.handle_reap_style_and_layout_data(dead_data) + drop_style_and_layout_data(dead_data) } } Msg::CollectReports(reports_chan) => { @@ -756,7 +755,7 @@ impl LayoutThread { match self.port.recv().unwrap() { Msg::ReapStyleAndLayoutData(dead_data) => { unsafe { - self.handle_reap_style_and_layout_data(dead_data) + drop_style_and_layout_data(dead_data) } } Msg::ExitNow => { @@ -1483,14 +1482,6 @@ impl LayoutThread { } } - /// Handles a message to destroy layout data. Layout data must be destroyed on *this* thread - /// because the struct type is transmuted to a different type on the script side. - unsafe fn handle_reap_style_and_layout_data(&self, data: OpaqueStyleAndLayoutData) { - let ptr: *mut AtomicRefCell = *data.ptr; - let non_opaque: *mut NonOpaqueStyleAndLayoutData = ptr as *mut _; - let _ = Box::from_raw(non_opaque); - } - /// Returns profiling information which is passed to the time profiler. fn profiler_metadata(&self) -> Option { Some(TimerMetadata { diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index ec2c45bfe8c..767a3132eb3 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -952,6 +952,7 @@ pub trait LayoutNodeHelpers { unsafe fn get_style_and_layout_data(&self) -> Option; unsafe fn init_style_and_layout_data(&self, OpaqueStyleAndLayoutData); + unsafe fn take_style_and_layout_data(&self) -> OpaqueStyleAndLayoutData; fn text_content(&self) -> String; fn selection(&self) -> Option>; @@ -1051,6 +1052,14 @@ impl LayoutNodeHelpers for LayoutJS { (*self.unsafe_get()).style_and_layout_data.set(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 + } + #[allow(unsafe_code)] fn text_content(&self) -> String { if let Some(text) = self.downcast::() { diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 2120f4ae5cc..ace0e7f40f7 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -256,6 +256,10 @@ impl<'ln> LayoutNode for ServoLayoutNode<'ln> { self.get_jsmanaged().init_style_and_layout_data(data); } + unsafe fn take_style_and_layout_data(&self) -> OpaqueStyleAndLayoutData { + self.get_jsmanaged().take_style_and_layout_data() + } + fn has_changed(&self) -> bool { unsafe { self.node.get_flag(HAS_CHANGED) } } diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 5f712046eea..04372274cfc 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -85,6 +85,7 @@ pub trait LayoutNode: GetLayoutData + TNode { fn type_id(&self) -> LayoutNodeType; unsafe fn init_style_and_layout_data(&self, data: OpaqueStyleAndLayoutData); + unsafe fn take_style_and_layout_data(&self) -> OpaqueStyleAndLayoutData; fn has_changed(&self) -> bool; diff --git a/components/style/gecko/traversal.rs b/components/style/gecko/traversal.rs index 671f25a9072..6827f4e57b0 100644 --- a/components/style/gecko/traversal.rs +++ b/components/style/gecko/traversal.rs @@ -58,6 +58,10 @@ impl<'lc, 'ln> DomTraversalContext> for RecalcStyleOnly<'lc> { element.ensure_data() } + unsafe fn clear_element_data<'a>(element: &'a GeckoElement<'ln>) { + element.clear_data() + } + fn local_context(&self) -> &LocalStyleContext { self.context.local_context() } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index ede38e82522..be69690f471 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -215,6 +215,12 @@ pub trait DomTraversalContext { prepare_for_styling(*element, Self::ensure_element_data(element)) } + /// Clears the ElementData attached to this element, if any. + /// + /// This is only safe to call in top-down traversal before processing the + /// children of |element|. + unsafe fn clear_element_data(element: &N::ConcreteElement); + fn local_context(&self) -> &LocalStyleContext; } @@ -378,10 +384,26 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C, } } - // If we restyled this node, conservatively mark all our children as needing - // processing. The eventual algorithm we're designing does this in a more granular - // fashion. - if mode == StylingMode::Restyle && !element.is_display_none() { + if element.is_display_none() { + // If this element is display:none, throw away all style data in the subtree. + fn clear_descendant_data>(el: E) { + for kid in el.as_node().children() { + if let Some(kid) = kid.as_element() { + // We maintain an invariant that, if an element has data, all its ancestors + // have data as well. By consequence, any element without data has no + // descendants with data. + if kid.get_data().is_some() { + unsafe { D::clear_element_data(&kid) }; + clear_descendant_data::<_, D>(kid); + } + } + } + }; + clear_descendant_data::<_, D>(element); + } else if mode == StylingMode::Restyle { + // If we restyled this node, conservatively mark all our children as needing + // processing. The eventual algorithm we're designing does this in a more granular + // fashion. for kid in element.as_node().children() { if let Some(kid) = kid.as_element() { unsafe { let _ = D::prepare_for_styling(&kid); }