style: Record names that appear in attribute selectors. draft
authorCameron McCormack <cam@mcc.id.au>
Mon, 08 May 2017 16:06:15 +0800
changeset 574699 3513c6732b81fd8e6f60a1cd54bfee65c4eab7d1
parent 573891 c3e5497cff1c995821b1c9320fa71f1ef9a8c30e
child 574700 51e286623d22a4dcedd7641ebf35d822bd2d15c2
push id57806
push userbmo:cam@mcc.id.au
push dateTue, 09 May 2017 10:15:53 +0000
milestone55.0a1
style: Record names that appear in attribute selectors. MozReview-Commit-ID: 11JFey3gSfv
servo/components/style/stylist.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -22,17 +22,17 @@ use properties::INHERIT_ALL;
 use properties::PropertyDeclarationBlock;
 use restyle_hints::{RestyleHint, DependencySet};
 use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource};
 use selector_parser::{SelectorImpl, PseudoElement, Snapshot};
 use selectors::Element;
 use selectors::bloom::BloomFilter;
 use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS};
 use selectors::matching::{ElementSelectorFlags, StyleRelations, matches_selector};
-use selectors::parser::{Combinator, Component, Selector, SelectorInner, SelectorIter};
+use selectors::parser::{AttrSelector, Combinator, Component, Selector, SelectorInner, SelectorIter};
 use selectors::parser::{SelectorMethods, LocalName as LocalNameSelector};
 use selectors::visitor::SelectorVisitor;
 use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
 use sink::Push;
 use smallvec::VecLike;
 use std::borrow::Borrow;
 use std::collections::HashMap;
 use std::fmt;
@@ -108,16 +108,32 @@ pub struct Stylist {
 
     /// A monotonically increasing counter to represent the order on which a
     /// style rule appears in a stylesheet, needed to sort them by source order.
     rules_source_order: usize,
 
     /// Selector dependencies used to compute restyle hints.
     dependencies: DependencySet,
 
+    /// The attribute local names that appear in attribute selectors.  Used
+    /// to avoid taking element snapshots when an irrelevant attribute changes.
+    /// (We don't bother storing the namespace, since namespaced attributes
+    /// are rare.)
+    ///
+    /// FIXME(heycam): This doesn't really need to be a counting Bloom filter.
+    attribute_dependencies: BloomFilter,
+
+    /// Whether `"style"` appears in an attribute selector.  This is not common,
+    /// and by tracking this explicitly, we can avoid taking an element snapshot
+    /// in the common case of style=""` changing due to modifying
+    /// `element.style`.  (We could track this in `attribute_dependencies`, like
+    /// all other attributes, but we should probably not risk incorrectly
+    /// returning `true` for `"style"` just due to a hash collision.)
+    style_attribute_dependency: bool,
+
     /// Selectors that require explicit cache revalidation (i.e. which depend
     /// on state that is not otherwise visible to the cache, like attributes or
     /// tree-structural state like child index and pseudos).
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
     selectors_for_cache_revalidation: SelectorMap<SelectorInner<SelectorImpl>>,
 
     /// The total number of selectors.
     num_selectors: usize,
@@ -172,16 +188,18 @@ impl Stylist {
 
             element_map: PerPseudoElementSelectorMap::new(),
             pseudos_map: Default::default(),
             animations: Default::default(),
             precomputed_pseudo_element_decls: Default::default(),
             rules_source_order: 0,
             rule_tree: RuleTree::new(),
             dependencies: DependencySet::new(),
+            attribute_dependencies: BloomFilter::new(),
+            style_attribute_dependency: false,
             selectors_for_cache_revalidation: SelectorMap::new(),
             num_selectors: 0,
             num_declarations: 0,
             num_rebuilds: 0,
         };
 
         SelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| {
             stylist.pseudos_map.insert(pseudo, PerPseudoElementSelectorMap::new());
@@ -254,16 +272,18 @@ impl Stylist {
         self.animations = Default::default();
         SelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| {
             self.pseudos_map.insert(pseudo, PerPseudoElementSelectorMap::new());
         });
 
         self.precomputed_pseudo_element_decls = Default::default();
         self.rules_source_order = 0;
         self.dependencies.clear();
+        self.attribute_dependencies.clear();
+        self.style_attribute_dependency = false;
         self.animations.clear();
         self.selectors_for_cache_revalidation = SelectorMap::new();
         self.num_selectors = 0;
         self.num_declarations = 0;
 
         extra_data.clear_font_faces();
 
         if let Some(ua_stylesheets) = ua_stylesheets {
@@ -314,16 +334,17 @@ impl Stylist {
                 CssRule::Style(ref locked) => {
                     let style_rule = locked.read_with(&guard);
                     self.num_declarations += style_rule.block.read_with(&guard).len();
                     for selector in &style_rule.selectors.0 {
                         self.num_selectors += 1;
                         self.add_rule_to_map(guard, selector, locked, stylesheet);
                         self.dependencies.note_selector(selector);
                         self.note_for_revalidation(selector);
+                        self.note_attribute_dependencies(selector);
                     }
                     self.rules_source_order += 1;
                 }
                 CssRule::Import(ref import) => {
                     let import = import.read_with(guard);
                     self.add_stylesheet(&import.stylesheet, guard, extra_data)
                 }
                 CssRule::Keyframes(ref keyframes_rule) => {
@@ -375,16 +396,31 @@ impl Stylist {
 
     #[inline]
     fn note_for_revalidation(&mut self, selector: &Selector<SelectorImpl>) {
         if needs_revalidation(selector) {
             self.selectors_for_cache_revalidation.insert(selector.inner.clone());
         }
     }
 
+    /// Returns whether the given attribute might appear in an attribute
+    /// selector of some rule in the stylist.
+    pub fn might_have_attribute_dependency(&self, local_name: &Atom) -> bool {
+        if *local_name == atom!("style") {
+            self.style_attribute_dependency
+        } else {
+            self.attribute_dependencies.might_contain(local_name)
+        }
+    }
+
+    #[inline]
+    fn note_attribute_dependencies(&mut self, selector: &Selector<SelectorImpl>) {
+        selector.visit(&mut AttributeDependencyVisitor(self));
+    }
+
     /// Computes the style for a given "precomputed" pseudo-element, taking the
     /// universal rules and applying them.
     ///
     /// If `inherit_all` is true, then all properties are inherited from the
     /// parent; otherwise, non-inherited properties are reset to their initial
     /// values. The flow constructor uses this flag when constructing anonymous
     /// flows.
     pub fn precomputed_values_for_pseudo(&self,
@@ -916,16 +952,35 @@ impl Drop for Stylist {
         // after this point.
         //
         // TODO(emilio): We can at least assert all the elements in the free
         // list are indeed free.
         unsafe { self.rule_tree.gc(); }
     }
 }
 
+/// Visitor to collect names that appear in attribute selectors.
+struct AttributeDependencyVisitor<'a>(&'a mut Stylist);
+
+impl<'a> SelectorVisitor for AttributeDependencyVisitor<'a> {
+    type Impl = SelectorImpl;
+
+    fn visit_attribute_selector(&mut self, selector: &AttrSelector<Self::Impl>) -> bool {
+        use precomputed_hash::PrecomputedHash;
+
+        if selector.lower_name == atom!("style") {
+            self.0.style_attribute_dependency = true;
+        } else {
+            self.0.attribute_dependencies.insert_hash(selector.name.precomputed_hash());
+            self.0.attribute_dependencies.insert_hash(selector.lower_name.precomputed_hash());
+        }
+        true
+    }
+}
+
 /// Visitor determine whether a selector requires cache revalidation.
 ///
 /// Note that we just check simple selectors and eagerly return when the first
 /// need for revalidation is found, so we don't need to store state on the
 /// visitor.
 ///
 /// Also, note that it's important to check the whole selector, due to cousins
 /// sharing arbitrarily deep in the DOM, not just the rightmost part of it
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -2261,8 +2261,15 @@ pub extern "C" fn Servo_StyleSet_Resolve
     };
 
     let declarations = Locked::<PropertyDeclarationBlock>::as_arc(&declarations);
 
     doc_data.stylist.compute_for_declarations(&guards,
                                               parent_style,
                                               declarations.clone()).into_strong()
 }
+
+#[no_mangle]
+pub extern "C" fn Servo_StyleSet_MightHaveAttributeDependency(raw_data: RawServoStyleSetBorrowed,
+                                                              local_name: *mut nsIAtom) -> bool {
+    let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
+    unsafe { Atom::with(local_name, &mut |atom| data.stylist.might_have_attribute_dependency(atom)) }
+}