diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs index c44cfd4367d..29c3279ad08 100644 --- a/components/script/dom/attr.rs +++ b/components/script/dom/attr.rs @@ -170,7 +170,7 @@ impl AttrMethods for Attr { impl Attr { pub fn set_value(&self, mut value: AttrValue, owner: &Element) { assert!(Some(owner) == self.owner().r()); - owner.will_mutate_attr(); + owner.will_mutate_attr(self); self.swap_value(&mut value); if self.identifier.namespace == ns!() { vtable_for(owner.upcast()) diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index f04bd4017ee..12bf4386433 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -11,12 +11,13 @@ use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object}; use dom::bindings::str::DOMString; use dom::cssrule::CSSRule; use dom::element::Element; -use dom::node::{Node, NodeDamage, window_from_node}; +use dom::node::{Node, window_from_node}; use dom::window::Window; use parking_lot::RwLock; use servo_url::ServoUrl; use std::ascii::AsciiExt; use std::sync::Arc; +use style::attr::AttrValue; use style::parser::ParserContextExtraData; use style::properties::{Importance, PropertyDeclarationBlock, PropertyId, LonghandId, ShorthandId}; use style::properties::{parse_one_declaration, parse_style_attribute}; @@ -53,16 +54,12 @@ impl CSSStyleOwner { let mut changed = true; match *self { CSSStyleOwner::Element(ref el) => { - let mut attr = el.style_attribute().borrow_mut(); - let (result, needs_clear) = if attr.is_some() { + let mut attr = el.style_attribute().borrow_mut().take(); + let result = if attr.is_some() { let lock = attr.as_ref().unwrap(); let mut pdb = lock.write(); let result = f(&mut pdb, &mut changed); - if changed { - el.set_style_attr(pdb.to_css_string()); - el.upcast::().dirty(NodeDamage::NodeStyleDamaged); - } - (result, pdb.declarations.is_empty()) + result } else { let mut pdb = PropertyDeclarationBlock { important_count: 0, @@ -72,18 +69,32 @@ impl CSSStyleOwner { // Here `changed` is somewhat silly, because we know the // exact conditions under it changes. - if !pdb.declarations.is_empty() { - el.set_style_attr(pdb.to_css_string()); - el.upcast::().dirty(NodeDamage::NodeStyleDamaged); - *attr = Some(Arc::new(RwLock::new(pdb))); + changed = !pdb.declarations.is_empty(); + if changed { + attr = Some(Arc::new(RwLock::new(pdb))); } - (result, false) + result }; - if needs_clear { - *attr = None; + if changed { + // Note that there's no need to remove the attribute here if + // the declaration block is empty[1], and if `attr` is + // `None` it means that it necessarily didn't change, so no + // need to go through all the set_attribute machinery. + // + // [1]: https://github.com/whatwg/html/issues/2306 + if let Some(pdb) = attr { + let serialization = pdb.read().to_css_string(); + el.set_attribute(&local_name!("style"), + AttrValue::Declaration(serialization, + pdb)); + } + } else { + // Remember to put it back. + *el.style_attribute().borrow_mut() = attr; } + result } CSSStyleOwner::CSSRule(ref rule, ref pdb) => { diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index be44ae0a91c..b2e93325236 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -129,7 +129,7 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use style::attr::AttrValue; use style::context::{QuirksMode, ReflowGoal}; -use style::restyle_hints::RestyleHint; +use style::restyle_hints::{RestyleHint, RESTYLE_STYLE_ATTRIBUTE}; use style::selector_parser::{RestyleDamage, Snapshot}; use style::str::{split_html_space_chars, str_join}; use style::stylesheets::Stylesheet; @@ -179,7 +179,7 @@ impl PendingRestyle { } } -// https://dom.spec.whatwg.org/#document +/// https://dom.spec.whatwg.org/#document #[dom_struct] pub struct Document { node: Node, @@ -2142,23 +2142,32 @@ impl Document { RefMut::map(map, |m| m.entry(JS::from_ref(el)).or_insert_with(PendingRestyle::new)) } - pub fn ensure_snapshot(&self, el: &Element) -> RefMut { + pub fn element_state_will_change(&self, el: &Element) { let mut entry = self.ensure_pending_restyle(el); if entry.snapshot.is_none() { entry.snapshot = Some(Snapshot::new(el.html_element_in_html_document())); } - RefMut::map(entry, |e| e.snapshot.as_mut().unwrap()) - } - - pub fn element_state_will_change(&self, el: &Element) { - let mut snapshot = self.ensure_snapshot(el); + let mut snapshot = entry.snapshot.as_mut().unwrap(); if snapshot.state.is_none() { snapshot.state = Some(el.state()); } } - pub fn element_attr_will_change(&self, el: &Element) { - let mut snapshot = self.ensure_snapshot(el); + pub fn element_attr_will_change(&self, el: &Element, attr: &Attr) { + // FIXME(emilio): Kind of a shame we have to duplicate this. + // + // I'm getting rid of the whole hashtable soon anyway, since all it does + // right now is populate the element restyle data in layout, and we + // could in theory do it in the DOM I think. + let mut entry = self.ensure_pending_restyle(el); + if entry.snapshot.is_none() { + entry.snapshot = Some(Snapshot::new(el.html_element_in_html_document())); + } + if attr.local_name() == &local_name!("style") { + entry.hint |= RESTYLE_STYLE_ATTRIBUTE; + } + + let mut snapshot = entry.snapshot.as_mut().unwrap(); if snapshot.attrs.is_none() { let attrs = el.attrs() .iter() diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index f85baf3811e..e93becfffdd 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -105,6 +105,7 @@ use style::properties::{DeclaredValue, Importance}; use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, parse_style_attribute}; use style::properties::longhands::{background_image, border_spacing, font_family, font_size, overflow_x}; use style::restyle_hints::RESTYLE_SELF; +use style::rule_tree::CascadeLevel; use style::selector_parser::{NonTSPseudoClass, RestyleDamage, SelectorImpl, SelectorParser}; use style::sink::Push; use style::stylist::ApplicableDeclarationBlock; @@ -380,7 +381,7 @@ impl LayoutElementHelpers for LayoutJS { declarations: vec![(declaration, Importance::Normal)], important_count: 0, })), - Importance::Normal) + CascadeLevel::PresHints) } let bgcolor = if let Some(this) = self.downcast::() { @@ -824,31 +825,6 @@ impl Element { } } - // this sync method is called upon modification of the style_attribute property, - // therefore, it should not trigger subsequent mutation events - pub fn set_style_attr(&self, new_value: String) { - let mut new_style = AttrValue::String(new_value); - - if let Some(style_attr) = self.attrs.borrow().iter().find(|a| a.name() == &local_name!("style")) { - style_attr.swap_value(&mut new_style); - return; - } - - // explicitly not calling the push_new_attribute convenience method - // in order to avoid triggering mutation events - let window = window_from_node(self); - let attr = Attr::new(&window, - local_name!("style"), - new_style, - local_name!("style"), - ns!(), - None, - Some(self)); - - assert!(attr.GetOwnerElement().r() == Some(self)); - self.attrs.borrow_mut().push(JS::from_ref(&attr)); - } - pub fn serialize(&self, traversal_scope: TraversalScope) -> Fallible { let mut writer = vec![]; match serialize(&mut writer, @@ -962,7 +938,7 @@ impl Element { pub fn push_attribute(&self, attr: &Attr) { assert!(attr.GetOwnerElement().r() == Some(self)); - self.will_mutate_attr(); + self.will_mutate_attr(attr); self.attrs.borrow_mut().push(JS::from_ref(attr)); if attr.namespace() == &ns!() { vtable_for(self.upcast()).attribute_mutated(attr, AttributeMutation::Set(None)); @@ -1088,8 +1064,8 @@ impl Element { let idx = self.attrs.borrow().iter().position(|attr| find(&attr)); idx.map(|idx| { - self.will_mutate_attr(); let attr = Root::from_ref(&*(*self.attrs.borrow())[idx]); + self.will_mutate_attr(&attr); self.attrs.borrow_mut().remove(idx); attr.set_owner(None); if attr.namespace() == &ns!() { @@ -1227,9 +1203,9 @@ impl Element { self.set_attribute(local_name, AttrValue::UInt(value.to_string(), value)); } - pub fn will_mutate_attr(&self) { + pub fn will_mutate_attr(&self, attr: &Attr) { let node = self.upcast::(); - node.owner_doc().element_attr_will_change(self); + node.owner_doc().element_attr_will_change(self, attr); } // https://dom.spec.whatwg.org/#insert-adjacent @@ -1502,7 +1478,7 @@ impl ElementMethods for Element { } // Step 4. - self.will_mutate_attr(); + self.will_mutate_attr(attr); attr.set_owner(Some(self)); self.attrs.borrow_mut()[position] = JS::from_ref(attr); old_attr.set_owner(None); @@ -2121,18 +2097,43 @@ impl VirtualMethods for Element { match attr.local_name() { &local_name!("style") => { // Modifying the `style` attribute might change style. - *self.style_attribute.borrow_mut() = - mutation.new_value(attr).map(|value| { - let win = window_from_node(self); - Arc::new(RwLock::new(parse_style_attribute( - &value, - &doc.base_url(), - win.css_error_reporter(), - ParserContextExtraData::default()))) - }); - if node.is_in_doc() { - node.dirty(NodeDamage::NodeStyleDamaged); - } + *self.style_attribute.borrow_mut() = match mutation { + AttributeMutation::Set(..) => { + // This is the fast path we use from + // CSSStyleDeclaration. + // + // Juggle a bit to keep the borrow checker happy + // while avoiding the extra clone. + let is_declaration = match *attr.value() { + AttrValue::Declaration(..) => true, + _ => false, + }; + + let block = if is_declaration { + let mut value = AttrValue::String(String::new()); + attr.swap_value(&mut value); + let (serialization, block) = match value { + AttrValue::Declaration(s, b) => (s, b), + _ => unreachable!(), + }; + let mut value = AttrValue::String(serialization); + attr.swap_value(&mut value); + block + } else { + let win = window_from_node(self); + Arc::new(RwLock::new(parse_style_attribute( + &attr.value(), + &doc.base_url(), + win.css_error_reporter(), + ParserContextExtraData::default()))) + }; + + Some(block) + } + AttributeMutation::Removed => { + None + } + }; }, &local_name!("id") => { *self.id_attribute.borrow_mut() = @@ -2716,7 +2717,7 @@ impl Element { } } -#[derive(Clone, Copy, PartialEq)] +#[derive(Clone, Copy)] pub enum AttributeMutation<'a> { /// The attribute is set, keep track of old value. /// https://dom.spec.whatwg.org/#attribute-is-set @@ -2728,6 +2729,13 @@ pub enum AttributeMutation<'a> { } impl<'a> AttributeMutation<'a> { + pub fn is_removal(&self) -> bool { + match *self { + AttributeMutation::Removed => true, + AttributeMutation::Set(..) => false, + } + } + pub fn new_value<'b>(&self, attr: &'b Attr) -> Option> { match *self { AttributeMutation::Set(_) => Some(attr.value()), diff --git a/components/script/dom/htmllinkelement.rs b/components/script/dom/htmllinkelement.rs index 4db3949d4f9..20e560e93ad 100644 --- a/components/script/dom/htmllinkelement.rs +++ b/components/script/dom/htmllinkelement.rs @@ -166,7 +166,7 @@ impl VirtualMethods for HTMLLinkElement { fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) { self.super_type().unwrap().attribute_mutated(attr, mutation); - if !self.upcast::().is_in_doc() || mutation == AttributeMutation::Removed { + if !self.upcast::().is_in_doc() || mutation.is_removal() { return; } diff --git a/components/style/attr.rs b/components/style/attr.rs index 17517da2dd9..ab3097d5ae7 100644 --- a/components/style/attr.rs +++ b/components/style/attr.rs @@ -11,9 +11,12 @@ use app_units::Au; use cssparser::{self, Color, RGBA}; use euclid::num::Zero; use num_traits::ToPrimitive; +use parking_lot::RwLock; +use properties::PropertyDeclarationBlock; use servo_url::ServoUrl; use std::ascii::AsciiExt; use std::str::FromStr; +use std::sync::Arc; use str::{HTML_SPACE_CHARACTERS, read_exponent, read_fraction}; use str::{read_numbers, split_commas, split_html_space_chars}; #[cfg(not(feature = "gecko"))] use str::str_join; @@ -30,7 +33,7 @@ pub enum LengthOrPercentageOrAuto { Length(Au), } -#[derive(PartialEq, Clone, Debug)] +#[derive(Clone, Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum AttrValue { String(String), @@ -43,6 +46,22 @@ pub enum AttrValue { Color(String, Option), Dimension(String, LengthOrPercentageOrAuto), Url(String, Option), + + /// Note that this variant is only used transitively as a fast path to set + /// the property declaration block relevant to the style of an element when + /// set from the inline declaration of that element (that is, + /// `element.style`). + /// + /// This can, as of this writing, only correspond to the value of the + /// `style` element, and is set from its relevant CSSInlineStyleDeclaration, + /// and then converted to a string in Element::attribute_mutated. + /// + /// Note that we don't necessarily need to do that (we could just clone the + /// declaration block), but that avoids keeping a refcounted + /// declarationblock for longer than needed. + Declaration(String, + #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] + Arc>) } /// Shared implementation to parse an integer according to @@ -330,6 +349,7 @@ impl ::std::ops::Deref for AttrValue { AttrValue::Color(ref value, _) | AttrValue::Int(ref value, _) | AttrValue::Url(ref value, _) | + AttrValue::Declaration(ref value, _) | AttrValue::Dimension(ref value, _) => &value, AttrValue::Atom(ref value) => &value, } diff --git a/components/style/data.rs b/components/style/data.rs index 475a04b7023..0c097b103a1 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -203,7 +203,7 @@ impl From for StoredRestyleHint { use self::DescendantRestyleHint::*; debug_assert!(!hint.contains(RESTYLE_LATER_SIBLINGS), "Caller should apply sibling hints"); StoredRestyleHint { - restyle_self: hint.contains(RESTYLE_SELF), + restyle_self: hint.contains(RESTYLE_SELF) || hint.contains(RESTYLE_STYLE_ATTRIBUTE), descendants: if hint.contains(RESTYLE_DESCENDANTS) { Descendants } else { Empty }, } } diff --git a/components/style/matching.rs b/components/style/matching.rs index 11c1a85b382..51f05b8bdec 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -573,7 +573,7 @@ fn compute_rule_node(context: &StyleContext, applicable_declarations: &mut Vec) -> StrongRuleNode { - let rules = applicable_declarations.drain(..).map(|d| (d.source, d.importance)); + let rules = applicable_declarations.drain(..).map(|d| (d.source, d.level)); let rule_node = context.shared.stylist.rule_tree.insert_ordered_rules(rules); rule_node } @@ -583,10 +583,14 @@ impl PrivateMatchMethods for E {} /// The public API that elements expose for selector matching. pub trait MatchMethods : TElement { /// Runs selector matching of this element, and returns the result. - fn match_element(&self, context: &StyleContext, parent_bf: Option<&BloomFilter>) + fn match_element(&self, + context: &StyleContext, + parent_bf: Option<&BloomFilter>) -> MatchResults { - let mut applicable_declarations: Vec = Vec::with_capacity(16); + let mut applicable_declarations = + Vec::::with_capacity(16); + let stylist = &context.shared.stylist; let style_attribute = self.style_attribute(); let animation_rules = self.get_animation_rules(None); @@ -609,7 +613,7 @@ pub trait MatchMethods : TElement { let pseudo_animation_rules = self.get_animation_rules(Some(&pseudo)); stylist.push_applicable_declarations(self, parent_bf, None, pseudo_animation_rules, - Some(&pseudo.clone()), + Some(&pseudo), &mut applicable_declarations, MatchingReason::ForStyling); diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index 89a8d273595..8528ef116b6 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -30,14 +30,23 @@ bitflags! { /// expensive, and so we use RestyleHints to short-circuit work we know is /// unnecessary. pub flags RestyleHint: u32 { - #[doc = "Rerun selector matching on the element."] + /// Rerun selector matching on the element. const RESTYLE_SELF = 0x01, - #[doc = "Rerun selector matching on all of the element's descendants."] - // NB: In Gecko, we have RESTYLE_SUBTREE which is inclusive of self, but heycam isn't aware - // of a good reason for that. + + /// Rerun selector matching on all of the element's descendants. + /// + /// NB: In Gecko, we have RESTYLE_SUBTREE which is inclusive of self, + /// but heycam isn't aware of a good reason for that. const RESTYLE_DESCENDANTS = 0x02, - #[doc = "Rerun selector matching on all later siblings of the element and all of their descendants."] + + /// Rerun selector matching on all later siblings of the element and all + /// of their descendants. const RESTYLE_LATER_SIBLINGS = 0x08, + + /// Don't re-run selector-matching on the element, only the style + /// attribute has changed, and this change didn't have any other + /// dependencies. + const RESTYLE_STYLE_ATTRIBUTE = 0x10, } } diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 932b30a8125..ee5ac080b49 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -151,11 +151,11 @@ impl RuleTree { /// Insert the given rules, that must be in proper order by specifity, and /// return the corresponding rule node representing the last inserted one. pub fn insert_ordered_rules<'a, I>(&self, iter: I) -> StrongRuleNode - where I: Iterator, + where I: Iterator, { let mut current = self.root.clone(); - for (source, importance) in iter { - current = current.ensure_child(self.root.downgrade(), source, importance); + for (source, level) in iter { + current = current.ensure_child(self.root.downgrade(), source, level); } current } @@ -176,6 +176,66 @@ impl RuleTree { /// where it likely did not result from a rigorous performance analysis.) const RULE_TREE_GC_INTERVAL: usize = 300; +/// The cascade level these rules are relevant at, as per[1]. +/// +/// The order of variants declared here is significant, and must be in +/// _ascending_ order of precedence. +/// +/// [1]: https://drafts.csswg.org/css-cascade/#cascade-origin +#[repr(u8)] +#[derive(Eq, PartialEq, Copy, Clone, Debug)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub enum CascadeLevel { + /// Normal User-Agent rules. + UANormal = 0, + /// Presentational hints. + PresHints, + /// User normal rules. + UserNormal, + /// Author normal rules. + AuthorNormal, + /// Style attribute normal rules. + StyleAttributeNormal, + /// CSS animations and script-generated animations. + Animations, + /// Author-supplied important rules. + AuthorImportant, + /// Style attribute important rules. + StyleAttributeImportant, + /// User important rules. + UserImportant, + /// User-agent important rules. + UAImportant, + /// Transitions + Transitions, +} + +impl CascadeLevel { + /// Returns whether this cascade level represents important rules of some + /// sort. + #[inline] + pub fn is_important(&self) -> bool { + match *self { + CascadeLevel::AuthorImportant | + CascadeLevel::StyleAttributeImportant | + CascadeLevel::UserImportant | + CascadeLevel::UAImportant => true, + _ => false, + } + } + + /// Returns the importance relevant for this rule. Pretty similar to + /// `is_important`. + #[inline] + pub fn importance(&self) -> Importance { + if self.is_important() { + Importance::Important + } else { + Importance::Normal + } + } +} + struct RuleNode { /// The root node. Only the root has no root pointer, for obvious reasons. root: Option, @@ -187,9 +247,8 @@ struct RuleNode { /// or a raw property declaration block (like the style attribute). source: Option, - /// The importance of the declarations relevant in the style rule, - /// meaningless in the root node. - importance: Importance, + /// The cascade level this rule is positioned at. + cascade_level: CascadeLevel, refcount: AtomicUsize, first_child: AtomicPtr, @@ -215,13 +274,13 @@ impl RuleNode { fn new(root: WeakRuleNode, parent: StrongRuleNode, source: StyleSource, - importance: Importance) -> Self { + cascade_level: CascadeLevel) -> Self { debug_assert!(root.upgrade().parent().is_none()); RuleNode { root: Some(root), parent: Some(parent), source: Some(source), - importance: importance, + cascade_level: cascade_level, refcount: AtomicUsize::new(1), first_child: AtomicPtr::new(ptr::null_mut()), next_sibling: AtomicPtr::new(ptr::null_mut()), @@ -236,7 +295,7 @@ impl RuleNode { root: None, parent: None, source: None, - importance: Importance::Normal, + cascade_level: CascadeLevel::UANormal, refcount: AtomicUsize::new(1), first_child: AtomicPtr::new(ptr::null_mut()), next_sibling: AtomicPtr::new(ptr::null_mut()), @@ -385,10 +444,10 @@ impl StrongRuleNode { fn ensure_child(&self, root: WeakRuleNode, source: StyleSource, - importance: Importance) -> StrongRuleNode { + cascade_level: CascadeLevel) -> StrongRuleNode { let mut last = None; for child in self.get().iter_children() { - if child .get().importance == importance && + if child .get().cascade_level == cascade_level && child.get().source.as_ref().unwrap().ptr_equals(&source) { return child; } @@ -398,7 +457,7 @@ impl StrongRuleNode { let mut node = Box::new(RuleNode::new(root, self.clone(), source.clone(), - importance)); + cascade_level)); let new_ptr: *mut RuleNode = &mut *node; loop { @@ -462,7 +521,7 @@ impl StrongRuleNode { /// Get the importance that this rule node represents. pub fn importance(&self) -> Importance { - self.get().importance + self.get().cascade_level.importance() } /// Get an iterator for this rule node and its ancestors. diff --git a/components/style/stylist.rs b/components/style/stylist.rs index ab2a9fc14fe..3d81e3897bc 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -14,10 +14,10 @@ use keyframes::KeyframesAnimation; use media_queries::Device; use parking_lot::RwLock; use pdqsort::sort_by; -use properties::{self, CascadeFlags, ComputedValues, INHERIT_ALL, Importance}; -use properties::{PropertyDeclaration, PropertyDeclarationBlock}; +use properties::{self, CascadeFlags, ComputedValues, INHERIT_ALL}; +use properties::PropertyDeclarationBlock; use restyle_hints::{RestyleHint, DependencySet}; -use rule_tree::{RuleTree, StrongRuleNode, StyleSource}; +use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource}; use selector_parser::{ElementExt, SelectorImpl, PseudoElement, Snapshot}; use selectors::Element; use selectors::bloom::BloomFilter; @@ -31,7 +31,6 @@ use std::borrow::Borrow; use std::collections::HashMap; use std::fmt; use std::hash::Hash; -use std::slice; use std::sync::Arc; use style_traits::viewport::ViewportConstraints; use stylesheets::{CssRule, Origin, StyleRule, Stylesheet, UserAgentStylesheets}; @@ -215,7 +214,9 @@ impl Stylist { // extra field, and avoid this precomputation entirely. if let Some(map) = self.pseudos_map.remove(&pseudo) { let mut declarations = vec![]; - map.user_agent.get_universal_rules(&mut declarations); + map.user_agent.get_universal_rules(&mut declarations, + CascadeLevel::UANormal, + CascadeLevel::UAImportant); self.precomputed_pseudo_element_decls.insert(pseudo, declarations); } }); @@ -311,7 +312,7 @@ impl Stylist { // FIXME(emilio): When we've taken rid of the cascade we can just // use into_iter. self.rule_tree.insert_ordered_rules( - declarations.into_iter().map(|a| (a.source.clone(), a.importance))) + declarations.into_iter().map(|a| (a.source.clone(), a.level))) } None => self.rule_tree.root(), }; @@ -394,7 +395,7 @@ impl Stylist { let rule_node = self.rule_tree.insert_ordered_rules( - declarations.into_iter().map(|a| (a.source, a.importance))); + declarations.into_iter().map(|a| (a.source, a.level))); let computed = properties::cascade(self.device.au_viewport_size(), @@ -527,13 +528,18 @@ impl Stylist { applicable_declarations, &mut relations, reason, - Importance::Normal); + CascadeLevel::UANormal); debug!("UA normal: {:?}", relations); // Step 2: Presentational hints. - let length = applicable_declarations.len(); + let length_before_preshints = applicable_declarations.len(); element.synthesize_presentational_hints_for_legacy_attributes(applicable_declarations); - if applicable_declarations.len() != length { + if applicable_declarations.len() != length_before_preshints { + if cfg!(debug_assertions) { + for declaration in &applicable_declarations[length_before_preshints..] { + assert_eq!(declaration.level, CascadeLevel::PresHints); + } + } // Never share style for elements with preshints relations |= AFFECTED_BY_PRESENTATIONAL_HINTS; } @@ -546,14 +552,14 @@ impl Stylist { applicable_declarations, &mut relations, reason, - Importance::Normal); + CascadeLevel::UserNormal); debug!("user normal: {:?}", relations); map.author.get_all_matching_rules(element, parent_bf, applicable_declarations, &mut relations, reason, - Importance::Normal); + CascadeLevel::AuthorNormal); debug!("author normal: {:?}", relations); // Step 4: Normal style attributes. @@ -562,7 +568,8 @@ impl Stylist { relations |= AFFECTED_BY_STYLE_ATTRIBUTE; Push::push( applicable_declarations, - ApplicableDeclarationBlock::from_declarations(sa.clone(), Importance::Normal)); + ApplicableDeclarationBlock::from_declarations(sa.clone(), + CascadeLevel::StyleAttributeNormal)); } } @@ -575,7 +582,8 @@ impl Stylist { relations |= AFFECTED_BY_ANIMATIONS; Push::push( applicable_declarations, - ApplicableDeclarationBlock::from_declarations(anim.clone(), Importance::Normal)); + ApplicableDeclarationBlock::from_declarations(anim.clone(), + CascadeLevel::Animations)); } debug!("animation: {:?}", relations); @@ -585,7 +593,7 @@ impl Stylist { applicable_declarations, &mut relations, reason, - Importance::Important); + CascadeLevel::AuthorImportant); debug!("author important: {:?}", relations); @@ -595,7 +603,8 @@ impl Stylist { relations |= AFFECTED_BY_STYLE_ATTRIBUTE; Push::push( applicable_declarations, - ApplicableDeclarationBlock::from_declarations(sa.clone(), Importance::Important)); + ApplicableDeclarationBlock::from_declarations(sa.clone(), + CascadeLevel::StyleAttributeImportant)); } } @@ -607,7 +616,7 @@ impl Stylist { applicable_declarations, &mut relations, reason, - Importance::Important); + CascadeLevel::UserImportant); debug!("user important: {:?}", relations); } else { @@ -620,7 +629,7 @@ impl Stylist { applicable_declarations, &mut relations, reason, - Importance::Important); + CascadeLevel::UAImportant); debug!("UA important: {:?}", relations); @@ -630,7 +639,7 @@ impl Stylist { relations |= AFFECTED_BY_TRANSITIONS; Push::push( applicable_declarations, - ApplicableDeclarationBlock::from_declarations(anim.clone(), Importance::Normal)); + ApplicableDeclarationBlock::from_declarations(anim.clone(), CascadeLevel::Transitions)); } debug!("transition: {:?}", relations); @@ -858,7 +867,7 @@ impl SelectorMap { matching_rules_list: &mut V, relations: &mut StyleRelations, reason: MatchingReason, - importance: Importance) + cascade_level: CascadeLevel) where E: Element, V: VecLike { @@ -876,7 +885,7 @@ impl SelectorMap { matching_rules_list, relations, reason, - importance) + cascade_level) } element.each_class(|class| { @@ -887,7 +896,7 @@ impl SelectorMap { matching_rules_list, relations, reason, - importance); + cascade_level); }); let local_name_hash = if element.is_html_element_in_html_document() { @@ -902,7 +911,7 @@ impl SelectorMap { matching_rules_list, relations, reason, - importance); + cascade_level); SelectorMap::get_matching_rules(element, parent_bf, @@ -910,7 +919,7 @@ impl SelectorMap { matching_rules_list, relations, reason, - importance); + cascade_level); // Sort only the rules we just added. sort_by_key(&mut matching_rules_list[init_len..], @@ -918,11 +927,15 @@ impl SelectorMap { } /// Append to `rule_list` all universal Rules (rules with selector `*|*`) in - /// `self` sorted by specifity and source order. + /// `self` sorted by specificity and source order. pub fn get_universal_rules(&self, - matching_rules_list: &mut V) + matching_rules_list: &mut V, + cascade_level: CascadeLevel, + important_cascade_level: CascadeLevel) where V: VecLike { + debug_assert!(!cascade_level.is_important()); + debug_assert!(important_cascade_level.is_important()); if self.empty { return } @@ -936,11 +949,11 @@ impl SelectorMap { let block = guard.block.read(); if block.any_normal() { matching_rules_list.push( - rule.to_applicable_declaration_block(Importance::Normal)); + rule.to_applicable_declaration_block(cascade_level)); } if block.any_important() { matching_rules_list.push( - rule.to_applicable_declaration_block(Importance::Important)); + rule.to_applicable_declaration_block(important_cascade_level)); } } } @@ -957,7 +970,7 @@ impl SelectorMap { matching_rules: &mut Vector, relations: &mut StyleRelations, reason: MatchingReason, - importance: Importance) + cascade_level: CascadeLevel) where E: Element, Str: Borrow + Eq + Hash, BorrowedStr: Eq + Hash, @@ -970,7 +983,7 @@ impl SelectorMap { matching_rules, relations, reason, - importance) + cascade_level) } } @@ -981,14 +994,14 @@ impl SelectorMap { matching_rules: &mut V, relations: &mut StyleRelations, reason: MatchingReason, - importance: Importance) + cascade_level: CascadeLevel) where E: Element, V: VecLike { for rule in rules.iter() { let guard = rule.style_rule.read(); let block = guard.block.read(); - let any_declaration_for_importance = if importance.important() { + let any_declaration_for_importance = if cascade_level.is_important() { block.any_important() } else { block.any_normal() @@ -997,7 +1010,7 @@ impl SelectorMap { matches_complex_selector(&*rule.selector, element, parent_bf, relations, reason) { matching_rules.push( - rule.to_applicable_declaration_block(importance)); + rule.to_applicable_declaration_block(cascade_level)); } } } @@ -1093,10 +1106,10 @@ pub struct Rule { } impl Rule { - fn to_applicable_declaration_block(&self, importance: Importance) -> ApplicableDeclarationBlock { + fn to_applicable_declaration_block(&self, level: CascadeLevel) -> ApplicableDeclarationBlock { ApplicableDeclarationBlock { source: StyleSource::Style(self.style_rule.clone()), - importance: importance, + level: level, source_order: self.source_order, specificity: self.specificity, } @@ -1114,8 +1127,8 @@ pub struct ApplicableDeclarationBlock { /// The style source, either a style rule, or a property declaration block. #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] pub source: StyleSource, - /// The importance of this declaration block. - pub importance: Importance, + /// The cascade level this applicable declaration block is in. + pub level: CascadeLevel, /// The source order of this block. pub source_order: usize, /// The specificity of the selector this block is represented by. @@ -1127,50 +1140,17 @@ impl ApplicableDeclarationBlock { /// declaration block and importance. #[inline] pub fn from_declarations(declarations: Arc>, - importance: Importance) + level: CascadeLevel) -> Self { ApplicableDeclarationBlock { source: StyleSource::Declarations(declarations), + level: level, source_order: 0, specificity: 0, - importance: importance, } } } -/// An iterator over the declarations that a given block represent, which is -/// effectively a filter by importance. -pub struct ApplicableDeclarationBlockIter<'a> { - iter: slice::Iter<'a, (PropertyDeclaration, Importance)>, - importance: Importance, -} - -impl<'a> Iterator for ApplicableDeclarationBlockIter<'a> { - type Item = &'a PropertyDeclaration; - - #[inline] - fn next(&mut self) -> Option { - while let Some(&(ref declaration, importance)) = self.iter.next() { - if importance == self.importance { - return Some(declaration) - } - } - None - } -} - -impl<'a> DoubleEndedIterator for ApplicableDeclarationBlockIter<'a> { - #[inline] - fn next_back(&mut self) -> Option { - while let Some(&(ref declaration, importance)) = self.iter.next_back() { - if importance == self.importance { - return Some(declaration) - } - } - None - } -} - #[inline] fn find_push(map: &mut FnvHashMap>, key: Str, value: Rule) { diff --git a/tests/unit/style/rule_tree/bench.rs b/tests/unit/style/rule_tree/bench.rs index 2504c6f2893..de68d99b71d 100644 --- a/tests/unit/style/rule_tree/bench.rs +++ b/tests/unit/style/rule_tree/bench.rs @@ -11,7 +11,7 @@ use style::error_reporting::ParseErrorReporter; use style::media_queries::MediaList; use style::parser::ParserContextExtraData; use style::properties::{longhands, DeclaredValue, Importance, PropertyDeclaration, PropertyDeclarationBlock}; -use style::rule_tree::{RuleTree, StrongRuleNode, StyleSource}; +use style::rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource}; use style::stylesheets::{Origin, Stylesheet, CssRule}; use test::{self, Bencher}; @@ -40,7 +40,7 @@ impl<'a> Drop for AutoGCRuleTree<'a> { } } -fn parse_rules(css: &str) -> Vec<(StyleSource, Importance)> { +fn parse_rules(css: &str) -> Vec<(StyleSource, CascadeLevel)> { let s = Stylesheet::from_str(css, ServoUrl::parse("http://localhost").unwrap(), Origin::Author, @@ -57,15 +57,15 @@ fn parse_rules(css: &str) -> Vec<(StyleSource, Importance)> { _ => None, } }).cloned().map(StyleSource::Style).map(|s| { - (s, Importance::Normal) + (s, CascadeLevel::UserNormal) }).collect() } -fn test_insertion(rule_tree: &RuleTree, rules: Vec<(StyleSource, Importance)>) -> StrongRuleNode { +fn test_insertion(rule_tree: &RuleTree, rules: Vec<(StyleSource, CascadeLevel)>) -> StrongRuleNode { rule_tree.insert_ordered_rules(rules.into_iter()) } -fn test_insertion_style_attribute(rule_tree: &RuleTree, rules: &[(StyleSource, Importance)]) -> StrongRuleNode { +fn test_insertion_style_attribute(rule_tree: &RuleTree, rules: &[(StyleSource, CascadeLevel)]) -> StrongRuleNode { let mut rules = rules.to_vec(); rules.push((StyleSource::Declarations(Arc::new(RwLock::new(PropertyDeclarationBlock { declarations: vec![ @@ -74,7 +74,7 @@ fn test_insertion_style_attribute(rule_tree: &RuleTree, rules: &[(StyleSource, I Importance::Normal), ], important_count: 0, - }))), Importance::Normal)); + }))), CascadeLevel::UserNormal)); test_insertion(rule_tree, rules) } diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index 98c5b68a2ef..fe330d865c7 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -9,6 +9,7 @@ use servo_atoms::Atom; use std::sync::Arc; use style::properties::{PropertyDeclarationBlock, PropertyDeclaration, DeclaredValue}; use style::properties::{longhands, Importance}; +use style::rule_tree::CascadeLevel; use style::selector_parser::SelectorParser; use style::stylesheets::StyleRule; use style::stylist::{Rule, SelectorMap}; @@ -113,7 +114,9 @@ fn test_get_universal_rules() { let map = get_mock_map(&["*|*", "#foo > *|*", ".klass", "#id"]); let mut decls = vec![]; - map.get_universal_rules(&mut decls); + map.get_universal_rules(&mut decls, + CascadeLevel::UserNormal, + CascadeLevel::UserImportant); assert_eq!(decls.len(), 1); }