diff --git a/src/components/main/layout/construct.rs b/src/components/main/layout/construct.rs index 539240f53fa..80809899a89 100644 --- a/src/components/main/layout/construct.rs +++ b/src/components/main/layout/construct.rs @@ -26,7 +26,7 @@ use layout::box_::{Box, GenericBox, IframeBox, IframeBoxInfo, ImageBox, ImageBox use layout::box_::{UnscannedTextBox, UnscannedTextBoxInfo, InlineInfo, InlineParentInfo}; use layout::context::LayoutContext; use layout::float_context::FloatType; -use layout::flow::{BaseFlow, Flow, MutableOwnedFlowUtils}; +use layout::flow::{BaseFlow, Flow, LeafSet, MutableOwnedFlowUtils}; use layout::inline::InlineFlow; use layout::text::TextRunScanner; use layout::util::LayoutDataAccess; @@ -56,6 +56,16 @@ pub enum ConstructionResult { ConstructionItemConstructionResult(ConstructionItem), } +impl ConstructionResult { + fn destroy(&mut self, leaf_set: &mut LeafSet) { + match *self { + NoConstructionResult => {} + FlowConstructionResult(ref mut flow) => flow.destroy(leaf_set), + ConstructionItemConstructionResult(ref mut item) => item.destroy(leaf_set), + } + } +} + /// Represents the output of flow construction for a DOM node that has not yet resulted in a /// complete flow. Construction items bubble up the tree until they find a `Flow` to be /// attached to. @@ -64,6 +74,20 @@ enum ConstructionItem { InlineBoxesConstructionItem(InlineBoxesConstructionResult), } +impl ConstructionItem { + fn destroy(&mut self, leaf_set: &mut LeafSet) { + match *self { + InlineBoxesConstructionItem(ref mut result) => { + for splits in result.splits.mut_iter() { + for split in splits.mut_iter() { + split.destroy(leaf_set) + } + } + } + } + } +} + /// Represents inline boxes and {ib} splits that are bubbling up from an inline. struct InlineBoxesConstructionResult { /// Any {ib} splits that we're bubbling up. @@ -107,6 +131,12 @@ struct InlineBlockSplit { flow: ~Flow, } +impl InlineBlockSplit { + fn destroy(&mut self, leaf_set: &mut LeafSet) { + self.flow.destroy(leaf_set) + } +} + /// Methods on optional vectors. /// /// TODO(pcwalton): I think this will no longer be necessary once Rust #8981 lands. @@ -539,9 +569,12 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> { // `display: none` contributes no flow construction result. Nuke the flow construction // results of children. (display::none, _, _) => { - for child in node.children() { - child.set_flow_construction_result(NoConstructionResult) - } + self.layout_context.shared.leaf_set.access(|leaf_set| { + for child in node.children() { + let mut old_result = child.swap_out_construction_result(); + old_result.destroy(leaf_set) + } + }) } // Inline items contribute inline box construction results. diff --git a/src/components/main/layout/flow.rs b/src/components/main/layout/flow.rs index f8cb46dada1..3a7931aaf0c 100644 --- a/src/components/main/layout/flow.rs +++ b/src/components/main/layout/flow.rs @@ -219,6 +219,9 @@ pub trait MutableOwnedFlowUtils { /// Adds a new flow as a child of this flow. Removes the flow from the given leaf set if /// it's present. fn add_new_child(&mut self, new_child: ~Flow, leaf_set: &mut LeafSet); + + /// Destroys the flow. + fn destroy(&mut self, leaf_set: &mut LeafSet); } pub enum FlowClass { @@ -470,9 +473,6 @@ impl FlowFlags { } /// Data common to all flows. -/// -/// TODO(pcwalton): Plant a destructor bomb on this type. It is bad if it goes out of scope, -/// because of the leaf list. pub struct BaseFlow { restyle_damage: RestyleDamage, @@ -506,10 +506,24 @@ pub struct BaseFlow { num_floats: uint, abs_position: Point2D, + /// Whether this flow has been destroyed. + /// + /// TODO(pcwalton): Pack this into the flags? Need to be careful because manipulation of this + /// flag can have memory safety implications. + priv destroyed: bool, + /// Various flags for flows and some info flags_info: FlowFlagsInfo, } +impl Drop for BaseFlow { + fn drop(&mut self) { + if !self.destroyed { + fail!("Flow destroyed by going out of scope—this is unsafe! Use `destroy()` instead!") + } + } +} + pub struct BoxIterator { priv boxes: ~[@Box], priv index: uint, @@ -550,6 +564,8 @@ impl BaseFlow { num_floats: 0, abs_position: Point2D(Au::new(0), Au::new(0)), + destroyed: false, + flags_info: FlowFlagsInfo::new(style.get()), } } @@ -722,6 +738,7 @@ impl<'a> MutableFlowUtils for &'a mut Flow { }); true } + } impl MutableOwnedFlowUtils for ~Flow { @@ -741,6 +758,25 @@ impl MutableOwnedFlowUtils for ~Flow { base.children.push_back(new_child); let _ = base.parallel.children_count.fetch_add(1, Relaxed); } + + /// Destroys the flow. + fn destroy(&mut self, leaf_set: &mut LeafSet) { + let is_leaf = { + let base = mut_base(*self); + base.children.len() == 0 + }; + + if is_leaf { + leaf_set.remove(self); + } else { + for kid in child_iter(*self) { + kid.destroy(leaf_set) + } + } + + let base = mut_base(*self); + base.destroyed = true + } } /// Keeps track of the leaves of the flow tree. This is used to efficiently start bottom-up @@ -766,13 +802,18 @@ impl LeafSet { /// Removes a flow from the leaf set. Asserts that the flow was indeed in the leaf set. (This /// invariant is needed for memory safety, as there must always be exactly one leaf set.) fn remove(&mut self, flow: &~Flow) { - let flow = parallel::owned_flow_to_unsafe_flow(flow); - if !self.set.contains(&flow) { + if !self.contains(flow) { fail!("attempted to remove a flow from the leaf set that wasn't in the set!") } + let flow = parallel::owned_flow_to_unsafe_flow(flow); self.set.remove(&flow); } + pub fn contains(&mut self, flow: &~Flow) -> bool { + let flow = parallel::owned_flow_to_unsafe_flow(flow); + self.set.contains(&flow) + } + pub fn clear(&mut self) { self.set.clear() } diff --git a/src/components/main/layout/layout_task.rs b/src/components/main/layout/layout_task.rs index 2fbea22074f..4a10dac03c3 100644 --- a/src/components/main/layout/layout_task.rs +++ b/src/components/main/layout/layout_task.rs @@ -12,8 +12,8 @@ use layout::construct::{FlowConstructionResult, FlowConstructor, NoConstructionR use layout::context::{LayoutContext, SharedLayoutInfo}; use layout::display_list_builder::{DisplayListBuilder, ToGfxColor}; use layout::extra::LayoutAuxMethods; -use layout::flow::{Flow, ImmutableFlowUtils, LeafSet, MutableFlowUtils, PreorderFlowTraversal}; -use layout::flow::{PostorderFlowTraversal}; +use layout::flow::{Flow, ImmutableFlowUtils, LeafSet, MutableFlowUtils, MutableOwnedFlowUtils}; +use layout::flow::{PreorderFlowTraversal, PostorderFlowTraversal}; use layout::flow; use layout::incremental::RestyleDamage; use layout::parallel::{AssignHeightsAndStoreOverflowTraversalKind, BubbleWidthsTraversalKind}; @@ -590,10 +590,7 @@ impl LayoutTask { }); } - // FIXME(pcwalton): Hack because we don't yet reference count flows. When we do reference - // count them, then the destructor should remove the flow from the leaf set once the count - // hits zero. - self.leaf_set.access(|leaf_set| leaf_set.clear()); + self.leaf_set.access(|leaf_set| layout_root.destroy(leaf_set)); // Tell script that we're done. //