diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 8dd3cd17aec..ee79d710563 100644 --- a/components/script/dom/document.rs +++ b/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() diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 3dbb35aa624..4b3a6010a9a 100644 --- a/components/script/dom/element.rs +++ b/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 { unsafe fn synthesize_presentational_hints_for_legacy_attributes(&self, hints: &mut V) where V: Push { + // 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 { } else if let Some(this) = self.downcast::() { // https://html.spec.whatwg.org/multipage/#the-hr-element-2:attr-hr-width this.get_width() + } else if let Some(this) = self.downcast::() { + 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 { this.get_height() } else if let Some(this) = self.downcast::() { this.get_height() + } else if let Some(this) = self.downcast::() { + 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::() 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 diff --git a/components/script/dom/htmlcanvaselement.rs b/components/script/dom/htmlcanvaselement.rs index dad4ebcc207..577537b421f 100644 --- a/components/script/dom/htmlcanvaselement.rs +++ b/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 { @@ -124,6 +126,26 @@ impl LayoutHTMLCanvasElementHelpers for LayoutJS { } } } + + #[allow(unsafe_code)] + fn get_width(&self) -> LengthOrPercentageOrAuto { + unsafe { + (&*self.upcast::().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::().unsafe_get()) + .get_attr_for_layout(&ns!(), &local_name!("height")) + .map(AttrValue::as_uint_px_dimension) + .unwrap_or(LengthOrPercentageOrAuto::Auto) + } + } } diff --git a/components/selectors/lib.rs b/components/selectors/lib.rs index 84bfecca058..fd8ccc6e52d 100644 --- a/components/selectors/lib.rs +++ b/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; diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 628ec971f5c..a0a2dc46aa0 100644 --- a/components/selectors/matching.rs +++ b/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( 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(..) | diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index d3987900301..fe0b5079988 100644 --- a/components/selectors/parser.rs +++ b/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; /// 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) -> 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, - _value: &Self::AttrValue) - -> bool { - false - } } } } @@ -143,93 +128,91 @@ pub struct Selector { pub specificity: u32, } -fn affects_sibling(simple_selector: &SimpleSelector) -> 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(simple_selector: &SimpleSelector) -> 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(&self, visitor: &mut V) -> bool + where V: SelectorVisitor; } impl SelectorMethods for Selector { - /// 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(&self, visitor: &mut V) -> bool + where V: SelectorVisitor, + { + self.complex_selector.visit(visitor) } } -impl SelectorMethods for ComplexSelector { - /// 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 SelectorMethods for Arc> { + type Impl = Impl; + + fn visit(&self, visitor: &mut V) -> bool + where V: SelectorVisitor, + { + let mut current = self; + let mut combinator = None; + loop { + if !visitor.visit_complex_selector(current, combinator) { + return false; + } + + for selector in ¤t.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 SelectorMethods for SimpleSelector { + type Impl = Impl; + + fn visit(&self, visitor: &mut V) -> bool + where V: SelectorVisitor, + { + 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(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(parser: &P, name: Cow) -> Result 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(&self, visitor: &mut V) -> bool + where V: SelectorVisitor { true } } #[derive(PartialEq, Debug)] diff --git a/components/selectors/visitor.rs b/components/selectors/visitor.rs new file mode 100644 index 00000000000..429c70c1cc8 --- /dev/null +++ b/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) -> 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>, + _combinator_to_right: Option) + -> bool { + true + } +} diff --git a/components/style/attr.rs b/components/style/attr.rs index 90154c151c7..04d37a37a70 100644 --- a/components/style/attr.rs +++ b/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 { diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index c6e4a524142..18f56ed1228 100644 --- a/components/style/gecko/selector_parser.rs +++ b/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>), + MozAny(Vec>>), } } } @@ -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(&self, visitor: &mut V) -> bool + where V: SelectorVisitor, + { + 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) -> bool { - attr_exists_selector_is_shareable(attr_selector) - } - - fn attr_equals_selector_is_shareable(attr_selector: &AttrSelector, - 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()) { diff --git a/components/style/matching.rs b/components/style/matching.rs index 3f2aa3c1e24..f0f288ef00f 100644 --- a/components/style/matching.rs +++ b/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(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 { /// The element. We use SendElement here so that the cache may live in /// ScopedTLS. element: SendElement, - /// The cached common style affecting attribute info. - common_style_affecting_attributes: Option, /// The cached class names. class_attributes: Option>, } impl PartialEq> for StyleSharingCandidate { 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(element: &E, @@ -175,12 +149,6 @@ fn element_matches_candidate(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(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(element: &E, Ok(current_styles.primary.clone()) } -fn have_same_common_style_affecting_attributes(element: &E, - candidate: &mut StyleSharingCandidate, - 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(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(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(element: &E, candidate: &mut StyleSharingCandidate, candidate_element: &E) -> bool { @@ -322,10 +204,10 @@ fn have_same_class(element: &E, // TODO: These re-match the candidate every time, which is suboptimal. #[inline] -fn match_same_not_common_style_affecting_attributes_rules(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(element: &E, + candidate: &E, + ctx: &SharedStyleContext) -> bool { + ctx.stylist.match_same_style_affecting_attributes_rules(element, candidate) } #[inline] @@ -387,7 +269,6 @@ impl StyleSharingCandidateCache { 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, _ => {} } } diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index c0827b788bb..dd04079c783 100644 --- a/components/style/restyle_hints.rs +++ b/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) -> bool { } } +fn is_sibling_affecting_selector(sel: &SimpleSelector) -> 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) -> 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>, + combinator: Option) + -> 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>) { - let mut cur = selector; - let mut combinator: Option = 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(); diff --git a/components/style/selector_parser.rs b/components/style/selector_parser.rs index 52f0a6b6d51..0664483cbe7 100644 --- a/components/style/selector_parser.rs +++ b/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) -> 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, - 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, - } - }) -} diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index a2be7c46415..e0277b31480 100644 --- a/components/style/servo/selector_parser.rs +++ b/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(&self, _: &mut V) -> bool + where V: SelectorVisitor + { + 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) -> bool { - attr_exists_selector_is_shareable(attr_selector) - } - - fn attr_equals_selector_is_shareable(attr_selector: &AttrSelector, - value: &Self::AttrValue) -> bool { - attr_equals_selector_is_shareable(attr_selector, value) - } } impl<'a> ::selectors::Parser for SelectorParser<'a> { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index afd78e1b4d2..2982457364a 100644 --- a/components/style/stylist.rs +++ b/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>, + style_affecting_attributes_selectors: Vec>, } /// 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(&self, - element: &E, - candidate: &E) -> bool + pub fn match_same_style_affecting_attributes_rules(&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(),