servo: Merge #16293 - style: Refactor the selectors crate to remove ad-hoc logic (from emilio:refactor-selectors); r=bholley

Also refactors a lot of the restyle hint code to use these new hooks, and removes a lot of hacks in servo that shouldn't be needed because of correct handling of restyles due to attribute changes.

I just smoketested this, so going for a full try run now.

Source-Repo: https://github.com/servo/servo
Source-Revision: 949931ec0caa20b9f38a12863f610da562950b90

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

servo/components/script/dom/document.rs

@@ -133,7 +133,7 @@ use std::sync::Arc;
 use std::time::{Duration, Instant};
 use style::attr::AttrValue;
 use style::context::{QuirksMode, ReflowGoal};
-use style::restyle_hints::{RestyleHint, RESTYLE_STYLE_ATTRIBUTE};
+use style::restyle_hints::{RestyleHint, RESTYLE_SELF, RESTYLE_STYLE_ATTRIBUTE};
 use style::selector_parser::{RestyleDamage, Snapshot};
 use style::shared_lock::SharedRwLock as StyleSharedRwLock;
 use style::str::{HTML_SPACE_CHARACTERS, split_html_space_chars, str_join};
@@ -2355,6 +2355,13 @@ impl Document {
             entry.hint |= RESTYLE_STYLE_ATTRIBUTE;
         }
 
+        // FIXME(emilio): This should become something like
+        // element.is_attribute_mapped(attr.local_name()).
+        if attr.local_name() == &local_name!("width") ||
+           attr.local_name() == &local_name!("height") {
+            entry.hint |= RESTYLE_SELF;
+       }
+
         let mut snapshot = entry.snapshot.as_mut().unwrap();
         if snapshot.attrs.is_none() {
             let attrs = el.attrs()

servo/components/script/dom/element.rs

@@ -41,6 +41,7 @@ use dom::eventtarget::EventTarget;
 use dom::htmlanchorelement::HTMLAnchorElement;
 use dom::htmlbodyelement::{HTMLBodyElement, HTMLBodyElementLayoutHelpers};
 use dom::htmlbuttonelement::HTMLButtonElement;
+use dom::htmlcanvaselement::{HTMLCanvasElement, LayoutHTMLCanvasElementHelpers};
 use dom::htmlcollection::HTMLCollection;
 use dom::htmlelement::HTMLElement;
 use dom::htmlfieldsetelement::HTMLFieldSetElement;
@@ -100,7 +101,6 @@ use std::sync::Arc;
 use style::attr::{AttrValue, LengthOrPercentageOrAuto};
 use style::context::{QuirksMode, ReflowGoal};
 use style::element_state::*;
-use style::matching::{common_style_affecting_attributes, rare_style_affecting_attributes};
 use style::properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock, parse_style_attribute};
 use style::properties::longhands::{self, background_image, border_spacing, font_family, font_size, overflow_x};
 use style::restyle_hints::RESTYLE_SELF;
@@ -382,6 +382,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
     unsafe fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, hints: &mut V)
         where V: Push<ApplicableDeclarationBlock>
     {
+        // FIXME(emilio): Just a single PDB should be enough.
         #[inline]
         fn from_declaration(shared_lock: &SharedRwLock, declaration: PropertyDeclaration)
                             -> ApplicableDeclarationBlock {
@@ -542,10 +543,13 @@ impl LayoutElementHelpers for LayoutJS<Element> {
         } else if let Some(this) = self.downcast::<HTMLHRElement>() {
             // https://html.spec.whatwg.org/multipage/#the-hr-element-2:attr-hr-width
             this.get_width()
+        } else if let Some(this) = self.downcast::<HTMLCanvasElement>() {
+            this.get_width()
         } else {
             LengthOrPercentageOrAuto::Auto
         };
 
+        // FIXME(emilio): Use from_computed value here and below.
         match width {
             LengthOrPercentageOrAuto::Auto => {}
             LengthOrPercentageOrAuto::Percentage(percentage) => {
@@ -569,6 +573,8 @@ impl LayoutElementHelpers for LayoutJS<Element> {
             this.get_height()
         } else if let Some(this) = self.downcast::<HTMLImageElement>() {
             this.get_height()
+        } else if let Some(this) = self.downcast::<HTMLCanvasElement>() {
+            this.get_height()
         } else {
             LengthOrPercentageOrAuto::Auto
         };
@@ -2153,10 +2159,6 @@ impl ElementMethods for Element {
     }
 }
 
-pub fn fragment_affecting_attributes() -> [LocalName; 3] {
-    [local_name!("width"), local_name!("height"), local_name!("src")]
-}
-
 impl VirtualMethods for Element {
     fn super_type(&self) -> Option<&VirtualMethods> {
         Some(self.upcast::<Node>() as &VirtualMethods)
@@ -2236,15 +2238,14 @@ impl VirtualMethods for Element {
                     }
                 }
             },
-            _ if attr.namespace() == &ns!() => {
-                if fragment_affecting_attributes().iter().any(|a| a == attr.local_name()) ||
-                   common_style_affecting_attributes().iter().any(|a| &a.attr_name == attr.local_name()) ||
-                   rare_style_affecting_attributes().iter().any(|a| a == attr.local_name())
-                {
+            _ => {
+                // FIXME(emilio): This is pretty dubious, and should be done in
+                // the relevant super-classes.
+                if attr.namespace() == &ns!() &&
+                   attr.local_name() == &local_name!("src") {
                     node.dirty(NodeDamage::OtherNodeDamage);
                 }
             },
-            _ => {},
         };
 
         // Make sure we rev the version even if we didn't dirty the node. If we

servo/components/script/dom/htmlcanvaselement.rs

@@ -36,7 +36,7 @@ use js::jsapi::{HandleValue, JSContext};
 use offscreen_gl_context::GLContextAttributes;
 use script_layout_interface::HTMLCanvasData;
 use std::iter::repeat;
-use style::attr::AttrValue;
+use style::attr::{AttrValue, LengthOrPercentageOrAuto};
 
 const DEFAULT_WIDTH: u32 = 300;
 const DEFAULT_HEIGHT: u32 = 150;
@@ -97,6 +97,8 @@ impl HTMLCanvasElement {
 
 pub trait LayoutHTMLCanvasElementHelpers {
     fn data(&self) -> HTMLCanvasData;
+    fn get_width(&self) -> LengthOrPercentageOrAuto;
+    fn get_height(&self) -> LengthOrPercentageOrAuto;
 }
 
 impl LayoutHTMLCanvasElementHelpers for LayoutJS<HTMLCanvasElement> {
@@ -124,6 +126,26 @@ impl LayoutHTMLCanvasElementHelpers for LayoutJS<HTMLCanvasElement> {
             }
         }
     }
+
+    #[allow(unsafe_code)]
+    fn get_width(&self) -> LengthOrPercentageOrAuto {
+        unsafe {
+            (&*self.upcast::<Element>().unsafe_get())
+                .get_attr_for_layout(&ns!(), &local_name!("width"))
+                .map(AttrValue::as_uint_px_dimension)
+                .unwrap_or(LengthOrPercentageOrAuto::Auto)
+        }
+    }
+
+    #[allow(unsafe_code)]
+    fn get_height(&self) -> LengthOrPercentageOrAuto {
+        unsafe {
+            (&*self.upcast::<Element>().unsafe_get())
+                .get_attr_for_layout(&ns!(), &local_name!("height"))
+                .map(AttrValue::as_uint_px_dimension)
+                .unwrap_or(LengthOrPercentageOrAuto::Auto)
+        }
+    }
 }
 
 

servo/components/selectors/lib.rs

@@ -12,6 +12,7 @@ pub mod bloom;
 pub mod matching;
 pub mod parser;
 mod tree;
+pub mod visitor;
 
 pub use parser::{SelectorImpl, Parser, SelectorList};
 pub use tree::Element;

servo/components/selectors/matching.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 use bloom::BloomFilter;
 use parser::{CaseSensitivity, Combinator, ComplexSelector, LocalName};
-use parser::{SimpleSelector, Selector, SelectorImpl};
+use parser::{SimpleSelector, Selector};
 use precomputed_hash::PrecomputedHash;
 use std::borrow::Borrow;
 use tree::Element;
@@ -35,10 +35,6 @@ bitflags! {
         /// Whether this element is affected by an ID selector.
         const AFFECTED_BY_ID_SELECTOR = 1 << 3,
 
-        /// Whether this element is affected by a non-common style-affecting
-        /// attribute.
-        const AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE_SELECTOR = 1 << 4,
-
         /// Whether this element matches the :empty pseudo class.
         const AFFECTED_BY_EMPTY = 1 << 5,
 
@@ -367,45 +363,28 @@ fn matches_simple_selector<E, F>(
             element.has_class(class)
         }
         SimpleSelector::AttrExists(ref attr) => {
-            let matches = element.match_attr_has(attr);
-
-            if matches && !E::Impl::attr_exists_selector_is_shareable(attr) {
-                *relations |= AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE_SELECTOR;
-            }
-
-            matches
+            element.match_attr_has(attr)
         }
         SimpleSelector::AttrEqual(ref attr, ref value, case_sensitivity) => {
-            let matches = match case_sensitivity {
+            match case_sensitivity {
                 CaseSensitivity::CaseSensitive => element.match_attr_equals(attr, value),
                 CaseSensitivity::CaseInsensitive => element.match_attr_equals_ignore_ascii_case(attr, value),
-            };
-
-            if matches && !E::Impl::attr_equals_selector_is_shareable(attr, value) {
-                *relations |= AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE_SELECTOR;
             }
-
-            matches
         }
         SimpleSelector::AttrIncludes(ref attr, ref value) => {
-            relation_if!(element.match_attr_includes(attr, value),
-                         AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE_SELECTOR)
+            element.match_attr_includes(attr, value)
         }
         SimpleSelector::AttrDashMatch(ref attr, ref value) => {
-            relation_if!(element.match_attr_dash(attr, value),
-                         AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE_SELECTOR)
+            element.match_attr_dash(attr, value)
         }
         SimpleSelector::AttrPrefixMatch(ref attr, ref value) => {
-            relation_if!(element.match_attr_prefix(attr, value),
-                         AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE_SELECTOR)
+            element.match_attr_prefix(attr, value)
         }
         SimpleSelector::AttrSubstringMatch(ref attr, ref value) => {
-            relation_if!(element.match_attr_substring(attr, value),
-                         AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE_SELECTOR)
+            element.match_attr_substring(attr, value)
         }
         SimpleSelector::AttrSuffixMatch(ref attr, ref value) => {
-            relation_if!(element.match_attr_suffix(attr, value),
-                         AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE_SELECTOR)
+            element.match_attr_suffix(attr, value)
         }
         SimpleSelector::AttrIncludesNeverMatch(..) |
         SimpleSelector::AttrPrefixNeverMatch(..) |

servo/components/selectors/parser.rs

@@ -12,6 +12,7 @@ use std::hash::Hash;
 use std::ops::Add;
 use std::sync::Arc;
 use tree::SELECTOR_WHITESPACE;
+use visitor::SelectorVisitor;
 
 macro_rules! with_all_bounds {
     (
@@ -50,26 +51,10 @@ macro_rules! with_all_bounds {
 
             /// non tree-structural pseudo-classes
             /// (see: https://drafts.csswg.org/selectors/#structural-pseudos)
-            type NonTSPseudoClass: $($CommonBounds)* + Sized + ToCss + SelectorMethods;
+            type NonTSPseudoClass: $($CommonBounds)* + Sized + ToCss + SelectorMethods<Impl = Self>;
 
             /// pseudo-elements
             type PseudoElement: $($CommonBounds)* + Sized + ToCss;
-
-            /// Declares if the following "attribute exists" selector is considered
-            /// "common" enough to be shareable. If that's not the case, when matching
-            /// over an element, the relation
-            /// AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE would be set.
-            fn attr_exists_selector_is_shareable(_attr_selector: &AttrSelector<Self>) -> bool {
-                false
-            }
-
-            /// Declares if the following "equals" attribute selector is considered
-            /// "common" enough to be shareable.
-            fn attr_equals_selector_is_shareable(_attr_selector: &AttrSelector<Self>,
-                                                 _value: &Self::AttrValue)
-                                                 -> bool {
-                false
-            }
         }
     }
 }
@@ -143,93 +128,91 @@ pub struct Selector<Impl: SelectorImpl> {
     pub specificity: u32,
 }
 
-fn affects_sibling<Impl: SelectorImpl>(simple_selector: &SimpleSelector<Impl>) -> bool {
-    match *simple_selector {
-        SimpleSelector::Negation(ref negated) => {
-            negated.iter().any(|ref selector| selector.affects_siblings())
-        }
-
-        SimpleSelector::FirstChild |
-        SimpleSelector::LastChild |
-        SimpleSelector::OnlyChild |
-        SimpleSelector::NthChild(..) |
-        SimpleSelector::NthLastChild(..) |
-        SimpleSelector::NthOfType(..) |
-        SimpleSelector::NthLastOfType(..) |
-        SimpleSelector::FirstOfType |
-        SimpleSelector::LastOfType |
-        SimpleSelector::OnlyOfType => true,
-
-        SimpleSelector::NonTSPseudoClass(ref pseudo_class) => pseudo_class.affects_siblings(),
-
-        _ => false,
-    }
-}
-
-fn matches_non_common_style_affecting_attribute<Impl: SelectorImpl>(simple_selector: &SimpleSelector<Impl>) -> bool {
-    match *simple_selector {
-        SimpleSelector::Negation(ref negated) => {
-            negated.iter().any(|ref selector| selector.matches_non_common_style_affecting_attribute())
-        }
-        SimpleSelector::AttrEqual(ref attr, ref val, _) => {
-            !Impl::attr_equals_selector_is_shareable(attr, val)
-        }
-        SimpleSelector::AttrExists(ref attr) => {
-            !Impl::attr_exists_selector_is_shareable(attr)
-        }
-        SimpleSelector::AttrIncludes(..) |
-        SimpleSelector::AttrDashMatch(..) |
-        SimpleSelector::AttrPrefixMatch(..) |
-        SimpleSelector::AttrSuffixMatch(..) |
-        SimpleSelector::AttrSubstringMatch(..) => true,
-
-        SimpleSelector::NonTSPseudoClass(ref pseudo_class) =>
-            pseudo_class.matches_non_common_style_affecting_attribute(),
-
-        // This deliberately includes Attr*NeverMatch
-        // which never match regardless of element attributes.
-        _ => false,
-    }
-}
-
 pub trait SelectorMethods {
-    fn affects_siblings(&self) -> bool;
-    fn matches_non_common_style_affecting_attribute(&self) -> bool;
+    type Impl: SelectorImpl;
+
+    fn visit<V>(&self, visitor: &mut V) -> bool
+        where V: SelectorVisitor<Impl = Self::Impl>;
 }
 
 impl<Impl: SelectorImpl> SelectorMethods for Selector<Impl> {
-    /// Whether this selector, if matching on a set of siblings, could affect
-    /// other sibling's style.
-    fn affects_siblings(&self) -> bool {
-        self.complex_selector.affects_siblings()
-    }
+    type Impl = Impl;
 
-    fn matches_non_common_style_affecting_attribute(&self) -> bool {
-        self.complex_selector.matches_non_common_style_affecting_attribute()
+    fn visit<V>(&self, visitor: &mut V) -> bool
+        where V: SelectorVisitor<Impl = Impl>,
+    {
+        self.complex_selector.visit(visitor)
     }
 }
 
-impl<Impl: SelectorImpl> SelectorMethods for ComplexSelector<Impl> {
-    /// Whether this complex selector, if matching on a set of siblings,
-    /// could affect other sibling's style.
-    fn affects_siblings(&self) -> bool {
-        match self.next {
-            Some((_, Combinator::NextSibling)) |
-            Some((_, Combinator::LaterSibling)) => return true,
-            _ => {},
+impl<Impl: SelectorImpl> SelectorMethods for Arc<ComplexSelector<Impl>> {
+    type Impl = Impl;
+
+    fn visit<V>(&self, visitor: &mut V) -> bool
+        where V: SelectorVisitor<Impl = Impl>,
+    {
+        let mut current = self;
+        let mut combinator = None;
+        loop {
+            if !visitor.visit_complex_selector(current, combinator) {
+                return false;
+            }
+
+            for selector in &current.compound_selector {
+                if !selector.visit(visitor) {
+                    return false;
+                }
+            }
+
+            match current.next {
+                Some((ref next, next_combinator)) => {
+                    current = next;
+                    combinator = Some(next_combinator);
+                }
+                None => break,
+            }
         }
 
-        match self.compound_selector.last() {
-            Some(ref selector) => affects_sibling(selector),
-            None => false,
-        }
+        true
     }
+}
 
-    fn matches_non_common_style_affecting_attribute(&self) -> bool {
-        match self.compound_selector.last() {
-            Some(ref selector) => matches_non_common_style_affecting_attribute(selector),
-            None => false,
+impl<Impl: SelectorImpl> SelectorMethods for SimpleSelector<Impl> {
+    type Impl = Impl;
+
+    fn visit<V>(&self, visitor: &mut V) -> bool
+        where V: SelectorVisitor<Impl = Impl>,
+    {
+        use self::SimpleSelector::*;
+
+        match *self {
+            Negation(ref negated) => {
+                for selector in negated {
+                    if !selector.visit(visitor) {
+                        return false;
+                    }
+                }
+            }
+            AttrExists(ref selector) |
+            AttrEqual(ref selector, _, _) |
+            AttrIncludes(ref selector, _) |
+            AttrDashMatch(ref selector, _) |
+            AttrPrefixMatch(ref selector, _) |
+            AttrSubstringMatch(ref selector, _) |
+            AttrSuffixMatch(ref selector, _) => {
+                if !visitor.visit_attribute_selector(selector) {
+                    return false;
+                }
+            }
+            NonTSPseudoClass(ref pseudo_class) => {
+                if !pseudo_class.visit(visitor) {
+                    return false;
+                }
+            },
+            _ => {}
         }
+
+        true
     }
 }
 
@@ -1023,9 +1006,8 @@ fn parse_functional_pseudo_class<P, Impl>(parser: &P,
         "not" => {
             if inside_negation {
                 return Err(())
-            } else {
-                return parse_negation(parser, input)
             }
+            return parse_negation(parser, input)
         },
         _ => {}
     }
@@ -1127,7 +1109,8 @@ fn parse_simple_pseudo_class<P, Impl>(parser: &P, name: Cow<str>) -> Result<Simp
         "only-of-type"  => Ok(SimpleSelector::OnlyOfType),
         _ => Err(())
     }).or_else(|()| {
-        P::parse_non_ts_pseudo_class(parser, name).map(|pc| SimpleSelector::NonTSPseudoClass(pc))
+        P::parse_non_ts_pseudo_class(parser, name)
+            .map(SimpleSelector::NonTSPseudoClass)
     })
 }
 
@@ -1176,8 +1159,10 @@ pub mod tests {
     }
 
     impl SelectorMethods for PseudoClass {
-        fn affects_siblings(&self) -> bool { false }
-        fn matches_non_common_style_affecting_attribute(&self) -> bool { false }
+        type Impl = DummySelectorImpl;
+
+        fn visit<V>(&self, visitor: &mut V) -> bool
+            where V: SelectorVisitor<Impl = Self::Impl> { true }
     }
 
     #[derive(PartialEq, Debug)]

servo/components/selectors/visitor.rs

@@ -0,0 +1,37 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+//! Visitor traits for selectors.
+
+#![deny(missing_docs)]
+
+use parser::{AttrSelector, Combinator, ComplexSelector, SelectorImpl};
+use std::sync::Arc;
+
+/// A trait to visit selector properties.
+///
+/// All the `visit_foo` methods return a boolean indicating whether the
+/// traversal should continue or not.
+pub trait SelectorVisitor {
+    /// The selector implementation this visitor wants to visit.
+    type Impl: SelectorImpl;
+
+    /// Visit an attribute selector that may match (there are other selectors
+    /// that may never match, like those containing whitespace or the empty
+    /// string).
+    fn visit_attribute_selector(&mut self, _: &AttrSelector<Self::Impl>) -> bool {
+        true
+    }
+
+    /// Visits a complex selector.
+    ///
+    /// Gets the combinator to the right of the selector, or `None` if the
+    /// selector is the leftmost one.
+    fn visit_complex_selector(&mut self,
+                              _: &Arc<ComplexSelector<Self::Impl>>,
+                              _combinator_to_right: Option<Combinator>)
+                              -> bool {
+        true
+    }
+}

servo/components/style/attr.rs

@@ -332,6 +332,23 @@ impl AttrValue {
             panic!("Uint not found");
         }
     }
+
+    /// Return the AttrValue as a dimension computed from its integer
+    /// representation, assuming that integer representation specifies pixels.
+    ///
+    /// This corresponds to attribute values returned as `AttrValue::UInt(_)`
+    /// by `VirtualMethods::parse_plain_attribute()`.
+    ///
+    /// ## Panics
+    ///
+    /// Panics if the `AttrValue` is not a `UInt`
+    pub fn as_uint_px_dimension(&self) -> LengthOrPercentageOrAuto {
+        if let AttrValue::UInt(_, value) = *self {
+            LengthOrPercentageOrAuto::Length(Au::from_px(value as i32))
+        } else {
+            panic!("Uint not found");
+        }
+    }
 }
 
 impl ::std::ops::Deref for AttrValue {

servo/components/style/gecko/selector_parser.rs

@@ -10,11 +10,12 @@ use gecko_bindings::structs::CSSPseudoClassType;
 use gecko_bindings::structs::nsIAtom;
 use restyle_hints::complex_selector_to_state;
 use selector_parser::{SelectorParser, PseudoElementCascadeType};
-use selector_parser::{attr_equals_selector_is_shareable, attr_exists_selector_is_shareable};
-use selectors::parser::{AttrSelector, ComplexSelector, SelectorMethods};
+use selectors::parser::{ComplexSelector, SelectorMethods};
+use selectors::visitor::SelectorVisitor;
 use std::borrow::Cow;
 use std::fmt;
 use std::ptr;
+use std::sync::Arc;
 use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 
 /// A representation of a CSS pseudo-element.
@@ -218,7 +219,7 @@ macro_rules! pseudo_class_name {
             ///
             /// TODO(emilio): We disallow combinators and pseudos here, so we
             /// should use SimpleSelector instead
-            MozAny(Vec<ComplexSelector<SelectorImpl>>),
+            MozAny(Vec<Arc<ComplexSelector<SelectorImpl>>>),
         }
     }
 }
@@ -261,24 +262,20 @@ impl ToCss for NonTSPseudoClass {
 }
 
 impl SelectorMethods for NonTSPseudoClass {
-    #[inline]
-    fn affects_siblings(&self) -> bool {
-        match *self {
-            NonTSPseudoClass::MozAny(ref selectors) => {
-                selectors.iter().any(|s| s.affects_siblings())
-            }
-            _ => false
-        }
-    }
+    type Impl = SelectorImpl;
 
-    #[inline]
-    fn matches_non_common_style_affecting_attribute(&self) -> bool {
-        match *self {
-            NonTSPseudoClass::MozAny(ref selectors) => {
-                selectors.iter().any(|s| s.matches_non_common_style_affecting_attribute())
+    fn visit<V>(&self, visitor: &mut V) -> bool
+        where V: SelectorVisitor<Impl = Self::Impl>,
+    {
+        if let NonTSPseudoClass::MozAny(ref selectors) = *self {
+            for selector in selectors {
+                if !selector.visit(visitor) {
+                    return false;
+                }
             }
-            _ => false
         }
+
+        true
     }
 }
 
@@ -364,15 +361,6 @@ impl ::selectors::SelectorImpl for SelectorImpl {
 
     type PseudoElement = PseudoElement;
     type NonTSPseudoClass = NonTSPseudoClass;
-
-    fn attr_exists_selector_is_shareable(attr_selector: &AttrSelector<Self>) -> bool {
-        attr_exists_selector_is_shareable(attr_selector)
-    }
-
-    fn attr_equals_selector_is_shareable(attr_selector: &AttrSelector<Self>,
-                                         value: &Self::AttrValue) -> bool {
-        attr_equals_selector_is_shareable(attr_selector, value)
-    }
 }
 
 impl<'a> ::selectors::Parser for SelectorParser<'a> {
@@ -413,7 +401,7 @@ impl<'a> ::selectors::Parser for SelectorParser<'a> {
                     }, )*
                     "-moz-any" => {
                         let selectors = parser.parse_comma_separated(|input| {
-                            ComplexSelector::parse(self, input)
+                            ComplexSelector::parse(self, input).map(Arc::new)
                         })?;
                         // Selectors inside `:-moz-any` may not include combinators.
                         if selectors.iter().any(|s| s.next.is_some()) {

servo/components/style/matching.rs

@@ -7,7 +7,7 @@
 #![allow(unsafe_code)]
 #![deny(missing_docs)]
 
-use {Atom, LocalName};
+use Atom;
 use animation::{self, Animation, PropertyAnimation};
 use atomic_refcell::AtomicRefMut;
 use cache::{LRUCache, LRUCacheMutIterator};
@@ -38,26 +38,6 @@ fn relations_are_shareable(relations: &StyleRelations) -> bool {
                           AFFECTED_BY_PRESENTATIONAL_HINTS)
 }
 
-fn create_common_style_affecting_attributes_from_element<E: TElement>(element: &E)
-                                                         -> CommonStyleAffectingAttributes {
-    let mut flags = CommonStyleAffectingAttributes::empty();
-    for attribute_info in &common_style_affecting_attributes() {
-        match attribute_info.mode {
-            CommonStyleAffectingAttributeMode::IsPresent(flag) => {
-                if element.has_attr(&ns!(), &attribute_info.attr_name) {
-                    flags.insert(flag)
-                }
-            }
-            CommonStyleAffectingAttributeMode::IsEqual(ref target_value, flag) => {
-                if element.attr_equals(&ns!(), &attribute_info.attr_name, target_value) {
-                    flags.insert(flag)
-                }
-            }
-        }
-    }
-    flags
-}
-
 /// Information regarding a style sharing candidate.
 ///
 /// Note that this information is stored in TLS and cleared after the traversal,
@@ -70,16 +50,13 @@ struct StyleSharingCandidate<E: TElement> {
     /// The element. We use SendElement here so that the cache may live in
     /// ScopedTLS.
     element: SendElement<E>,
-    /// The cached common style affecting attribute info.
-    common_style_affecting_attributes: Option<CommonStyleAffectingAttributes>,
     /// The cached class names.
     class_attributes: Option<Vec<Atom>>,
 }
 
 impl<E: TElement> PartialEq<StyleSharingCandidate<E>> for StyleSharingCandidate<E> {
     fn eq(&self, other: &Self) -> bool {
-        self.element == other.element &&
-            self.common_style_affecting_attributes == other.common_style_affecting_attributes
+        self.element == other.element
     }
 }
 
@@ -115,17 +92,14 @@ pub enum CacheMiss {
     StyleAttr,
     /// The element and the candidate class names didn't match.
     Class,
-    /// The element and the candidate common style affecting attributes didn't
-    /// match.
-    CommonStyleAffectingAttributes,
     /// The presentation hints didn't match.
     PresHints,
     /// The element and the candidate didn't match the same set of
     /// sibling-affecting rules.
     SiblingRules,
-    /// The element and the candidate didn't match the same set of non-common
-    /// style affecting attribute selectors.
-    NonCommonAttrRules,
+    /// The element and the candidate didn't match the same set of style
+    /// affecting attribute selectors.
+    AttrRules,
 }
 
 fn element_matches_candidate<E: TElement>(element: &E,
@@ -175,12 +149,6 @@ fn element_matches_candidate<E: TElement>(element: &E,
         miss!(Class)
     }
 
-    if !have_same_common_style_affecting_attributes(element,
-                                                    candidate,
-                                                    candidate_element) {
-        miss!(CommonStyleAffectingAttributes)
-    }
-
     if !have_same_presentational_hints(element, candidate_element) {
         miss!(PresHints)
     }
@@ -191,10 +159,10 @@ fn element_matches_candidate<E: TElement>(element: &E,
         miss!(SiblingRules)
     }
 
-    if !match_same_not_common_style_affecting_attributes_rules(element,
-                                                               candidate_element,
-                                                               shared_context) {
-        miss!(NonCommonAttrRules)
+    if !match_same_style_affecting_attributes_rules(element,
+                                                    candidate_element,
+                                                    shared_context) {
+        miss!(AttrRules)
     }
 
     let data = candidate_element.borrow_data().unwrap();
@@ -204,20 +172,10 @@ fn element_matches_candidate<E: TElement>(element: &E,
     Ok(current_styles.primary.clone())
 }
 
-fn have_same_common_style_affecting_attributes<E: TElement>(element: &E,
-                                                            candidate: &mut StyleSharingCandidate<E>,
-                                                            candidate_element: &E) -> bool {
-    if candidate.common_style_affecting_attributes.is_none() {
-        candidate.common_style_affecting_attributes =
-            Some(create_common_style_affecting_attributes_from_element(candidate_element))
-    }
-    create_common_style_affecting_attributes_from_element(element) ==
-        candidate.common_style_affecting_attributes.unwrap()
-}
-
 fn have_same_presentational_hints<E: TElement>(element: &E, candidate: &E) -> bool {
     let mut first = ForgetfulSink::new();
     element.synthesize_presentational_hints_for_legacy_attributes(&mut first);
+
     if cfg!(debug_assertions) {
         let mut second = vec![];
         candidate.synthesize_presentational_hints_for_legacy_attributes(&mut second);
@@ -228,82 +186,6 @@ fn have_same_presentational_hints<E: TElement>(element: &E, candidate: &E) -> bo
     first.is_empty()
 }
 
-bitflags! {
-    /// A set of common style-affecting attributes we check separately to
-    /// optimize the style sharing cache.
-    pub flags CommonStyleAffectingAttributes: u8 {
-        /// The `hidden` attribute.
-        const HIDDEN_ATTRIBUTE = 0x01,
-        /// The `nowrap` attribute.
-        const NO_WRAP_ATTRIBUTE = 0x02,
-        /// The `align="left"` attribute.
-        const ALIGN_LEFT_ATTRIBUTE = 0x04,
-        /// The `align="center"` attribute.
-        const ALIGN_CENTER_ATTRIBUTE = 0x08,
-        /// The `align="right"` attribute.
-        const ALIGN_RIGHT_ATTRIBUTE = 0x10,
-    }
-}
-
-/// The information of how to match a given common-style affecting attribute.
-pub struct CommonStyleAffectingAttributeInfo {
-    /// The attribute name.
-    pub attr_name: LocalName,
-    /// The matching mode for the attribute.
-    pub mode: CommonStyleAffectingAttributeMode,
-}
-
-/// How should we match a given common style-affecting attribute?
-#[derive(Clone)]
-pub enum CommonStyleAffectingAttributeMode {
-    /// Just for presence?
-    IsPresent(CommonStyleAffectingAttributes),
-    /// For presence and equality with a given value.
-    IsEqual(Atom, CommonStyleAffectingAttributes),
-}
-
-/// The common style affecting attribute array.
-///
-/// TODO: This should be a `const static` or similar, but couldn't be because
-/// `Atom`s have destructors.
-#[inline]
-pub fn common_style_affecting_attributes() -> [CommonStyleAffectingAttributeInfo; 5] {
-    [
-        CommonStyleAffectingAttributeInfo {
-            attr_name: local_name!("hidden"),
-            mode: CommonStyleAffectingAttributeMode::IsPresent(HIDDEN_ATTRIBUTE),
-        },
-        CommonStyleAffectingAttributeInfo {
-            attr_name: local_name!("nowrap"),
-            mode: CommonStyleAffectingAttributeMode::IsPresent(NO_WRAP_ATTRIBUTE),
-        },
-        CommonStyleAffectingAttributeInfo {
-            attr_name: local_name!("align"),
-            mode: CommonStyleAffectingAttributeMode::IsEqual(atom!("left"), ALIGN_LEFT_ATTRIBUTE),
-        },
-        CommonStyleAffectingAttributeInfo {
-            attr_name: local_name!("align"),
-            mode: CommonStyleAffectingAttributeMode::IsEqual(atom!("center"), ALIGN_CENTER_ATTRIBUTE),
-        },
-        CommonStyleAffectingAttributeInfo {
-            attr_name: local_name!("align"),
-            mode: CommonStyleAffectingAttributeMode::IsEqual(atom!("right"), ALIGN_RIGHT_ATTRIBUTE),
-        }
-    ]
-}
-
-/// Attributes that, if present, disable style sharing. All legacy HTML
-/// attributes must be in either this list or
-/// `common_style_affecting_attributes`. See the comment in
-/// `synthesize_presentational_hints_for_legacy_attributes`.
-///
-/// TODO(emilio): This is not accurate now, we don't disable style sharing for
-/// this now since we check for attribute selectors in the stylesheet. Consider
-/// removing this.
-pub fn rare_style_affecting_attributes() -> [LocalName; 4] {
-    [local_name!("bgcolor"), local_name!("border"), local_name!("colspan"), local_name!("rowspan")]
-}
-
 fn have_same_class<E: TElement>(element: &E,
                                 candidate: &mut StyleSharingCandidate<E>,
                                 candidate_element: &E) -> bool {
@@ -322,10 +204,10 @@ fn have_same_class<E: TElement>(element: &E,
 
 // TODO: These re-match the candidate every time, which is suboptimal.
 #[inline]
-fn match_same_not_common_style_affecting_attributes_rules<E: TElement>(element: &E,
-                                                                       candidate: &E,
-                                                                       ctx: &SharedStyleContext) -> bool {
-    ctx.stylist.match_same_not_common_style_affecting_attributes_rules(element, candidate)
+fn match_same_style_affecting_attributes_rules<E: TElement>(element: &E,
+                                                            candidate: &E,
+                                                            ctx: &SharedStyleContext) -> bool {
+    ctx.stylist.match_same_style_affecting_attributes_rules(element, candidate)
 }
 
 #[inline]
@@ -387,7 +269,6 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
 
         self.cache.insert(StyleSharingCandidate {
             element: unsafe { SendElement::new(*element) },
-            common_style_affecting_attributes: None,
             class_attributes: None,
         });
     }
@@ -1143,10 +1024,9 @@ pub trait MatchMethods : TElement {
                         },
                         // Too expensive failure, give up, we don't want another
                         // one of these.
-                        CacheMiss::CommonStyleAffectingAttributes |
                         CacheMiss::PresHints |
                         CacheMiss::SiblingRules |
-                        CacheMiss::NonCommonAttrRules => break,
+                        CacheMiss::AttrRules => break,
                         _ => {}
                     }
                 }

servo/components/style/restyle_hints.rs

@@ -18,6 +18,7 @@ use selectors::{Element, MatchAttr};
 use selectors::matching::{ElementSelectorFlags, StyleRelations};
 use selectors::matching::matches_complex_selector;
 use selectors::parser::{AttrSelector, Combinator, ComplexSelector, SimpleSelector};
+use selectors::visitor::SelectorVisitor;
 use std::clone::Clone;
 use std::sync::Arc;
 
@@ -399,6 +400,22 @@ fn is_attr_selector(sel: &SimpleSelector<SelectorImpl>) -> bool {
     }
 }
 
+fn is_sibling_affecting_selector(sel: &SimpleSelector<SelectorImpl>) -> bool {
+    match *sel {
+        SimpleSelector::FirstChild |
+        SimpleSelector::LastChild |
+        SimpleSelector::OnlyChild |
+        SimpleSelector::NthChild(..) |
+        SimpleSelector::NthLastChild(..) |
+        SimpleSelector::NthOfType(..) |
+        SimpleSelector::NthLastOfType(..) |
+        SimpleSelector::FirstOfType |
+        SimpleSelector::LastOfType |
+        SimpleSelector::OnlyOfType => true,
+        _ => false,
+    }
+}
+
 fn combinator_to_restyle_hint(combinator: Option<Combinator>) -> RestyleHint {
     match combinator {
         None => RESTYLE_SELF,
@@ -458,6 +475,74 @@ struct Dependency {
     sensitivities: Sensitivities,
 }
 
+/// A visitor struct that collects information for a given selector.
+///
+/// This is the struct responsible of adding dependencies for a given complex
+/// selector.
+pub struct SelectorDependencyVisitor<'a> {
+    dependency_set: &'a mut DependencySet,
+    affects_siblings: bool,
+    affected_by_attribute: bool,
+}
+
+impl<'a> SelectorDependencyVisitor<'a> {
+    /// Create a new `SelectorDependencyVisitor`.
+    pub fn new(dependency_set: &'a mut DependencySet) -> Self {
+        SelectorDependencyVisitor {
+            dependency_set: dependency_set,
+            affects_siblings: false,
+            affected_by_attribute: false,
+        }
+    }
+
+    /// Returns whether this visitor has known of a sibling-dependent selector.
+    pub fn affects_siblings(&self) -> bool {
+        self.affects_siblings
+    }
+
+    /// Returns whether this visitor has known of a attribute-dependent
+    /// selector.
+    pub fn affected_by_attribute(&self) -> bool {
+        self.affected_by_attribute
+    }
+}
+
+impl<'a> SelectorVisitor for SelectorDependencyVisitor<'a> {
+    type Impl = SelectorImpl;
+
+    fn visit_complex_selector(&mut self,
+                              selector: &Arc<ComplexSelector<SelectorImpl>>,
+                              combinator: Option<Combinator>)
+                              -> bool
+    {
+        let mut sensitivities = Sensitivities::new();
+        for s in &selector.compound_selector {
+            sensitivities.states.insert(selector_to_state(s));
+            if !self.affects_siblings {
+                self.affects_siblings = is_sibling_affecting_selector(s);
+            }
+            if !sensitivities.attrs {
+                sensitivities.attrs = is_attr_selector(s);
+            }
+        }
+
+        let hint = combinator_to_restyle_hint(combinator);
+
+        self.affected_by_attribute |= sensitivities.attrs;
+        self.affects_siblings |= hint.intersects(RESTYLE_LATER_SIBLINGS);
+
+        if !sensitivities.is_empty() {
+            self.dependency_set.add_dependency(Dependency {
+                selector: selector.clone(),
+                hint: hint,
+                sensitivities: sensitivities,
+            });
+        }
+
+        true
+    }
+}
+
 /// A set of dependencies for a given stylist.
 ///
 /// Note that there are measurable perf wins from storing them separately
@@ -476,12 +561,12 @@ pub struct DependencySet {
 
 impl DependencySet {
     fn add_dependency(&mut self, dep: Dependency) {
-        let affects_attrs = dep.sensitivities.attrs;
+        let affected_by_attribute = dep.sensitivities.attrs;
         let affects_states = !dep.sensitivities.states.is_empty();
 
-        if affects_attrs && affects_states {
+        if affected_by_attribute && affects_states {
             self.common_deps.push(dep)
-        } else if affects_attrs {
+        } else if affected_by_attribute {
             self.attr_deps.push(dep)
         } else {
             self.state_deps.push(dep)
@@ -502,46 +587,6 @@ impl DependencySet {
         self.common_deps.len() + self.attr_deps.len() + self.state_deps.len()
     }
 
-    /// Create the needed dependencies that a given selector creates, and add
-    /// them to the set.
-    pub fn note_selector(&mut self, selector: &Arc<ComplexSelector<SelectorImpl>>) {
-        let mut cur = selector;
-        let mut combinator: Option<Combinator> = None;
-        loop {
-            let mut sensitivities = Sensitivities::new();
-            for s in &cur.compound_selector {
-                sensitivities.states.insert(selector_to_state(s));
-                if !sensitivities.attrs {
-                    sensitivities.attrs = is_attr_selector(s);
-                }
-
-                // NOTE(emilio): I haven't thought this thoroughly, but we may
-                // not need to do anything for combinators inside negations.
-                //
-                // Or maybe we do, and need to call note_selector recursively
-                // here to account for them correctly, but keep the
-                // sensitivities of the parent?
-                //
-                // In any case, perhaps we should just drop it, see bug 1348802.
-            }
-            if !sensitivities.is_empty() {
-                self.add_dependency(Dependency {
-                    selector: cur.clone(),
-                    hint: combinator_to_restyle_hint(combinator),
-                    sensitivities: sensitivities,
-                });
-            }
-
-            cur = match cur.next {
-                Some((ref sel, comb)) => {
-                    combinator = Some(comb);
-                    sel
-                }
-                None => break,
-            }
-        }
-    }
-
     /// Clear this dependency set.
     pub fn clear(&mut self) {
         self.common_deps.clear();

servo/components/style/selector_parser.rs

@@ -7,9 +7,8 @@
 #![deny(missing_docs)]
 
 use cssparser::Parser as CssParser;
-use matching::{common_style_affecting_attributes, CommonStyleAffectingAttributeMode};
 use selectors::Element;
-use selectors::parser::{AttrSelector, SelectorList};
+use selectors::parser::SelectorList;
 use std::fmt::Debug;
 use stylesheets::{Origin, Namespaces};
 
@@ -130,41 +129,3 @@ impl SelectorImpl {
         })
     }
 }
-
-/// Checks whether we can share style if an element matches a given
-/// attribute-selector that checks for existence (`[attr_name]`) easily.
-///
-/// We could do the same thing that we do for sibling rules and keep optimizing
-/// these common attributes, but we'd have to measure how common it is.
-pub fn attr_exists_selector_is_shareable(attr_selector: &AttrSelector<SelectorImpl>) -> bool {
-    // NB(pcwalton): If you update this, remember to update the corresponding list in
-    // `can_share_style_with()` as well.
-    common_style_affecting_attributes().iter().any(|common_attr_info| {
-        common_attr_info.attr_name == attr_selector.name && match common_attr_info.mode {
-            CommonStyleAffectingAttributeMode::IsPresent(_) => true,
-            CommonStyleAffectingAttributeMode::IsEqual(..) => false,
-        }
-    })
-}
-
-/// Checks whether we can share style if an element matches a given
-/// attribute-selector that checks for equality (`[attr_name="attr_value"]`)
-/// easily.
-///
-/// We could do the same thing that we do for sibling rules and keep optimizing
-/// these common attributes, but we'd have to measure how common it is.
-pub fn attr_equals_selector_is_shareable(attr_selector: &AttrSelector<SelectorImpl>,
-                                         value: &AttrValue) -> bool {
-    // FIXME(pcwalton): Remove once we start actually supporting RTL text. This is in
-    // here because the UA style otherwise disables all style sharing completely.
-    // FIXME(SimonSapin): should this be the attribute *name* rather than value?
-    atom!("dir") == *value ||
-    common_style_affecting_attributes().iter().any(|common_attr_info| {
-        common_attr_info.attr_name == attr_selector.name && match common_attr_info.mode {
-            CommonStyleAffectingAttributeMode::IsEqual(ref target_value, _) => {
-                *target_value == *value
-            }
-            CommonStyleAffectingAttributeMode::IsPresent(_) => false,
-        }
-    })
-}

servo/components/style/servo/selector_parser.rs

@@ -12,10 +12,10 @@ use cssparser::{Parser as CssParser, ToCss, serialize_identifier};
 use element_state::ElementState;
 use restyle_hints::ElementSnapshot;
 use selector_parser::{ElementExt, PseudoElementCascadeType, SelectorParser};
-use selector_parser::{attr_equals_selector_is_shareable, attr_exists_selector_is_shareable};
 use selectors::{Element, MatchAttrGeneric};
 use selectors::matching::StyleRelations;
 use selectors::parser::{AttrSelector, SelectorMethods};
+use selectors::visitor::SelectorVisitor;
 use std::borrow::Cow;
 use std::fmt;
 use std::fmt::Debug;
@@ -199,10 +199,14 @@ impl ToCss for NonTSPseudoClass {
 }
 
 impl SelectorMethods for NonTSPseudoClass {
-    #[inline]
-    fn affects_siblings(&self) -> bool { false }
-    #[inline]
-    fn matches_non_common_style_affecting_attribute(&self) -> bool { false }
+    type Impl = SelectorImpl;
+
+
+    fn visit<V>(&self, _: &mut V) -> bool
+        where V: SelectorVisitor<Impl = Self::Impl>
+    {
+        true
+    }
 }
 
 impl NonTSPseudoClass {
@@ -251,15 +255,6 @@ impl ::selectors::SelectorImpl for SelectorImpl {
     type NamespaceUrl = Namespace;
     type BorrowedLocalName = LocalName;
     type BorrowedNamespaceUrl = Namespace;
-
-    fn attr_exists_selector_is_shareable(attr_selector: &AttrSelector<Self>) -> bool {
-        attr_exists_selector_is_shareable(attr_selector)
-    }
-
-    fn attr_equals_selector_is_shareable(attr_selector: &AttrSelector<Self>,
-                                         value: &Self::AttrValue) -> bool {
-        attr_equals_selector_is_shareable(attr_selector, value)
-    }
 }
 
 impl<'a> ::selectors::Parser for SelectorParser<'a> {

servo/components/style/stylist.rs

@@ -17,7 +17,7 @@ use properties::{self, CascadeFlags, ComputedValues};
 #[cfg(feature = "servo")]
 use properties::INHERIT_ALL;
 use properties::PropertyDeclarationBlock;
-use restyle_hints::{RestyleHint, DependencySet};
+use restyle_hints::{RestyleHint, DependencySet, SelectorDependencyVisitor};
 use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource};
 use selector_parser::{SelectorImpl, PseudoElement, Snapshot};
 use selectors::Element;
@@ -117,7 +117,7 @@ pub struct Stylist {
     /// Selectors in the page matching elements with non-common style-affecting
     /// attributes.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
-    non_common_style_affecting_attributes_selectors: Vec<Selector<SelectorImpl>>,
+    style_affecting_attributes_selectors: Vec<Selector<SelectorImpl>>,
 }
 
 /// This struct holds data which user of Stylist may want to extract
@@ -169,9 +169,8 @@ impl Stylist {
             rule_tree: RuleTree::new(),
             state_deps: DependencySet::new(),
 
-            // XXX remember resetting them!
             sibling_affecting_selectors: vec![],
-            non_common_style_affecting_attributes_selectors: vec![]
+            style_affecting_attributes_selectors: vec![]
         };
 
         SelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| {
@@ -226,7 +225,7 @@ impl Stylist {
         self.animations.clear();
 
         self.sibling_affecting_selectors.clear();
-        self.non_common_style_affecting_attributes_selectors.clear();
+        self.style_affecting_attributes_selectors.clear();
 
         extra_data.clear_font_faces();
 
@@ -249,7 +248,7 @@ impl Stylist {
         debug!(" - Got {} sibling-affecting selectors",
                self.sibling_affecting_selectors.len());
         debug!(" - Got {} non-common-style-attribute-affecting selectors",
-               self.non_common_style_affecting_attributes_selectors.len());
+               self.style_affecting_attributes_selectors.len());
         debug!(" - Got {} deps for style-hint calculation",
                self.state_deps.len());
 
@@ -300,13 +299,16 @@ impl Stylist {
                     self.rules_source_order += 1;
 
                     for selector in &style_rule.selectors.0 {
-                        self.state_deps.note_selector(&selector.complex_selector);
-                        if selector.affects_siblings() {
+                        let mut visitor =
+                            SelectorDependencyVisitor::new(&mut self.state_deps);
+                        selector.visit(&mut visitor);
+
+                        if visitor.affects_siblings() {
                             self.sibling_affecting_selectors.push(selector.clone());
                         }
 
-                        if selector.matches_non_common_style_affecting_attribute() {
-                            self.non_common_style_affecting_attributes_selectors.push(selector.clone());
+                        if visitor.affected_by_attribute() {
+                            self.style_affecting_attributes_selectors.push(selector.clone());
                         }
                     }
                 }
@@ -765,9 +767,9 @@ impl Stylist {
     ///
     /// This is used to test elements and candidates in the style-sharing
     /// candidate cache.
-    pub fn match_same_not_common_style_affecting_attributes_rules<E>(&self,
-                                                                     element: &E,
-                                                                     candidate: &E) -> bool
+    pub fn match_same_style_affecting_attributes_rules<E>(&self,
+                                                          element: &E,
+                                                          candidate: &E) -> bool
         where E: TElement,
     {
         use selectors::matching::StyleRelations;
@@ -779,7 +781,7 @@ impl Stylist {
         //
         // TODO(emilio): Use the bloom filter, since they contain the element's
         // ancestor chain and it's correct for the candidate too.
-        for ref selector in self.non_common_style_affecting_attributes_selectors.iter() {
+        for ref selector in self.style_affecting_attributes_selectors.iter() {
             let element_matches =
                 matches_complex_selector(&selector.complex_selector, element,
                                          None, &mut StyleRelations::empty(),