style: Also cache the class list in the CurrentElementInfo.

This patch also removes all notion of style sharing from matching.rs, which is
nice.

components/style/context.rs

@@ -7,7 +7,6 @@
 #[cfg(feature = "servo")] use animation::Animation;
 use animation::PropertyAnimation;
 use app_units::Au;
-use bit_vec::BitVec;
 use bloom::StyleBloom;
 use cache::LRUCache;
 use data::ElementData;
@@ -23,7 +22,7 @@ use selector_parser::SnapshotMap;
 use selectors::matching::ElementSelectorFlags;
 #[cfg(feature = "servo")] use servo_config::opts;
 use shared_lock::StylesheetGuards;
-use sharing::StyleSharingCandidateCache;
+use sharing::{CachedStyleSharingData, StyleSharingCandidateCache};
 #[cfg(feature = "servo")] use std::collections::HashMap;
 #[cfg(feature = "gecko")] use std::env;
 use std::fmt;
@@ -165,9 +164,8 @@ pub struct CurrentElementInfo {
     element: OpaqueNode,
     /// Whether the element is being styled for the first time.
     is_initial_style: bool,
-    /// Lazy cache of the result of matching the current element against the
+    /// Lazy cache of the different data used for style sharing.
-    /// revalidation selectors.
+    pub cached_style_sharing_data: CachedStyleSharingData,
-    pub revalidation_match_results: Option<BitVec>,
     /// A Vec of possibly expired animations. Used only by Servo.
     #[allow(dead_code)]
     pub possibly_expired_animations: Vec<PropertyAnimation>,
@@ -465,7 +463,7 @@ impl<E: TElement> ThreadLocalStyleContext<E> {
         self.current_element_info = Some(CurrentElementInfo {
             element: element.as_node().opaque(),
             is_initial_style: !data.has_styles(),
-            revalidation_match_results: None,
+            cached_style_sharing_data: CachedStyleSharingData::new(),
             possibly_expired_animations: Vec::new(),
         });
     }

components/style/matching.rs

@@ -23,7 +23,7 @@ use rule_tree::{CascadeLevel, StrongRuleNode};
 use selector_parser::{PseudoElement, RestyleDamage, SelectorImpl};
 use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode, StyleRelations};
 use selectors::matching::{VisitedHandlingMode, AFFECTED_BY_PSEUDO_ELEMENTS};
-use sharing::{StyleSharingBehavior, StyleSharingResult};
+use sharing::StyleSharingBehavior;
 use stylearc::Arc;
 use stylist::{ApplicableDeclarationList, RuleInclusion};
 
@@ -858,7 +858,7 @@ pub trait MatchMethods : TElement {
             let revalidation_match_results = context.thread_local
                                                     .current_element_info
                                                     .as_mut().unwrap()
-                                                    .revalidation_match_results
+                                                    .cached_style_sharing_data
                                                     .take();
             context.thread_local
                    .style_sharing_candidate_cache
@@ -1341,30 +1341,6 @@ pub trait MatchMethods : TElement {
         false
     }
 
-    /// Attempts to share a style with another node. This method is unsafe
-    /// because it depends on the `style_sharing_candidate_cache` having only
-    /// live nodes in it, and we have no way to guarantee that at the type
-    /// system level yet.
-    unsafe fn share_style_if_possible(&self,
-                                      context: &mut StyleContext<Self>,
-                                      data: &mut ElementData)
-                                      -> StyleSharingResult {
-        let shared_context = &context.shared;
-        let current_element_info =
-            context.thread_local.current_element_info.as_mut().unwrap();
-        let selector_flags_map = &mut context.thread_local.selector_flags;
-        let bloom_filter = context.thread_local.bloom_filter.filter();
-
-        context.thread_local
-            .style_sharing_candidate_cache
-            .share_style_if_possible(shared_context,
-                                     current_element_info,
-                                     selector_flags_map,
-                                     bloom_filter,
-                                     *self,
-                                     data)
-    }
-
     /// 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.

components/style/sharing/checks.rs

@@ -6,13 +6,12 @@
 //! quickly whether it's worth to share style, and whether two different
 //! elements can indeed share the same style.
 
-use context::{CurrentElementInfo, SelectorFlagsMap, SharedStyleContext};
+use context::{SelectorFlagsMap, SharedStyleContext};
 use dom::TElement;
 use element_state::*;
-use matching::MatchMethods;
 use selectors::bloom::BloomFilter;
-use selectors::matching::{ElementSelectorFlags, StyleRelations};
+use selectors::matching::StyleRelations;
-use sharing::StyleSharingCandidate;
+use sharing::{StyleSharingCandidate, StyleSharingTarget};
 use sink::ForgetfulSink;
 use stylearc::Arc;
 
@@ -63,16 +62,12 @@ pub fn has_presentational_hints<E>(element: E) -> bool
 /// Whether a given element has the same class attribute than a given candidate.
 ///
 /// We don't try to share style across elements with different class attributes.
-pub fn have_same_class<E>(element: E,
+pub fn have_same_class<E>(target: &mut StyleSharingTarget<E>,
                           candidate: &mut StyleSharingCandidate<E>)
                           -> bool
     where E: TElement,
 {
-    // XXX Efficiency here, I'm only validating ideas.
+    target.class_list() == candidate.class_list()
-    let mut element_class_attributes = vec![];
-    element.each_class(|c| element_class_attributes.push(c.clone()));
-
-    element_class_attributes == candidate.class_list()
 }
 
 /// Compare element and candidate state, but ignore visitedness.  Styles don't
@@ -96,42 +91,19 @@ pub fn have_same_state_ignoring_visitedness<E>(element: E,
 /// :first-child, etc, or on attributes that we don't check off-hand (pretty
 /// much every attribute selector except `id` and `class`.
 #[inline]
-pub fn revalidate<E>(element: E,
+pub fn revalidate<E>(target: &mut StyleSharingTarget<E>,
                      candidate: &mut StyleSharingCandidate<E>,
                      shared_context: &SharedStyleContext,
                      bloom: &BloomFilter,
-                     info: &mut CurrentElementInfo,
                      selector_flags_map: &mut SelectorFlagsMap<E>)
                      -> bool
     where E: TElement,
 {
     let stylist = &shared_context.stylist;
 
-    if info.revalidation_match_results.is_none() {
+    let for_element =
-        // It's important to set the selector flags. Otherwise, if we succeed in
+        target.revalidation_match_results(stylist, bloom, selector_flags_map);
-        // sharing the style, we may not set the slow selector flags for the
-        // right elements (which may not necessarily be |element|), causing
-        // missed restyles after future DOM mutations.
-        //
-        // Gecko's test_bug534804.html exercises this. A minimal testcase is:
-        // <style> #e:empty + span { ... } </style>
-        // <span id="e">
-        //   <span></span>
-        // </span>
-        // <span></span>
-        //
-        // The style sharing cache will get a hit for the second span. When the
-        // child span is subsequently removed from the DOM, missing selector
-        // flags would cause us to miss the restyle on the second span.
-        let mut set_selector_flags = |el: &E, flags: ElementSelectorFlags| {
-            element.apply_selector_flags(selector_flags_map, el, flags);
-        };
-        info.revalidation_match_results =
-            Some(stylist.match_revalidation_selectors(&element, bloom,
-                                                      &mut set_selector_flags));
-    }
 
-    let for_element = info.revalidation_match_results.as_ref().unwrap();
     let for_candidate = candidate.revalidation_match_results(stylist, bloom);
 
     // This assert "ensures", to some extent, that the two candidates have

components/style/sharing/mod.rs

@@ -8,7 +8,7 @@
 use Atom;
 use bit_vec::BitVec;
 use cache::{LRUCache, LRUCacheMutIterator};
-use context::{CurrentElementInfo, SelectorFlagsMap, SharedStyleContext};
+use context::{SelectorFlagsMap, SharedStyleContext, StyleContext};
 use data::{ComputedStyle, ElementData, ElementStyles};
 use dom::{TElement, SendElement};
 use matching::{ChildCascadeRequirement, MatchMethods};
@@ -17,6 +17,7 @@ use selectors::bloom::BloomFilter;
 use selectors::matching::{ElementSelectorFlags, StyleRelations};
 use sink::ForgetfulSink;
 use smallvec::SmallVec;
+use std::ops::Deref;
 use stylist::Stylist;
 
 mod checks;
@@ -50,6 +51,23 @@ pub struct CachedStyleSharingData {
 }
 
 impl CachedStyleSharingData {
+    /// Trivially construct an empty `CachedStyleSharingData` with nothing on
+    /// it.
+    pub fn new() -> Self {
+        Self {
+            class_list: None,
+            revalidation_match_results: None,
+        }
+    }
+
+    /// Move the cached data to a new instance, and return it.
+    pub fn take(&mut self) -> Self {
+        Self {
+            class_list: self.class_list.take(),
+            revalidation_match_results: self.revalidation_match_results.take(),
+        }
+    }
+
     /// Get or compute the class-list associated with this element.
     pub fn class_list<E>(&mut self, element: E) -> &[Atom]
         where E: TElement,
@@ -122,6 +140,97 @@ impl<E: TElement> PartialEq<StyleSharingCandidate<E>> for StyleSharingCandidate<
     }
 }
 
+/// An element we want to test against the style sharing cache.
+pub struct StyleSharingTarget<E: TElement> {
+    element: E,
+    cache: CachedStyleSharingData,
+}
+
+impl<E: TElement> Deref for StyleSharingTarget<E> {
+    type Target = E;
+
+    fn deref(&self) -> &Self::Target {
+        &self.element
+    }
+}
+
+impl<E: TElement> StyleSharingTarget<E> {
+    /// Trivially construct a new StyleSharingTarget to test against the cache.
+    pub fn new(element: E) -> Self {
+        Self {
+            element: element,
+            cache: CachedStyleSharingData::new(),
+        }
+    }
+
+    fn class_list(&mut self) -> &[Atom] {
+        self.cache.class_list(self.element)
+    }
+
+    fn revalidation_match_results(
+        &mut self,
+        stylist: &Stylist,
+        bloom: &BloomFilter,
+        selector_flags_map: &mut SelectorFlagsMap<E>
+    ) -> &BitVec {
+        // It's important to set the selector flags. Otherwise, if we succeed in
+        // sharing the style, we may not set the slow selector flags for the
+        // right elements (which may not necessarily be |element|), causing
+        // missed restyles after future DOM mutations.
+        //
+        // Gecko's test_bug534804.html exercises this. A minimal testcase is:
+        // <style> #e:empty + span { ... } </style>
+        // <span id="e">
+        //   <span></span>
+        // </span>
+        // <span></span>
+        //
+        // The style sharing cache will get a hit for the second span. When the
+        // child span is subsequently removed from the DOM, missing selector
+        // flags would cause us to miss the restyle on the second span.
+        let element = self.element;
+        let mut set_selector_flags = |el: &E, flags: ElementSelectorFlags| {
+            element.apply_selector_flags(selector_flags_map, el, flags);
+        };
+
+        self.cache.revalidation_match_results(self.element,
+                                              stylist,
+                                              bloom,
+                                              &mut set_selector_flags)
+    }
+
+    /// Attempts to share a style with another node.
+    pub fn share_style_if_possible(
+        mut self,
+        context: &mut StyleContext<E>,
+        data: &mut ElementData)
+        -> StyleSharingResult
+    {
+        use std::mem;
+
+        let shared_context = &context.shared;
+        let selector_flags_map = &mut context.thread_local.selector_flags;
+        let bloom_filter = context.thread_local.bloom_filter.filter();
+
+        let result = context.thread_local
+            .style_sharing_candidate_cache
+            .share_style_if_possible(shared_context,
+                                     selector_flags_map,
+                                     bloom_filter,
+                                     &mut self,
+                                     data);
+
+        // FIXME(emilio): Do this in a cleaner way.
+        mem::swap(&mut self.cache,
+                  &mut context
+                      .thread_local
+                      .current_element_info.as_mut().unwrap()
+                      .cached_style_sharing_data);
+
+        result
+    }
+}
+
 /// A cache miss result.
 #[derive(Clone, Debug)]
 pub enum CacheMiss {
@@ -198,7 +307,7 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
                               element: &E,
                               style: &ComputedValues,
                               relations: StyleRelations,
-                              revalidation_match_results: Option<BitVec>) {
+                              cache: CachedStyleSharingData) {
         let parent = match element.parent_element() {
             Some(element) => element,
             None => {
@@ -242,10 +351,7 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
 
         self.cache.insert(StyleSharingCandidate {
             element: unsafe { SendElement::new(*element) },
-            cache: CachedStyleSharingData {
+            cache: cache,
-                class_list: None,
-                revalidation_match_results: revalidation_match_results,
-            }
         });
     }
 
@@ -260,43 +366,39 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
     }
 
     /// Attempts to share a style with another node.
-    ///
+    fn share_style_if_possible(
-    /// This method is unsafe because it depends on the
-    /// `style_sharing_candidate_cache` having only live nodes in it, and we
-    /// have no way to guarantee that at the type system level yet.
-    pub unsafe fn share_style_if_possible(
         &mut self,
         shared_context: &SharedStyleContext,
-        current_element_info: &mut CurrentElementInfo,
         selector_flags_map: &mut SelectorFlagsMap<E>,
         bloom_filter: &BloomFilter,
-        element: E,
+        target: &mut StyleSharingTarget<E>,
         data: &mut ElementData
     ) -> StyleSharingResult {
         if shared_context.options.disable_style_sharing_cache {
             debug!("{:?} Cannot share style: style sharing cache disabled",
-                   element);
+                   target.element);
             return StyleSharingResult::CannotShare
         }
 
-        if element.parent_element().is_none() {
+        if target.parent_element().is_none() {
-            debug!("{:?} Cannot share style: element has no parent", element);
+            debug!("{:?} Cannot share style: element has no parent",
+                   target.element);
             return StyleSharingResult::CannotShare
         }
 
-        if element.is_native_anonymous() {
+        if target.is_native_anonymous() {
-            debug!("{:?} Cannot share style: NAC", element);
+            debug!("{:?} Cannot share style: NAC", target.element);
             return StyleSharingResult::CannotShare;
         }
 
-        if element.style_attribute().is_some() {
+        if target.style_attribute().is_some() {
             debug!("{:?} Cannot share style: element has style attribute",
-                   element);
+                   target.element);
             return StyleSharingResult::CannotShare
         }
 
-        if element.get_id().is_some() {
+        if target.get_id().is_some() {
-            debug!("{:?} Cannot share style: element has id", element);
+            debug!("{:?} Cannot share style: element has id", target.element);
             return StyleSharingResult::CannotShare
         }
 
@@ -304,11 +406,10 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
         for (i, candidate) in self.iter_mut().enumerate() {
             let sharing_result =
                 Self::test_candidate(
-                    element,
+                    target,
                     candidate,
                     &shared_context,
                     bloom_filter,
-                    current_element_info,
                     selector_flags_map
                 );
 
@@ -321,7 +422,7 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
                     let old_values = data.get_styles_mut()
                                          .and_then(|s| s.primary.values.take());
                     let child_cascade_requirement =
-                        element.accumulate_damage(
+                        target.accumulate_damage(
                             &shared_context,
                             data.get_restyle_mut(),
                             old_values.as_ref().map(|v| &**v),
@@ -360,7 +461,7 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
             }
         }
 
-        debug!("{:?} Cannot share style: {} cache entries", element,
+        debug!("{:?} Cannot share style: {} cache entries", target.element,
                self.cache.num_entries());
 
         if should_clear_cache {
@@ -370,11 +471,10 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
         StyleSharingResult::CannotShare
     }
 
-    fn test_candidate(element: E,
+    fn test_candidate(target: &mut StyleSharingTarget<E>,
                       candidate: &mut StyleSharingCandidate<E>,
                       shared: &SharedStyleContext,
                       bloom: &BloomFilter,
-                      info: &mut CurrentElementInfo,
                       selector_flags_map: &mut SelectorFlagsMap<E>)
                       -> Result<ComputedStyle, CacheMiss> {
         macro_rules! miss {
@@ -386,66 +486,66 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
         // Check that we have the same parent, or at least the same pointer
         // identity for parent computed style. The latter check allows us to
         // share style between cousins if the parents shared style.
-        let parent = element.parent_element();
+        let parent = target.parent_element();
         let candidate_parent = candidate.element.parent_element();
         if parent != candidate_parent &&
            !checks::same_computed_values(parent, candidate_parent) {
             miss!(Parent)
         }
 
-        if element.is_native_anonymous() {
+        if target.is_native_anonymous() {
             debug_assert!(!candidate.element.is_native_anonymous(),
                           "Why inserting NAC into the cache?");
             miss!(NativeAnonymousContent)
         }
 
-        if *element.get_local_name() != *candidate.element.get_local_name() {
+        if *target.get_local_name() != *candidate.element.get_local_name() {
             miss!(LocalName)
         }
 
-        if *element.get_namespace() != *candidate.element.get_namespace() {
+        if *target.get_namespace() != *candidate.element.get_namespace() {
             miss!(Namespace)
         }
 
-        if element.is_link() != candidate.element.is_link() {
+        if target.is_link() != candidate.element.is_link() {
             miss!(Link)
         }
 
-        if element.matches_user_and_author_rules() !=
+        if target.matches_user_and_author_rules() !=
             candidate.element.matches_user_and_author_rules() {
             miss!(UserAndAuthorRules)
         }
 
-        if !checks::have_same_state_ignoring_visitedness(element, candidate) {
+        if !checks::have_same_state_ignoring_visitedness(target.element, candidate) {
             miss!(State)
         }
 
-        if element.get_id() != candidate.element.get_id() {
+        if target.get_id() != candidate.element.get_id() {
             miss!(IdAttr)
         }
 
-        if element.style_attribute().is_some() {
+        if target.style_attribute().is_some() {
             miss!(StyleAttr)
         }
 
-        if !checks::have_same_class(element, candidate) {
+        if !checks::have_same_class(target, candidate) {
             miss!(Class)
         }
 
-        if checks::has_presentational_hints(element) {
+        if checks::has_presentational_hints(target.element) {
             miss!(PresHints)
         }
 
-        if !checks::revalidate(element, candidate, shared, bloom, info,
+        if !checks::revalidate(target, candidate, shared, bloom,
                                selector_flags_map) {
             miss!(Revalidation)
         }
 
         let data = candidate.element.borrow_data().unwrap();
-        debug_assert!(element.has_current_styles(&data));
+        debug_assert!(target.has_current_styles(&data));
 
         debug!("Sharing style between {:?} and {:?}",
-               element, candidate.element);
+               target.element, candidate.element);
         Ok(data.styles().primary.clone())
     }
 }

components/style/traversal.rs

@@ -11,7 +11,7 @@ use dom::{DirtyDescendants, NodeInfo, OpaqueNode, TElement, TNode};
 use matching::{ChildCascadeRequirement, MatchMethods};
 use restyle_hints::{HintComputationContext, RestyleHint};
 use selector_parser::RestyleDamage;
-use sharing::StyleSharingBehavior;
+use sharing::{StyleSharingBehavior, StyleSharingTarget};
 #[cfg(feature = "servo")] use servo_config::opts;
 use smallvec::SmallVec;
 use std::borrow::BorrowMut;
@@ -795,9 +795,9 @@ fn compute_style<E, D>(_traversal: &D,
     // First, try the style sharing cache. If we get a match we can skip the rest
     // of the work.
     if let MatchAndCascade = kind {
-        let sharing_result = unsafe {
+        let target = StyleSharingTarget::new(element);
-            element.share_style_if_possible(context, data)
+        let sharing_result = target.share_style_if_possible(context, data);
-        };
+
         if let StyleWasShared(index, had_damage) = sharing_result {
             context.thread_local.statistics.styles_shared += 1;
             context.thread_local.style_sharing_candidate_cache.touch(index);