Auto merge of #6657 - Ms2ger:ThreadSafeLayoutNodeChildrenIterator, r=…

…pcwalton

Cleanup ThreadSafeLayoutNodeChildrenIterator.



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

components/layout/wrapper.rs

@@ -656,35 +656,9 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
         self.node.flow_debug_id()
     }
 
-    fn first_child(&self) -> Option<ThreadSafeLayoutNode<'ln>> {
-        if self.pseudo != PseudoElementType::Normal {
-            return None
-        }
-
-        if self.has_before_pseudo() {
-            return Some(self.with_pseudo(PseudoElementType::Before(self.get_before_display())));
-        }
-
-        unsafe {
-            self.get_jsmanaged().first_child_ref().map(|node| self.new_with_this_lifetime(&node))
-        }
-    }
-
-    /// Returns the next sibling of this node. Unsafe and private because this can lead to races.
-    unsafe fn next_sibling(&self) -> Option<ThreadSafeLayoutNode<'ln>> {
-        if self.pseudo.is_before() {
-            return self.get_jsmanaged().first_child_ref().map(|node| self.new_with_this_lifetime(&node))
-        }
-
-        self.get_jsmanaged().next_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
-    }
-
     /// Returns an iterator over this node's children.
     pub fn children(&self) -> ThreadSafeLayoutNodeChildrenIterator<'ln> {
-        ThreadSafeLayoutNodeChildrenIterator {
-            current_node: self.first_child(),
-            parent_node: self.clone(),
-        }
+        ThreadSafeLayoutNodeChildrenIterator::new(*self)
     }
 
     /// If this is an element, accesses the element data. Fails if this is not an element node.
@@ -970,39 +944,79 @@ pub struct ThreadSafeLayoutNodeChildrenIterator<'a> {
     parent_node: ThreadSafeLayoutNode<'a>,
 }
 
+impl<'a> ThreadSafeLayoutNodeChildrenIterator<'a> {
+    fn new(parent: ThreadSafeLayoutNode<'a>) -> ThreadSafeLayoutNodeChildrenIterator<'a> {
+        fn first_child<'a>(parent: ThreadSafeLayoutNode<'a>)
+                           -> Option<ThreadSafeLayoutNode<'a>> {
+            if parent.pseudo != PseudoElementType::Normal {
+                return None
+            }
+
+            if parent.has_before_pseudo() {
+                let pseudo = PseudoElementType::Before(parent.get_before_display());
+                return Some(parent.with_pseudo(pseudo));
+            }
+
+            unsafe {
+                parent.get_jsmanaged().first_child_ref()
+                      .map(|node| parent.new_with_this_lifetime(&node))
+            }
+        }
+
+        ThreadSafeLayoutNodeChildrenIterator {
+            current_node: first_child(parent),
+            parent_node: parent,
+        }
+    }
+}
+
 impl<'a> Iterator for ThreadSafeLayoutNodeChildrenIterator<'a> {
     type Item = ThreadSafeLayoutNode<'a>;
     fn next(&mut self) -> Option<ThreadSafeLayoutNode<'a>> {
         let node = self.current_node.clone();
 
-        match node {
-            Some(ref node) => {
-                if node.pseudo.is_after() {
-                    return None
-                }
-
-                self.current_node = if self.parent_node.pseudo == PseudoElementType::Normal {
-                    self.current_node.clone().and_then(|node| {
-                        unsafe {
-                            node.next_sibling()
+        if let Some(ref node) = node {
+            self.current_node = match node.pseudo {
+                PseudoElementType::Before(_) => {
+                    match unsafe { self.parent_node.get_jsmanaged().first_child_ref() } {
+                        Some(first) => {
+                            Some(unsafe {
+                                self.parent_node.new_with_this_lifetime(&first)
+                            })
+                        },
+                        None => {
+                            if self.parent_node.has_after_pseudo() {
+                                let pseudo = PseudoElementType::After(
+                                    self.parent_node.get_after_display());
+                                Some(self.parent_node.with_pseudo(pseudo))
+                            } else {
+                                None
+                            }
                         }
-                    })
-                } else {
+                    }
+                },
+                PseudoElementType::Normal => {
+                    match unsafe { node.get_jsmanaged().next_sibling_ref() } {
+                        Some(next) => {
+                            Some(unsafe {
+                                self.parent_node.new_with_this_lifetime(&next)
+                            })
+                        },
+                        None => {
+                            if self.parent_node.has_after_pseudo() {
+                                let pseudo = PseudoElementType::After(
+                                    self.parent_node.get_after_display());
+                                Some(self.parent_node.with_pseudo(pseudo))
+                            } else {
+                                None
+                            }
+                        }
+                    }
+                },
+                PseudoElementType::After(_) => {
                     None
-                };
-            }
-            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()
-                }
-            }
+                },
+            };
         }
 
         node