Auto merge of #16745 - bholley:more_selector_maps, r=emilio

Use selector maps for revalidation and dependency selectors

https://bugzilla.mozilla.org/show_bug.cgi?id=1360088

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

components/style/context.rs

@@ -94,11 +94,7 @@ impl Default for StyleSystemOptions {
     #[cfg(feature = "gecko")]
     fn default() -> Self {
         StyleSystemOptions {
-            disable_style_sharing_cache:
+            disable_style_sharing_cache: get_env("DISABLE_STYLE_SHARING_CACHE"),
-                // Disable the style sharing cache on opt builds until
-                // bug 1358693 is fixed, but keep it on debug builds to make
-                // sure we don't introduce correctness bugs.
-                if cfg!(debug_assertions) { get_env("DISABLE_STYLE_SHARING_CACHE") } else { true },
             dump_style_statistics: get_env("DUMP_STYLE_STATISTICS"),
         }
     }

components/style/restyle_hints.rs

@@ -20,7 +20,10 @@ use selectors::matching::matches_selector;
 use selectors::parser::{AttrSelector, Combinator, Component, Selector};
 use selectors::parser::{SelectorInner, SelectorMethods};
 use selectors::visitor::SelectorVisitor;
+use smallvec::SmallVec;
+use std::borrow::Borrow;
 use std::clone::Clone;
+use stylist::SelectorMap;
 
 bitflags! {
     /// When the ElementState of an element (like IN_HOVER_STATE) changes,
@@ -429,7 +432,7 @@ fn combinator_to_restyle_hint(combinator: Option<Combinator>) -> RestyleHint {
     }
 }
 
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 /// The aspects of an selector which are sensitive.
 pub struct Sensitivities {
@@ -450,6 +453,10 @@ impl Sensitivities {
             attrs: false,
         }
     }
+
+    fn sensitive_to(&self, attrs: bool, states: ElementState) -> bool {
+        (attrs && self.attrs) || self.states.intersects(states)
+    }
 }
 
 /// Mapping between (partial) CompoundSelectors (and the combinator to their
@@ -470,7 +477,7 @@ impl Sensitivities {
 /// This allows us to quickly scan through the dependency sites of all style
 /// rules and determine the maximum effect that a given state or attribute
 /// change may have on the style of elements in the document.
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct Dependency {
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
@@ -481,6 +488,11 @@ pub struct Dependency {
     pub sensitivities: Sensitivities,
 }
 
+impl Borrow<SelectorInner<SelectorImpl>> for Dependency {
+    fn borrow(&self) -> &SelectorInner<SelectorImpl> {
+        &self.selector
+    }
+}
 
 /// The following visitor visits all the simple selectors for a given complex
 /// selector, taking care of :not and :any combinators, collecting whether any
@@ -500,32 +512,17 @@ impl SelectorVisitor for SensitivitiesVisitor {
 
 /// A set of dependencies for a given stylist.
 ///
-/// Note that there are measurable perf wins from storing them separately
+/// Note that we can have many dependencies, often more than the total number
-/// depending on what kind of change they affect, and its also not a big deal to
+/// of selectors given that we can get multiple partial selectors for a given
-/// do it, since the dependencies are per-document.
+/// selector. As such, we want all the usual optimizations, including the
+/// SelectorMap and the bloom filter.
 #[derive(Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-pub struct DependencySet {
+pub struct DependencySet(pub SelectorMap<Dependency>);
-    /// Dependencies only affected by state.
-    state_deps: Vec<Dependency>,
-    /// Dependencies only affected by attributes.
-    attr_deps: Vec<Dependency>,
-    /// Dependencies affected by both.
-    common_deps: Vec<Dependency>,
-}
 
 impl DependencySet {
     fn add_dependency(&mut self, dep: Dependency) {
-        let affected_by_attribute = dep.sensitivities.attrs;
+        self.0.insert(dep);
-        let affects_states = !dep.sensitivities.states.is_empty();
-
-        if affected_by_attribute && affects_states {
-            self.common_deps.push(dep)
-        } else if affected_by_attribute {
-            self.attr_deps.push(dep)
-        } else {
-            self.state_deps.push(dep)
-        }
     }
 
     /// Adds a selector to this `DependencySet`.
@@ -592,23 +589,17 @@ impl DependencySet {
 
     /// Create an empty `DependencySet`.
     pub fn new() -> Self {
-        DependencySet {
+        DependencySet(SelectorMap::new())
-            state_deps: vec![],
-            attr_deps: vec![],
-            common_deps: vec![],
-        }
     }
 
     /// Return the total number of dependencies that this set contains.
     pub fn len(&self) -> usize {
-        self.common_deps.len() + self.attr_deps.len() + self.state_deps.len()
+        self.0.len()
     }
 
     /// Clear this dependency set.
     pub fn clear(&mut self) {
-        self.common_deps.clear();
+        self.0 = SelectorMap::new();
-        self.attr_deps.clear();
-        self.state_deps.clear();
     }
 
     /// Compute a restyle hint given an element and a snapshot, per the rules
@@ -631,64 +622,46 @@ impl DependencySet {
         let mut hint = RestyleHint::empty();
         let snapshot_el = ElementWrapper::new_with_snapshot(el.clone(), snapshot);
 
-        Self::compute_partial_hint(&self.common_deps, el, &snapshot_el,
+        // Compute whether the snapshot has any different id or class attributes
-                                   &state_changes, attrs_changed, &mut hint);
+        // from the element. If it does, we need to pass those to the lookup, so
-
+        // that we get all the possible applicable selectors from the rulehash.
-        if !state_changes.is_empty() {
+        let mut additional_id = None;
-            Self::compute_partial_hint(&self.state_deps, el, &snapshot_el,
+        let mut additional_classes = SmallVec::<[Atom; 8]>::new();
-                                       &state_changes, attrs_changed, &mut hint);
+        if snapshot.has_attrs() {
+            let id = snapshot.id_attr();
+            if id.is_some() && id != el.get_id() {
+                additional_id = id;
             }
 
-        if attrs_changed {
+            snapshot.each_class(|c| if !el.has_class(c) { additional_classes.push(c.clone()) });
-            Self::compute_partial_hint(&self.attr_deps, el, &snapshot_el,
-                                       &state_changes, attrs_changed, &mut hint);
         }
 
+        self.0.lookup_with_additional(*el, additional_id, &additional_classes, &mut |dep| {
+            if !dep.sensitivities.sensitive_to(attrs_changed, state_changes) || hint.contains(dep.hint) {
+                return true;
+            }
+
+            // We can ignore the selector flags, since they would have already been set during
+            // original matching for any element that might change its matching behavior here.
+            let matched_then =
+                matches_selector(&dep.selector, &snapshot_el, None,
+                                 &mut StyleRelations::empty(),
+                                 &mut |_, _| {});
+            let matches_now =
+                matches_selector(&dep.selector, el, None,
+                                 &mut StyleRelations::empty(),
+                                 &mut |_, _| {});
+            if matched_then != matches_now {
+                hint.insert(dep.hint);
+            }
+
+            !hint.is_all()
+        });
+
         debug!("Calculated restyle hint: {:?}. (Element={:?}, State={:?}, Snapshot={:?}, {} Deps)",
                hint, el, current_state, snapshot, self.len());
         trace!("Deps: {:?}", self);
 
         hint
     }
-
-    fn compute_partial_hint<E>(deps: &[Dependency],
-                               element: &E,
-                               snapshot: &ElementWrapper<E>,
-                               state_changes: &ElementState,
-                               attrs_changed: bool,
-                               hint: &mut RestyleHint)
-        where E: TElement,
-    {
-        if hint.is_all() {
-            return;
-        }
-        for dep in deps {
-            debug_assert!((!state_changes.is_empty() && !dep.sensitivities.states.is_empty()) ||
-                          (attrs_changed && dep.sensitivities.attrs),
-                          "Testing a known ineffective dependency?");
-            if (attrs_changed || state_changes.intersects(dep.sensitivities.states)) && !hint.contains(dep.hint) {
-                // We can ignore the selector flags, since they would have already been set during
-                // original matching for any element that might change its matching behavior here.
-                let matched_then =
-                    matches_selector(&dep.selector, snapshot, None,
-                                     &mut StyleRelations::empty(),
-                                     &mut |_, _| {});
-                let matches_now =
-                    matches_selector(&dep.selector, element, None,
-                                     &mut StyleRelations::empty(),
-                                     &mut |_, _| {});
-                if matched_then != matches_now {
-                    hint.insert(dep.hint);
-                }
-                if hint.is_all() {
-                    break;
-                }
-            }
-        }
-    }
-
-    /// Get the dependencies affected by state.
-    pub fn get_state_deps(&self) -> &[Dependency] {
-        &self.state_deps
-    }
 }

components/style/stylist.rs

@@ -117,7 +117,7 @@ pub struct Stylist {
     /// on state that is not otherwise visible to the cache, like attributes or
     /// tree-structural state like child index and pseudos).
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
-    selectors_for_cache_revalidation: Vec<SelectorInner<SelectorImpl>>,
+    selectors_for_cache_revalidation: SelectorMap<SelectorInner<SelectorImpl>>,
 
     /// The total number of selectors.
     num_selectors: usize,
@@ -177,7 +177,7 @@ impl Stylist {
             rules_source_order: 0,
             rule_tree: RuleTree::new(),
             dependencies: DependencySet::new(),
-            selectors_for_cache_revalidation: vec![],
+            selectors_for_cache_revalidation: SelectorMap::new(),
             num_selectors: 0,
             num_declarations: 0,
             num_rebuilds: 0,
@@ -260,7 +260,7 @@ impl Stylist {
         self.rules_source_order = 0;
         self.dependencies.clear();
         self.animations.clear();
-        self.selectors_for_cache_revalidation.clear();
+        self.selectors_for_cache_revalidation = SelectorMap::new();
         self.num_selectors = 0;
         self.num_declarations = 0;
 
@@ -376,7 +376,7 @@ impl Stylist {
     #[inline]
     fn note_for_revalidation(&mut self, selector: &Selector<SelectorImpl>) {
         if needs_revalidation(selector) {
-            self.selectors_for_cache_revalidation.push(selector.inner.clone());
+            self.selectors_for_cache_revalidation.insert(selector.inner.clone());
         }
     }
 
@@ -847,17 +847,20 @@ impl Stylist {
         use selectors::matching::StyleRelations;
         use selectors::matching::matches_selector;
 
-        let len = self.selectors_for_cache_revalidation.len();
+        // Note that, by the time we're revalidating, we're guaranteed that the
-        let mut results = BitVec::from_elem(len, false);
+        // candidate and the entry have the same id, classes, and local name.
-
+        // This means we're guaranteed to get the same rulehash buckets for all
-        for (i, ref selector) in self.selectors_for_cache_revalidation
+        // the lookups, which means that the bitvecs are comparable. We verify
-                                     .iter().enumerate() {
+        // this in the caller by asserting that the bitvecs are same-length.
-            results.set(i, matches_selector(selector,
+        let mut results = BitVec::new();
+        self.selectors_for_cache_revalidation.lookup(*element, &mut |selector| {
+            results.push(matches_selector(selector,
                                           element,
                                           Some(bloom),
                                           &mut StyleRelations::empty(),
                                           flags_setter));
-        }
+            true
+        });
 
         results
     }
@@ -1005,11 +1008,11 @@ pub fn needs_revalidation(selector: &Selector<SelectorImpl>) -> bool {
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 struct PerPseudoElementSelectorMap {
     /// Rules from user agent stylesheets
-    user_agent: SelectorMap,
+    user_agent: SelectorMap<Rule>,
     /// Rules from author stylesheets
-    author: SelectorMap,
+    author: SelectorMap<Rule>,
     /// Rules from user stylesheets
-    user: SelectorMap,
+    user: SelectorMap<Rule>,
 }
 
 impl PerPseudoElementSelectorMap {
@@ -1023,7 +1026,7 @@ impl PerPseudoElementSelectorMap {
     }
 
     #[inline]
-    fn borrow_for_origin(&mut self, origin: &Origin) -> &mut SelectorMap {
+    fn borrow_for_origin(&mut self, origin: &Origin) -> &mut SelectorMap<Rule> {
         match *origin {
             Origin::UserAgent => &mut self.user_agent,
             Origin::Author => &mut self.author,
@@ -1032,39 +1035,41 @@ impl PerPseudoElementSelectorMap {
     }
 }
 
-/// Map element data to Rules whose last simple selector starts with them.
+/// Map element data to selector-providing objects for which the last simple
+/// selector starts with them.
 ///
 /// e.g.,
-/// "p > img" would go into the set of Rules corresponding to the
+/// "p > img" would go into the set of selectors corresponding to the
 /// element "img"
-/// "a .foo .bar.baz" would go into the set of Rules corresponding to
+/// "a .foo .bar.baz" would go into the set of selectors corresponding to
 /// the class "bar"
 ///
-/// Because we match Rules right-to-left (i.e., moving up the tree
+/// Because we match selectors right-to-left (i.e., moving up the tree
 /// from an element), we need to compare the last simple selector in the
-/// Rule with the element.
+/// selector with the element.
 ///
 /// So, if an element has ID "id1" and classes "foo" and "bar", then all
 /// the rules it matches will have their last simple selector starting
 /// either with "#id1" or with ".foo" or with ".bar".
 ///
 /// Hence, the union of the rules keyed on each of element's classes, ID,
-/// element name, etc. will contain the Rules that actually match that
+/// element name, etc. will contain the Selectors that actually match that
 /// element.
 ///
 /// TODO: Tune the initial capacity of the HashMap
+#[derive(Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-pub struct SelectorMap {
+pub struct SelectorMap<T: Clone + Borrow<SelectorInner<SelectorImpl>>> {
     /// A hash from an ID to rules which contain that ID selector.
-    pub id_hash: FnvHashMap<Atom, Vec<Rule>>,
+    pub id_hash: FnvHashMap<Atom, Vec<T>>,
     /// A hash from a class name to rules which contain that class selector.
-    pub class_hash: FnvHashMap<Atom, Vec<Rule>>,
+    pub class_hash: FnvHashMap<Atom, Vec<T>>,
     /// A hash from local name to rules which contain that local name selector.
-    pub local_name_hash: FnvHashMap<LocalName, Vec<Rule>>,
+    pub local_name_hash: FnvHashMap<LocalName, Vec<T>>,
     /// Rules that don't have ID, class, or element selectors.
-    pub other_rules: Vec<Rule>,
+    pub other: Vec<T>,
-    /// Whether this hash is empty.
+    /// The number of entries in this map.
-    pub empty: bool,
+    pub count: usize,
 }
 
 #[inline]
@@ -1072,18 +1077,30 @@ fn sort_by_key<T, F: Fn(&T) -> K, K: Ord>(v: &mut [T], f: F) {
     sort_by(v, |a, b| f(a).cmp(&f(b)))
 }
 
-impl SelectorMap {
+impl<T> SelectorMap<T> where T: Clone + Borrow<SelectorInner<SelectorImpl>> {
     /// Trivially constructs an empty `SelectorMap`.
     pub fn new() -> Self {
         SelectorMap {
             id_hash: HashMap::default(),
             class_hash: HashMap::default(),
             local_name_hash: HashMap::default(),
-            other_rules: Vec::new(),
+            other: Vec::new(),
-            empty: true,
+            count: 0,
         }
     }
 
+    /// Returns whether there are any entries in the map.
+    pub fn is_empty(&self) -> bool {
+        self.count == 0
+    }
+
+    /// Returns the number of entries.
+    pub fn len(&self) -> usize {
+        self.count
+    }
+}
+
+impl SelectorMap<Rule> {
     /// Append to `rule_list` all Rules in `self` that match element.
     ///
     /// Extract matching rules as per element's ID, classes, tag name, etc..
@@ -1099,7 +1116,7 @@ impl SelectorMap {
               V: VecLike<ApplicableDeclarationBlock>,
               F: FnMut(&E, ElementSelectorFlags),
     {
-        if self.empty {
+        if self.is_empty() {
             return
         }
 
@@ -1138,7 +1155,7 @@ impl SelectorMap {
 
         SelectorMap::get_matching_rules(element,
                                         parent_bf,
-                                        &self.other_rules,
+                                        &self.other,
                                         matching_rules_list,
                                         relations,
                                         flags_setter,
@@ -1158,7 +1175,7 @@ impl SelectorMap {
                                -> Vec<ApplicableDeclarationBlock> {
         debug_assert!(!cascade_level.is_important());
         debug_assert!(important_cascade_level.is_important());
-        if self.empty {
+        if self.is_empty() {
             return vec![];
         }
 
@@ -1167,7 +1184,7 @@ impl SelectorMap {
         // We need to insert important rules _after_ normal rules for this to be
         // correct, and also to not trigger rule tree assertions.
         let mut important = vec![];
-        for rule in self.other_rules.iter() {
+        for rule in self.other.iter() {
             if rule.selector.complex.iter_raw().next().is_none() {
                 let style_rule = rule.style_rule.read_with(guard);
                 let block = style_rule.block.read_with(guard);
@@ -1245,23 +1262,24 @@ impl SelectorMap {
             }
         }
     }
+}
 
-    /// Insert rule into the correct hash.
+impl<T> SelectorMap<T> where T: Clone + Borrow<SelectorInner<SelectorImpl>> {
-    /// Order in which to try: id_hash, class_hash, local_name_hash, other_rules.
+    /// Inserts into the correct hash, trying id, class, and localname.
-    pub fn insert(&mut self, rule: Rule) {
+    pub fn insert(&mut self, entry: T) {
-        self.empty = false;
+        self.count += 1;
 
-        if let Some(id_name) = SelectorMap::get_id_name(&rule) {
+        if let Some(id_name) = get_id_name(entry.borrow()) {
-            find_push(&mut self.id_hash, id_name, rule);
+            find_push(&mut self.id_hash, id_name, entry);
             return;
         }
 
-        if let Some(class_name) = SelectorMap::get_class_name(&rule) {
+        if let Some(class_name) = get_class_name(entry.borrow()) {
-            find_push(&mut self.class_hash, class_name, rule);
+            find_push(&mut self.class_hash, class_name, entry);
             return;
         }
 
-        if let Some(LocalNameSelector { name, lower_name }) = SelectorMap::get_local_name(&rule) {
+        if let Some(LocalNameSelector { name, lower_name }) = get_local_name(entry.borrow()) {
             // If the local name in the selector isn't lowercase, insert it into
             // the rule hash twice. This means that, during lookup, we can always
             // find the rules based on the local name of the element, regardless
@@ -1274,19 +1292,129 @@ impl SelectorMap {
             // lookup may produce superfluous selectors, but the subsequent
             // selector matching work will filter them out.
             if name != lower_name {
-                find_push(&mut self.local_name_hash, lower_name, rule.clone());
+                find_push(&mut self.local_name_hash, lower_name, entry.clone());
             }
-            find_push(&mut self.local_name_hash, name, rule);
+            find_push(&mut self.local_name_hash, name, entry);
 
             return;
         }
 
-        self.other_rules.push(rule);
+        self.other.push(entry);
     }
 
-    /// Retrieve the first ID name in Rule, or None otherwise.
+    /// Looks up entries by id, class, local name, and other (in order).
-    pub fn get_id_name(rule: &Rule) -> Option<Atom> {
+    ///
-        for ss in rule.selector.complex.iter() {
+    /// Each entry is passed to the callback, which returns true to continue
+    /// iterating entries, or false to terminate the lookup.
+    ///
+    /// Returns false if the callback ever returns false.
+    ///
+    /// FIXME(bholley) This overlaps with SelectorMap<Rule>::get_all_matching_rules,
+    /// but that function is extremely hot and I'd rather not rearrange it.
+    #[inline]
+    pub fn lookup<E, F>(&self, element: E, f: &mut F) -> bool
+        where E: TElement,
+              F: FnMut(&T) -> bool
+    {
+        // Id.
+        if let Some(id) = element.get_id() {
+            if let Some(v) = self.id_hash.get(&id) {
+                for entry in v.iter() {
+                    if !f(&entry) {
+                        return false;
+                    }
+                }
+            }
+        }
+
+        // Class.
+        let mut done = false;
+        element.each_class(|class| {
+            if !done {
+                if let Some(v) = self.class_hash.get(class) {
+                    for entry in v.iter() {
+                        if !f(&entry) {
+                            done = true;
+                            return;
+                        }
+                    }
+                }
+            }
+        });
+        if done {
+            return false;
+        }
+
+        // Local name.
+        if let Some(v) = self.local_name_hash.get(element.get_local_name()) {
+            for entry in v.iter() {
+                if !f(&entry) {
+                    return false;
+                }
+            }
+        }
+
+        // Other.
+        for entry in self.other.iter() {
+            if !f(&entry) {
+                return false;
+            }
+        }
+
+        true
+    }
+
+    /// Performs a normal lookup, and also looks up entries for the passed-in
+    /// id and classes.
+    ///
+    /// Each entry is passed to the callback, which returns true to continue
+    /// iterating entries, or false to terminate the lookup.
+    ///
+    /// Returns false if the callback ever returns false.
+    #[inline]
+    pub fn lookup_with_additional<E, F>(&self,
+                                        element: E,
+                                        additional_id: Option<Atom>,
+                                        additional_classes: &[Atom],
+                                        f: &mut F)
+                                        -> bool
+        where E: TElement,
+              F: FnMut(&T) -> bool
+    {
+        // Do the normal lookup.
+        if !self.lookup(element, f) {
+            return false;
+        }
+
+        // Check the additional id.
+        if let Some(id) = additional_id {
+            if let Some(v) = self.id_hash.get(&id) {
+                for entry in v.iter() {
+                    if !f(&entry) {
+                        return false;
+                    }
+                }
+            }
+        }
+
+        // Check the additional classes.
+        for class in additional_classes {
+            if let Some(v) = self.class_hash.get(class) {
+                for entry in v.iter() {
+                    if !f(&entry) {
+                        return false;
+                    }
+                }
+            }
+        }
+
+        true
+    }
+}
+
+/// Retrieve the first ID name in the selector, or None otherwise.
+pub fn get_id_name(selector: &SelectorInner<SelectorImpl>) -> Option<Atom> {
+    for ss in selector.complex.iter() {
         // TODO(pradeep): Implement case-sensitivity based on the
         // document type and quirks mode.
         if let Component::ID(ref id) = *ss {
@@ -1295,11 +1423,11 @@ impl SelectorMap {
     }
 
     None
-    }
+}
 
-    /// Retrieve the FIRST class name in Rule, or None otherwise.
+/// Retrieve the FIRST class name in the selector, or None otherwise.
-    pub fn get_class_name(rule: &Rule) -> Option<Atom> {
+pub fn get_class_name(selector: &SelectorInner<SelectorImpl>) -> Option<Atom> {
-        for ss in rule.selector.complex.iter() {
+    for ss in selector.complex.iter() {
         // TODO(pradeep): Implement case-sensitivity based on the
         // document type and quirks mode.
         if let Component::Class(ref class) = *ss {
@@ -1308,11 +1436,12 @@ impl SelectorMap {
     }
 
     None
-    }
+}
 
-    /// Retrieve the name if it is a type selector, or None otherwise.
+/// Retrieve the name if it is a type selector, or None otherwise.
-    pub fn get_local_name(rule: &Rule) -> Option<LocalNameSelector<SelectorImpl>> {
+pub fn get_local_name(selector: &SelectorInner<SelectorImpl>)
-        for ss in rule.selector.complex.iter() {
+                      -> Option<LocalNameSelector<SelectorImpl>> {
+    for ss in selector.complex.iter() {
         if let Component::LocalName(ref n) = *ss {
             return Some(LocalNameSelector {
                 name: n.name.clone(),
@@ -1322,7 +1451,6 @@ impl SelectorMap {
     }
 
     None
-    }
 }
 
 /// A rule, that wraps a style rule, but represents a single selector of the
@@ -1347,6 +1475,12 @@ pub struct Rule {
     specificity_and_bits: u32,
 }
 
+impl Borrow<SelectorInner<SelectorImpl>> for Rule {
+    fn borrow(&self) -> &SelectorInner<SelectorImpl> {
+        &self.selector
+    }
+}
+
 /// Masks for specificity_and_bits.
 const SPECIFICITY_MASK: u32 = 0x3fffffff;
 const ANY_IMPORTANT_DECLARATIONS_BIT: u32 = 1 << 30;
@@ -1441,7 +1575,8 @@ impl ApplicableDeclarationBlock {
 }
 
 #[inline]
-fn find_push<Str: Eq + Hash>(map: &mut FnvHashMap<Str, Vec<Rule>>, key: Str,
+fn find_push<Str: Eq + Hash, V>(map: &mut FnvHashMap<Str, Vec<V>>,
-                             value: Rule) {
+                                key: Str,
+                                value: V) {
     map.entry(key).or_insert_with(Vec::new).push(value)
 }

tests/unit/style/restyle_hints.rs

@@ -24,8 +24,7 @@ fn smoke_restyle_hints() {
     let selector = (selectors.0).first().unwrap();
     dependencies.note_selector(selector);
     assert_eq!(dependencies.len(), 1);
-    let state_deps = dependencies.get_state_deps();
+    let dep = &dependencies.0.other[0];
-    assert_eq!(state_deps.len(), 1);
+    assert!(!dep.sensitivities.states.is_empty());
-    assert!(!state_deps[0].sensitivities.states.is_empty());
+    assert!(dep.hint.contains(RESTYLE_LATER_SIBLINGS));
-    assert!(state_deps[0].hint.contains(RESTYLE_LATER_SIBLINGS));
 }

tests/unit/style/stylist.rs

@@ -13,6 +13,7 @@ use style::selector_parser::{SelectorImpl, SelectorParser};
 use style::shared_lock::SharedRwLock;
 use style::stylearc::Arc;
 use style::stylesheets::StyleRule;
+use style::stylist;
 use style::stylist::{Rule, SelectorMap};
 use style::stylist::needs_revalidation;
 use style::thread_state;
@@ -45,8 +46,8 @@ fn get_mock_rules(css_selectors: &[&str]) -> (Vec<Vec<Rule>>, SharedRwLock) {
     }).collect(), shared_lock)
 }
 
-fn get_mock_map(selectors: &[&str]) -> (SelectorMap, SharedRwLock) {
+fn get_mock_map(selectors: &[&str]) -> (SelectorMap<Rule>, SharedRwLock) {
-    let mut map = SelectorMap::new();
+    let mut map = SelectorMap::<Rule>::new();
     let (selector_rules, shared_lock) = get_mock_rules(selectors);
 
     for rules in selector_rules.into_iter() {
@@ -170,22 +171,22 @@ fn test_rule_ordering_same_specificity() {
 #[test]
 fn test_get_id_name() {
     let (rules_list, _) = get_mock_rules(&[".intro", "#top"]);
-    assert_eq!(SelectorMap::get_id_name(&rules_list[0][0]), None);
+    assert_eq!(stylist::get_id_name(&rules_list[0][0].selector), None);
-    assert_eq!(SelectorMap::get_id_name(&rules_list[1][0]), Some(Atom::from("top")));
+    assert_eq!(stylist::get_id_name(&rules_list[1][0].selector), Some(Atom::from("top")));
 }
 
 #[test]
 fn test_get_class_name() {
     let (rules_list, _) = get_mock_rules(&[".intro.foo", "#top"]);
-    assert_eq!(SelectorMap::get_class_name(&rules_list[0][0]), Some(Atom::from("foo")));
+    assert_eq!(stylist::get_class_name(&rules_list[0][0].selector), Some(Atom::from("foo")));
-    assert_eq!(SelectorMap::get_class_name(&rules_list[1][0]), None);
+    assert_eq!(stylist::get_class_name(&rules_list[1][0].selector), None);
 }
 
 #[test]
 fn test_get_local_name() {
     let (rules_list, _) = get_mock_rules(&["img.foo", "#top", "IMG", "ImG"]);
     let check = |i: usize, names: Option<(&str, &str)>| {
-        assert!(SelectorMap::get_local_name(&rules_list[i][0])
+        assert!(stylist::get_local_name(&rules_list[i][0].selector)
                 == names.map(|(name, lower_name)| LocalNameSelector {
                         name: LocalName::from(name),
                         lower_name: LocalName::from(lower_name) }))