auto merge of #468 : jdm/servo/investigate, r=metajack

By the power vested in me through the agency of gdb, massif, and valgrind, here are some changes that fix various crashes that occur when shutting down at arbitrary points, along with some honest-to-goodness shutdown leaks reported by valgrind, and the layout cycles that were causing 1gb/s memory explosions when running test_hammer_layout.html.

src/components/servo-gfx/font.rs

@@ -177,6 +177,10 @@ pub impl FontGroup {
         }
     }
 
+    fn teardown(&mut self) {
+        self.fonts = ~[];
+    }
+
     fn create_textrun(&self, text: ~str) -> TextRun {
         assert!(self.fonts.len() > 0);
 
@@ -283,6 +287,11 @@ pub impl Font {
         return result;
     }
 
+    fn teardown(&mut self) {
+        self.shaper = None;
+        self.azure_font = None;
+    }
+
     // TODO: this should return a borrowed pointer, but I can't figure
     // out why borrowck doesn't like my implementation.
 

src/components/servo-gfx/font_list.rs

@@ -132,7 +132,6 @@ impl FontFamily {
 /// In the common case, each FontFamily will have a singleton FontEntry, or it will have the
 /// standard four faces: Normal, Bold, Italic, BoldItalic.
 pub struct FontEntry {
-    family: @mut FontFamily,
     face_name: ~str,
     priv weight: CSSFontWeight,
     priv italic: bool,
@@ -141,9 +140,8 @@ pub struct FontEntry {
 }
 
 impl FontEntry {
-    pub fn new(family: @mut FontFamily, handle: FontHandle) -> FontEntry {
+    pub fn new(handle: FontHandle) -> FontEntry {
         FontEntry {
-            family: family,
             face_name: handle.face_name(),
             weight: handle.boldness(),
             italic: handle.is_italic(),

src/components/servo-gfx/platform/linux/font.rs

@@ -53,6 +53,7 @@ pub struct FontHandle {
     // if the font is created using FT_Memory_Face.
     source: FontSource,
     face: FT_Face,
+    handle: FontContextHandle
 }
 
 #[unsafe_destructor]
@@ -81,7 +82,14 @@ impl FontHandleMethods for FontHandle {
         // and moving buf into the struct ctor, but cant' move out of
         // captured binding.
         return match face_result {
-            Ok(face) => Ok(FontHandle { face: face, source: FontSourceMem(buf) }),
+            Ok(face) => {
+              let handle = FontHandle {
+                  face: face,
+                  source: FontSourceMem(buf),
+                  handle: *fctx
+              };
+              Ok(handle)
+            }
             Err(()) => Err(())
         };
 
@@ -247,7 +255,11 @@ pub impl<'self> FontHandle {
             return Err(());
         }
         if FontHandle::set_char_size(face, style.pt_size).is_ok() {
-            Ok(FontHandle { source: FontSourceFile(file), face: face })
+            Ok(FontHandle {
+                source: FontSourceFile(file),
+                face: face,
+                handle: *fctx
+            })
         } else {
             Err(())
         }
@@ -268,7 +280,11 @@ pub impl<'self> FontHandle {
             return Err(());
         }
 
-        Ok(FontHandle { source: FontSourceFile(file), face: face })
+        Ok(FontHandle {
+            source: FontSourceFile(file),
+            face: face,
+            handle: *fctx
+        })
     }
 
     priv fn get_face_rec(&'self self) -> &'self FT_FaceRec {

src/components/servo-gfx/platform/linux/font_list.rs

@@ -6,7 +6,7 @@ extern mod freetype;
 extern mod fontconfig;
 
 use fontconfig::fontconfig::{
-    FcChar8, FcResultMatch, FcSetSystem,
+    FcChar8, FcResultMatch, FcSetSystem, FcPattern,
     FcResultNoMatch, FcMatchPattern, FC_SLANT_ITALIC, FC_WEIGHT_BOLD
 };
 use fontconfig::fontconfig::bindgen::{
@@ -116,7 +116,7 @@ pub impl FontListHandle {
                 let font_handle = font_handle.unwrap();
 
                 debug!("Creating new FontEntry for face: %s", font_handle.face_name());
-                let entry = @FontEntry::new(family, font_handle);
+                let entry = @FontEntry::new(font_handle);
                 family.entries.push(entry);
             }
 
@@ -127,10 +127,21 @@ pub impl FontListHandle {
     }
 }
 
+struct AutoPattern {
+    pattern: *FcPattern
+}
+
+impl Drop for AutoPattern {
+    fn finalize(&self) {
+        FcPatternDestroy(self.pattern);
+    }
+}
+
 pub fn path_from_identifier(name: ~str, style: &UsedFontStyle) -> Result<~str, ()> {
     unsafe {
         let config = FcConfigGetCurrent();
-        let pattern = FcPatternCreate();
+        let wrapper = AutoPattern { pattern: FcPatternCreate() };
+        let pattern = wrapper.pattern;
         let res = do str::as_c_str("family") |FC_FAMILY| {
             do str::as_c_str(name) |family| {
                 FcPatternAddString(pattern, FC_FAMILY, family as *FcChar8)
@@ -166,7 +177,8 @@ pub fn path_from_identifier(name: ~str, style: &UsedFontStyle) -> Result<~str, (
         }
         FcDefaultSubstitute(pattern);
         let result = FcResultNoMatch;
-        let result_pattern = FcFontMatch(config, pattern, &result);
+        let result_wrapper = AutoPattern { pattern: FcFontMatch(config, pattern, &result) };
+        let result_pattern = result_wrapper.pattern;
         if result != FcResultMatch && result_pattern.is_null() {
             debug!("obtaining match to pattern failed");
             return Err(());

src/components/servo-gfx/platform/macos/font_list.rs

@@ -51,7 +51,7 @@ pub impl FontListHandle {
             let handle = result::unwrap(FontHandle::new_from_CTFont(&self.fctx, font));
 
             debug!("Creating new FontEntry for face: %s", handle.face_name());
-            let entry = @FontEntry::new(family, handle);
+            let entry = @FontEntry::new(handle);
             family.entries.push(entry)
         }
     }

src/components/servo-gfx/text/text_run.rs

@@ -51,6 +51,10 @@ pub impl<'self> TextRun {
         return run;
     }
 
+    fn teardown(&self) {
+        self.font.teardown();
+    }
+
     fn compute_potential_breaks(text: &str, glyphs: &mut GlyphStore) {
         // TODO(Issue #230): do a better job. See Gecko's LineBreaker.
 

src/components/servo/dom/document.rs

@@ -34,18 +34,6 @@ pub fn Document(root: AbstractNode, window: Option<@mut Window>) -> @mut Documen
     doc
 }
 
-#[unsafe_destructor]
-impl Drop for Document {
-    fn finalize(&self) {
-        let compartment = global_script_context().js_compartment;
-        do self.root.with_base |base| {
-            assert!(base.wrapper.get_wrapper().is_not_null());
-            let rootable = base.wrapper.get_rootable();
-            JS_RemoveObjectRoot(compartment.cx.ptr, rootable);
-        }
-    }
-}
-
 pub impl Document {
     fn getElementsByTagName(&self, tag: DOMString) -> Option<@mut HTMLCollection> {
         let mut elements = ~[];
@@ -67,5 +55,14 @@ pub impl Document {
             window.content_changed()
         }
     }
+
+    fn teardown(&self) {
+        let compartment = global_script_context().js_compartment;
+        do self.root.with_base |node| {
+            assert!(node.wrapper.get_wrapper().is_not_null());
+            let rootable = node.wrapper.get_rootable();
+            JS_RemoveObjectRoot(compartment.cx.ptr, rootable);
+        }
+    }
 }
 

src/components/servo/layout/aux.rs

@@ -18,6 +18,14 @@ impl LayoutAuxMethods for AbstractNode {
     /// box in the COW model) and populates it with an empty style object.
     fn initialize_layout_data(self) -> Option<@mut LayoutData> {
         if self.has_layout_data() {
+            {
+                let layout_data = &mut self.layout_data().flow;
+                match *layout_data {
+                  Some(ref flow) => flow.teardown(),
+                  None => ()
+                }
+            }
+            self.layout_data().flow = None;
             None
         } else {
             let data = @mut LayoutData::new();

src/components/servo/layout/block.rs

@@ -45,6 +45,14 @@ impl BlockFlowData {
             is_root: true
         }
     }
+
+    pub fn teardown(&mut self) {
+        self.common.teardown();
+        for self.box.each |box| {
+            box.teardown();
+        }
+        self.box = None;
+    }
 }
 
 pub trait BlockLayout {

src/components/servo/layout/box.rs

@@ -59,6 +59,15 @@ pub enum RenderBox {
     UnscannedTextRenderBoxClass(@mut UnscannedTextRenderBox),
 }
 
+impl RenderBox {
+    pub fn teardown(&self) {
+        match *self {
+            TextRenderBoxClass(box) => box.teardown(),
+            _ => ()
+        }
+    }
+}
+
 /// A box that represents a (replaced content) image and its accompanying borders, shadows, etc.
 pub struct ImageRenderBox {
     base: RenderBoxBase,
@@ -88,6 +97,12 @@ pub struct TextRenderBox {
     text_data: TextBoxData,
 }
 
+impl TextRenderBox {
+    fn teardown(&self) {
+        self.text_data.teardown();
+    }
+}
+
 /// The data for an unscanned text box.
 pub struct UnscannedTextRenderBox {
     base: RenderBoxBase,

src/components/servo/layout/flow.rs

@@ -67,6 +67,43 @@ impl Clone for FlowContext {
     }
 }
 
+impl FlowContext {
+    pub fn teardown(&self) {
+        match *self {
+          AbsoluteFlow(data) |
+          FloatFlow(data) |
+          InlineBlockFlow(data) |
+          TableFlow(data) => data.teardown(),
+          BlockFlow(data) => data.teardown(),
+          InlineFlow(data) => data.teardown()
+        }
+    }
+}
+
+impl FlowData {
+    pub fn teardown(&mut self) {
+        // Under the assumption that all flows exist in a tree,
+        // we must restrict ourselves to finalizing flows that
+        // are descendents and subsequent siblings to ourselves,
+        // or we risk dynamic borrow failures.
+        self.parent = None;
+
+        for self.first_child.each |flow| {
+            flow.teardown();
+        }
+        self.first_child = None;
+
+        self.last_child = None;
+
+        for self.next_sibling.each |flow| {
+            flow.teardown();
+        }
+        self.next_sibling = None;
+
+        self.prev_sibling = None;
+    }
+}
+
 impl TreeNodeRef<FlowData> for FlowContext {
     fn with_base<R>(&self, callback: &fn(&FlowData) -> R) -> R {
         match *self {

src/components/servo/layout/inline.rs

@@ -316,11 +316,18 @@ impl TextRunScanner {
                 // and then letting `FontGroup` decide which `Font` to stick into the text run.
                 let font_style = in_boxes[self.clump.begin()].font_style();
                 let fontgroup = ctx.font_ctx.get_resolved_font_for_style(&font_style);
-                let run = @TextRun::new(fontgroup.fonts[0], run_str);
+
+                // TextRuns contain a cycle which is usually resolved by the teardown
+                // sequence. If no clump takes ownership, however, it will leak.
+                let clump = self.clump;
+                let run = if clump.length() != 0 {
+                    Some(@TextRun::new(fontgroup.fonts[0], run_str))
+                } else {
+                    None
+                };
 
                 // Make new boxes with the run and adjusted text indices.
                 debug!("TextRunScanner: pushing box(es) in range: %?", self.clump);
-                let clump = self.clump;
                 for clump.eachi |i| {
                     let range = new_ranges[i - self.clump.begin()];
                     if range.length() == 0 { 
@@ -331,7 +338,7 @@ impl TextRunScanner {
                     }
 
                     do in_boxes[i].with_imm_base |base| {
-                        let new_box = @mut adapt_textbox_with_range(*base, run, range);
+                        let new_box = @mut adapt_textbox_with_range(*base, run.get(), range);
                         out_boxes.push(TextRenderBoxClass(new_box));
                     }
                 }
@@ -638,6 +645,14 @@ impl InlineFlowData {
             elems: ElementMapping::new(),
         }
     }
+
+    pub fn teardown(&mut self) {
+        self.common.teardown();
+        for self.boxes.each |box| {
+            box.teardown();
+        }
+        self.boxes = ~[];
+    }
 }
 
 pub trait InlineLayout {

src/components/servo/layout/text.rs

@@ -21,6 +21,10 @@ impl TextBoxData {
             range: range,
         }
     }
+
+    pub fn teardown(&self) {
+        self.run.teardown();
+    }
 }
 
 pub fn adapt_textbox_with_range(mut base: RenderBoxBase, run: @TextRun, range: Range)

src/components/servo/scripting/script_task.rs

@@ -285,6 +285,10 @@ impl ScriptContext {
 
     /// Handles a request to exit the script task and shut down layout.
     fn handle_exit_msg(&mut self) {
+        self.join_layout();
+        for self.root_frame.each |frame| {
+            frame.document.teardown();
+        }
         self.layout_task.send(layout_task::ExitMsg)
     }
 

src/test/test_hammer_layout.js

@@ -5,7 +5,8 @@ var count = 1000000;
 var start = new Date();
 for (var i = 0; i < count; i++) {
   div.setAttribute('id', 'styled');
-  div.getBoundingClientRect();
+  //div.getBoundingClientRect();
 }
 var stop = new Date();
 window.alert((stop - start) / count * 1e6);
+window.close();