Auto merge of #17063 - emilio:pres-hints-sharing, r=bholley

style: Allow sharing styles across elements with presentational hints.

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

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| {

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) {

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(),
         });
     }

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
-                                                    .current_element_info
-                                                    .as_mut().unwrap()
-                                                    .revalidation_match_results
-                                                    .take();
+            let validation_data =
+                context.thread_local
+                    .current_element_info
+                    .as_mut().unwrap()
+                    .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.

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 {

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));
-    }
-
-    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 =
+        target.revalidation_match_results(stylist, bloom, selector_flags_map);
 
-    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