servo: Merge #6650 - Various layout cleanup (from Ms2ger:layout-cleanup); r=jdm

Source-Repo: https://github.com/servo/servo
Source-Revision: e6538c0f2d887bfb244313009490ca67bc1c2298

servo/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)

servo/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) -> &'a Arc<ComputedValues> {
-        unsafe {
-            let layout_data_ref = self.borrow_layout_data();
+    fn style<'a>(&'a self) -> Ref<'a, Arc<ComputedValues>> {
+        Ref::map(self.borrow_layout_data(), |layout_data_ref| {
             let layout_data = layout_data_ref.as_ref().expect("no layout data");
-            mem::transmute::<&Arc<ComputedValues>,
-                             &'a Arc<ComputedValues>>(self.get_style(&layout_data))
-        }
+            self.get_style(layout_data)
+        })
     }
 
     fn has_style(&self) -> bool {

servo/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)]

servo/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::{PreorderDomTraversal, PostorderDomTraversal};
+use wrapper::UnsafeLayoutNode;
 
 use profile_traits::time::{self, ProfilerMetadata, profile};
 use std::mem;
@@ -241,41 +242,43 @@ 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);
+            // Get a real flow.
+            let flow: &mut FlowRef = unsafe {
+                mem::transmute(&mut unsafe_flow)
+            };
 
-                // Perform the appropriate traversal.
-                if self.should_process(&mut **flow) {
-                    self.process(&mut **flow);
-                }
+            // Perform the appropriate traversal.
+            if self.should_process(&mut **flow) {
+                self.process(&mut **flow);
+            }
 
 
-                let base = flow::mut_base(&mut **flow);
+            let base = flow::mut_base(&mut **flow);
 
-                // Reset the count of children for the next layout traversal.
-                base.parallel.children_count.store(base.children.len() as isize,
-                                                   Ordering::Relaxed);
+            // Reset the count of children for the next layout traversal.
+            base.parallel.children_count.store(base.children.len() as isize,
+                                               Ordering::Relaxed);
 
-                // Possibly enqueue the parent.
-                let mut unsafe_parent = base.parallel.parent;
-                if unsafe_parent == null_unsafe_flow() {
-                    // We're done!
-                    break
-                }
+            // Possibly enqueue the parent.
+            let mut unsafe_parent = base.parallel.parent;
+            if unsafe_parent == null_unsafe_flow() {
+                // We're done!
+                break
+            }
 
-                // 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_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.
-                    unsafe_flow = unsafe_parent
-                } else {
-                    // Stop.
-                    break
-                }
+            // 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 = 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.
+                unsafe_flow = unsafe_parent
+            } else {
+                // Stop.
+                break
             }
         }
     }

servo/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};

servo/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::{PreorderDomTraversal, PostorderDomTraversal};
+use wrapper::{ThreadSafeLayoutNode, UnsafeLayoutNode};
 
 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.
         {

servo/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,36 +981,26 @@ impl<'a> Iterator for ThreadSafeLayoutNodeChildrenIterator<'a> {
                     return None
                 }
 
-                match self.parent_node {
-                    Some(ref parent_node) => {
-                        self.current_node = if parent_node.pseudo == PseudoElementType::Normal {
-                            self.current_node.clone().and_then(|node| {
-                                unsafe {
-                                    node.next_sibling()
-                                }
-                            })
-                        } else {
-                            None
-                        };
-                    }
-                    None => {}
-                }
+                self.current_node = if self.parent_node.pseudo == PseudoElementType::Normal {
+                    self.current_node.clone().and_then(|node| {
+                        unsafe {
+                            node.next_sibling()
+                        }
+                    })
+                } else {
+                    None
+                };
             }
             None => {
-                match self.parent_node {
-                    Some(ref parent_node) => {
-                        if parent_node.has_after_pseudo() {
-                            let pseudo_after_node = if parent_node.pseudo == PseudoElementType::Normal {
-                                let pseudo = PseudoElementType::After(parent_node.get_after_display());
-                                Some(parent_node.with_pseudo(pseudo))
-                            } else {
-                                None
-                            };
-                            self.current_node = pseudo_after_node;
-                            return self.current_node.clone()
-                        }
-                   }
-                   None => {}
+                if self.parent_node.has_after_pseudo() {
+                    let pseudo_after_node = if self.parent_node.pseudo == PseudoElementType::Normal {
+                        let pseudo = PseudoElementType::After(self.parent_node.get_after_display());
+                        Some(self.parent_node.with_pseudo(pseudo))
+                    } else {
+                        None
+                    };
+                    self.current_node = pseudo_after_node;
+                    return self.current_node.clone()
                 }
             }
         }
@@ -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);
-}