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

style: Move some bloom filter code outside of matching.rs

Also simplify it, while we're at it.

<!-- 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/16961)
<!-- Reviewable:end -->

2 files changed, 34 insertions, 54 deletions

diff --git a/components/style/bloom.rs b/components/style/bloom.rs
index e8ba95aeb94..7633d154fe8 100644
--- a/components/style/bloom.rs
+++ b/components/style/bloom.rs
@@ -8,7 +8,6 @@
 #![deny(missing_docs)]
 
 use dom::{SendElement, TElement};
-use matching::MatchMethods;
 use selectors::bloom::BloomFilter;
 
 /// A struct that allows us to fast-reject deep descendant selectors avoiding
@@ -50,6 +49,26 @@ pub struct StyleBloom<E: TElement> {
     elements: Vec<SendElement<E>>,
 }
 
+fn each_relevant_element_hash<E, F>(element: E, mut f: F)
+    where E: TElement,
+          F: FnMut(u32),
+{
+    f(element.get_local_name().get_hash());
+    f(element.get_namespace().get_hash());
+
+    if let Some(id) = element.get_id() {
+        f(id.get_hash());
+    }
+
+    // TODO: case-sensitivity depends on the document type and quirks mode.
+    //
+    // TODO(emilio): It's not clear whether that's relevant here though?
+    // Classes and ids should be normalized already I think.
+    element.each_class(|class| {
+        f(class.get_hash())
+    });
+}
+
 impl<E: TElement> StyleBloom<E> {
     /// Create an empty `StyleBloom`.
     pub fn new() -> Self {
@@ -72,15 +91,26 @@ impl<E: TElement> StyleBloom<E> {
                 assert!(element.parent_element().is_none());
             }
         }
-        element.insert_into_bloom_filter(&mut *self.filter);
+        self.push_internal(element);
+    }
+
+    /// Same as `push`, but without asserting, in order to use it from
+    /// `rebuild`.
+    fn push_internal(&mut self, element: E) {
+        each_relevant_element_hash(element, |hash| {
+            self.filter.insert_hash(hash);
+        });
         self.elements.push(unsafe { SendElement::new(element) });
     }
 
     /// Pop the last element in the bloom filter and return it.
     fn pop(&mut self) -> Option<E> {
         let popped = self.elements.pop().map(|el| *el);
+
         if let Some(popped) = popped {
-            popped.remove_from_bloom_filter(&mut self.filter);
+            each_relevant_element_hash(popped, |hash| {
+                self.filter.remove_hash(hash);
+            })
         }
 
         popped
@@ -103,8 +133,7 @@ impl<E: TElement> StyleBloom<E> {
         self.clear();
 
         while let Some(parent) = element.parent_element() {
-            parent.insert_into_bloom_filter(&mut *self.filter);
-            self.elements.push(unsafe { SendElement::new(parent) });
+            self.push_internal(parent);
             element = parent;
         }
 
diff --git a/components/style/matching.rs b/components/style/matching.rs
index 0c4e53cfc4c..8c762905990 100644
--- a/components/style/matching.rs
+++ b/components/style/matching.rs
@@ -1388,55 +1388,6 @@ pub trait MatchMethods : TElement {
         StyleSharingResult::CannotShare
     }
 
-    // The below two functions are copy+paste because I can't figure out how to
-    // write a function which takes a generic function. I don't think it can
-    // be done.
-    //
-    // Ideally, I'd want something like:
-    //
-    //   > fn with_really_simple_selectors(&self, f: <H: Hash>|&H|);
-
-
-    // In terms of `Component`s, these two functions will insert and remove:
-    //   - `Component::LocalName`
-    //   - `Component::Namepace`
-    //   - `Component::ID`
-    //   - `Component::Class`
-
-    /// Inserts and removes the matching `Descendant` selectors from a bloom
-    /// filter. This is used to speed up CSS selector matching to remove
-    /// unnecessary tree climbs for `Descendant` queries.
-    ///
-    /// A bloom filter of the local names, namespaces, IDs, and classes is kept.
-    /// Therefore, each node must have its matching selectors inserted _after_
-    /// its own selector matching and _before_ its children start.
-    fn insert_into_bloom_filter(&self, bf: &mut BloomFilter) {
-        bf.insert_hash(self.get_local_name().get_hash());
-        bf.insert_hash(self.get_namespace().get_hash());
-        if let Some(id) = self.get_id() {
-            bf.insert_hash(id.get_hash());
-        }
-        // TODO: case-sensitivity depends on the document type and quirks mode
-        self.each_class(|class| {
-            bf.insert_hash(class.get_hash())
-        });
-    }
-
-    /// After all the children are done css selector matching, this must be
-    /// called to reset the bloom filter after an `insert`.
-    fn remove_from_bloom_filter(&self, bf: &mut BloomFilter) {
-        bf.remove_hash(self.get_local_name().get_hash());
-        bf.remove_hash(self.get_namespace().get_hash());
-        if let Some(id) = self.get_id() {
-            bf.remove_hash(id.get_hash());
-        }
-
-        // TODO: case-sensitivity depends on the document type and quirks mode
-        self.each_class(|class| {
-            bf.remove_hash(class.get_hash())
-        });
-    }
-
     /// Given the old and new style of this element, and whether it's a
     /// pseudo-element, compute the restyle damage used to determine which
     /// kind of layout or painting operations we'll need.