From 4a2c4b65ccbe583289bd354607ee88e4f9a09c1a Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 13 Dec 2014 10:34:38 +0100 Subject: [PATCH 1/2] Factor out part of the traversal routine in HTMLCollection. I would move the filter() call into the traverse function as well, but the callback can't outlive its stack frame. --- components/script/dom/htmlcollection.rs | 43 +++++++++++-------------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/components/script/dom/htmlcollection.rs b/components/script/dom/htmlcollection.rs index 91034fd4423..8a95b558793 100644 --- a/components/script/dom/htmlcollection.rs +++ b/components/script/dom/htmlcollection.rs @@ -10,12 +10,13 @@ use dom::bindings::js::{JS, JSRef, Temporary}; use dom::bindings::trace::JSTraceable; use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object}; use dom::element::{Element, AttributeHandlers, ElementHelpers}; -use dom::node::{Node, NodeHelpers}; +use dom::node::{Node, NodeHelpers, TreeIterator}; use dom::window::Window; use servo_util::namespace; use servo_util::str::{DOMString, split_html_space_chars}; use std::ascii::AsciiExt; +use std::iter::FilterMap; use string_cache::{Atom, Namespace}; pub trait CollectionFilter : JSTraceable { @@ -171,6 +172,14 @@ impl HTMLCollection { } HTMLCollection::create(window, root, box ElementChildFilter) } + + fn traverse<'a>(root: JSRef<'a, Node>) + -> FilterMap<'a, JSRef<'a, Node>, + JSRef<'a, Element>, + TreeIterator<'a>> { + root.traverse_preorder() + .filter_map(ElementCast::to_ref) + } } impl<'a> HTMLCollectionMethods for JSRef<'a, HTMLCollection> { @@ -180,11 +189,9 @@ impl<'a> HTMLCollectionMethods for JSRef<'a, HTMLCollection> { Static(ref elems) => elems.len() as u32, Live(ref root, ref filter) => { let root = root.root(); - root.traverse_preorder() - .filter(|&child| { - let elem: Option> = ElementCast::to_ref(child); - elem.map_or(false, |elem| filter.filter(elem, *root)) - }).count() as u32 + HTMLCollection::traverse(*root) + .filter(|element| filter.filter(*element, *root)) + .count() as u32 } } } @@ -198,17 +205,11 @@ impl<'a> HTMLCollectionMethods for JSRef<'a, HTMLCollection> { .map(|elem| Temporary::new(elem.clone())), Live(ref root, ref filter) => { let root = root.root(); - root.traverse_preorder() - .filter_map(|node| { - let elem: Option> = ElementCast::to_ref(node); - match elem { - Some(ref elem) if filter.filter(*elem, *root) => Some(elem.clone()), - _ => None - } - }) + HTMLCollection::traverse(*root) + .filter(|element| filter.filter(*element, *root)) .nth(index as uint) .clone() - .map(|elem| Temporary::from_rooted(elem)) + .map(Temporary::from_rooted) } } } @@ -230,18 +231,12 @@ impl<'a> HTMLCollectionMethods for JSRef<'a, HTMLCollection> { .map(|maybe_elem| Temporary::from_rooted(*maybe_elem)), Live(ref root, ref filter) => { let root = root.root(); - root.traverse_preorder() - .filter_map(|node| { - let elem: Option> = ElementCast::to_ref(node); - match elem { - Some(ref elem) if filter.filter(*elem, *root) => Some(elem.clone()), - _ => None - } - }) + HTMLCollection::traverse(*root) + .filter(|element| filter.filter(*element, *root)) .find(|elem| { elem.get_string_attribute(&atom!("name")) == key || elem.get_string_attribute(&atom!("id")) == key }) - .map(|maybe_elem| Temporary::from_rooted(maybe_elem)) + .map(Temporary::from_rooted) } } } From 14e1455119fde58a9e4ab9232efa816d2585c900 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 13 Dec 2014 10:38:17 +0100 Subject: [PATCH 2/2] Skip the root node in live HTMLCollections. The root node is never included in the collection, and omitting it here simplifies and speeds up the filter implementations. --- components/script/dom/htmlcollection.rs | 24 +++++++------------- components/script/dom/htmlfieldsetelement.rs | 7 +++--- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/components/script/dom/htmlcollection.rs b/components/script/dom/htmlcollection.rs index 8a95b558793..86f977c843a 100644 --- a/components/script/dom/htmlcollection.rs +++ b/components/script/dom/htmlcollection.rs @@ -16,7 +16,7 @@ use servo_util::namespace; use servo_util::str::{DOMString, split_html_space_chars}; use std::ascii::AsciiExt; -use std::iter::FilterMap; +use std::iter::{FilterMap, Skip}; use string_cache::{Atom, Namespace}; pub trait CollectionFilter : JSTraceable { @@ -63,10 +63,7 @@ impl HTMLCollection { namespace_filter: Option } impl CollectionFilter for AllElementFilter { - fn filter(&self, elem: JSRef, root: JSRef) -> bool { - if NodeCast::from_ref(elem) == root { - return false - } + fn filter(&self, elem: JSRef, _root: JSRef) -> bool { match self.namespace_filter { None => true, Some(ref namespace) => *elem.namespace() == *namespace @@ -89,10 +86,7 @@ impl HTMLCollection { ascii_lower_tag: Atom, } impl CollectionFilter for TagNameFilter { - fn filter(&self, elem: JSRef, root: JSRef) -> bool { - if NodeCast::from_ref(elem) == root { - return false - } + fn filter(&self, elem: JSRef, _root: JSRef) -> bool { if elem.html_element_in_html_document() { *elem.local_name() == self.ascii_lower_tag } else { @@ -123,10 +117,7 @@ impl HTMLCollection { namespace_filter: Option } impl CollectionFilter for TagNameNSFilter { - fn filter(&self, elem: JSRef, root: JSRef) -> bool { - if NodeCast::from_ref(elem) == root { - return false - } + fn filter(&self, elem: JSRef, _root: JSRef) -> bool { let ns_match = match self.namespace_filter { Some(ref namespace) => { *elem.namespace() == *namespace @@ -150,8 +141,8 @@ impl HTMLCollection { classes: Vec } impl CollectionFilter for ClassNameFilter { - fn filter(&self, elem: JSRef, root: JSRef) -> bool { - (NodeCast::from_ref(elem) != root) && self.classes.iter().all(|class| elem.has_class(class)) + fn filter(&self, elem: JSRef, _root: JSRef) -> bool { + self.classes.iter().all(|class| elem.has_class(class)) } } let filter = ClassNameFilter { @@ -176,8 +167,9 @@ impl HTMLCollection { fn traverse<'a>(root: JSRef<'a, Node>) -> FilterMap<'a, JSRef<'a, Node>, JSRef<'a, Element>, - TreeIterator<'a>> { + Skip>> { root.traverse_preorder() + .skip(1) .filter_map(ElementCast::to_ref) } } diff --git a/components/script/dom/htmlfieldsetelement.rs b/components/script/dom/htmlfieldsetelement.rs index 1dfdaa063b9..3ed4a82200c 100644 --- a/components/script/dom/htmlfieldsetelement.rs +++ b/components/script/dom/htmlfieldsetelement.rs @@ -6,7 +6,7 @@ use dom::attr::Attr; use dom::attr::AttrHelpers; use dom::bindings::codegen::Bindings::HTMLFieldSetElementBinding; use dom::bindings::codegen::Bindings::HTMLFieldSetElementBinding::HTMLFieldSetElementMethods; -use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLFieldSetElementDerived, NodeCast}; +use dom::bindings::codegen::InheritTypes::{HTMLFieldSetElementDerived, NodeCast}; use dom::bindings::codegen::InheritTypes::{HTMLElementCast, HTMLLegendElementDerived}; use dom::bindings::js::{JSRef, Temporary}; use dom::bindings::utils::{Reflectable, Reflector}; @@ -54,11 +54,10 @@ impl<'a> HTMLFieldSetElementMethods for JSRef<'a, HTMLFieldSetElement> { #[jstraceable] struct ElementsFilter; impl CollectionFilter for ElementsFilter { - fn filter<'a>(&self, elem: JSRef<'a, Element>, root: JSRef<'a, Node>) -> bool { + fn filter<'a>(&self, elem: JSRef<'a, Element>, _root: JSRef<'a, Node>) -> bool { static TAG_NAMES: StaticStringVec = &["button", "fieldset", "input", "keygen", "object", "output", "select", "textarea"]; - let root: JSRef = ElementCast::to_ref(root).unwrap(); - elem != root && TAG_NAMES.iter().any(|&tag_name| tag_name == elem.local_name().as_slice()) + TAG_NAMES.iter().any(|&tag_name| tag_name == elem.local_name().as_slice()) } } let node: JSRef = NodeCast::from_ref(self);