Auto merge of #6650 - Ms2ger:layout-cleanup, r=jdm

Various layout cleanup.



<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6650)
<!-- Reviewable:end -->

components/layout/construct.rs

@@ -40,7 +40,8 @@ use table_row::TableRowFlow;
 use table_rowgroup::TableRowGroupFlow;
 use table_wrapper::TableWrapperFlow;
 use text::TextRunScanner;
-use wrapper::{PostorderNodeMutTraversal, PseudoElementType, ThreadSafeLayoutNode};
+use traversal::PostorderNodeMutTraversal;
+use wrapper::{PseudoElementType, ThreadSafeLayoutNode};
 
 use gfx::display_list::OpaqueNode;
 use script::dom::characterdata::CharacterDataTypeId;
@@ -666,7 +667,7 @@ impl<'a> FlowConstructor<'a> {
 
             self.create_fragments_for_node_text_content(&mut initial_fragments,
                                                         node,
-                                                        node.style());
+                                                        &*node.style());
         }
 
         self.build_flow_for_block_starting_with_fragments(flow, node, initial_fragments)

components/layout/css/node_style.rs

@@ -7,16 +7,17 @@
 use data::LayoutDataWrapper;
 use wrapper::{PseudoElementType, ThreadSafeLayoutNode};
 
-use std::mem;
 use style::properties::ComputedValues;
+
+use std::cell::Ref;
 use std::sync::Arc;
 
 /// Node mixin providing `style` method that returns a `NodeStyle`
 pub trait StyledNode {
-    fn get_style<'a>(&'a self, layout_data_ref: &'a LayoutDataWrapper) -> &'a Arc<ComputedValues>;
+    fn get_style<'a>(&self, layout_data_ref: &'a LayoutDataWrapper) -> &'a Arc<ComputedValues>;
     /// Returns the style results for the given node. If CSS selector matching has not yet been
     /// performed, fails.
-    fn style<'a>(&'a self) -> &'a Arc<ComputedValues>;
+    fn style<'a>(&'a self) -> Ref<'a, Arc<ComputedValues>>;
     /// Does this node have a computed style yet?
     fn has_style(&self) -> bool;
     /// Removes the style from this node.
@@ -34,14 +35,11 @@ impl<'ln> StyledNode for ThreadSafeLayoutNode<'ln> {
     }
 
     #[inline]
-    #[allow(unsafe_code)]
+    fn style<'a>(&'a self) -> Ref<'a, Arc<ComputedValues>> {
-    fn style<'a>(&'a self) -> &'a Arc<ComputedValues> {
+        Ref::map(self.borrow_layout_data(), |layout_data_ref| {
-        unsafe {
-            let layout_data_ref = self.borrow_layout_data();
             let layout_data = layout_data_ref.as_ref().expect("no layout data");
-            mem::transmute::<&Arc<ComputedValues>,
+            self.get_style(layout_data)
-                             &'a Arc<ComputedValues>>(self.get_style(&layout_data))
+        })
-        }
     }
 
     fn has_style(&self) -> bool {

components/layout/lib.rs

@@ -5,6 +5,7 @@
 #![feature(append)]
 #![feature(arc_unique)]
 #![feature(box_syntax)]
+#![feature(cell_extras)]
 #![feature(custom_derive)]
 #![feature(filling_drop)]
 #![feature(hashmap_hasher)]

components/layout/parallel.rs

@@ -12,12 +12,13 @@ use context::{LayoutContext, SharedLayoutContext};
 use flow::{Flow, MutableFlowUtils, PreorderFlowTraversal, PostorderFlowTraversal};
 use flow;
 use flow_ref::FlowRef;
+use traversal::{PreorderDomTraversal, PostorderDomTraversal};
+use traversal::PostorderNodeMutTraversal;
 use traversal::{BubbleISizes, AssignISizes, AssignBSizesAndStoreOverflow};
 use traversal::{ComputeAbsolutePositions, BuildDisplayList};
 use traversal::{RecalcStyleForNode, ConstructFlows};
 use wrapper::{layout_node_to_unsafe_layout_node, layout_node_from_unsafe_layout_node, LayoutNode};
-use wrapper::{PostorderNodeMutTraversal, UnsafeLayoutNode};
+use wrapper::UnsafeLayoutNode;
-use wrapper::{PreorderDomTraversal, PostorderDomTraversal};
 
 use profile_traits::time::{self, ProfilerMetadata, profile};
 use std::mem;
@@ -241,9 +242,10 @@ trait ParallelPostorderFlowTraversal : PostorderFlowTraversal {
                     mut unsafe_flow: UnsafeFlow,
                     _: &mut WorkerProxy<SharedLayoutContext,UnsafeFlowList>) {
         loop {
-            unsafe {
             // Get a real flow.
-                let flow: &mut FlowRef = mem::transmute(&mut unsafe_flow);
+            let flow: &mut FlowRef = unsafe {
+                mem::transmute(&mut unsafe_flow)
+            };
 
             // Perform the appropriate traversal.
             if self.should_process(&mut **flow) {
@@ -267,7 +269,9 @@ trait ParallelPostorderFlowTraversal : PostorderFlowTraversal {
             // No, we're not at the root yet. Then are we the last child
             // of our parent to finish processing? If so, we can continue
             // on with our parent; otherwise, we've gotta wait.
-                let parent: &mut FlowRef = mem::transmute(&mut unsafe_parent);
+            let parent: &mut FlowRef = unsafe {
+                mem::transmute(&mut unsafe_parent)
+            };
             let parent_base = flow::mut_base(&mut **parent);
             if parent_base.parallel.children_count.fetch_sub(1, Ordering::Relaxed) == 1 {
                 // We were the last child of our parent. Reflow our parent.
@@ -278,7 +282,6 @@ trait ParallelPostorderFlowTraversal : PostorderFlowTraversal {
             }
         }
     }
-    }
 }
 
 /// A parallel top-down flow traversal.

components/layout/sequential.rs

@@ -10,12 +10,12 @@ use flow::{PostorderFlowTraversal, PreorderFlowTraversal};
 use flow_ref::FlowRef;
 use fragment::FragmentBorderBoxIterator;
 use generated_content::ResolveGeneratedContent;
+use traversal::{PreorderDomTraversal, PostorderDomTraversal};
+use traversal::PostorderNodeMutTraversal;
 use traversal::{BubbleISizes, RecalcStyleForNode, ConstructFlows};
 use traversal::{AssignBSizesAndStoreOverflow, AssignISizes};
 use traversal::{ComputeAbsolutePositions, BuildDisplayList};
 use wrapper::LayoutNode;
-use wrapper::{PostorderNodeMutTraversal};
-use wrapper::{PreorderDomTraversal, PostorderDomTraversal};
 
 use euclid::point::Point2D;
 use util::geometry::{Au, ZERO_POINT};

components/layout/traversal.rs

@@ -4,8 +4,6 @@
 
 //! Traversals over the DOM and flow trees, running the layout computations.
 
-#![allow(unsafe_code)]
-
 use css::node_style::StyledNode;
 use css::matching::{ApplicableDeclarations, MatchMethods, StyleSharingResult};
 use construct::FlowConstructor;
@@ -15,8 +13,7 @@ use flow::{PreorderFlowTraversal, PostorderFlowTraversal};
 use incremental::{self, BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, RestyleDamage};
 use script::layout_interface::ReflowGoal;
 use wrapper::{layout_node_to_unsafe_layout_node, LayoutNode};
-use wrapper::{PostorderNodeMutTraversal, ThreadSafeLayoutNode, UnsafeLayoutNode};
+use wrapper::{ThreadSafeLayoutNode, UnsafeLayoutNode};
-use wrapper::{PreorderDomTraversal, PostorderDomTraversal};
 
 use selectors::bloom::BloomFilter;
 use selectors::Node;
@@ -118,6 +115,32 @@ fn insert_ancestors_into_bloom_filter(bf: &mut Box<BloomFilter>,
     debug!("[{}] Inserted {} ancestors.", tid(), ancestors);
 }
 
+
+/// A top-down traversal.
+pub trait PreorderDomTraversal {
+    /// The operation to perform. Return true to continue or false to stop.
+    fn process(&self, node: LayoutNode);
+}
+
+/// A bottom-up traversal, with a optional in-order pass.
+pub trait PostorderDomTraversal {
+    /// The operation to perform. Return true to continue or false to stop.
+    fn process(&self, node: LayoutNode);
+}
+
+/// A bottom-up, parallelizable traversal.
+pub trait PostorderNodeMutTraversal {
+    /// The operation to perform. Return true to continue or false to stop.
+    fn process<'a>(&'a mut self, node: &ThreadSafeLayoutNode<'a>) -> bool;
+
+    /// Returns true if this node should be pruned. If this returns true, we skip the operation
+    /// entirely and do not process any descendant nodes. This is called *before* child nodes are
+    /// visited. The default implementation never prunes any nodes.
+    fn should_prune<'a>(&'a self, _node: &ThreadSafeLayoutNode<'a>) -> bool {
+        false
+    }
+}
+
 /// The recalc-style-for-node traversal, which styles each node and must run before
 /// layout computation. This computes the styles applied to each node.
 #[derive(Copy, Clone)]
@@ -127,6 +150,7 @@ pub struct RecalcStyleForNode<'a> {
 
 impl<'a> PreorderDomTraversal for RecalcStyleForNode<'a> {
     #[inline]
+    #[allow(unsafe_code)]
     fn process(&self, node: LayoutNode) {
         // Initialize layout data.
         //
@@ -219,6 +243,7 @@ pub struct ConstructFlows<'a> {
 
 impl<'a> PostorderDomTraversal for ConstructFlows<'a> {
     #[inline]
+    #[allow(unsafe_code)]
     fn process(&self, node: LayoutNode) {
         // Construct flows for this node.
         {

components/layout/wrapper.rs

@@ -683,7 +683,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
     pub fn children(&self) -> ThreadSafeLayoutNodeChildrenIterator<'ln> {
         ThreadSafeLayoutNodeChildrenIterator {
             current_node: self.first_child(),
-            parent_node: Some(self.clone()),
+            parent_node: self.clone(),
         }
     }
 
@@ -770,28 +770,6 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
         self.node.mutate_layout_data()
     }
 
-    /// Traverses the tree in postorder.
-    ///
-    /// TODO(pcwalton): Offer a parallel version with a compatible API.
-    pub fn traverse_postorder_mut<T:PostorderNodeMutTraversal>(&mut self, traversal: &mut T)
-                                  -> bool {
-        if traversal.should_prune(self) {
-            return true
-        }
-
-        let mut opt_kid = self.first_child();
-        while let Some(mut kid) = opt_kid {
-            if !kid.traverse_postorder_mut(traversal) {
-                return false
-            }
-            unsafe {
-                opt_kid = kid.next_sibling()
-            }
-        }
-
-        traversal.process(self)
-    }
-
     pub fn is_ignorable_whitespace(&self) -> bool {
         unsafe {
             let text: LayoutJS<Text> = match TextCast::to_layout_js(self.get_jsmanaged()) {
@@ -989,7 +967,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
 
 pub struct ThreadSafeLayoutNodeChildrenIterator<'a> {
     current_node: Option<ThreadSafeLayoutNode<'a>>,
-    parent_node: Option<ThreadSafeLayoutNode<'a>>,
+    parent_node: ThreadSafeLayoutNode<'a>,
 }
 
 impl<'a> Iterator for ThreadSafeLayoutNodeChildrenIterator<'a> {
@@ -1003,9 +981,7 @@ impl<'a> Iterator for ThreadSafeLayoutNodeChildrenIterator<'a> {
                     return None
                 }
 
-                match self.parent_node {
+                self.current_node = if self.parent_node.pseudo == PseudoElementType::Normal {
-                    Some(ref parent_node) => {
-                        self.current_node = if parent_node.pseudo == PseudoElementType::Normal {
                     self.current_node.clone().and_then(|node| {
                         unsafe {
                             node.next_sibling()
@@ -1015,16 +991,11 @@ impl<'a> Iterator for ThreadSafeLayoutNodeChildrenIterator<'a> {
                     None
                 };
             }
-                    None => {}
-                }
-            }
             None => {
-                match self.parent_node {
+                if self.parent_node.has_after_pseudo() {
-                    Some(ref parent_node) => {
+                    let pseudo_after_node = if self.parent_node.pseudo == PseudoElementType::Normal {
-                        if parent_node.has_after_pseudo() {
+                        let pseudo = PseudoElementType::After(self.parent_node.get_after_display());
-                            let pseudo_after_node = if parent_node.pseudo == PseudoElementType::Normal {
+                        Some(self.parent_node.with_pseudo(pseudo))
-                                let pseudo = PseudoElementType::After(parent_node.get_after_display());
-                                Some(parent_node.with_pseudo(pseudo))
                     } else {
                         None
                     };
@@ -1032,9 +1003,6 @@ impl<'a> Iterator for ThreadSafeLayoutNodeChildrenIterator<'a> {
                     return self.current_node.clone()
                 }
             }
-                   None => {}
-                }
-            }
         }
 
         node
@@ -1056,19 +1024,6 @@ impl<'le> ThreadSafeLayoutElement<'le> {
     }
 }
 
-/// A bottom-up, parallelizable traversal.
-pub trait PostorderNodeMutTraversal {
-    /// The operation to perform. Return true to continue or false to stop.
-    fn process<'a>(&'a mut self, node: &ThreadSafeLayoutNode<'a>) -> bool;
-
-    /// Returns true if this node should be pruned. If this returns true, we skip the operation
-    /// entirely and do not process any descendant nodes. This is called *before* child nodes are
-    /// visited. The default implementation never prunes any nodes.
-    fn should_prune<'a>(&'a self, _node: &ThreadSafeLayoutNode<'a>) -> bool {
-        false
-    }
-}
-
 /// Opaque type stored in type-unsafe work queues for parallel layout.
 /// Must be transmutable to and from LayoutNode.
 pub type UnsafeLayoutNode = (usize, usize);
@@ -1086,15 +1041,3 @@ pub unsafe fn layout_node_from_unsafe_layout_node(node: &UnsafeLayoutNode) -> La
     let (node, _) = *node;
     mem::transmute(node)
 }
-
-/// A top-down traversal.
-pub trait PreorderDomTraversal {
-    /// The operation to perform. Return true to continue or false to stop.
-    fn process(&self, node: LayoutNode);
-}
-
-/// A bottom-up traversal, with a optional in-order pass.
-pub trait PostorderDomTraversal {
-    /// The operation to perform. Return true to continue or false to stop.
-    fn process(&self, node: LayoutNode);
-}