Auto merge of #15890 - emilio:bloom, r=heycam

selectors: Check the bloom filter at most once per complex selector.

Fixes https://github.com/servo/rust-selectors/issues/107

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15890)
<!-- Reviewable:end -->

components/selectors/matching.rs

@@ -112,12 +112,56 @@ pub fn matches<E>(selector_list: &[Selector<E::Impl>],
     })
 }
 
+fn may_match<E>(mut selector: &ComplexSelector<E::Impl>,
+                bf: &BloomFilter)
+                -> bool
+    where E: Element,
+{
+    // See if the bloom filter can exclude any of the descendant selectors, and
+    // reject if we can.
+    loop {
+         match selector.next {
+             None => break,
+             Some((ref cs, Combinator::Descendant)) => selector = &**cs,
+             Some((ref cs, _)) => {
+                 selector = &**cs;
+                 continue;
+             }
+         };
+
+        for ss in selector.compound_selector.iter() {
+            match *ss {
+                SimpleSelector::LocalName(LocalName { ref name, ref lower_name })  => {
+                    if !bf.might_contain(name) &&
+                       !bf.might_contain(lower_name) {
+                       return false
+                    }
+                },
+                SimpleSelector::Namespace(ref namespace) => {
+                    if !bf.might_contain(&namespace.url) {
+                        return false
+                    }
+                },
+                SimpleSelector::ID(ref id) => {
+                    if !bf.might_contain(id) {
+                        return false
+                    }
+                },
+                SimpleSelector::Class(ref class) => {
+                    if !bf.might_contain(class) {
+                        return false
+                    }
+                },
+                _ => {},
+            }
+        }
+    }
+
+    // If we haven't proven otherwise, it may match.
+    true
+}
+
 /// Determines whether the given element matches the given complex selector.
-///
-/// NB: If you add support for any new kinds of selectors to this routine, be sure to set
-/// `shareable` to false unless you are willing to update the style sharing logic. Otherwise things
-/// will almost certainly break as elements will start mistakenly sharing styles. (See
-/// `can_share_style_with` in `servo/components/style/matching.rs`.)
 pub fn matches_complex_selector<E>(selector: &ComplexSelector<E::Impl>,
                                    element: &E,
                                    parent_bf: Option<&BloomFilter>,
@@ -126,7 +170,13 @@ pub fn matches_complex_selector<E>(selector: &ComplexSelector<E::Impl>,
                                    -> bool
     where E: Element
 {
-    match matches_complex_selector_internal(selector, element, parent_bf, relations, flags) {
+    if let Some(filter) = parent_bf {
+        if !may_match::<E>(selector, filter) {
+            return false;
+        }
+    }
+
+    match matches_complex_selector_internal(selector, element, relations, flags) {
         SelectorMatchingResult::Matched => {
             match selector.next {
                 Some((_, Combinator::NextSibling)) |
@@ -190,81 +240,19 @@ enum SelectorMatchingResult {
     NotMatchedGlobally,
 }
 
-/// Quickly figures out whether or not the complex selector is worth doing more
-/// work on. If the simple selectors don't match, or there's a child selector
-/// that does not appear in the bloom parent bloom filter, we can exit early.
-fn can_fast_reject<E>(mut selector: &ComplexSelector<E::Impl>,
-                      element: &E,
-                      parent_bf: Option<&BloomFilter>,
-                      relations: &mut StyleRelations,
-                      flags: &mut ElementSelectorFlags)
-                      -> Option<SelectorMatchingResult>
-    where E: Element
-{
-    if !selector.compound_selector.iter().all(|simple_selector| {
-      matches_simple_selector(simple_selector, element, parent_bf, relations, flags) }) {
-        return Some(SelectorMatchingResult::NotMatchedAndRestartFromClosestLaterSibling);
-    }
-
-    let bf: &BloomFilter = match parent_bf {
-        None => return None,
-        Some(ref bf) => bf,
-    };
-
-    // See if the bloom filter can exclude any of the descendant selectors, and
-    // reject if we can.
-    loop {
-         match selector.next {
-             None => break,
-             Some((ref cs, Combinator::Descendant)) => selector = &**cs,
-             Some((ref cs, _)) => {
-                 selector = &**cs;
-                 continue;
-             }
-         };
-
-        for ss in selector.compound_selector.iter() {
-            match *ss {
-                SimpleSelector::LocalName(LocalName { ref name, ref lower_name })  => {
-                    if !bf.might_contain(name) &&
-                       !bf.might_contain(lower_name) {
-                        return Some(SelectorMatchingResult::NotMatchedGlobally);
-                    }
-                },
-                SimpleSelector::Namespace(ref namespace) => {
-                    if !bf.might_contain(&namespace.url) {
-                        return Some(SelectorMatchingResult::NotMatchedGlobally);
-                    }
-                },
-                SimpleSelector::ID(ref id) => {
-                    if !bf.might_contain(id) {
-                        return Some(SelectorMatchingResult::NotMatchedGlobally);
-                    }
-                },
-                SimpleSelector::Class(ref class) => {
-                    if !bf.might_contain(class) {
-                        return Some(SelectorMatchingResult::NotMatchedGlobally);
-                    }
-                },
-                _ => {},
-            }
-        }
-    }
-
-    // Can't fast reject.
-    None
-}
-
 fn matches_complex_selector_internal<E>(selector: &ComplexSelector<E::Impl>,
-                                         element: &E,
+                                        element: &E,
-                                         parent_bf: Option<&BloomFilter>,
+                                        relations: &mut StyleRelations,
-                                         relations: &mut StyleRelations,
+                                        flags: &mut ElementSelectorFlags)
-                                         flags: &mut ElementSelectorFlags)
+                                        -> SelectorMatchingResult
-                                         -> SelectorMatchingResult
      where E: Element
 {
-    if let Some(result) = can_fast_reject(selector, element, parent_bf, relations, flags) {
+    let matches_all_simple_selectors = selector.compound_selector.iter().all(|simple| {
-        return result;
+        matches_simple_selector(simple, element, relations, flags)
+    });
+
+    if !matches_all_simple_selectors {
+        return SelectorMatchingResult::NotMatchedAndRestartFromClosestLaterSibling;
     }
 
     match selector.next {
@@ -287,10 +275,9 @@ fn matches_complex_selector_internal<E>(selector: &ComplexSelector<E::Impl>,
                     Some(next_element) => next_element,
                 };
                 let result = matches_complex_selector_internal(&**next_selector,
-                                                                &element,
+                                                               &element,
-                                                                parent_bf,
+                                                               relations,
-                                                                relations,
+                                                               flags);
-                                                                flags);
                 match (result, combinator) {
                     // Return the status immediately.
                     (SelectorMatchingResult::Matched, _) => return result,
@@ -332,7 +319,6 @@ fn matches_complex_selector_internal<E>(selector: &ComplexSelector<E::Impl>,
 fn matches_simple_selector<E>(
         selector: &SimpleSelector<E::Impl>,
         element: &E,
-        parent_bf: Option<&BloomFilter>,
         relations: &mut StyleRelations,
         flags: &mut ElementSelectorFlags)
         -> bool
@@ -466,7 +452,10 @@ fn matches_simple_selector<E>(
         }
         SimpleSelector::Negation(ref negated) => {
             !negated.iter().all(|s| {
-                matches_complex_selector(s, element, parent_bf, relations, flags)
+                match matches_complex_selector_internal(s, element, relations, flags) {
+                    SelectorMatchingResult::Matched => true,
+                    _ => false,
+                }
             })
         }
     }