servo: Merge #17063 - style: Allow sharing styles across elements with presentational hints (from emilio:pres-hints-sharing); r=bholley

Source-Repo: https://github.com/servo/servo
Source-Revision: 38a6a3bff6f6e0e35eb592891d5e70e7cb897b69

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 5c6316d592dfa5ef57e7fc03f4c5211cbbc1b1d7

servo/components/layout/animation.rs

@@ -6,13 +6,13 @@
 
 use context::LayoutContext;
 use flow::{self, Flow};
+use fnv::FnvHashMap;
 use gfx::display_list::OpaqueNode;
 use ipc_channel::ipc::IpcSender;
 use msg::constellation_msg::PipelineId;
 use opaque_node::OpaqueNodeMethods;
 use script_traits::{AnimationState, ConstellationControlMsg, LayoutMsg as ConstellationMsg};
 use script_traits::UntrustedNodeAddress;
-use std::collections::HashMap;
 use std::sync::mpsc::Receiver;
 use style::animation::{Animation, update_style_for_animation};
 use style::font_metrics::ServoMetricsProvider;
@@ -24,8 +24,8 @@ use style::timer::Timer;
 /// `expired_animations`.
 pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
                               script_chan: &IpcSender<ConstellationControlMsg>,
-                              running_animations: &mut HashMap<OpaqueNode, Vec<Animation>>,
-                              expired_animations: &mut HashMap<OpaqueNode, Vec<Animation>>,
+                              running_animations: &mut FnvHashMap<OpaqueNode, Vec<Animation>>,
+                              expired_animations: &mut FnvHashMap<OpaqueNode, Vec<Animation>>,
                               mut newly_transitioning_nodes: Option<&mut Vec<UntrustedNodeAddress>>,
                               new_animations_receiver: &Receiver<Animation>,
                               pipeline_id: PipelineId,
@@ -149,7 +149,7 @@ pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
 // this should be made generic.
 pub fn recalc_style_for_animations(context: &LayoutContext,
                                    flow: &mut Flow,
-                                   animations: &HashMap<OpaqueNode,
+                                   animations: &FnvHashMap<OpaqueNode,
                                                         Vec<Animation>>) {
     let mut damage = RestyleDamage::empty();
     flow.mutate_fragments(&mut |fragment| {

servo/components/layout_thread/lib.rs

@@ -44,7 +44,7 @@ use euclid::point::Point2D;
 use euclid::rect::Rect;
 use euclid::scale_factor::ScaleFactor;
 use euclid::size::Size2D;
-use fnv::FnvHasher;
+use fnv::FnvHashMap;
 use gfx::display_list::{OpaqueNode, WebRenderImageInfo};
 use gfx::font;
 use gfx::font_cache_thread::FontCacheThread;
@@ -99,7 +99,6 @@ use servo_url::ServoUrl;
 use std::borrow::ToOwned;
 use std::cell::{Cell, RefCell};
 use std::collections::HashMap;
-use std::hash::BuildHasherDefault;
 use std::marker::PhantomData;
 use std::mem as std_mem;
 use std::ops::{Deref, DerefMut};
@@ -203,10 +202,10 @@ pub struct LayoutThread {
     document_shared_lock: Option<SharedRwLock>,
 
     /// The list of currently-running animations.
-    running_animations: StyleArc<RwLock<HashMap<OpaqueNode, Vec<Animation>>>>,
+    running_animations: StyleArc<RwLock<FnvHashMap<OpaqueNode, Vec<Animation>>>>,
 
     /// The list of animations that have expired since the last style recalculation.
-    expired_animations: StyleArc<RwLock<HashMap<OpaqueNode, Vec<Animation>>>>,
+    expired_animations: StyleArc<RwLock<FnvHashMap<OpaqueNode, Vec<Animation>>>>,
 
     /// A counter for epoch messages
     epoch: Cell<Epoch>,
@@ -224,9 +223,8 @@ pub struct LayoutThread {
     /// The CSS error reporter for all CSS loaded in this layout thread
     error_reporter: CSSErrorReporter,
 
-    webrender_image_cache: Arc<RwLock<HashMap<(ServoUrl, UsePlaceholder),
-                                              WebRenderImageInfo,
-                                              BuildHasherDefault<FnvHasher>>>>,
+    webrender_image_cache: Arc<RwLock<FnvHashMap<(ServoUrl, UsePlaceholder),
+                                                 WebRenderImageInfo>>>,
 
     /// Webrender interface.
     webrender_api: webrender_traits::RenderApi,
@@ -492,8 +490,8 @@ impl LayoutThread {
             outstanding_web_fonts: outstanding_web_fonts_counter,
             root_flow: RefCell::new(None),
             document_shared_lock: None,
-            running_animations: StyleArc::new(RwLock::new(HashMap::new())),
-            expired_animations: StyleArc::new(RwLock::new(HashMap::new())),
+            running_animations: StyleArc::new(RwLock::new(FnvHashMap::default())),
+            expired_animations: StyleArc::new(RwLock::new(FnvHashMap::default())),
             epoch: Cell::new(Epoch(0)),
             viewport_size: Size2D::new(Au(0), Au(0)),
             webrender_api: webrender_api_sender.create_api(),
@@ -521,7 +519,7 @@ impl LayoutThread {
                 script_chan: Arc::new(Mutex::new(script_chan)),
             },
             webrender_image_cache:
-                Arc::new(RwLock::new(HashMap::with_hasher(Default::default()))),
+                Arc::new(RwLock::new(FnvHashMap::default())),
             timer:
                 if PREFS.get("layout.animations.test.enabled")
                            .as_boolean().unwrap_or(false) {

servo/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;
@@ -21,11 +20,8 @@ use font_metrics::FontMetricsProvider;
 #[cfg(feature = "gecko")] use properties::ComputedValues;
 use selector_parser::SnapshotMap;
 use selectors::matching::ElementSelectorFlags;
-#[cfg(feature = "servo")] use servo_config::opts;
 use shared_lock::StylesheetGuards;
-use sharing::StyleSharingCandidateCache;
-#[cfg(feature = "servo")] use std::collections::HashMap;
-#[cfg(feature = "gecko")] use std::env;
+use sharing::{ValidationData, StyleSharingCandidateCache};
 use std::fmt;
 use std::ops::Add;
 #[cfg(feature = "servo")] use std::sync::Mutex;
@@ -79,6 +75,7 @@ pub struct StyleSystemOptions {
 
 #[cfg(feature = "gecko")]
 fn get_env(name: &str) -> bool {
+    use std::env;
     match env::var(name) {
         Ok(s) => !s.is_empty(),
         Err(_) => false,
@@ -88,6 +85,8 @@ fn get_env(name: &str) -> bool {
 impl Default for StyleSystemOptions {
     #[cfg(feature = "servo")]
     fn default() -> Self {
+        use servo_config::opts;
+
         StyleSystemOptions {
             disable_style_sharing_cache: opts::get().disable_share_style_cache,
             dump_style_statistics: opts::get().style_sharing_stats,
@@ -135,11 +134,11 @@ pub struct SharedStyleContext<'a> {
 
     /// The animations that are currently running.
     #[cfg(feature = "servo")]
-    pub running_animations: Arc<RwLock<HashMap<OpaqueNode, Vec<Animation>>>>,
+    pub running_animations: Arc<RwLock<FnvHashMap<OpaqueNode, Vec<Animation>>>>,
 
     /// The list of animations that have expired since the last style recalculation.
     #[cfg(feature = "servo")]
-    pub expired_animations: Arc<RwLock<HashMap<OpaqueNode, Vec<Animation>>>>,
+    pub expired_animations: Arc<RwLock<FnvHashMap<OpaqueNode, Vec<Animation>>>>,
 
     /// Data needed to create the thread-local style context from the shared one.
     #[cfg(feature = "servo")]
@@ -154,26 +153,27 @@ impl<'a> SharedStyleContext<'a> {
     }
 }
 
-/// Information about the current element being processed. We group this together
-/// into a single struct within ThreadLocalStyleContext so that we can instantiate
-/// and destroy it easily at the beginning and end of element processing.
+/// Information about the current element being processed. We group this
+/// together into a single struct within ThreadLocalStyleContext so that we can
+/// instantiate and destroy it easily at the beginning and end of element
+/// processing.
 pub struct CurrentElementInfo {
-    /// The element being processed. Currently we use an OpaqueNode since we only
-    /// use this for identity checks, but we could use SendElement if there were
-    /// a good reason to.
+    /// The element being processed. Currently we use an OpaqueNode since we
+    /// only use this for identity checks, but we could use SendElement if there
+    /// were a good reason to.
     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
-    /// revalidation selectors.
-    pub revalidation_match_results: Option<BitVec>,
+    /// Lazy cache of the different data used for style sharing.
+    pub validation_data: ValidationData,
     /// A Vec of possibly expired animations. Used only by Servo.
     #[allow(dead_code)]
     pub possibly_expired_animations: Vec<PropertyAnimation>,
 }
 
-/// Statistics gathered during the traversal. We gather statistics on each thread
-/// and then combine them after the threads join via the Add implementation below.
+/// Statistics gathered during the traversal. We gather statistics on each
+/// thread and then combine them after the threads join via the Add
+/// implementation below.
 #[derive(Default)]
 pub struct TraversalStatistics {
     /// The total number of elements traversed.
@@ -463,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,
+            validation_data: ValidationData::new(),
             possibly_expired_animations: Vec::new(),
         });
     }

servo/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};
 
@@ -855,17 +855,19 @@ pub trait MatchMethods : TElement {
             //
             // If we do have the results, grab them here to satisfy the borrow
             // checker.
-            let revalidation_match_results = context.thread_local
+            let validation_data =
+                context.thread_local
                     .current_element_info
                     .as_mut().unwrap()
-                                                    .revalidation_match_results
+                    .validation_data
                     .take();
+
             context.thread_local
                    .style_sharing_candidate_cache
                    .insert_if_possible(self,
                                        data.styles().primary.values(),
                                        primary_results.relations,
-                                       revalidation_match_results);
+                                       validation_data);
         }
 
         child_cascade_requirement
@@ -1341,30 +1343,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.

servo/components/style/rule_tree/mod.rs

@@ -61,6 +61,12 @@ pub enum StyleSource {
     Declarations(Arc<Locked<PropertyDeclarationBlock>>),
 }
 
+impl PartialEq for StyleSource {
+    fn eq(&self, other: &Self) -> bool {
+        self.ptr_equals(other)
+    }
+}
+
 impl StyleSource {
     #[inline]
     fn ptr_equals(&self, other: &Self) -> bool {

servo/components/style/sharing/checks.rs

@@ -6,14 +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 sharing::StyleSharingCandidate;
-use sink::ForgetfulSink;
+use selectors::matching::StyleRelations;
+use sharing::{StyleSharingCandidate, StyleSharingTarget};
 use stylearc::Arc;
 
 /// Determines, based on the results of selector matching, whether it's worth to
@@ -24,8 +22,7 @@ pub fn relations_are_shareable(relations: &StyleRelations) -> bool {
     use selectors::matching::*;
     !relations.intersects(AFFECTED_BY_ID_SELECTOR |
                           AFFECTED_BY_PSEUDO_ELEMENTS |
-                          AFFECTED_BY_STYLE_ATTRIBUTE |
-                          AFFECTED_BY_PRESENTATIONAL_HINTS)
+                          AFFECTED_BY_STYLE_ATTRIBUTE)
 }
 
 /// Whether, given two elements, they have pointer-equal computed values.
@@ -52,33 +49,24 @@ pub fn same_computed_values<E>(first: Option<E>, second: Option<E>) -> bool
 /// We consider not worth to share style with an element that has presentational
 /// hints, both because implementing the code that compares that the hints are
 /// equal is somewhat annoying, and also because it'd be expensive enough.
-pub fn has_presentational_hints<E>(element: E) -> bool
+pub fn have_same_presentational_hints<E>(
+    target: &mut StyleSharingTarget<E>,
+    candidate: &mut StyleSharingCandidate<E>
+) -> bool
     where E: TElement,
 {
-    let mut hints = ForgetfulSink::new();
-    element.synthesize_presentational_hints_for_legacy_attributes(&mut hints);
-    !hints.is_empty()
+    target.pres_hints() == candidate.pres_hints()
 }
 
 /// 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.
-    let mut element_class_attributes = vec![];
-    element.each_class(|c| element_class_attributes.push(c.clone()));
-
-    if candidate.class_attributes.is_none() {
-        let mut attrs = vec![];
-        candidate.element.each_class(|c| attrs.push(c.clone()));
-        candidate.class_attributes = Some(attrs)
-    }
-
-    element_class_attributes == *candidate.class_attributes.as_ref().unwrap()
+    target.class_list() == candidate.class_list()
 }
 
 /// Compare element and candidate state, but ignore visitedness.  Styles don't
@@ -102,50 +90,20 @@ 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() {
-        // 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 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 =
+        target.revalidation_match_results(stylist, bloom, selector_flags_map);
 
-    if candidate.revalidation_match_results.is_none() {
-        let results =
-            stylist.match_revalidation_selectors(&*candidate.element, bloom,
-                                                 &mut |_, _| {});
-        candidate.revalidation_match_results = Some(results);
-    }
-
-    let for_element = info.revalidation_match_results.as_ref().unwrap();
-    let for_candidate = candidate.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
     // matched the same rulehash buckets, and as such, that the bits we're

servo/components/style/sharing/mod.rs

@@ -8,14 +8,16 @@
 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};
 use properties::ComputedValues;
 use selectors::bloom::BloomFilter;
-use selectors::matching::StyleRelations;
-use sink::ForgetfulSink;
+use selectors::matching::{ElementSelectorFlags, StyleRelations};
+use smallvec::SmallVec;
+use std::ops::Deref;
+use stylist::{ApplicableDeclarationBlock, Stylist};
 
 mod checks;
 
@@ -32,23 +34,127 @@ pub enum StyleSharingBehavior {
     Disallow,
 }
 
+/// Some data we want to avoid recomputing all the time while trying to share
+/// style.
+#[derive(Debug)]
+pub struct ValidationData {
+    /// The class list of this element.
+    ///
+    /// TODO(emilio): See if it's worth to sort them, or doing something else in
+    /// a similar fashion as what Boris is doing for the ID attribute.
+    class_list: Option<SmallVec<[Atom; 5]>>,
+
+    /// The list of presentational attributes of the element.
+    pres_hints: Option<SmallVec<[ApplicableDeclarationBlock; 5]>>,
+
+    /// The cached result of matching this entry against the revalidation
+    /// selectors.
+    revalidation_match_results: Option<BitVec>,
+}
+
+impl ValidationData {
+    /// Trivially construct an empty `ValidationData` with nothing on
+    /// it.
+    pub fn new() -> Self {
+        Self {
+            class_list: None,
+            pres_hints: 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(),
+            pres_hints: self.pres_hints.take(),
+            revalidation_match_results: self.revalidation_match_results.take(),
+        }
+    }
+
+    /// Get or compute the list of presentational attributes associated with
+    /// this element.
+    pub fn pres_hints<E>(&mut self, element: E) -> &[ApplicableDeclarationBlock]
+        where E: TElement,
+    {
+        if self.pres_hints.is_none() {
+            let mut pres_hints = SmallVec::new();
+            element.synthesize_presentational_hints_for_legacy_attributes(&mut pres_hints);
+            self.pres_hints = Some(pres_hints);
+        }
+        &*self.pres_hints.as_ref().unwrap()
+    }
+
+    /// Get or compute the class-list associated with this element.
+    pub fn class_list<E>(&mut self, element: E) -> &[Atom]
+        where E: TElement,
+    {
+        if self.class_list.is_none() {
+            let mut class_list = SmallVec::new();
+            element.each_class(|c| class_list.push(c.clone()));
+            self.class_list = Some(class_list);
+        }
+        &*self.class_list.as_ref().unwrap()
+    }
+
+    /// Computes the revalidation results if needed, and returns it.
+    fn revalidation_match_results<E, F>(
+        &mut self,
+        element: E,
+        stylist: &Stylist,
+        bloom: &BloomFilter,
+        flags_setter: &mut F
+    ) -> &BitVec
+        where E: TElement,
+              F: FnMut(&E, ElementSelectorFlags),
+    {
+        if self.revalidation_match_results.is_none() {
+            self.revalidation_match_results =
+                Some(stylist.match_revalidation_selectors(&element, bloom,
+                                                          flags_setter));
+        }
+
+        self.revalidation_match_results.as_ref().unwrap()
+    }
+}
+
 /// Information regarding a style sharing candidate, that is, an entry in the
 /// style sharing cache.
 ///
 /// Note that this information is stored in TLS and cleared after the traversal,
 /// and once here, the style information of the element is immutable, so it's
 /// safe to access.
-///
-/// TODO: We can stick a lot more info here.
 #[derive(Debug)]
 pub struct StyleSharingCandidate<E: TElement> {
     /// The element. We use SendElement here so that the cache may live in
     /// ScopedTLS.
     element: SendElement<E>,
-    /// The cached class names.
-    class_attributes: Option<Vec<Atom>>,
-    /// The cached result of matching this entry against the revalidation selectors.
-    revalidation_match_results: Option<BitVec>,
+    validation_data: ValidationData,
+}
+
+impl<E: TElement> StyleSharingCandidate<E> {
+    /// Get the classlist of this candidate.
+    fn class_list(&mut self) -> &[Atom] {
+        self.validation_data.class_list(*self.element)
+    }
+
+    /// Get the pres hints of this candidate.
+    fn pres_hints(&mut self) -> &[ApplicableDeclarationBlock] {
+        self.validation_data.pres_hints(*self.element)
+    }
+
+    /// Get the classlist of this candidate.
+    fn revalidation_match_results(
+        &mut self,
+        stylist: &Stylist,
+        bloom: &BloomFilter,
+    ) -> &BitVec {
+        self.validation_data.revalidation_match_results(
+            *self.element,
+            stylist,
+            bloom,
+            &mut |_, _| {})
+    }
 }
 
 impl<E: TElement> PartialEq<StyleSharingCandidate<E>> for StyleSharingCandidate<E> {
@@ -57,6 +163,102 @@ 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,
+    validation_data: ValidationData,
+}
+
+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,
+            validation_data: ValidationData::new(),
+        }
+    }
+
+    fn class_list(&mut self) -> &[Atom] {
+        self.validation_data.class_list(self.element)
+    }
+
+    /// Get the pres hints of this candidate.
+    fn pres_hints(&mut self) -> &[ApplicableDeclarationBlock] {
+        self.validation_data.pres_hints(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.validation_data.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);
+
+        mem::swap(&mut self.validation_data,
+                  &mut context
+                      .thread_local
+                      .current_element_info.as_mut().unwrap()
+                      .validation_data);
+
+        result
+    }
+}
+
 /// A cache miss result.
 #[derive(Clone, Debug)]
 pub enum CacheMiss {
@@ -133,7 +335,9 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
                               element: &E,
                               style: &ComputedValues,
                               relations: StyleRelations,
-                              revalidation_match_results: Option<BitVec>) {
+                              mut validation_data: ValidationData) {
+        use selectors::matching::AFFECTED_BY_PRESENTATIONAL_HINTS;
+
         let parent = match element.parent_element() {
             Some(element) => element,
             None => {
@@ -154,14 +358,6 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
             return;
         }
 
-        // Make sure we noted any presentational hints in the StyleRelations.
-        if cfg!(debug_assertions) {
-            let mut hints = ForgetfulSink::new();
-            element.synthesize_presentational_hints_for_legacy_attributes(&mut hints);
-            debug_assert!(hints.is_empty(),
-                          "Style relations should not be shareable!");
-        }
-
         let box_style = style.get_box();
         if box_style.specifies_transitions() {
             debug!("Failing to insert to the cache: transitions");
@@ -173,12 +369,18 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
             return;
         }
 
+        // Take advantage of the information we've learned during
+        // selector-matching.
+        if !relations.intersects(AFFECTED_BY_PRESENTATIONAL_HINTS) {
+            debug_assert!(validation_data.pres_hints.as_ref().map_or(true, |v| v.is_empty()));
+            validation_data.pres_hints = Some(SmallVec::new());
+        }
+
         debug!("Inserting into cache: {:?} with parent {:?}", element, parent);
 
         self.cache.insert(StyleSharingCandidate {
             element: unsafe { SendElement::new(*element) },
-            class_attributes: None,
-            revalidation_match_results: revalidation_match_results,
+            validation_data: validation_data,
         });
     }
 
@@ -193,43 +395,39 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
     }
 
     /// 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.
-    pub unsafe fn share_style_if_possible(
+    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() {
-            debug!("{:?} Cannot share style: element has no parent", element);
+        if target.parent_element().is_none() {
+            debug!("{:?} Cannot share style: element has no parent",
+                   target.element);
             return StyleSharingResult::CannotShare
         }
 
-        if element.is_native_anonymous() {
-            debug!("{:?} Cannot share style: NAC", element);
+        if target.is_native_anonymous() {
+            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() {
-            debug!("{:?} Cannot share style: element has id", element);
+        if target.get_id().is_some() {
+            debug!("{:?} Cannot share style: element has id", target.element);
             return StyleSharingResult::CannotShare
         }
 
@@ -237,11 +435,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
                 );
 
@@ -254,7 +451,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),
@@ -293,7 +490,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 {
@@ -303,11 +500,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 {
@@ -319,66 +515,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::have_same_presentational_hints(target, candidate) {
             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())
     }
 }

servo/components/style/stylist.rs

@@ -1397,7 +1397,7 @@ impl Rule {
 /// This represents the declarations in a given declaration block for a given
 /// importance.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct ApplicableDeclarationBlock {
     /// The style source, either a style rule, or a property declaration block.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]

servo/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;
@@ -800,9 +800,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 {
-            element.share_style_if_possible(context, data)
-        };
+        let target = StyleSharingTarget::new(element);
+        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);