Bug 1907393: When checking for @scope, avoid redundant use of bloom filter. r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D216321

servo/components/selectors/matching.rs

@@ -132,8 +132,10 @@ where
     false
 }
 
+/// Given the ancestor hashes from a selector, see if the current element,
+/// represented by the bloom filter, has a chance of matching at all.
 #[inline(always)]
-fn may_match(hashes: &AncestorHashes, bf: &BloomFilter) -> bool {
+pub fn selector_may_match(hashes: &AncestorHashes, bf: &BloomFilter) -> bool {
     // Check the first three hashes. Note that we can check for zero before
     // masking off the high bits, since if any of the first three hashes is
     // zero the fourth will be as well. We also take care to avoid the
@@ -281,7 +283,7 @@ where
     // Use the bloom filter to fast-reject.
     if let Some(hashes) = hashes {
         if let Some(filter) = context.bloom_filter {
-            if !may_match(hashes, filter) {
+            if !selector_may_match(hashes, filter) {
                 return KleeneValue::False;
             }
         }

servo/components/style/stylesheets/scope_rule.rs

@@ -22,7 +22,7 @@ use malloc_size_of::{
 };
 use selectors::context::MatchingContext;
 use selectors::matching::matches_selector;
-use selectors::parser::{AncestorHashes, ParseRelative, Selector, SelectorList};
+use selectors::parser::{ParseRelative, Selector, SelectorList};
 use selectors::OpaqueElement;
 use servo_arc::Arc;
 use std::fmt::{self, Write};
@@ -199,7 +199,7 @@ impl ImplicitScopeRoot {
 /// Target of this scope.
 pub enum ScopeTarget<'a> {
     /// Target matches an element matching the specified selector list.
-    Selector(&'a SelectorList<SelectorImpl>, &'a [AncestorHashes]),
+    Selector(&'a SelectorList<SelectorImpl>),
     /// Target matches only the specified element.
     Element(OpaqueElement),
 }
@@ -213,10 +213,10 @@ impl<'a> ScopeTarget<'a> {
         context: &mut MatchingContext<E::Impl>,
     ) -> bool {
         match self {
-            Self::Selector(list, hashes_list) => {
+            Self::Selector(list) => {
                 context.nest_for_scope_condition(scope, |context| {
-                    for (selector, hashes) in list.slice().iter().zip(hashes_list.iter()) {
-                        if matches_selector(selector, 0, Some(hashes), &element, context) {
+                    for selector in list.slice().iter() {
+                        if matches_selector(selector, 0, None, &element, context) {
                             return true;
                         }
                     }
@@ -279,7 +279,6 @@ where
 /// That is, check if any ancestor to the root matches the scope-end selector.
 pub fn element_is_outside_of_scope<E>(
     selector: &Selector<E::Impl>,
-    hashes: &AncestorHashes,
     element: E,
     root: OpaqueElement,
     context: &mut MatchingContext<E::Impl>,
@@ -291,7 +290,7 @@ where
     let mut parent = Some(element);
     context.nest_for_scope_condition(Some(root), |context| {
         while let Some(p) = parent {
-            if matches_selector(selector, 0, Some(hashes), &p, context) {
+            if matches_selector(selector, 0, None, &p, context) {
                 return true;
             }
             if p.opaque() == root {

servo/components/style/stylist.rs

@@ -65,7 +65,7 @@ use malloc_size_of::{MallocShallowSizeOf, MallocSizeOfOps, MallocUnconditionalSh
 use selectors::attr::{CaseSensitivity, NamespaceConstraint};
 use selectors::bloom::BloomFilter;
 use selectors::matching::{
-    matches_selector, MatchingContext, MatchingMode, NeedsSelectorFlags, SelectorCaches,
+    matches_selector, selector_may_match, MatchingContext, MatchingMode, NeedsSelectorFlags, SelectorCaches
 };
 use selectors::matching::{MatchingForInvalidation, VisitedHandlingMode};
 use selectors::parser::{
@@ -2910,8 +2910,17 @@ impl CascadeData {
         }
 
         let (root_target, matches_shadow_host) = if let Some(start) = bounds.start.as_ref() {
+            if let Some(filter) = context.bloom_filter {
+                // Use the bloom filter here. If our ancestors do not have the right hashes,
+                // there's no point in traversing up. Besides, the filter is built for this depth,
+                // so the filter contains more data than it should, the further we go up the ancestor
+                // chain. It wouldn't generate wrong results, but makes the traversal even more pointless.
+                if !start.hashes.iter().any(|entry| selector_may_match(entry, filter)) {
+                    return vec![];
+                }
+            }
             (
-                ScopeTarget::Selector(&start.selectors, &start.hashes),
+                ScopeTarget::Selector(&start.selectors),
                 start.selectors.slice().iter().any(|s| {
                     !s.matches_featureless_host_selector_or_pseudo_element()
                         .is_empty()
@@ -2975,9 +2984,16 @@ impl CascadeData {
             // If any scope-end selector matches, we're not in scope.
             for scope_root in potential_scope_roots {
                 if end.selectors.slice().iter().zip(end.hashes.iter()).all(|(selector, hashes)| {
+                    // Like checking for scope-start, use the bloom filter here.
+                    if let Some(filter) = context.bloom_filter {
+                        if !selector_may_match(hashes, filter) {
+                            // Selector this hash belongs to won't cause us to be out of this scope.
+                            return true;
+                        }
+                    }
+
                     !element_is_outside_of_scope(
                         selector,
-                        hashes,
                         element,
                         scope_root.root,
                         context,