style: Extract a bit better the logic for finding elements with an ID under a subtree. draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 27 Oct 2017 13:52:06 +0200
changeset 690482 ba91c33d481a1d03cbb030a9e5a4c4291159e865
parent 690481 84dc6b1413c6ec8c75e2e22a41011026eeb05c4f
child 690483 0339476e13f3089c888434298715d69f4334c18a
child 690492 a4ad47c0c780aa2f8cc559607de6571a72d40d2d
push id87308
push userbmo:emilio@crisal.io
push dateThu, 02 Nov 2017 01:16:37 +0000
milestone58.0a1
style: Extract a bit better the logic for finding elements with an ID under a subtree. MozReview-Commit-ID: Hj9yxBk1OvS
servo/components/style/dom_apis.rs
--- a/servo/components/style/dom_apis.rs
+++ b/servo/components/style/dom_apis.rs
@@ -235,68 +235,92 @@ where
             return true;
         }
 
         current = n.parent_node();
     }
     false
 }
 
-fn find_elements_with_id<E, Q, F>(
+/// Execute `callback` on each element with a given `id` under `root`.
+///
+/// If `callback` returns false, iteration will stop immediately.
+fn each_element_with_id_under<E, F>(
+    root: E::ConcreteNode,
+    id: &Atom,
+    quirks_mode: QuirksMode,
+    mut callback: F,
+)
+where
+    E: TElement,
+    F: FnMut(E) -> bool,
+{
+    let case_sensitivity = quirks_mode.classes_and_ids_case_sensitivity();
+    if case_sensitivity == CaseSensitivity::CaseSensitive &&
+       root.is_in_document()
+    {
+        let doc = root.owner_doc();
+        if let Ok(elements) = doc.elements_with_id(id) {
+            if root == doc.as_node() {
+                for element in elements {
+                    if !callback(*element) {
+                        return;
+                    }
+                }
+            } else {
+                for element in elements {
+                    if !element_is_descendant_of(*element, root) {
+                        continue;
+                    }
+
+                    if !callback(*element) {
+                        return;
+                    }
+                }
+            }
+        }
+        return;
+    }
+
+    for node in root.dom_descendants() {
+        let element = match node.as_element() {
+            Some(e) => e,
+            None => continue,
+        };
+
+        if !element.has_id(id, case_sensitivity) {
+            continue;
+        }
+
+        if !callback(element) {
+            return;
+        }
+    }
+}
+
+fn collect_elements_with_id<E, Q, F>(
     root: E::ConcreteNode,
     id: &Atom,
     results: &mut Q::Output,
     quirks_mode: QuirksMode,
     mut filter: F,
 )
 where
     E: TElement,
     Q: SelectorQuery<E>,
     F: FnMut(E) -> bool,
 {
-    let case_sensitivity = quirks_mode.classes_and_ids_case_sensitivity();
-    if case_sensitivity == CaseSensitivity::CaseSensitive &&
-       root.is_in_document()
-    {
-        let doc = root.owner_doc();
-        if let Ok(elements) = doc.elements_with_id(id) {
-            if root == doc.as_node() {
-                for element in elements {
-                    if !filter(*element) {
-                        continue;
-                    }
+    each_element_with_id_under::<E, _>(root, id, quirks_mode, |element| {
+        if !filter(element) {
+            return true;
+        }
 
-                    Q::append_element(results, *element);
-                    if Q::should_stop_after_first_match() {
-                        break;
-                    }
-                }
-            } else {
-                for element in elements {
-                    if !element_is_descendant_of(*element, root) {
-                        continue;
-                    }
-
-                    if !filter(*element) {
-                        continue;
-                    }
-
-                    Q::append_element(results, *element);
-                    if Q::should_stop_after_first_match() {
-                        break;
-                    }
-                }
-            }
-        }
-        return;
-    }
-
-    collect_all_elements::<E, Q, _>(root, results, |element| {
-        element.has_id(id, case_sensitivity) && filter(element)
-    });
+        Q::append_element(results, element);
+        return !Q::should_stop_after_first_match()
+    })
 }
 
 
 /// Fast paths for querySelector with a single simple selector.
 fn query_selector_single_query<E, Q>(
     root: E::ConcreteNode,
     component: &Component<E::Impl>,
     results: &mut Q::Output,
@@ -306,17 +330,17 @@ where
     E: TElement,
     Q: SelectorQuery<E>,
 {
     match *component {
         Component::ExplicitUniversalType => {
             collect_all_elements::<E, Q, _>(root, results, |_| true)
         }
         Component::ID(ref id) => {
-            find_elements_with_id::<E, Q, _>(
+            collect_elements_with_id::<E, Q, _>(
                 root,
                 id,
                 results,
                 quirks_mode,
                 |_| true,
             )
         }
         Component::Class(ref class) => {
@@ -383,17 +407,17 @@ where
         debug_assert!(combinator.map_or(true, |c| !c.is_sibling()));
 
         for component in &mut iter {
             match *component {
                 Component::ID(ref id) => {
                     if combinator.is_none() {
                         // In the rightmost compound, just find descednants of
                         // root that match the selector list with that id.
-                        find_elements_with_id::<E, Q, _>(
+                        collect_elements_with_id::<E, Q, _>(
                             root,
                             id,
                             results,
                             quirks_mode,
                             |e| {
                                 matching::matches_selector_list(
                                     selector_list,
                                     &e,