Auto merge of #5931 - nox:rm-helpers, r=jdm

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

components/script/dom/document.rs

@@ -548,7 +548,7 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> {
         let el = match ElementCast::to_ref(node.r()) {
             Some(el) => Temporary::from_rooted(el),
             None => {
-                let parent = node.r().parent_node();
+                let parent = node.r().GetParentNode();
                 match parent.and_then(ElementCast::to_temporary) {
                     Some(parent) => parent,
                     None => return,

components/script/dom/element.rs

@@ -1214,11 +1214,13 @@ impl<'a> ElementMethods for JSRef<'a, Element> {
         let context_document = document_from_node(self).root();
         let context_node: JSRef<Node> = NodeCast::from_ref(self);
         // Step 1.
-        let context_parent = match context_node.parent_node() {
+        let context_parent = match context_node.GetParentNode() {
-            // Step 2.
+            None => {
-            None => return Ok(()),
+                // Step 2.
-            Some(parent) => parent.root()
+                return Ok(());
-        };
+            },
+            Some(parent) => parent,
+        }.root();
 
         let parent = match context_parent.r().type_id() {
             // Step 3.
@@ -1229,11 +1231,10 @@ impl<'a> ElementMethods for JSRef<'a, Element> {
                 let body_elem = Element::create(QualName::new(ns!(HTML), atom!(body)),
                                                 None, context_document.r(),
                                                 ElementCreator::ScriptCreated);
-                let body_node: Temporary<Node> = NodeCast::from_temporary(body_elem);
+                NodeCast::from_temporary(body_elem)
-                body_node.root()
             },
-            _ => context_node.parent_node().unwrap().root()
+            _ => context_node.GetParentNode().unwrap()
-        };
+        }.root();
 
         // Step 5.
         let frag = try!(parent.r().parse_fragment(value));

components/script/dom/htmloptionelement.rs

@@ -175,7 +175,7 @@ impl<'a> VirtualMethods for JSRef<'a, HTMLOptionElement> {
         }
 
         let node: JSRef<Node> = NodeCast::from_ref(*self);
-        if node.parent_node().is_some() {
+        if node.GetParentNode().is_some() {
             node.check_parent_disabled_state_for_option();
         } else {
             node.check_disabled_attribute();

components/script/dom/node.rs

@@ -285,7 +285,7 @@ trait PrivateNodeHelpers {
 impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
     // https://dom.spec.whatwg.org/#node-is-inserted
     fn node_inserted(self) {
-        assert!(self.parent_node().is_some());
+        assert!(self.parent_node.get().is_some());
         let document = document_from_node(self).root();
         let is_in_doc = self.is_in_doc();
 
@@ -294,14 +294,14 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
             vtable_for(&node.r()).bind_to_tree(is_in_doc);
         }
 
-        let parent = self.parent_node().root();
+        let parent = self.parent_node.get().root();
         parent.r().map(|parent| vtable_for(&parent).child_inserted(self));
         document.r().content_and_heritage_changed(self, NodeDamage::OtherNodeDamage);
     }
 
     // https://dom.spec.whatwg.org/#node-is-removed
     fn node_removed(self, parent_in_doc: bool) {
-        assert!(self.parent_node().is_none());
+        assert!(self.parent_node.get().is_none());
         for node in self.traverse_preorder() {
             let node = node.root();
             vtable_for(&node.r()).unbind_from_tree(parent_in_doc);
@@ -317,15 +317,15 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
     ///
     /// Fails unless `new_child` is disconnected from the tree.
     fn add_child(self, new_child: JSRef<Node>, before: Option<JSRef<Node>>) {
-        assert!(new_child.parent_node().is_none());
+        assert!(new_child.parent_node.get().is_none());
-        assert!(new_child.prev_sibling().is_none());
+        assert!(new_child.prev_sibling.get().is_none());
-        assert!(new_child.next_sibling().is_none());
+        assert!(new_child.next_sibling.get().is_none());
         match before {
             Some(ref before) => {
-                assert!(before.parent_node().root().r() == Some(self));
+                assert!(before.parent_node.get().root().r() == Some(self));
-                match before.prev_sibling().root() {
+                match before.prev_sibling.get().root() {
                     None => {
-                        assert!(Some(*before) == self.first_child().root().r());
+                        assert!(Some(*before) == self.first_child.get().root().r());
                         self.first_child.set(Some(JS::from_rooted(new_child)));
                     },
                     Some(ref prev_sibling) => {
@@ -337,10 +337,10 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
                 new_child.next_sibling.set(Some(JS::from_rooted(*before)));
             },
             None => {
-                match self.last_child().root() {
+                match self.last_child.get().root() {
                     None => self.first_child.set(Some(JS::from_rooted(new_child))),
                     Some(ref last_child) => {
-                        assert!(last_child.r().next_sibling().is_none());
+                        assert!(last_child.r().next_sibling.get().is_none());
                         last_child.r().next_sibling.set(Some(JS::from_rooted(new_child)));
                         new_child.prev_sibling.set(Some(JS::from_rooted(last_child.r())));
                     }
@@ -357,7 +357,7 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
     ///
     /// Fails unless `child` is a child of this node.
     fn remove_child(self, child: JSRef<Node>) {
-        assert!(child.parent_node().root().r() == Some(self));
+        assert!(child.parent_node.get().root().r() == Some(self));
 
         match child.prev_sibling.get().root() {
             None => {
@@ -429,12 +429,6 @@ pub trait NodeHelpers {
     fn len(self) -> u32;
     fn index(self) -> u32;
 
-    fn parent_node(self) -> Option<Temporary<Node>>;
-    fn first_child(self) -> Option<Temporary<Node>>;
-    fn last_child(self) -> Option<Temporary<Node>>;
-    fn prev_sibling(self) -> Option<Temporary<Node>>;
-    fn next_sibling(self) -> Option<Temporary<Node>>;
-
     fn owner_doc(self) -> Temporary<Document>;
     fn set_owner_doc(self, document: JSRef<Document>);
     fn is_in_html_doc(self) -> bool;
@@ -584,28 +578,6 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
         self.preceding_siblings().count() as u32
     }
 
-    fn parent_node(self) -> Option<Temporary<Node>> {
-        self.parent_node.get().map(Temporary::from_rooted)
-    }
-
-    fn first_child(self) -> Option<Temporary<Node>> {
-        self.first_child.get().map(Temporary::from_rooted)
-    }
-
-    fn last_child(self) -> Option<Temporary<Node>> {
-        self.last_child.get().map(Temporary::from_rooted)
-    }
-
-    /// Returns the previous sibling of this node. Fails if this node is borrowed mutably.
-    fn prev_sibling(self) -> Option<Temporary<Node>> {
-        self.prev_sibling.get().map(Temporary::from_rooted)
-    }
-
-    /// Returns the next sibling of this node. Fails if this node is borrowed mutably.
-    fn next_sibling(self) -> Option<Temporary<Node>> {
-        self.next_sibling.get().map(Temporary::from_rooted)
-    }
-
     #[inline]
     fn is_element(self) -> bool {
         match self.type_id {
@@ -756,12 +728,12 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
         // selectors. Maybe we can do something smarter in the future.
         if !self.get_has_dirty_siblings() {
             let parent =
-                match self.parent_node() {
+                match self.parent_node.get() {
                     None         => return,
                     Some(parent) => parent,
-                };
+                }.root();
 
-            for sibling in parent.root().r().children() {
+            for sibling in parent.r().children() {
                 let sibling = sibling.root();
                 sibling.r().set_has_dirty_siblings(true);
             }
@@ -798,20 +770,20 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
 
     fn following_siblings(self) -> NodeSiblingIterator {
         NodeSiblingIterator {
-            current: self.next_sibling(),
+            current: self.GetNextSibling(),
         }
     }
 
     fn preceding_siblings(self) -> ReverseSiblingIterator {
         ReverseSiblingIterator {
-            current: self.prev_sibling(),
+            current: self.GetPreviousSibling(),
         }
     }
 
     fn is_parent_of(self, child: JSRef<Node>) -> bool {
-        match child.parent_node() {
+        match child.parent_node.get().root() {
-            Some(ref parent) if parent == &Temporary::from_rooted(self) => true,
+            Some(ref parent) => parent.r() == self,
-            _ => false
+            None => false,
         }
     }
 
@@ -829,7 +801,7 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
 
     // https://dom.spec.whatwg.org/#dom-childnode-before
     fn before(self, nodes: Vec<NodeOrString>) -> ErrorResult {
-        match self.parent_node().root() {
+        match self.parent_node.get().root() {
             None => {
                 // Step 1.
                 Ok(())
@@ -847,7 +819,7 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
 
     // https://dom.spec.whatwg.org/#dom-childnode-after
     fn after(self, nodes: Vec<NodeOrString>) -> ErrorResult {
-        match self.parent_node().root() {
+        match self.parent_node.get().root() {
             None => {
                 // Step 1.
                 Ok(())
@@ -858,7 +830,7 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
                 let node = try!(doc.r().node_from_nodes_and_strings(nodes)).root();
                 // Step 3.
                 // FIXME(https://github.com/servo/servo/issues/5720)
-                let next_sibling = self.next_sibling().root();
+                let next_sibling = self.next_sibling.get().root();
                 Node::pre_insert(node.r(), parent_node.r(),
                                  next_sibling.r()).map(|_| ())
             },
@@ -867,7 +839,7 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
 
     // https://dom.spec.whatwg.org/#dom-childnode-replacewith
     fn replace_with(self, nodes: Vec<NodeOrString>) -> ErrorResult {
-        match self.parent_node().root() {
+        match self.parent_node.get().root() {
             None => {
                 // Step 1.
                 Ok(())
@@ -888,7 +860,7 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
         let doc = self.owner_doc().root();
         let node = try!(doc.r().node_from_nodes_and_strings(nodes)).root();
         // Step 2.
-        let first_child = self.first_child().root();
+        let first_child = self.first_child.get().root();
         Node::pre_insert(node.r(), self, first_child.r()).map(|_| ())
     }
 
@@ -951,7 +923,7 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
 
     fn ancestors(self) -> AncestorIterator {
         AncestorIterator {
-            current: self.parent_node()
+            current: self.GetParentNode()
         }
     }
 
@@ -975,13 +947,13 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
 
     fn children(self) -> NodeSiblingIterator {
         NodeSiblingIterator {
-            current: self.first_child(),
+            current: self.GetFirstChild(),
         }
     }
 
     fn rev_children(self) -> ReverseSiblingIterator {
         ReverseSiblingIterator {
-            current: self.last_child(),
+            current: self.GetLastChild(),
         }
     }
 
@@ -995,7 +967,7 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
     }
 
     fn remove_self(self) {
-        match self.parent_node().root() {
+        match self.parent_node.get().root() {
             Some(ref parent) => parent.r().remove_child(self),
             None => ()
         }
@@ -1243,7 +1215,7 @@ impl Iterator for NodeSiblingIterator {
             None => return None,
             Some(current) => current,
         }.root();
-        self.current = current.r().next_sibling();
+        self.current = current.r().GetNextSibling();
         Some(Temporary::from_rooted(current.r()))
     }
 }
@@ -1260,7 +1232,7 @@ impl Iterator for ReverseSiblingIterator {
             None => return None,
             Some(current) => current,
         }.root();
-        self.current = current.r().prev_sibling();
+        self.current = current.r().GetPreviousSibling();
         Some(Temporary::from_rooted(current.r()))
     }
 }
@@ -1277,7 +1249,7 @@ impl Iterator for AncestorIterator {
             None => return None,
             Some(current) => current,
         }.root();
-        self.current = current.r().parent_node();
+        self.current = current.r().GetParentNode();
         Some(Temporary::from_rooted(current.r()))
     }
 }
@@ -1306,7 +1278,7 @@ impl Iterator for TreeIterator {
             Some(current) => current,
         };
         let node = current.root();
-        if let Some(first_child) = node.r().first_child() {
+        if let Some(first_child) = node.r().GetFirstChild() {
             self.current = Some(first_child);
             self.depth += 1;
             return Some(current);
@@ -1315,7 +1287,7 @@ impl Iterator for TreeIterator {
             if self.depth == 0 {
                 break;
             }
-            if let Some(next_sibling) = ancestor.root().r().next_sibling() {
+            if let Some(next_sibling) = ancestor.root().r().GetNextSibling() {
                 self.current = Some(next_sibling);
                 return Some(current);
             }
@@ -1394,7 +1366,7 @@ impl Node {
     // https://dom.spec.whatwg.org/#concept-node-adopt
     pub fn adopt(node: JSRef<Node>, document: JSRef<Document>) {
         // Step 1.
-        match node.parent_node().root() {
+        match node.parent_node.get().root() {
             Some(ref parent) => {
                 Node::remove(node, parent.r(), SuppressObserver::Unsuppressed);
             }
@@ -1534,7 +1506,7 @@ impl Node {
 
         // Step 7-8.
         let reference_child = match child {
-            Some(child) if child == node => node.next_sibling(),
+            Some(child) if child == node => node.GetNextSibling(),
             _ => None
         }.root();
         let reference_child = reference_child.r().or(child);
@@ -1672,7 +1644,7 @@ impl Node {
     // https://dom.spec.whatwg.org/#concept-node-pre-remove
     fn pre_remove(child: JSRef<Node>, parent: JSRef<Node>) -> Fallible<Temporary<Node>> {
         // Step 1.
-        match child.parent_node() {
+        match child.GetParentNode() {
             Some(ref node) if node != &Temporary::from_rooted(parent) => return Err(NotFound),
             None => return Err(NotFound),
             _ => ()
@@ -1687,7 +1659,7 @@ impl Node {
 
     // https://dom.spec.whatwg.org/#concept-node-remove
     fn remove(node: JSRef<Node>, parent: JSRef<Node>, suppress_observers: SuppressObserver) {
-        assert!(node.parent_node().map_or(false, |node_parent| node_parent == Temporary::from_rooted(parent)));
+        assert!(node.GetParentNode().map_or(false, |node_parent| node_parent == Temporary::from_rooted(parent)));
 
         // Step 1-5: ranges.
         // Step 6-7: mutation observers.
@@ -1892,13 +1864,7 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
 
     // https://dom.spec.whatwg.org/#dom-node-parentelement
     fn GetParentElement(self) -> Option<Temporary<Element>> {
-        self.parent_node.get()
+        self.GetParentNode().and_then(ElementCast::to_temporary)
-                        .and_then(|parent| {
-                            let parent = parent.root();
-                            ElementCast::to_ref(parent.r()).map(|elem| {
-                                Temporary::from_rooted(elem)
-                            })
-                        })
     }
 
     // https://dom.spec.whatwg.org/#dom-node-haschildnodes
@@ -2124,8 +2090,8 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
         }
 
         // Step 7-8.
-        let child_next_sibling = child.next_sibling().root();
+        let child_next_sibling = child.next_sibling.get().root();
-        let node_next_sibling = node.next_sibling().root();
+        let node_next_sibling = node.next_sibling.get().root();
         let reference_child = if child_next_sibling.r() == Some(node) {
             node_next_sibling.r()
         } else {
@@ -2433,53 +2399,28 @@ impl<'a> style::node::TNode<'a> for JSRef<'a, Node> {
     type Element = JSRef<'a, Element>;
 
     fn parent_node(self) -> Option<JSRef<'a, Node>> {
-        // FIXME(zwarich): Remove this when UFCS lands and there is a better way
+        (*self).parent_node.get()
-        // of disambiguating methods.
+               .map(|node| node.root().get_unsound_ref_forever())
-        fn parent_node<'a, T: NodeHelpers>(this: T) -> Option<Temporary<Node>> {
-            this.parent_node()
-        }
-
-        parent_node(self).map(|node| node.root().get_unsound_ref_forever())
     }
 
     fn first_child(self) -> Option<JSRef<'a, Node>> {
-        // FIXME(zwarich): Remove this when UFCS lands and there is a better way
+        (*self).first_child.get()
-        // of disambiguating methods.
+               .map(|node| node.root().get_unsound_ref_forever())
-        fn first_child<'a, T: NodeHelpers>(this: T) -> Option<Temporary<Node>> {
-            this.first_child()
-        }
-
-        first_child(self).map(|node| node.root().get_unsound_ref_forever())
     }
 
     fn last_child(self) -> Option<JSRef<'a, Node>> {
-        // FIXME(zwarich): Remove this when UFCS lands and there is a better way
+        (*self).last_child.get()
-        // of disambiguating methods.
+               .map(|node| node.root().get_unsound_ref_forever())
-        fn last_child<'a, T: NodeHelpers>(this: T) -> Option<Temporary<Node>> {
-            this.last_child()
-        }
-
-        last_child(self).map(|node| node.root().get_unsound_ref_forever())
     }
 
     fn prev_sibling(self) -> Option<JSRef<'a, Node>> {
-        // FIXME(zwarich): Remove this when UFCS lands and there is a better way
+        (*self).prev_sibling.get()
-        // of disambiguating methods.
+               .map(|node| node.root().get_unsound_ref_forever())
-        fn prev_sibling<'a, T: NodeHelpers>(this: T) -> Option<Temporary<Node>> {
-            this.prev_sibling()
-        }
-
-        prev_sibling(self).map(|node| node.root().get_unsound_ref_forever())
     }
 
     fn next_sibling(self) -> Option<JSRef<'a, Node>> {
-        // FIXME(zwarich): Remove this when UFCS lands and there is a better way
+        (*self).next_sibling.get()
-        // of disambiguating methods.
+               .map(|node| node.root().get_unsound_ref_forever())
-        fn next_sibling<'a, T: NodeHelpers>(this: T) -> Option<Temporary<Node>> {
-            this.next_sibling()
-        }
-
-        next_sibling(self).map(|node| node.root().get_unsound_ref_forever())
     }
 
     fn is_document(self) -> bool {
@@ -2597,7 +2538,7 @@ impl<'a> DisabledStateHelpers for JSRef<'a, Node> {
 
     fn check_parent_disabled_state_for_option(self) {
         if self.get_disabled_state() { return; }
-        if let Some(ref parent) = self.parent_node().root() {
+        if let Some(ref parent) = self.GetParentNode().root() {
             if parent.r().is_htmloptgroupelement() && parent.r().get_disabled_state() {
                 self.set_disabled_state(true);
                 self.set_enabled_state(false);

components/script/dom/range.rs

@@ -133,25 +133,25 @@ impl<'a> RangeMethods for JSRef<'a, Range> {
 
     // https://dom.spec.whatwg.org/#dom-range-setstartbeforenode
     fn SetStartBefore(self, node: JSRef<Node>) -> ErrorResult {
-        let parent = try!(node.parent_node().ok_or(Error::InvalidNodeType)).root();
+        let parent = try!(node.GetParentNode().ok_or(Error::InvalidNodeType)).root();
         self.SetStart(parent.r(), node.index())
     }
 
     // https://dom.spec.whatwg.org/#dom-range-setstartafternode
     fn SetStartAfter(self, node: JSRef<Node>) -> ErrorResult {
-        let parent = try!(node.parent_node().ok_or(Error::InvalidNodeType)).root();
+        let parent = try!(node.GetParentNode().ok_or(Error::InvalidNodeType)).root();
         self.SetStart(parent.r(), node.index() + 1)
     }
 
     // https://dom.spec.whatwg.org/#dom-range-setendbeforenode
     fn SetEndBefore(self, node: JSRef<Node>) -> ErrorResult {
-        let parent = try!(node.parent_node().ok_or(Error::InvalidNodeType)).root();
+        let parent = try!(node.GetParentNode().ok_or(Error::InvalidNodeType)).root();
         self.SetEnd(parent.r(), node.index())
     }
 
     // https://dom.spec.whatwg.org/#dom-range-setendafternode
     fn SetEndAfter(self, node: JSRef<Node>) -> ErrorResult {
-        let parent = try!(node.parent_node().ok_or(Error::InvalidNodeType)).root();
+        let parent = try!(node.GetParentNode().ok_or(Error::InvalidNodeType)).root();
         self.SetEnd(parent.r(), node.index() + 1)
     }
 
@@ -259,7 +259,7 @@ impl<'a> RangeMethods for JSRef<'a, Range> {
             // Step 1.
             return false;
         }
-        let parent = match node.parent_node() {
+        let parent = match node.GetParentNode() {
             Some(parent) => parent,
             None => {
                 // Step 3.
@@ -364,7 +364,7 @@ impl RangeInner {
     // https://dom.spec.whatwg.org/#dom-range-selectnodenode
     fn select_node(&mut self, node: JSRef<Node>) -> ErrorResult {
         // Steps 1, 2.
-        let parent = try!(node.parent_node().ok_or(Error::InvalidNodeType)).root();
+        let parent = try!(node.GetParentNode().ok_or(Error::InvalidNodeType)).root();
         // Step 3.
         let index = node.index();
         // Step 4.
@@ -497,7 +497,7 @@ fn bp_position(a_node: JSRef<Node>, a_offset: u32,
         // Step 3-1, 3-2.
         let b_ancestors = b_node.inclusive_ancestors();
         let ref child = b_ancestors.map(|child| child.root()).find(|child| {
-            child.r().parent_node().unwrap().root().r() == a_node
+            child.r().GetParentNode().unwrap().root().r() == a_node
         }).unwrap();
         // Step 3-3.
         if child.r().index() < a_offset {

components/script/dom/text.rs

@@ -68,11 +68,11 @@ impl<'a> TextMethods for JSRef<'a, Text> {
         let owner_doc = node.owner_doc().root();
         let new_node = owner_doc.r().CreateTextNode(new_data).root();
         // Step 6.
-        let parent = node.parent_node().root();
+        let parent = node.GetParentNode().root();
         if let Some(ref parent) = parent {
             // Step 7.
             parent.r().InsertBefore(NodeCast::from_ref(new_node.r()),
-                                    node.next_sibling().root().r())
+                                    node.GetNextSibling().root().r())
                   .unwrap();
             // TODO: Ranges.
         }

components/script/dom/treewalker.rs

@@ -14,7 +14,7 @@ use dom::bindings::js::{JS, JSRef, MutHeap, OptionalRootable, Rootable};
 use dom::bindings::js::Temporary;
 use dom::bindings::utils::{Reflector, reflect_dom_object};
 use dom::document::{Document, DocumentHelpers};
-use dom::node::{Node, NodeHelpers};
+use dom::node::Node;
 
 // https://dom.spec.whatwg.org/#interface-treewalker
 #[dom_struct]
@@ -93,37 +93,175 @@ impl<'a> TreeWalkerMethods for JSRef<'a, TreeWalker> {
 
     // https://dom.spec.whatwg.org/#dom-treewalker-parentnode
     fn ParentNode(self) -> Fallible<Option<Temporary<Node>>> {
-        self.parent_node()
+        // "1. Let node be the value of the currentNode attribute."
+        let mut node = self.current_node.get().root().get_unsound_ref_forever();
+        // "2. While node is not null and is not root, run these substeps:"
+        while !self.is_root_node(node) {
+            // "1. Let node be node's parent."
+            match node.GetParentNode() {
+                Some(n) => {
+                    node = n.root().get_unsound_ref_forever();
+                    // "2. If node is not null and filtering node returns FILTER_ACCEPT,
+                    //     then set the currentNode attribute to node, return node."
+                    match try!(self.accept_node(node)) {
+                        NodeFilterConstants::FILTER_ACCEPT => {
+                            self.current_node.set(JS::from_rooted(node));
+                            return Ok(Some(Temporary::from_rooted(node)))
+                        },
+                        _ => {}
+                    }
+                },
+                None => break,
+            }
+        }
+        // "3. Return null."
+        Ok(None)
     }
 
     // https://dom.spec.whatwg.org/#dom-treewalker-firstchild
     fn FirstChild(self) -> Fallible<Option<Temporary<Node>>> {
-        self.first_child()
+        // "The firstChild() method must traverse children of type first."
+        self.traverse_children(|node| node.GetFirstChild(),
+                               |node| node.GetNextSibling())
     }
 
     // https://dom.spec.whatwg.org/#dom-treewalker-lastchild
     fn LastChild(self) -> Fallible<Option<Temporary<Node>>> {
-        self.last_child()
+        // "The lastChild() method must traverse children of type last."
+        self.traverse_children(|node| node.GetLastChild(),
+                               |node| node.GetPreviousSibling())
     }
 
     // https://dom.spec.whatwg.org/#dom-treewalker-previoussibling
     fn PreviousSibling(self) -> Fallible<Option<Temporary<Node>>> {
-        self.prev_sibling()
+        // "The nextSibling() method must traverse siblings of type next."
+        self.traverse_siblings(|node| node.GetLastChild(),
+                               |node| node.GetPreviousSibling())
     }
 
     // https://dom.spec.whatwg.org/#dom-treewalker-nextsibling
     fn NextSibling(self) -> Fallible<Option<Temporary<Node>>> {
-        self.next_sibling()
+        // "The previousSibling() method must traverse siblings of type previous."
+        self.traverse_siblings(|node| node.GetFirstChild(),
+                               |node| node.GetNextSibling())
     }
 
     // https://dom.spec.whatwg.org/#dom-treewalker-previousnode
     fn PreviousNode(self) -> Fallible<Option<Temporary<Node>>> {
-        self.prev_node()
+        // "1. Let node be the value of the currentNode attribute."
+        let mut node = self.current_node.get().root().get_unsound_ref_forever();
+        // "2. While node is not root, run these substeps:"
+        while !self.is_root_node(node) {
+            // "1. Let sibling be the previous sibling of node."
+            let mut sibling_op = node.GetPreviousSibling();
+            // "2. While sibling is not null, run these subsubsteps:"
+            while sibling_op.is_some() {
+                // "1. Set node to sibling."
+                node = sibling_op.unwrap().root().get_unsound_ref_forever();
+                // "2. Filter node and let result be the return value."
+                // "3. While result is not FILTER_REJECT and node has a child,
+                //     set node to its last child and then filter node and
+                //     set result to the return value."
+                // "4. If result is FILTER_ACCEPT, then
+                //     set the currentNode attribute to node and return node."
+                loop {
+                    let result = try!(self.accept_node(node));
+                    match result {
+                        NodeFilterConstants::FILTER_REJECT => break,
+                        _ if node.GetFirstChild().is_some() =>
+                            node = node.GetLastChild().unwrap().root()
+                                       .get_unsound_ref_forever(),
+                        NodeFilterConstants::FILTER_ACCEPT => {
+                            self.current_node.set(JS::from_rooted(node));
+                            return Ok(Some(Temporary::from_rooted(node)))
+                        },
+                        _ => break
+                    }
+                }
+                // "5. Set sibling to the previous sibling of node."
+                sibling_op = node.GetPreviousSibling()
+            }
+            // "3. If node is root or node's parent is null, return null."
+            if self.is_root_node(node) || node.GetParentNode().is_none() {
+                return Ok(None)
+            }
+            // "4. Set node to its parent."
+            match node.GetParentNode() {
+                None =>
+                    // This can happen if the user set the current node to somewhere
+                    // outside of the tree rooted at the original root.
+                    return Ok(None),
+                Some(n) => node = n.root().get_unsound_ref_forever()
+            }
+            // "5. Filter node and if the return value is FILTER_ACCEPT, then
+            //     set the currentNode attribute to node and return node."
+            match try!(self.accept_node(node)) {
+                NodeFilterConstants::FILTER_ACCEPT => {
+                    self.current_node.set(JS::from_rooted(node));
+                    return Ok(Some(Temporary::from_rooted(node)))
+                },
+                _ => {}
+            }
+        }
+        // "6. Return null."
+        Ok(None)
     }
 
     // https://dom.spec.whatwg.org/#dom-treewalker-nextnode
     fn NextNode(self) -> Fallible<Option<Temporary<Node>>> {
-        self.next_node()
+        // "1. Let node be the value of the currentNode attribute."
+        let mut node = self.current_node.get().root().get_unsound_ref_forever();
+        // "2. Let result be FILTER_ACCEPT."
+        let mut result = NodeFilterConstants::FILTER_ACCEPT;
+        // "3. Run these substeps:"
+        loop {
+            // "1. While result is not FILTER_REJECT and node has a child, run these subsubsteps:"
+            loop {
+                match result {
+                    NodeFilterConstants::FILTER_REJECT => break,
+                    _ => {}
+                }
+                match node.GetFirstChild() {
+                    None => break,
+                    Some (child) => {
+                        // "1. Set node to its first child."
+                        node = child.root().get_unsound_ref_forever();
+                        // "2. Filter node and set result to the return value."
+                        result = try!(self.accept_node(node));
+                        // "3. If result is FILTER_ACCEPT, then
+                        //     set the currentNode attribute to node and return node."
+                        match result {
+                            NodeFilterConstants::FILTER_ACCEPT => {
+                                self.current_node.set(JS::from_rooted(node));
+                                return Ok(Some(Temporary::from_rooted(node)))
+                            },
+                            _ => {}
+                        }
+                    }
+                }
+            }
+            // "2. If a node is following node and is not following root,
+            //     set node to the first such node."
+            // "Otherwise, return null."
+            match self.first_following_node_not_following_root(node) {
+                None => return Ok(None),
+                Some(n) => {
+                    node = n.root().get_unsound_ref_forever();
+                    // "3. Filter node and set result to the return value."
+                    result = try!(self.accept_node(node));
+                    // "4. If result is FILTER_ACCEPT, then
+                    //     set the currentNode attribute to node and return node."
+                    match result {
+                        NodeFilterConstants::FILTER_ACCEPT => {
+                            self.current_node.set(JS::from_rooted(node));
+                            return Ok(Some(Temporary::from_rooted(node)))
+                        },
+                        _ => {}
+                    }
+                }
+            }
+            // "5. Run these substeps again."
+        }
     }
 }
 
@@ -209,7 +347,7 @@ impl<'a> PrivateTreeWalkerHelpers for JSRef<'a, TreeWalker> {
                                     },
                                     None => {
                                         // "3. Let parent be node's parent."
-                                        match node.parent_node().map(|p| p.root().get_unsound_ref_forever()) {
+                                        match node.GetParentNode().map(|p| p.root().get_unsound_ref_forever()) {
                                             // "4. If parent is null, parent is root,
                                             //     or parent is currentNode attribute's value,
                                             //     return null."
@@ -280,7 +418,7 @@ impl<'a> PrivateTreeWalkerHelpers for JSRef<'a, TreeWalker> {
                 }
             }
             // "3. Set node to its parent."
-            match node.parent_node().map(|p| p.root().get_unsound_ref_forever()) {
+            match node.GetParentNode().map(|p| p.root().get_unsound_ref_forever()) {
                 // "4. If node is null or is root, return null."
                 None => return Ok(None),
                 Some(n) if self.is_root_node(n) => return Ok(None),
@@ -302,11 +440,11 @@ impl<'a> PrivateTreeWalkerHelpers for JSRef<'a, TreeWalker> {
                                                -> Option<Temporary<Node>> {
         // "An object A is following an object B if A and B are in the same tree
         //  and A comes after B in tree order."
-        match node.next_sibling() {
+        match node.GetNextSibling() {
             None => {
                 let mut candidate = node;
-                while !self.is_root_node(candidate) && candidate.next_sibling().is_none() {
+                while !self.is_root_node(candidate) && candidate.GetNextSibling().is_none() {
-                    match candidate.parent_node() {
+                    match candidate.GetParentNode() {
                         None =>
                             // This can happen if the user set the current node to somewhere
                             // outside of the tree rooted at the original root.
@@ -317,7 +455,7 @@ impl<'a> PrivateTreeWalkerHelpers for JSRef<'a, TreeWalker> {
                 if self.is_root_node(candidate) {
                     None
                 } else {
-                    candidate.next_sibling()
+                    candidate.GetNextSibling()
                 }
             },
             it => it
@@ -354,195 +492,11 @@ impl<'a> PrivateTreeWalkerHelpers for JSRef<'a, TreeWalker> {
     }
 }
 
-pub trait TreeWalkerHelpers {
-    fn parent_node(self) -> Fallible<Option<Temporary<Node>>>;
-    fn first_child(self) -> Fallible<Option<Temporary<Node>>>;
-    fn last_child(self) -> Fallible<Option<Temporary<Node>>>;
-    fn next_sibling(self) -> Fallible<Option<Temporary<Node>>>;
-    fn prev_sibling(self) -> Fallible<Option<Temporary<Node>>>;
-    fn next_node(self) -> Fallible<Option<Temporary<Node>>>;
-    fn prev_node(self) -> Fallible<Option<Temporary<Node>>>;
-}
-
-impl<'a> TreeWalkerHelpers for JSRef<'a, TreeWalker> {
-    // https://dom.spec.whatwg.org/#dom-treewalker-parentnode
-    fn parent_node(self) -> Fallible<Option<Temporary<Node>>> {
-        // "1. Let node be the value of the currentNode attribute."
-        let mut node = self.current_node.get().root().get_unsound_ref_forever();
-        // "2. While node is not null and is not root, run these substeps:"
-        while !self.is_root_node(node) {
-            // "1. Let node be node's parent."
-            match node.parent_node() {
-                Some(n) => {
-                    node = n.root().get_unsound_ref_forever();
-                    // "2. If node is not null and filtering node returns FILTER_ACCEPT,
-                    //     then set the currentNode attribute to node, return node."
-                    match try!(self.accept_node(node)) {
-                        NodeFilterConstants::FILTER_ACCEPT => {
-                            self.current_node.set(JS::from_rooted(node));
-                            return Ok(Some(Temporary::from_rooted(node)))
-                        },
-                        _ => {}
-                    }
-                },
-                None => break,
-            }
-        }
-        // "3. Return null."
-        Ok(None)
-    }
-
-    // https://dom.spec.whatwg.org/#dom-treewalker-firstchild
-    fn first_child(self) -> Fallible<Option<Temporary<Node>>> {
-        // "The firstChild() method must traverse children of type first."
-        self.traverse_children(|node| node.first_child(),
-                               |node| node.next_sibling())
-    }
-
-    // https://dom.spec.whatwg.org/#dom-treewalker-lastchild
-    fn last_child(self) -> Fallible<Option<Temporary<Node>>> {
-        // "The lastChild() method must traverse children of type last."
-        self.traverse_children(|node| node.last_child(),
-                               |node| node.prev_sibling())
-    }
-
-    // https://dom.spec.whatwg.org/#dom-treewalker-nextsibling
-    fn next_sibling(self) -> Fallible<Option<Temporary<Node>>> {
-        // "The nextSibling() method must traverse siblings of type next."
-        self.traverse_siblings(|node| node.first_child(),
-                               |node| node.next_sibling())
-    }
-
-    // https://dom.spec.whatwg.org/#dom-treewalker-previoussibling
-    fn prev_sibling(self) -> Fallible<Option<Temporary<Node>>> {
-        // "The previousSibling() method must traverse siblings of type previous."
-        self.traverse_siblings(|node| node.last_child(),
-                               |node| node.prev_sibling())
-    }
-
-    // https://dom.spec.whatwg.org/#dom-treewalker-previousnode
-    fn prev_node(self) -> Fallible<Option<Temporary<Node>>> {
-        // "1. Let node be the value of the currentNode attribute."
-        let mut node = self.current_node.get().root().get_unsound_ref_forever();
-        // "2. While node is not root, run these substeps:"
-        while !self.is_root_node(node) {
-            // "1. Let sibling be the previous sibling of node."
-            let mut sibling_op = node.prev_sibling();
-            // "2. While sibling is not null, run these subsubsteps:"
-            while sibling_op.is_some() {
-                // "1. Set node to sibling."
-                node = sibling_op.unwrap().root().get_unsound_ref_forever();
-                // "2. Filter node and let result be the return value."
-                // "3. While result is not FILTER_REJECT and node has a child,
-                //     set node to its last child and then filter node and
-                //     set result to the return value."
-                // "4. If result is FILTER_ACCEPT, then
-                //     set the currentNode attribute to node and return node."
-                loop {
-                    let result = try!(self.accept_node(node));
-                    match result {
-                        NodeFilterConstants::FILTER_REJECT => break,
-                        _ if node.first_child().is_some() =>
-                            node = node.last_child().unwrap().root().get_unsound_ref_forever(),
-                        NodeFilterConstants::FILTER_ACCEPT => {
-                            self.current_node.set(JS::from_rooted(node));
-                            return Ok(Some(Temporary::from_rooted(node)))
-                        },
-                        _ => break
-                    }
-                }
-                // "5. Set sibling to the previous sibling of node."
-                sibling_op = node.prev_sibling()
-            }
-            // "3. If node is root or node's parent is null, return null."
-            if self.is_root_node(node) || node.parent_node() == None {
-                return Ok(None)
-            }
-            // "4. Set node to its parent."
-            match node.parent_node() {
-                None =>
-                    // This can happen if the user set the current node to somewhere
-                    // outside of the tree rooted at the original root.
-                    return Ok(None),
-                Some(n) => node = n.root().get_unsound_ref_forever()
-            }
-            // "5. Filter node and if the return value is FILTER_ACCEPT, then
-            //     set the currentNode attribute to node and return node."
-            match try!(self.accept_node(node)) {
-                NodeFilterConstants::FILTER_ACCEPT => {
-                    self.current_node.set(JS::from_rooted(node));
-                    return Ok(Some(Temporary::from_rooted(node)))
-                },
-                _ => {}
-            }
-        }
-        // "6. Return null."
-        Ok(None)
-    }
-
-    // https://dom.spec.whatwg.org/#dom-treewalker-nextnode
-    fn next_node(self) -> Fallible<Option<Temporary<Node>>> {
-        // "1. Let node be the value of the currentNode attribute."
-        let mut node = self.current_node.get().root().get_unsound_ref_forever();
-        // "2. Let result be FILTER_ACCEPT."
-        let mut result = NodeFilterConstants::FILTER_ACCEPT;
-        // "3. Run these substeps:"
-        loop {
-            // "1. While result is not FILTER_REJECT and node has a child, run these subsubsteps:"
-            loop {
-                match result {
-                    NodeFilterConstants::FILTER_REJECT => break,
-                    _ => {}
-                }
-                match node.first_child() {
-                    None => break,
-                    Some (child) => {
-                        // "1. Set node to its first child."
-                        node = child.root().get_unsound_ref_forever();
-                        // "2. Filter node and set result to the return value."
-                        result = try!(self.accept_node(node));
-                        // "3. If result is FILTER_ACCEPT, then
-                        //     set the currentNode attribute to node and return node."
-                        match result {
-                            NodeFilterConstants::FILTER_ACCEPT => {
-                                self.current_node.set(JS::from_rooted(node));
-                                return Ok(Some(Temporary::from_rooted(node)))
-                            },
-                            _ => {}
-                        }
-                    }
-                }
-            }
-            // "2. If a node is following node and is not following root,
-            //     set node to the first such node."
-            // "Otherwise, return null."
-            match self.first_following_node_not_following_root(node) {
-                None => return Ok(None),
-                Some(n) => {
-                    node = n.root().get_unsound_ref_forever();
-                    // "3. Filter node and set result to the return value."
-                    result = try!(self.accept_node(node));
-                    // "4. If result is FILTER_ACCEPT, then
-                    //     set the currentNode attribute to node and return node."
-                    match result {
-                        NodeFilterConstants::FILTER_ACCEPT => {
-                            self.current_node.set(JS::from_rooted(node));
-                            return Ok(Some(Temporary::from_rooted(node)))
-                        },
-                        _ => {}
-                    }
-                }
-            }
-            // "5. Run these substeps again."
-        }
-    }
-}
-
 impl<'a> Iterator for JSRef<'a, TreeWalker> {
     type Item = JSRef<'a, Node>;
 
     fn next(&mut self) -> Option<JSRef<'a, Node>> {
-        match self.next_node() {
+        match self.NextNode() {
             Ok(node) => node.map(|n| n.root().get_unsound_ref_forever()),
             Err(_) =>
                 // The Err path happens only when a JavaScript

components/script/parse/html.rs

@@ -121,7 +121,7 @@ impl<'a> TreeSink for servohtmlparser::Sink {
             new_node: NodeOrText<JS<Node>>) -> Result<(), NodeOrText<JS<Node>>> {
         // If there is no parent, return the node to the parser.
         let sibling: Root<Node> = sibling.root();
-        let parent = match sibling.r().parent_node() {
+        let parent = match sibling.r().GetParentNode() {
             Some(p) => p.root(),
             None => return Err(new_node),
         };