Reap layout data whenever a node is removed from the tree.

Also introduce a clear() function to layout data which will be used to clear items such as compositor layouts.

Clear the layout data when a node becomes display:none.

components/layout/construct.rs

@@ -1326,6 +1326,13 @@ impl<'ln> NodeUtils for ThreadSafeLayoutNode<'ln> {
         let mut layout_data_ref = self.mutate_layout_data();
         let layout_data = layout_data_ref.as_mut().expect("no layout data");
 
+        match result {
+            ConstructionResult::None => {
+                layout_data.clear();
+            }
+            _ => {}
+        }
+
         let dst = self.get_construction_result(layout_data);
 
         *dst = result;

components/layout/layout_task.rs

@@ -35,7 +35,7 @@ use gfx::paint_task::Msg as PaintMsg;
 use layout_traits::{LayoutControlMsg, LayoutTaskFactory};
 use log;
 use script::dom::bindings::js::LayoutJS;
-use script::dom::node::{LayoutDataRef, Node, NodeTypeId};
+use script::dom::node::{LayoutData, Node, NodeTypeId};
 use script::dom::element::ElementTypeId;
 use script::dom::htmlelement::HTMLElementTypeId;
 use script::layout_interface::{ContentBoxResponse, ContentBoxesResponse};
@@ -937,11 +937,10 @@ impl LayoutTask {
     }
 
     /// Handles a message to destroy layout data. Layout data must be destroyed on *this* task
-    /// because it contains local managed pointers.
+    /// because the struct type is transmuted to a different type on the script side.
-    unsafe fn handle_reap_layout_data(layout_data: LayoutDataRef) {
+    unsafe fn handle_reap_layout_data(layout_data: LayoutData) {
-        let mut layout_data_ref = layout_data.borrow_mut();
+        let layout_data_wrapper: LayoutDataWrapper = mem::transmute(layout_data);
-        let _: Option<LayoutDataWrapper> = mem::transmute(
+        layout_data_wrapper.clear();
-            mem::replace(&mut *layout_data_ref, None));
     }
 
     /// Returns profiling information which is passed to the time profiler.

components/layout/util.rs

@@ -78,6 +78,12 @@ pub struct LayoutDataWrapper {
     pub data: Box<PrivateLayoutData>,
 }
 
+impl LayoutDataWrapper {
+    pub fn clear(&self) {
+        // TODO: Clear items related to this node, e.g. compositor layers
+    }
+}
+
 #[allow(dead_code)]
 fn static_assertion(x: Option<LayoutDataWrapper>) {
     unsafe {

components/script/dom/node.rs

@@ -177,9 +177,7 @@ impl NodeFlags {
 impl Drop for Node {
     #[allow(unsafe_blocks)]
     fn drop(&mut self) {
-        unsafe {
+        self.layout_data.dispose();
-            self.reap_layout_data();
-        }
     }
 }
 
@@ -205,6 +203,8 @@ pub struct LayoutData {
     _data: NonZero<*const ()>,
 }
 
+unsafe impl Send for LayoutData {}
+
 pub struct LayoutDataRef {
     pub data_cell: RefCell<Option<LayoutData>>,
 }
@@ -218,18 +218,17 @@ impl LayoutDataRef {
         }
     }
 
-    /// Returns true if there is layout data present.
+    /// Sends layout data, if any, back to the layout task to be destroyed.
-    #[inline]
+    pub fn dispose(&self) {
-    pub fn is_present(&self) -> bool {
+        if let Some(mut layout_data) = mem::replace(&mut *self.borrow_mut(), None) {
-        self.data_cell.borrow().is_some()
+            let layout_chan = layout_data.chan.take();
-    }
+            match layout_chan {
-
+                None => {}
-    /// Take the chan out of the layout data if it is present.
+                Some(chan) => {
-    pub fn take_chan(&self) -> Option<LayoutChan> {
+                    let LayoutChan(chan) = chan;
-        let mut layout_data = self.data_cell.borrow_mut();
+                    chan.send(Msg::ReapLayoutData(layout_data)).unwrap()
-        match &mut *layout_data {
+                }
-            &mut None => None,
+            }
-            &mut Some(ref mut layout_data) => Some(layout_data.chan.take().unwrap()),
         }
     }
 
@@ -258,8 +257,6 @@ impl LayoutDataRef {
     }
 }
 
-unsafe impl Send for LayoutDataRef {}
-
 /// The different types of nodes.
 #[derive(Copy, PartialEq, Debug)]
 #[jstraceable]
@@ -302,6 +299,7 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
         for node in self.traverse_preorder() {
             vtable_for(&node).unbind_from_tree(parent_in_doc);
         }
+        self.layout_data.dispose();
     }
 
     //
@@ -1648,21 +1646,6 @@ impl Node {
         Temporary::from_rooted(copy.r())
     }
 
-    /// Sends layout data, if any, back to the layout task to be destroyed.
-    unsafe fn reap_layout_data(&mut self) {
-        if self.layout_data.is_present() {
-            let layout_data = mem::replace(&mut self.layout_data, LayoutDataRef::new());
-            let layout_chan = layout_data.take_chan();
-            match layout_chan {
-                None => {}
-                Some(chan) => {
-                    let LayoutChan(chan) = chan;
-                    chan.send(Msg::ReapLayoutData(layout_data)).unwrap()
-                },
-            }
-        }
-    }
-
     pub fn collect_text_contents<'a, T: Iterator<Item=JSRef<'a, Node>>>(iterator: T) -> String {
         let mut content = String::new();
         for node in iterator {

components/script/layout_interface.rs

@@ -6,7 +6,7 @@
 //! interface helps reduce coupling between these two components, and enables
 //! the DOM to be placed in a separate crate from layout.
 
-use dom::node::LayoutDataRef;
+use dom::node::LayoutData;
 
 use geom::point::Point2D;
 use geom::rect::Rect;
@@ -41,7 +41,7 @@ pub enum Msg {
     /// Destroys layout data associated with a DOM node.
     ///
     /// TODO(pcwalton): Maybe think about batching to avoid message traffic.
-    ReapLayoutData(LayoutDataRef),
+    ReapLayoutData(LayoutData),
 
     /// Requests that the layout task enter a quiescent state in which no more messages are
     /// accepted except `ExitMsg`. A response message will be sent on the supplied channel when