Drop style data from descendants on display:none.

MozReview-Commit-ID: 8ls43oAGWRg

components/layout/traversal.rs

@@ -28,6 +28,7 @@ pub struct RecalcStyleAndConstructFlows<'lc> {
     root: OpaqueNode,
 }
 
+#[allow(unsafe_code)]
 impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
     where N: LayoutNode + TNode,
           N::ConcreteElement: LayoutElement
@@ -133,12 +134,15 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
         }
     }
 
-    #[allow(unsafe_code)]
     unsafe fn ensure_element_data(element: &N::ConcreteElement) -> &AtomicRefCell<ElementData> {
         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()
     }

components/layout/wrapper.rs

@@ -42,6 +42,12 @@ use style::traversal::prepare_for_styling;
 
 pub type NonOpaqueStyleAndLayoutData = AtomicRefCell<PersistentLayoutData>;
 
+pub unsafe fn drop_style_and_layout_data(data: OpaqueStyleAndLayoutData) {
+    let ptr: *mut AtomicRefCell<PartialPersistentLayoutData> = *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<T: GetLayoutData> GetRawData for T {
 
 pub trait LayoutNodeHelpers {
     fn initialize_data(&self);
+    fn clear_data(&self);
 }
 
 impl<T: LayoutNode> LayoutNodeHelpers for T {
@@ -95,6 +102,12 @@ impl<T: LayoutNode> 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 {

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<PartialPersistentLayoutData> = *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<TimerMetadata> {
         Some(TimerMetadata {

components/script/dom/node.rs

@@ -952,6 +952,7 @@ pub trait LayoutNodeHelpers {
 
     unsafe fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData>;
     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<Range<usize>>;
@@ -1051,6 +1052,14 @@ impl LayoutNodeHelpers for LayoutJS<Node> {
         (*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::<Text>() {

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) }
     }

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;
 

components/style/gecko/traversal.rs

@@ -58,6 +58,10 @@ impl<'lc, 'ln> DomTraversalContext<GeckoNode<'ln>> 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()
     }

components/style/traversal.rs

@@ -215,6 +215,12 @@ pub trait DomTraversalContext<N: TNode> {
         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 element.is_display_none() {
+        // If this element is display:none, throw away all style data in the subtree.
+        fn clear_descendant_data<E: TElement, D: DomTraversalContext<E::ConcreteNode>>(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.
-    if mode == StylingMode::Restyle && !element.is_display_none() {
         for kid in element.as_node().children() {
             if let Some(kid) = kid.as_element() {
                 unsafe { let _ = D::prepare_for_styling(&kid); }