Auto merge of #18087 - emilio:stylist-clear-cleanup, r=heycam

style: Cleanup a bit Stylist and SelectorMap.

Bug: 1390255
Reviewed-by: heycam

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

components/style/gecko/data.rs

@@ -17,7 +17,7 @@ use properties::ComputedValues;
 use servo_arc::Arc;
 use shared_lock::{Locked, StylesheetGuards, SharedRwLockReadGuard};
 use stylesheet_set::StylesheetSet;
-use stylesheets::{Origin, PerOrigin, StylesheetContents, StylesheetInDocument};
+use stylesheets::{PerOrigin, StylesheetContents, StylesheetInDocument};
 use stylist::{ExtraStyleData, Stylist};
 
 /// Little wrapper to a Gecko style sheet.
@@ -164,14 +164,14 @@ impl PerDocumentStyleDataImpl {
 
         let author_style_disabled = self.stylesheets.author_style_disabled();
 
-        let iter = self.stylesheets.flush(document_element);
+        let (iter, dirty_origins) = self.stylesheets.flush(document_element);
         self.stylist.rebuild(
             iter,
             &StylesheetGuards::same(guard),
             /* ua_sheets = */ None,
-            /* stylesheets_changed = */ true,
             author_style_disabled,
             &mut self.extra_style_data,
+            dirty_origins,
         );
     }
 
@@ -187,17 +187,6 @@ impl PerDocumentStyleDataImpl {
         self.stylist.device().default_computed_values_arc()
     }
 
-    /// Clear the stylist.  This will be a no-op if the stylist is
-    /// already cleared; the stylist handles that.
-    pub fn clear_stylist(&mut self) {
-        self.stylist.clear();
-    }
-
-    /// Clear the stylist's data for the specified origin.
-    pub fn clear_stylist_origin(&mut self, origin: &Origin) {
-        self.stylist.clear_origin(origin);
-    }
-
     /// Returns whether visited links are enabled.
     fn visited_links_enabled(&self) -> bool {
         unsafe { bindings::Gecko_AreVisitedLinksEnabled() }

components/style/gecko/generated/bindings.rs

@@ -1987,9 +1987,6 @@ extern "C" {
     pub fn Servo_StyleSet_Init(pres_context: RawGeckoPresContextOwned)
      -> *mut RawServoStyleSet;
 }
-extern "C" {
-    pub fn Servo_StyleSet_Clear(set: RawServoStyleSetBorrowed);
-}
 extern "C" {
     pub fn Servo_StyleSet_RebuildCachedData(set: RawServoStyleSetBorrowed);
 }

components/style/invalidation/element/invalidation_map.rs

@@ -195,8 +195,8 @@ impl InvalidationMap {
     pub fn clear(&mut self) {
         self.class_to_selector.clear();
         self.id_to_selector.clear();
-        self.state_affecting_selectors = SelectorMap::new();
+        self.state_affecting_selectors.clear();
-        self.other_attribute_affecting_selectors = SelectorMap::new();
+        self.other_attribute_affecting_selectors.clear();
         self.has_id_attribute_selectors = false;
         self.has_class_attribute_selectors = false;
     }

components/style/selector_map.rs

@@ -15,7 +15,6 @@ use rule_tree::CascadeLevel;
 use selector_parser::SelectorImpl;
 use selectors::matching::{matches_selector, MatchingContext, ElementSelectorFlags};
 use selectors::parser::{Component, Combinator, SelectorIter};
-use selectors::parser::LocalName as LocalNameSelector;
 use smallvec::{SmallVec, VecLike};
 use std::collections::{HashMap, HashSet};
 use std::collections::hash_map;
@@ -102,7 +101,7 @@ pub struct SelectorMap<T> {
     /// A hash from local name to rules which contain that local name selector.
     pub local_name_hash: PrecomputedHashMap<LocalName, SmallVec<[T; 1]>>,
     /// Rules that don't have ID, class, or element selectors.
-    pub other: Vec<T>,
+    pub other: SmallVec<[T; 1]>,
     /// The number of entries in this map.
     pub count: usize,
 }
@@ -119,11 +118,20 @@ impl<T> SelectorMap<T> {
             id_hash: MaybeCaseInsensitiveHashMap::new(),
             class_hash: MaybeCaseInsensitiveHashMap::new(),
             local_name_hash: HashMap::default(),
-            other: Vec::new(),
+            other: SmallVec::new(),
             count: 0,
         }
     }
 
+    /// Clears the hashmap retaining storage.
+    pub fn clear(&mut self) {
+        self.id_hash.clear();
+        self.class_hash.clear();
+        self.local_name_hash.clear();
+        self.other.clear();
+        self.count = 0;
+    }
+
     /// Returns whether there are any entries in the map.
     pub fn is_empty(&self) -> bool {
         self.count == 0
@@ -231,37 +239,42 @@ impl<T: SelectorMapEntry> SelectorMap<T> {
     pub fn insert(&mut self, entry: T, quirks_mode: QuirksMode) {
         self.count += 1;
 
-        if let Some(id_name) = get_id_name(entry.selector()) {
+        let vector = match find_bucket(entry.selector()) {
-            self.id_hash.entry(id_name, quirks_mode).or_insert_with(SmallVec::new).push(entry);
+            Bucket::ID(id) => {
-            return;
+                self.id_hash
-        }
+                    .entry(id.clone(), quirks_mode)
-
+                    .or_insert_with(SmallVec::new)
-        if let Some(class_name) = get_class_name(entry.selector()) {
-            self.class_hash.entry(class_name, quirks_mode).or_insert_with(SmallVec::new).push(entry);
-            return;
-        }
-
-        if let Some(LocalNameSelector { name, lower_name }) = get_local_name(entry.selector()) {
-            // 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
-            // of whether it's an html element in an html document (in which case
-            // we match against lower_name) or not (in which case we match against
-            // name).
-            //
-            // In the case of a non-html-element-in-html-document with a
-            // lowercase localname and a non-lowercase selector, the rulehash
-            // 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, entry.clone());
             }
-            find_push(&mut self.local_name_hash, name, entry);
+            Bucket::Class(class) => {
+                self.class_hash
+                    .entry(class.clone(), quirks_mode)
+                    .or_insert_with(SmallVec::new)
+            }
+            Bucket::LocalName { name, lower_name } => {
+                // 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 of whether it's an html element in an
+                // html document (in which case we match against lower_name) or
+                // not (in which case we match against name).
+                //
+                // In the case of a non-html-element-in-html-document with a
+                // lowercase localname and a non-lowercase selector, the
+                // rulehash 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.clone(), entry.clone());
+                }
+                self.local_name_hash
+                    .entry(name.clone())
+                    .or_insert_with(SmallVec::new)
+            }
+            Bucket::Universal => {
+                &mut self.other
+            }
+        };
 
-            return;
+        vector.push(entry);
-        }
-
-        self.other.push(entry);
     }
 
     /// Looks up entries by id, class, local name, and other (in order).
@@ -375,74 +388,65 @@ impl<T: SelectorMapEntry> SelectorMap<T> {
     }
 }
 
-/// Searches a compound selector from left to right. If the compound selector
+enum Bucket<'a> {
-/// is a pseudo-element, it's ignored.
+    ID(&'a Atom),
-///
+    Class(&'a Atom),
-/// The first non-None value returned from |f| is returned.
+    LocalName { name: &'a LocalName, lower_name: &'a LocalName, },
-#[inline(always)]
+    Universal,
-fn find_from_left<F, R>(
+}
-    mut iter: SelectorIter<SelectorImpl>,
-    mut f: F
-) -> Option<R>
-where
-    F: FnMut(&Component<SelectorImpl>) -> Option<R>,
-{
-    for ss in &mut iter {
-        if let Some(r) = f(ss) {
-            return Some(r)
-        }
-    }
 
-    // Effectively, pseudo-elements are ignored, given only state pseudo-classes
+fn specific_bucket_for<'a>(
-    // may appear before them.
+    component: &'a Component<SelectorImpl>
-    if iter.next_sequence() == Some(Combinator::PseudoElement) {
+) -> Bucket<'a> {
-        for ss in &mut iter {
+    match *component {
-            if let Some(r) = f(ss) {
+        Component::ID(ref id) => Bucket::ID(id),
-                return Some(r)
+        Component::Class(ref class) => Bucket::Class(class),
+        Component::LocalName(ref selector) => {
+            Bucket::LocalName {
+                name: &selector.name,
+                lower_name: &selector.lower_name,
             }
         }
+        _ => Bucket::Universal
+    }
+}
+
+/// Searches a compound selector from left to right, and returns the appropriate
+/// bucket for it.
+#[inline(always)]
+fn find_bucket<'a>(mut iter: SelectorIter<'a, SelectorImpl>) -> Bucket<'a> {
+    let mut current_bucket = Bucket::Universal;
+
+    loop {
+        // We basically want to find the most specific bucket,
+        // where:
+        //
+        //   id > class > local name > universal.
+        //
+        for ss in &mut iter {
+            let new_bucket = specific_bucket_for(ss);
+            match new_bucket {
+                Bucket::ID(..) => return new_bucket,
+                Bucket::Class(..) => {
+                    current_bucket = new_bucket;
+                }
+                Bucket::LocalName { .. } => {
+                    if matches!(current_bucket, Bucket::Universal) {
+                        current_bucket = new_bucket;
+                    }
+                }
+                Bucket::Universal => {},
+            }
+        }
+
+        // Effectively, pseudo-elements are ignored, given only state
+        // pseudo-classes may appear before them.
+        if iter.next_sequence() != Some(Combinator::PseudoElement) {
+            break;
+        }
     }
 
-    None
+    return current_bucket
-}
-
-/// Retrieve the first ID name in the selector, or None otherwise.
-#[inline(always)]
-pub fn get_id_name(iter: SelectorIter<SelectorImpl>)
-                   -> Option<Atom> {
-    find_from_left(iter, |ss| {
-        if let Component::ID(ref id) = *ss {
-            return Some(id.clone());
-        }
-        None
-    })
-}
-
-/// Retrieve the FIRST class name in the selector, or None otherwise.
-#[inline(always)]
-pub fn get_class_name(iter: SelectorIter<SelectorImpl>)
-                      -> Option<Atom> {
-    find_from_left(iter, |ss| {
-        if let Component::Class(ref class) = *ss {
-            return Some(class.clone());
-        }
-        None
-    })
-}
-
-/// Retrieve the name if it is a type selector, or None otherwise.
-#[inline(always)]
-pub fn get_local_name(iter: SelectorIter<SelectorImpl>)
-                      -> Option<LocalNameSelector<SelectorImpl>> {
-    find_from_left(iter, |ss| {
-        if let Component::LocalName(ref n) = *ss {
-            return Some(LocalNameSelector {
-                name: n.name.clone(),
-                lower_name: n.lower_name.clone(),
-            })
-        }
-        None
-    })
 }
 
 #[inline]

components/style/stylesheet_set.rs

@@ -8,7 +8,7 @@ use dom::TElement;
 use invalidation::stylesheets::StylesheetInvalidationSet;
 use shared_lock::SharedRwLockReadGuard;
 use std::slice;
-use stylesheets::{Origin, PerOrigin, StylesheetInDocument};
+use stylesheets::{OriginSet, PerOrigin, StylesheetInDocument};
 use stylist::Stylist;
 
 /// Entry for a StylesheetSet. We don't bother creating a constructor, because
@@ -169,21 +169,23 @@ where
     pub fn flush<E>(
         &mut self,
         document_element: Option<E>,
-    ) -> StylesheetIterator<S>
+    ) -> (StylesheetIterator<S>, OriginSet)
     where
         E: TElement,
     {
         debug!("StylesheetSet::flush");
         debug_assert!(self.has_changed());
 
-        for (data, _) in self.invalidation_data.iter_mut_origins() {
+        let mut origins = OriginSet::empty();
+        for (data, origin) in self.invalidation_data.iter_mut_origins() {
             if data.dirty {
                 data.invalidations.flush(document_element);
                 data.dirty = false;
+                origins |= origin;
             }
         }
 
-        self.iter()
+        (self.iter(), origins)
     }
 
     /// Returns an iterator over the current list of stylesheets.
@@ -191,24 +193,15 @@ where
         StylesheetIterator(self.entries.iter())
     }
 
-    /// Mark the stylesheets as dirty, because something external may have
+    /// Mark the stylesheets for the specified origin as dirty, because
-    /// invalidated it.
+    /// something external may have invalidated it.
-    ///
+    pub fn force_dirty(&mut self, origins: OriginSet) {
-    /// FIXME(emilio): Make this more granular.
+        for origin in origins.iter() {
-    pub fn force_dirty(&mut self) {
+            let data = self.invalidation_data.borrow_mut_for_origin(&origin);
-        for (data, _) in self.invalidation_data.iter_mut_origins() {
             data.invalidations.invalidate_fully();
             data.dirty = true;
         }
     }
-
-    /// Mark the stylesheets for the specified origin as dirty, because
-    /// something external may have invalidated it.
-    pub fn force_dirty_origin(&mut self, origin: &Origin) {
-        let data = self.invalidation_data.borrow_mut_for_origin(origin);
-        data.invalidations.invalidate_fully();
-        data.dirty = true;
-    }
 }
 
 struct InvalidationData {

components/style/stylesheets/mod.rs

@@ -44,7 +44,7 @@ pub use self::memory::{MallocSizeOf, MallocSizeOfFn, MallocSizeOfWithGuard};
 #[cfg(feature = "gecko")]
 pub use self::memory::{MallocSizeOfWithRepeats, SizeOfState};
 pub use self::namespace_rule::NamespaceRule;
-pub use self::origin::{Origin, OriginSet, PerOrigin, PerOriginClear};
+pub use self::origin::{Origin, OriginSet, PerOrigin};
 pub use self::page_rule::PageRule;
 pub use self::rule_parser::{State, TopLevelRuleParser};
 pub use self::rule_list::{CssRules, CssRulesHelpers};

components/style/stylesheets/origin.rs

@@ -168,20 +168,6 @@ impl<T> PerOrigin<T> {
     }
 }
 
-/// An object that can be cleared.
-pub trait PerOriginClear {
-    /// Clears the object.
-    fn clear(&mut self);
-}
-
-impl<T> PerOriginClear for PerOrigin<T> where T: PerOriginClear {
-    fn clear(&mut self) {
-        self.user_agent.clear();
-        self.user.clear();
-        self.author.clear();
-    }
-}
-
 /// Iterator over `PerOrigin<T>`, from highest level (author) to lowest
 /// (user agent).
 ///

components/style/stylist.rs

@@ -40,7 +40,7 @@ use style_traits::viewport::ViewportConstraints;
 #[cfg(feature = "gecko")]
 use stylesheets::{CounterStyleRule, FontFaceRule};
 use stylesheets::{CssRule, StyleRule};
-use stylesheets::{StylesheetInDocument, Origin, OriginSet, PerOrigin, PerOriginClear};
+use stylesheets::{StylesheetInDocument, Origin, OriginSet, PerOrigin};
 use stylesheets::UserAgentStylesheets;
 use stylesheets::keyframes_rule::KeyframesAnimation;
 use stylesheets::viewport_rule::{self, MaybeNew, ViewportRule};
@@ -182,52 +182,6 @@ impl Stylist {
         }
     }
 
-    /// Clear the stylist's state, effectively resetting it to more or less
-    /// the state Stylist::new creates.
-    ///
-    /// We preserve the state of the following members:
-    ///   device: Someone might have set this on us.
-    ///   quirks_mode: Again, someone might have set this on us.
-    ///   num_rebuilds: clear() followed by rebuild() should just increment this
-    ///   rule_tree: So we can re-use rule nodes across rebuilds.
-    ///
-    /// We don't just use struct update syntax with Stylist::new(self.device)
-    /// beause for some of our members we can clear them instead of creating new
-    /// objects.  This does cause unfortunate code duplication with
-    /// Stylist::new.
-    pub fn clear(&mut self) {
-        self.cascade_data.clear();
-        self.precomputed_pseudo_element_decls.clear();
-        self.viewport_constraints = None;
-
-        // XXX(heycam) Why do this, if we are preserving the Device?
-        self.is_device_dirty = true;
-    }
-
-    /// Clear the stylist's state for the specified origin.
-    pub fn clear_origin(&mut self, origin: &Origin) {
-        self.cascade_data.borrow_mut_for_origin(origin).clear();
-
-        if *origin == Origin::UserAgent {
-            // We only collect these declarations from UA sheets.
-            self.precomputed_pseudo_element_decls.clear();
-        }
-
-        // The stored `ViewportConstraints` contains data from rules across
-        // all origins.
-        self.viewport_constraints = None;
-
-        // XXX(heycam) Why do this, if we are preserving the Device?
-        self.is_device_dirty = true;
-    }
-
-    /// Returns whether any origin's `CascadeData` has been cleared.
-    fn any_origin_cleared(&self) -> bool {
-        self.cascade_data
-            .iter_origins()
-            .any(|(d, _)| d.is_cleared)
-    }
-
     /// Rebuild the stylist for the given document stylesheets, and optionally
     /// with a set of user agent stylesheets.
     ///
@@ -239,40 +193,17 @@ impl Stylist {
         doc_stylesheets: I,
         guards: &StylesheetGuards,
         ua_stylesheets: Option<&UserAgentStylesheets>,
-        stylesheets_changed: bool,
         author_style_disabled: bool,
-        extra_data: &mut PerOrigin<ExtraStyleData>
+        extra_data: &mut PerOrigin<ExtraStyleData>,
+        mut origins_to_rebuild: OriginSet,
     ) -> bool
     where
         I: Iterator<Item = &'a S> + Clone,
         S: StylesheetInDocument + ToMediaListKey + 'static,
     {
-        debug_assert!(!self.any_origin_cleared() || self.is_device_dirty);
+        debug_assert!(!origins_to_rebuild.is_empty() || self.is_device_dirty);
-
+        if self.is_device_dirty {
-        // Determine the origins that actually need updating.
+            origins_to_rebuild = OriginSet::all();
-        //
-        // XXX(heycam): What is the relationship between `stylesheets_changed`
-        // and the `is_cleared` fields on each origin's `CascadeData`?  Can
-        // we avoid passing in `stylesheets_changed`?
-        let mut to_update: PerOrigin<bool> = Default::default();
-
-        // If we're provided with a list of UA and user style sheets, then
-        // we must update those cascade levels. (Servo does this, but Gecko
-        // just includes the UA and User sheets in `doc_stylesheets`.)
-        if ua_stylesheets.is_some() {
-            to_update.user_agent = true;
-            to_update.user = true;
-        }
-
-        for (data, origin) in self.cascade_data.iter_mut_origins() {
-            if data.is_cleared {
-                data.is_cleared = false;
-                *to_update.borrow_mut_for_origin(&origin) = true;
-            }
-        }
-
-        if !(self.is_device_dirty || stylesheets_changed) {
-            return false;
         }
 
         self.num_rebuilds += 1;
@@ -307,13 +238,13 @@ impl Stylist {
             }
         }
 
-        // XXX(heycam): We should probably just move the `extra_data` to be
+        for origin in origins_to_rebuild.iter() {
-        // stored on the `Stylist` instead of Gecko's `PerDocumentStyleData`.
+            extra_data.borrow_mut_for_origin(&origin).clear();
-        // That would let us clear it inside `clear()` and `clear_origin()`.
+            self.cascade_data.borrow_mut_for_origin(&origin).clear();
-        for (update, origin) in to_update.iter_origins() {
+        }
-            if *update {
+
-                extra_data.borrow_mut_for_origin(&origin).clear();
+        if origins_to_rebuild.contains(Origin::UserAgent.into()) {
-            }
+            self.precomputed_pseudo_element_decls.clear();
         }
 
         if let Some(ua_stylesheets) = ua_stylesheets {
@@ -337,11 +268,10 @@ impl Stylist {
         // Only add stylesheets for origins we are updating, and only add
         // Author level sheets if author style is not disabled.
         let sheets_to_add = doc_stylesheets.filter(|s| {
-            match s.contents(guards.author).origin {
+            let sheet_origin = s.contents(guards.author).origin;
-                Origin::UserAgent => to_update.user_agent,
+
-                Origin::Author => to_update.author && !author_style_disabled,
+            origins_to_rebuild.contains(sheet_origin.into()) &&
-                Origin::User => to_update.user,
+                (!matches!(sheet_origin, Origin::Author) || !author_style_disabled)
-            }
         });
 
         for stylesheet in sheets_to_add {
@@ -367,16 +297,19 @@ impl Stylist {
         I: Iterator<Item = &'a S> + Clone,
         S: StylesheetInDocument + ToMediaListKey + 'static,
     {
-        debug_assert!(!self.any_origin_cleared() || self.is_device_dirty);
-
         // We have to do a dirtiness check before clearing, because if
         // we're not actually dirty we need to no-op here.
         if !(self.is_device_dirty || stylesheets_changed) {
             return false;
         }
-        self.clear();
+        self.rebuild(
-        self.rebuild(doc_stylesheets, guards, ua_stylesheets, stylesheets_changed,
+            doc_stylesheets,
-                     author_style_disabled, extra_data)
+            guards,
+            ua_stylesheets,
+            author_style_disabled,
+            extra_data,
+            OriginSet::all(),
+        )
     }
 
     fn add_stylesheet<S>(
@@ -529,11 +462,7 @@ impl Stylist {
     pub fn might_have_attribute_dependency(&self,
                                            local_name: &LocalName)
                                            -> bool {
-        if self.any_origin_cleared() || self.is_device_dirty {
+        if *local_name == local_name!("style") {
-            // We can't tell what attributes are in our style rules until
-            // we rebuild.
-            true
-        } else if *local_name == local_name!("style") {
             self.cascade_data
                 .iter_origins()
                 .any(|(d, _)| d.style_attribute_dependency)
@@ -550,13 +479,7 @@ impl Stylist {
     /// Returns whether the given ElementState bit might be relied upon by a
     /// selector of some rule in the stylist.
     pub fn might_have_state_dependency(&self, state: ElementState) -> bool {
-        if self.any_origin_cleared() || self.is_device_dirty {
+        self.has_state_dependency(state)
-            // We can't tell what states our style rules rely on until
-            // we rebuild.
-            true
-        } else {
-            self.has_state_dependency(state)
-        }
     }
 
     /// Returns whether the given ElementState bit is relied upon by a selector
@@ -1440,14 +1363,17 @@ impl ExtraStyleData {
     }
 
     /// Add the given @counter-style rule.
-    fn add_counter_style(&mut self, guard: &SharedRwLockReadGuard,
+    fn add_counter_style(
-                         rule: &Arc<Locked<CounterStyleRule>>) {
+        &mut self,
+        guard: &SharedRwLockReadGuard,
+        rule: &Arc<Locked<CounterStyleRule>>,
+    ) {
         let name = rule.read_with(guard).mName.raw::<nsIAtom>().into();
         self.counter_styles.insert(name, rule.clone());
     }
 }
 
-impl PerOriginClear for ExtraStyleData {
+impl ExtraStyleData {
     fn clear(&mut self) {
         #[cfg(feature = "gecko")]
         {
@@ -1689,11 +1615,6 @@ struct CascadeData {
 
     /// The total number of declarations.
     num_declarations: usize,
-
-    /// If true, the `CascadeData` is in a cleared state (e.g. just-constructed,
-    /// or had `clear()` called on it with no following `rebuild()` on the
-    /// `Stylist`).
-    is_cleared: bool,
 }
 
 impl CascadeData {
@@ -1712,7 +1633,6 @@ impl CascadeData {
             rules_source_order: 0,
             num_selectors: 0,
             num_declarations: 0,
-            is_cleared: true,
         }
     }
 
@@ -1727,28 +1647,21 @@ impl CascadeData {
     fn has_rules_for_pseudo(&self, pseudo: &PseudoElement) -> bool {
         self.pseudos_map.get(pseudo).is_some()
     }
-}
 
-impl PerOriginClear for CascadeData {
     fn clear(&mut self) {
-        if self.is_cleared {
+        self.element_map.clear();
-            return;
+        self.pseudos_map.clear();
-        }
+        self.animations.clear();
-
-        self.element_map = SelectorMap::new();
-        self.pseudos_map = Default::default();
-        self.animations = Default::default();
         self.invalidation_map.clear();
         self.attribute_dependencies.clear();
         self.style_attribute_dependency = false;
         self.state_dependencies = ElementState::empty();
         self.mapped_ids.clear();
-        self.selectors_for_cache_revalidation = SelectorMap::new();
+        self.selectors_for_cache_revalidation.clear();
         self.effective_media_query_results.clear();
         self.rules_source_order = 0;
         self.num_selectors = 0;
         self.num_declarations = 0;
-        self.is_cleared = true;
     }
 }
 

ports/geckolib/glue.rs

@@ -117,7 +117,7 @@ use style::style_adjuster::StyleAdjuster;
 use style::stylesheets::{CssRule, CssRules, CssRuleType, CssRulesHelpers, DocumentRule};
 use style::stylesheets::{FontFeatureValuesRule, ImportRule, KeyframesRule, MallocSizeOfWithGuard};
 use style::stylesheets::{MediaRule, NamespaceRule, Origin, OriginSet, PageRule, SizeOfState, StyleRule};
-use style::stylesheets::{StylesheetContents, StylesheetInDocument, SupportsRule};
+use style::stylesheets::{StylesheetContents, SupportsRule};
 use style::stylesheets::StylesheetLoader as StyleStylesheetLoader;
 use style::stylesheets::keyframes_rule::{Keyframe, KeyframeSelector, KeyframesStepValue};
 use style::stylesheets::supports_rule::parse_condition_or_declaration;
@@ -891,9 +891,7 @@ pub extern "C" fn Servo_StyleSet_AppendStyleSheet(
     let mut data = &mut *data;
     let guard = global_style_data.shared_lock.read();
     let sheet = unsafe { GeckoStyleSheet::new(sheet) };
-    let origin = sheet.contents(&guard).origin;
     data.stylesheets.append_stylesheet(&data.stylist, sheet, &guard);
-    data.clear_stylist_origin(&origin);
 }
 
 #[no_mangle]
@@ -942,9 +940,7 @@ pub extern "C" fn Servo_StyleSet_PrependStyleSheet(
     let mut data = &mut *data;
     let guard = global_style_data.shared_lock.read();
     let sheet = unsafe { GeckoStyleSheet::new(sheet) };
-    let origin = sheet.contents(&guard).origin;
     data.stylesheets.prepend_stylesheet(&data.stylist, sheet, &guard);
-    data.clear_stylist_origin(&origin);
 }
 
 #[no_mangle]
@@ -958,14 +954,12 @@ pub extern "C" fn Servo_StyleSet_InsertStyleSheetBefore(
     let mut data = &mut *data;
     let guard = global_style_data.shared_lock.read();
     let sheet = unsafe { GeckoStyleSheet::new(sheet) };
-    let origin = sheet.contents(&guard).origin;
     data.stylesheets.insert_stylesheet_before(
         &data.stylist,
         sheet,
         unsafe { GeckoStyleSheet::new(before_sheet) },
         &guard,
     );
-    data.clear_stylist_origin(&origin);
 }
 
 #[no_mangle]
@@ -978,9 +972,7 @@ pub extern "C" fn Servo_StyleSet_RemoveStyleSheet(
     let mut data = &mut *data;
     let guard = global_style_data.shared_lock.read();
     let sheet = unsafe { GeckoStyleSheet::new(sheet) };
-    let origin = sheet.contents(&guard).origin;
     data.stylesheets.remove_stylesheet(&data.stylist, sheet, &guard);
-    data.clear_stylist_origin(&origin);
 }
 
 #[no_mangle]
@@ -1002,10 +994,7 @@ pub extern "C" fn Servo_StyleSet_NoteStyleSheetsChanged(
     changed_origins: OriginFlags,
 ) {
     let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
-    for origin in OriginSet::from(changed_origins).iter() {
+    data.stylesheets.force_dirty(OriginSet::from(changed_origins));
-        data.stylesheets.force_dirty_origin(&origin);
-        data.clear_stylist_origin(&origin);
-    }
     data.stylesheets.set_author_style_disabled(author_style_disabled);
 }
 
@@ -1939,12 +1928,6 @@ pub extern "C" fn Servo_StyleSet_RebuildCachedData(raw_data: RawServoStyleSetBor
     data.stylist.device_mut().rebuild_cached_data();
 }
 
-#[no_mangle]
-pub extern "C" fn Servo_StyleSet_Clear(raw_data: RawServoStyleSetBorrowed) {
-    let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
-    data.clear_stylist();
-}
-
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_Drop(data: RawServoStyleSetOwned) {
     let _ = data.into_box::<PerDocumentStyleData>();

tests/unit/style/stylist.rs

@@ -167,36 +167,6 @@ fn test_rule_ordering_same_specificity() {
             "The rule that comes later should win.");
 }
 
-
-#[test]
-fn test_get_id_name() {
-    let (rules_list, _) = get_mock_rules(&[".intro", "#top"]);
-    assert_eq!(selector_map::get_id_name(rules_list[0][0].selector.iter()), None);
-    assert_eq!(selector_map::get_id_name(rules_list[1][0].selector.iter()), Some(Atom::from("top")));
-}
-
-#[test]
-fn test_get_class_name() {
-    let (rules_list, _) = get_mock_rules(&[".intro.foo", "#top"]);
-    assert_eq!(selector_map::get_class_name(rules_list[0][0].selector.iter()), Some(Atom::from("intro")));
-    assert_eq!(selector_map::get_class_name(rules_list[1][0].selector.iter()), 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!(selector_map::get_local_name(rules_list[i][0].selector.iter())
-                == names.map(|(name, lower_name)| LocalNameSelector {
-                        name: LocalName::from(name),
-                        lower_name: LocalName::from(lower_name) }))
-    };
-    check(0, Some(("img", "img")));
-    check(1, None);
-    check(2, Some(("IMG", "img")));
-    check(3, Some(("ImG", "img")));
-}
-
 #[test]
 fn test_insert() {
     let (rules_list, _) = get_mock_rules(&[".intro.foo", "#top"]);
@@ -204,8 +174,8 @@ fn test_insert() {
     selector_map.insert(rules_list[1][0].clone(), QuirksMode::NoQuirks);
     assert_eq!(1, selector_map.id_hash.get(&Atom::from("top"), QuirksMode::NoQuirks).unwrap()[0].source_order);
     selector_map.insert(rules_list[0][0].clone(), QuirksMode::NoQuirks);
-    assert_eq!(0, selector_map.class_hash.get(&Atom::from("intro"), QuirksMode::NoQuirks).unwrap()[0].source_order);
+    assert_eq!(0, selector_map.class_hash.get(&Atom::from("foo"), QuirksMode::NoQuirks).unwrap()[0].source_order);
-    assert!(selector_map.class_hash.get(&Atom::from("foo"), QuirksMode::NoQuirks).is_none());
+    assert!(selector_map.class_hash.get(&Atom::from("intro"), QuirksMode::NoQuirks).is_none());
 }
 
 fn mock_stylist() -> Stylist {