diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index 2ab50ba4073..076a2283b67 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -92,7 +92,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { fn Length(&self) -> u32 { let elem = self.owner.upcast::(); let len = match *elem.style_attribute().borrow() { - Some(ref declarations) => declarations.normal.len() + declarations.important.len(), + Some(ref declarations) => declarations.declarations.len(), None => 0, }; len as u32 @@ -103,19 +103,15 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { let index = index as usize; let elem = self.owner.upcast::(); let style_attribute = elem.style_attribute().borrow(); - let result = style_attribute.as_ref().and_then(|declarations| { - if index > declarations.normal.len() { - declarations.important - .get(index - declarations.normal.len()) - .map(|decl| format!("{:?} !important", decl)) - } else { - declarations.normal - .get(index) - .map(|decl| format!("{:?}", decl)) + style_attribute.as_ref().and_then(|declarations| { + declarations.declarations.get(index) + }).map(|&(ref declaration, importance)| { + let mut css = declaration.to_css_string(); + if importance.important() { + css += " !important"; } - }); - - result.map_or(DOMString::new(), DOMString::from) + DOMString::from(css) + }).unwrap_or_else(DOMString::new) } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue @@ -151,11 +147,11 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // Step 2.3 // Work around closures not being Clone #[derive(Clone)] - struct Map<'a, 'b: 'a>(slice::Iter<'a, Ref<'b, PropertyDeclaration>>); + struct Map<'a, 'b: 'a>(slice::Iter<'a, Ref<'b, (PropertyDeclaration, Importance)>>); impl<'a, 'b> Iterator for Map<'a, 'b> { type Item = &'a PropertyDeclaration; fn next(&mut self) -> Option { - self.0.next().map(|r| &**r) + self.0.next().map(|r| &r.0) } } @@ -167,7 +163,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // Step 3 & 4 match owner.get_inline_style_declaration(&property) { - Some(declaration) => DOMString::from(declaration.value()), + Some(declaration) => DOMString::from(declaration.0.value()), None => DOMString::new(), } } @@ -188,8 +184,10 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { } // Step 3 } else { - if self.owner.get_important_inline_style_declaration(&property).is_some() { - return DOMString::from("important"); + if let Some(decl) = self.owner.get_inline_style_declaration(&property) { + if decl.1.important() { + return DOMString::from("important"); + } } } @@ -366,7 +364,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // Step 3 let decl_block = parse_style_attribute(&value, &window.get_url(), window.css_error_reporter(), ParserContextExtraData::default()); - *element.style_attribute().borrow_mut() = if decl_block.normal.is_empty() && decl_block.important.is_empty() { + *element.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() { None // Step 2 } else { Some(decl_block) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 5141c8d5adf..999f4aa100a 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -80,7 +80,6 @@ use std::cell::{Cell, Ref}; use std::convert::TryFrom; use std::default::Default; use std::fmt; -use std::mem; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; use string_cache::{Atom, Namespace, QualName}; @@ -329,7 +328,9 @@ impl LayoutElementHelpers for LayoutJS { { #[inline] fn from_declaration(rule: PropertyDeclaration) -> DeclarationBlock { - DeclarationBlock::from_declarations(Arc::new(vec![rule])) + DeclarationBlock::from_declarations( + Arc::new(vec![(rule, Importance::Normal)]), + Importance::Normal) } let bgcolor = if let Some(this) = self.downcast::() { @@ -761,20 +762,11 @@ impl Element { fn remove(element: &Element, property: &str) { let mut inline_declarations = element.style_attribute.borrow_mut(); if let &mut Some(ref mut declarations) = &mut *inline_declarations { - let index = declarations.normal + let index = declarations.declarations .iter() - .position(|decl| decl.matches(property)); + .position(|&(ref decl, _)| decl.matches(property)); if let Some(index) = index { - Arc::make_mut(&mut declarations.normal).remove(index); - return; - } - - let index = declarations.important - .iter() - .position(|decl| decl.matches(property)); - if let Some(index) = index { - Arc::make_mut(&mut declarations.important).remove(index); - return; + Arc::make_mut(&mut declarations.declarations).remove(index); } } } @@ -786,47 +778,29 @@ impl Element { pub fn update_inline_style(&self, declarations: Vec, importance: Importance) { - fn update(element: &Element, mut declarations: Vec, + fn update(element: &Element, declarations: Vec, importance: Importance) { let mut inline_declarations = element.style_attribute().borrow_mut(); if let &mut Some(ref mut existing_declarations) = &mut *inline_declarations { - let existing_declarations = if importance.important() { - &mut existing_declarations.important - } else { - &mut existing_declarations.normal - }; - // Usually, the reference count will be 1 here. But transitions could make it greater // than that. - let existing_declarations = Arc::make_mut(existing_declarations); + let existing_declarations = Arc::make_mut(&mut existing_declarations.declarations); - for mut incoming_declaration in declarations { - let mut replaced = false; + 'outer: for incoming_declaration in declarations { for existing_declaration in &mut *existing_declarations { - if existing_declaration.name() == incoming_declaration.name() { - mem::swap(existing_declaration, &mut incoming_declaration); - replaced = true; - break; + if existing_declaration.0.name() == incoming_declaration.name() { + *existing_declaration = (incoming_declaration, importance); + continue 'outer; } } - - if !replaced { - existing_declarations.push(incoming_declaration); - } + existing_declarations.push((incoming_declaration, importance)); } return; } - let (important, normal) = if importance.important() { - (declarations, vec![]) - } else { - (vec![], declarations) - }; - *inline_declarations = Some(PropertyDeclarationBlock { - important: Arc::new(important), - normal: Arc::new(normal), + declarations: Arc::new(declarations.into_iter().map(|d| (d, importance)).collect()), }); } @@ -836,30 +810,18 @@ impl Element { pub fn set_inline_style_property_priority(&self, properties: &[&str], - importance: Importance) { + new_importance: Importance) { { let mut inline_declarations = self.style_attribute().borrow_mut(); if let &mut Some(ref mut declarations) = &mut *inline_declarations { - let (from, to) = if importance == Importance::Important { - (&mut declarations.normal, &mut declarations.important) - } else { - (&mut declarations.important, &mut declarations.normal) - }; - - // Usually, the reference counts of `from` and `to` will be 1 here. But transitions - // could make them greater than that. - let from = Arc::make_mut(from); - let to = Arc::make_mut(to); - let mut new_from = Vec::new(); - for declaration in from.drain(..) { - let name = declaration.name(); - if properties.iter().any(|p| name == **p) { - to.push(declaration) - } else { - new_from.push(declaration) - } - } - mem::replace(from, new_from); + // Usually, the reference counts of `from` and `to` will be 1 here. But transitions + // could make them greater than that. + let declarations = Arc::make_mut(&mut declarations.declarations); + for &mut (ref declaration, ref mut importance) in declarations { + if properties.iter().any(|p| declaration.name() == **p) { + *importance = new_importance + } + } } } @@ -868,25 +830,12 @@ impl Element { pub fn get_inline_style_declaration(&self, property: &Atom) - -> Option> { + -> Option> { ref_filter_map(self.style_attribute.borrow(), |inline_declarations| { inline_declarations.as_ref().and_then(|declarations| { - declarations.normal + declarations.declarations .iter() - .chain(declarations.important.iter()) - .find(|decl| decl.matches(&property)) - }) - }) - } - - pub fn get_important_inline_style_declaration(&self, - property: &Atom) - -> Option> { - ref_filter_map(self.style_attribute.borrow(), |inline_declarations| { - inline_declarations.as_ref().and_then(|declarations| { - declarations.important - .iter() - .find(|decl| decl.matches(&property)) + .find(|&&(ref decl, _)| decl.matches(&property)) }) }) } diff --git a/components/style/animation.rs b/components/style/animation.rs index 2764511c7f3..bdfd126ab15 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -15,7 +15,7 @@ use properties::longhands::animation_iteration_count::computed_value::AnimationI use properties::longhands::animation_play_state::computed_value::AnimationPlayState; use properties::longhands::transition_timing_function::computed_value::StartEnd; use properties::longhands::transition_timing_function::computed_value::TransitionTimingFunction; -use properties::{self, ComputedValues}; +use properties::{self, ComputedValues, Importance}; use selector_matching::DeclarationBlock; use std::sync::Arc; use std::sync::mpsc::Sender; @@ -385,7 +385,8 @@ fn compute_style_for_animation_step(context: &SharedStyleContext, KeyframesStepValue::ComputedValues => style_from_cascade.clone(), KeyframesStepValue::Declarations(ref declarations) => { let declaration_block = DeclarationBlock { - declarations: declarations.clone(), + mixed_declarations: declarations.clone(), + importance: Importance::Normal, source_order: 0, specificity: ::std::u32::MAX, }; diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index 3656ccee24d..56bf4607d4d 100644 --- a/components/style/keyframes.rs +++ b/components/style/keyframes.rs @@ -5,9 +5,9 @@ use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser}; use cssparser::{DeclarationListParser, DeclarationParser}; use parser::{ParserContext, log_css_error}; -use properties::PropertyDeclaration; use properties::PropertyDeclarationParseResult; use properties::animated_properties::TransitionProperty; +use properties::{PropertyDeclaration, Importance}; use std::sync::Arc; /// A number from 1 to 100, indicating the percentage of the animation where @@ -72,7 +72,12 @@ impl KeyframeSelector { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct Keyframe { pub selector: KeyframeSelector, - pub declarations: Arc>, + + /// `!important` is not allowed in keyframe declarations, + /// so the second value of these tuples is always `Importance::Normal`. + /// But including them enables `compute_style_for_animation_step` to create a `DeclarationBlock` + /// by cloning an `Arc<_>` (incrementing a reference count) rather than re-creating a `Vec<_>`. + pub declarations: Arc>, } /// A keyframes step value. This can be a synthetised keyframes animation, that @@ -82,7 +87,8 @@ pub struct Keyframe { #[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum KeyframesStepValue { - Declarations(Arc>), + /// See `Keyframe::declarations`’s docs about the presence of `Importance`. + Declarations(Arc>), ComputedValues, } @@ -108,7 +114,7 @@ impl KeyframesStep { value: KeyframesStepValue) -> Self { let declared_timing_function = match value { KeyframesStepValue::Declarations(ref declarations) => { - declarations.iter().any(|prop_decl| { + declarations.iter().any(|&(ref prop_decl, _)| { match *prop_decl { PropertyDeclaration::AnimationTimingFunction(..) => true, _ => false, @@ -148,8 +154,8 @@ fn get_animated_properties(keyframe: &Keyframe) -> Vec { let mut ret = vec![]; // NB: declarations are already deduplicated, so we don't have to check for // it here. - for declaration in keyframe.declarations.iter() { - if let Some(property) = TransitionProperty::from_declaration(&declaration) { + for &(ref declaration, _) in keyframe.declarations.iter() { + if let Some(property) = TransitionProperty::from_declaration(declaration) { ret.push(property); } } @@ -247,7 +253,7 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { let mut iter = DeclarationListParser::new(input, parser); while let Some(declaration) = iter.next() { match declaration { - Ok(d) => declarations.extend(d), + Ok(d) => declarations.extend(d.into_iter().map(|d| (d, Importance::Normal))), Err(range) => { let pos = range.start; let message = format!("Unsupported keyframe property declaration: '{}'", diff --git a/components/style/matching.rs b/components/style/matching.rs index cb1f70602ad..ffd69ebd529 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -14,7 +14,7 @@ use context::{StyleContext, SharedStyleContext}; use data::PrivateStyleData; use dom::{TElement, TNode, TRestyleDamage, UnsafeNode}; use properties::longhands::display::computed_value as display; -use properties::{ComputedValues, PropertyDeclaration, cascade}; +use properties::{ComputedValues, cascade}; use selector_impl::{TheSelectorImpl, PseudoElement}; use selector_matching::{DeclarationBlock, Stylist}; use selectors::bloom::BloomFilter; @@ -118,15 +118,11 @@ impl<'a> ApplicableDeclarationsCacheQuery<'a> { impl<'a> PartialEq for ApplicableDeclarationsCacheQuery<'a> { fn eq(&self, other: &ApplicableDeclarationsCacheQuery<'a>) -> bool { - if self.declarations.len() != other.declarations.len() { - return false - } - for (this, other) in self.declarations.iter().zip(other.declarations) { - if !arc_ptr_eq(&this.declarations, &other.declarations) { - return false - } - } - true + self.declarations.len() == other.declarations.len() && + self.declarations.iter().zip(other.declarations).all(|(this, other)| { + arc_ptr_eq(&this.mixed_declarations, &other.mixed_declarations) && + this.importance == other.importance + }) } } impl<'a> Eq for ApplicableDeclarationsCacheQuery<'a> {} @@ -143,8 +139,9 @@ impl<'a> Hash for ApplicableDeclarationsCacheQuery<'a> { for declaration in self.declarations { // Each declaration contians an Arc, which is a stable // pointer; we use that for hashing and equality. - let ptr = &*declaration.declarations as *const Vec; + let ptr: *const Vec<_> = &*declaration.mixed_declarations; ptr.hash(state); + declaration.importance.hash(state); } } } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index ff96363127b..45ee70ee3a1 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -14,8 +14,6 @@ use std::ascii::AsciiExt; use std::boxed::Box as StdBox; use std::collections::HashSet; use std::fmt::{self, Write}; -use std::iter::{Iterator, Chain, Zip, Repeat, repeat}; -use std::slice; use std::sync::Arc; use app_units::Au; @@ -264,7 +262,7 @@ mod property_bit_field { % endfor #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum Importance { /// Indicates a declaration without `!important`. Normal, @@ -288,21 +286,7 @@ impl Importance { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct PropertyDeclarationBlock { #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")] - pub important: Arc>, - #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")] - pub normal: Arc>, -} - -impl PropertyDeclarationBlock { - /// Provides an iterator of all declarations, with indication of !important value - pub fn declarations(&self) -> Chain< - Zip, Repeat>, - Zip, Repeat> - > { - let normal = self.normal.iter().zip(repeat(Importance::Normal)); - let important = self.important.iter().zip(repeat(Importance::Important)); - normal.chain(important) - } + pub declarations: Arc>, } impl ToCss for PropertyDeclarationBlock { @@ -316,7 +300,7 @@ impl ToCss for PropertyDeclarationBlock { let mut already_serialized = Vec::new(); // Step 3 - for (declaration, importance) in self.declarations() { + for &(ref declaration, importance) in &*self.declarations { // Step 3.1 let property = declaration.name(); @@ -330,7 +314,7 @@ impl ToCss for PropertyDeclarationBlock { if !shorthands.is_empty() { // Step 3.3.1 - let mut longhands = self.declarations() + let mut longhands = self.declarations.iter() .filter(|d| !already_serialized.contains(&d.0.name())) .collect::>(); @@ -342,7 +326,7 @@ impl ToCss for PropertyDeclarationBlock { let mut current_longhands = Vec::new(); let mut important_count = 0; - for &(longhand, longhand_importance) in longhands.iter() { + for &&(ref longhand, longhand_importance) in longhands.iter() { let longhand_name = longhand.name(); if properties.iter().any(|p| &longhand_name == *p) { current_longhands.push(longhand); @@ -386,7 +370,7 @@ impl ToCss for PropertyDeclarationBlock { for current_longhand in current_longhands { // Substep 9 already_serialized.push(current_longhand.name()); - let index_to_remove = longhands.iter().position(|l| l.0 == current_longhand); + let index_to_remove = longhands.iter().position(|l| l.0 == *current_longhand); if let Some(index) = index_to_remove { // Substep 10 longhands.remove(index); @@ -557,8 +541,7 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> { pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Parser) -> PropertyDeclarationBlock { - let mut important_declarations = Vec::new(); - let mut normal_declarations = Vec::new(); + let mut declarations = Vec::new(); let parser = PropertyDeclarationParser { context: context, }; @@ -566,11 +549,7 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars while let Some(declaration) = iter.next() { match declaration { Ok((results, importance)) => { - if importance.important() { - important_declarations.extend(results); - } else { - normal_declarations.extend(results); - } + declarations.extend(results.into_iter().map(|d| (d, importance))) } Err(range) => { let pos = range.start; @@ -581,46 +560,79 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars } } PropertyDeclarationBlock { - important: Arc::new(deduplicate_property_declarations(important_declarations)), - normal: Arc::new(deduplicate_property_declarations(normal_declarations)), + declarations: Arc::new(deduplicate_property_declarations(declarations)), } } -/// Only keep the last declaration for any given property. +/// Only keep the "winning" declaration for any given property, by importance then source order. /// The input and output are in source order -fn deduplicate_property_declarations(declarations: Vec) - -> Vec { - let mut deduplicated = vec![]; - let mut seen = PropertyBitField::new(); - let mut seen_custom = Vec::new(); - for declaration in declarations.into_iter().rev() { +fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Importance)>) + -> Vec<(PropertyDeclaration, Importance)> { + let mut deduplicated = Vec::new(); + let mut seen_normal = PropertyBitField::new(); + let mut seen_important = PropertyBitField::new(); + let mut seen_custom_normal = Vec::new(); + let mut seen_custom_important = Vec::new(); + for (declaration, importance) in declarations.into_iter().rev() { match declaration { % for property in data.longhands: PropertyDeclaration::${property.camel_case}(..) => { % if not property.derived_from: - if seen.get_${property.ident}() { - continue + if importance.important() { + if seen_important.get_${property.ident}() { + continue + } + if seen_normal.get_${property.ident}() { + remove_one(&mut deduplicated, |d| { + matches!(d, &(PropertyDeclaration::${property.camel_case}(..), _)) + }) + } + seen_important.set_${property.ident}() + } else { + if seen_normal.get_${property.ident}() || + seen_important.get_${property.ident}() { + continue + } + seen_normal.set_${property.ident}() } - seen.set_${property.ident}() % else: unreachable!(); % endif }, % endfor PropertyDeclaration::Custom(ref name, _) => { - if seen_custom.contains(name) { - continue + if importance.important() { + if seen_custom_important.contains(name) { + continue + } + if seen_custom_normal.contains(name) { + remove_one(&mut deduplicated, |d| { + matches!(d, &(PropertyDeclaration::Custom(ref n, _), _) if n == name) + }) + } + seen_custom_important.push(name.clone()) + } else { + if seen_custom_normal.contains(name) || + seen_custom_important.contains(name) { + continue + } + seen_custom_normal.push(name.clone()) } - seen_custom.push(name.clone()) } } - deduplicated.push(declaration) + deduplicated.push((declaration, importance)) } deduplicated.reverse(); deduplicated } +#[inline] +fn remove_one bool>(v: &mut Vec, mut remove_this: F) { + let previous_len = v.len(); + v.retain(|x| !remove_this(x)); + debug_assert_eq!(v.len(), previous_len - 1); +} #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum CSSWideKeyword { @@ -1701,7 +1713,7 @@ fn cascade_with_cached_declarations( // Declaration blocks are stored in increasing precedence order, // we want them in decreasing order here. for sub_list in applicable_declarations.iter().rev() { - for declaration in sub_list.declarations.iter().rev() { + for declaration in sub_list.iter().rev() { match *declaration { % for style_struct in data.active_style_structs(): % for property in style_struct.longhands: @@ -1832,7 +1844,7 @@ pub fn cascade(viewport_size: Size2D, let mut custom_properties = None; let mut seen_custom = HashSet::new(); for sub_list in applicable_declarations.iter().rev() { - for declaration in sub_list.declarations.iter().rev() { + for declaration in sub_list.iter().rev() { match *declaration { PropertyDeclaration::Custom(ref name, ref value) => { ::custom_properties::cascade( @@ -1890,7 +1902,7 @@ pub fn cascade(viewport_size: Size2D, ComputedValues::do_cascade_property(|cascade_property| { % for category_to_cascade_now in ["early", "other"]: for sub_list in applicable_declarations.iter().rev() { - for declaration in sub_list.declarations.iter().rev() { + for declaration in sub_list.iter().rev() { if let PropertyDeclaration::Custom(..) = *declaration { continue } diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs index 775f81915d4..c8f6dc089f1 100644 --- a/components/style/selector_matching.rs +++ b/components/style/selector_matching.rs @@ -9,7 +9,7 @@ use element_state::*; use error_reporting::StdoutErrorReporter; use keyframes::KeyframesAnimation; use media_queries::{Device, MediaType}; -use properties::{self, PropertyDeclaration, PropertyDeclarationBlock, ComputedValues}; +use properties::{self, PropertyDeclaration, PropertyDeclarationBlock, ComputedValues, Importance}; use quickersort::sort_by; use restyle_hints::{RestyleHint, DependencySet}; use selector_impl::{ElementExt, TheSelectorImpl, PseudoElement}; @@ -25,6 +25,7 @@ use std::collections::HashMap; use std::fmt; use std::hash::BuildHasherDefault; use std::hash::Hash; +use std::slice; use std::sync::Arc; use string_cache::Atom; use style_traits::viewport::ViewportConstraints; @@ -163,8 +164,8 @@ impl Stylist { // Take apart the StyleRule into individual Rules and insert // them into the SelectorMap of that priority. macro_rules! append( - ($style_rule: ident, $priority: ident) => { - if !$style_rule.declarations.$priority.is_empty() { + ($style_rule: ident, $priority: ident, $importance: expr) => { + if !$style_rule.declarations.declarations.is_empty() { for selector in &$style_rule.selectors { let map = if let Some(ref pseudo) = selector.pseudo_element { self.pseudos_map @@ -179,7 +180,8 @@ impl Stylist { selector: selector.complex_selector.clone(), declarations: DeclarationBlock { specificity: selector.specificity, - declarations: $style_rule.declarations.$priority.clone(), + mixed_declarations: $style_rule.declarations.declarations.clone(), + importance: $importance, source_order: rules_source_order, }, }); @@ -191,8 +193,8 @@ impl Stylist { for rule in stylesheet.effective_rules(&self.device) { match *rule { CSSRule::Style(ref style_rule) => { - append!(style_rule, normal); - append!(style_rule, important); + append!(style_rule, normal, Importance::Normal); + append!(style_rule, important, Importance::Important); rules_source_order += 1; for selector in &style_rule.selectors { @@ -397,7 +399,9 @@ impl Stylist { relations |= AFFECTED_BY_STYLE_ATTRIBUTE; Push::push( applicable_declarations, - DeclarationBlock::from_declarations(sa.normal.clone())); + DeclarationBlock::from_declarations( + sa.declarations.clone(), + Importance::Normal)); } debug!("style attr: {:?}", relations); @@ -414,7 +418,9 @@ impl Stylist { if let Some(ref sa) = style_attribute { Push::push( applicable_declarations, - DeclarationBlock::from_declarations(sa.important.clone())); + DeclarationBlock::from_declarations( + sa.declarations.clone(), + Importance::Important)); } debug!("style attr important: {:?}", relations); @@ -829,20 +835,64 @@ pub struct Rule { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] #[derive(Debug, Clone)] pub struct DeclarationBlock { - pub declarations: Arc>, + /// Contains declarations of either importance, but only those of self.importance are relevant. + /// Use DeclarationBlock::iter + pub mixed_declarations: Arc>, + pub importance: Importance, pub source_order: usize, pub specificity: u32, } impl DeclarationBlock { #[inline] - pub fn from_declarations(declarations: Arc>) -> Self { + pub fn from_declarations(declarations: Arc>, + importance: Importance) + -> Self { DeclarationBlock { - declarations: declarations, + mixed_declarations: declarations, + importance: importance, source_order: 0, specificity: 0, } } + + pub fn iter(&self) -> DeclarationBlockIter { + DeclarationBlockIter { + iter: self.mixed_declarations.iter(), + importance: self.importance, + } + } +} + +pub struct DeclarationBlockIter<'a> { + iter: slice::Iter<'a, (PropertyDeclaration, Importance)>, + importance: Importance, +} + +impl<'a> Iterator for DeclarationBlockIter<'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 DeclarationBlockIter<'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 + } } fn find_push(map: &mut HashMap>, key: Str, value: Rule) { diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index 551e7999154..6e4f137cfdc 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -45,8 +45,8 @@ use style::element_state::ElementState; use style::error_reporting::StdoutErrorReporter; use style::gecko_selector_impl::{GeckoSelectorImpl, NonTSPseudoClass, PseudoElement}; use style::parser::ParserContextExtraData; +use style::properties::PropertyDeclarationBlock; use style::properties::{ComputedValues, parse_style_attribute}; -use style::properties::{PropertyDeclaration, PropertyDeclarationBlock}; use style::refcell::{Ref, RefCell, RefMut}; use style::selector_impl::ElementExt; use style::selector_matching::DeclarationBlock; diff --git a/tests/unit/script/size_of.rs b/tests/unit/script/size_of.rs index b30304d5dac..472beb888c3 100644 --- a/tests/unit/script/size_of.rs +++ b/tests/unit/script/size_of.rs @@ -40,10 +40,10 @@ macro_rules! sizeof_checker ( // Update the sizes here sizeof_checker!(size_event_target, EventTarget, 40); sizeof_checker!(size_node, Node, 160); -sizeof_checker!(size_element, Element, 336); -sizeof_checker!(size_htmlelement, HTMLElement, 352); -sizeof_checker!(size_div, HTMLDivElement, 352); -sizeof_checker!(size_span, HTMLSpanElement, 352); +sizeof_checker!(size_element, Element, 328); +sizeof_checker!(size_htmlelement, HTMLElement, 344); +sizeof_checker!(size_div, HTMLDivElement, 344); +sizeof_checker!(size_span, HTMLSpanElement, 344); sizeof_checker!(size_text, Text, 192); sizeof_checker!(size_characterdata, CharacterData, 192); sizeof_checker!(size_servothreadsafelayoutnode, ServoThreadSafeLayoutNode, 16); diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index 98ffa07f722..2f2b5d39127 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -5,7 +5,7 @@ pub use cssparser::ToCss; pub use std::sync::Arc; pub use style::computed_values::display::T::inline_block; -pub use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarationBlock}; +pub use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarationBlock, Importance}; pub use style::values::specified::{BorderStyle, CSSColor, Length}; pub use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent}; pub use style::properties::longhands::outline_color::computed_value::T as ComputedColor; @@ -18,37 +18,41 @@ fn property_declaration_block_should_serialize_correctly() { use style::properties::longhands::overflow_x::computed_value::T as OverflowXValue; use style::properties::longhands::overflow_y::computed_value::T as OverflowYContainer; - let mut normal = Vec::new(); - let mut important = Vec::new(); + let declarations = vec![ + (PropertyDeclaration::Width( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(Length::from_px(70f32)))), + Importance::Normal), - let length = DeclaredValue::Value(LengthOrPercentageOrAuto::Length(Length::from_px(70f32))); - normal.push(PropertyDeclaration::Width(length)); + (PropertyDeclaration::MinHeight( + DeclaredValue::Value(LengthOrPercentage::Length(Length::from_px(20f32)))), + Importance::Normal), - let min_height = DeclaredValue::Value(LengthOrPercentage::Length(Length::from_px(20f32))); - normal.push(PropertyDeclaration::MinHeight(min_height)); + (PropertyDeclaration::Height( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(Length::from_px(20f32)))), + Importance::Important), - let value = DeclaredValue::Value(inline_block); - normal.push(PropertyDeclaration::Display(value)); + (PropertyDeclaration::Display( + DeclaredValue::Value(inline_block)), + Importance::Normal), - let overflow_x = DeclaredValue::Value(OverflowXValue::auto); - normal.push(PropertyDeclaration::OverflowX(overflow_x)); + (PropertyDeclaration::OverflowX( + DeclaredValue::Value(OverflowXValue::auto)), + Importance::Normal), - let overflow_y = DeclaredValue::Value(OverflowYContainer(OverflowXValue::auto)); - normal.push(PropertyDeclaration::OverflowY(overflow_y)); - - let height = DeclaredValue::Value(LengthOrPercentageOrAuto::Length(Length::from_px(20f32))); - important.push(PropertyDeclaration::Height(height)); + (PropertyDeclaration::OverflowY( + DeclaredValue::Value(OverflowYContainer(OverflowXValue::auto))), + Importance::Normal), + ]; let block = PropertyDeclarationBlock { - normal: Arc::new(normal), - important: Arc::new(important) + declarations: Arc::new(declarations), }; let css_string = block.to_css_string(); assert_eq!( css_string, - "width: 70px; min-height: 20px; display: inline-block; overflow: auto; height: 20px !important;" + "width: 70px; min-height: 20px; height: 20px !important; display: inline-block; overflow: auto;" ); } @@ -57,8 +61,7 @@ mod shorthand_serialization { pub fn shorthand_properties_to_string(properties: Vec) -> String { let block = PropertyDeclarationBlock { - normal: Arc::new(properties), - important: Arc::new(Vec::new()) + declarations: Arc::new(properties.into_iter().map(|d| (d, Importance::Normal)).collect()), }; block.to_css_string() diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 72273165b35..27d35c513cf 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -13,6 +13,7 @@ use style::error_reporting::ParseErrorReporter; use style::keyframes::{Keyframe, KeyframeSelector, KeyframePercentage}; use style::parser::ParserContextExtraData; use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, DeclaredValue, longhands}; +use style::properties::Importance; use style::properties::longhands::animation_play_state; use style::stylesheets::{Stylesheet, CSSRule, StyleRule, KeyframesRule, Origin}; use style::values::specified::{LengthOrPercentageOrAuto, Percentage}; @@ -87,10 +88,10 @@ fn test_parse_stylesheet() { }, ], declarations: PropertyDeclarationBlock { - normal: Arc::new(vec![]), - important: Arc::new(vec![ - PropertyDeclaration::Display(DeclaredValue::Value( + declarations: Arc::new(vec![ + (PropertyDeclaration::Display(DeclaredValue::Value( longhands::display::SpecifiedValue::none)), + Importance::Important), ]), }, }), @@ -132,11 +133,11 @@ fn test_parse_stylesheet() { }, ], declarations: PropertyDeclarationBlock { - normal: Arc::new(vec![ - PropertyDeclaration::Display(DeclaredValue::Value( + declarations: Arc::new(vec![ + (PropertyDeclaration::Display(DeclaredValue::Value( longhands::display::SpecifiedValue::block)), + Importance::Normal), ]), - important: Arc::new(vec![]), }, }), CSSRule::Style(StyleRule { @@ -166,24 +167,31 @@ fn test_parse_stylesheet() { }, ], declarations: PropertyDeclarationBlock { - normal: Arc::new(vec![ - PropertyDeclaration::BackgroundColor(DeclaredValue::Value( + declarations: Arc::new(vec![ + (PropertyDeclaration::BackgroundColor(DeclaredValue::Value( longhands::background_color::SpecifiedValue { authored: Some("blue".to_owned()), parsed: cssparser::Color::RGBA(cssparser::RGBA { red: 0., green: 0., blue: 1., alpha: 1. }), } - )), - PropertyDeclaration::BackgroundPosition(DeclaredValue::Initial), - PropertyDeclaration::BackgroundRepeat(DeclaredValue::Initial), - PropertyDeclaration::BackgroundAttachment(DeclaredValue::Initial), - PropertyDeclaration::BackgroundImage(DeclaredValue::Initial), - PropertyDeclaration::BackgroundSize(DeclaredValue::Initial), - PropertyDeclaration::BackgroundOrigin(DeclaredValue::Initial), - PropertyDeclaration::BackgroundClip(DeclaredValue::Initial), + )), + Importance::Normal), + (PropertyDeclaration::BackgroundPosition(DeclaredValue::Initial), + Importance::Normal), + (PropertyDeclaration::BackgroundRepeat(DeclaredValue::Initial), + Importance::Normal), + (PropertyDeclaration::BackgroundAttachment(DeclaredValue::Initial), + Importance::Normal), + (PropertyDeclaration::BackgroundImage(DeclaredValue::Initial), + Importance::Normal), + (PropertyDeclaration::BackgroundSize(DeclaredValue::Initial), + Importance::Normal), + (PropertyDeclaration::BackgroundOrigin(DeclaredValue::Initial), + Importance::Normal), + (PropertyDeclaration::BackgroundClip(DeclaredValue::Initial), + Importance::Normal), ]), - important: Arc::new(vec![]), }, }), CSSRule::Keyframes(KeyframesRule { @@ -193,19 +201,22 @@ fn test_parse_stylesheet() { selector: KeyframeSelector::new_for_unit_testing( vec![KeyframePercentage::new(0.)]), declarations: Arc::new(vec![ - PropertyDeclaration::Width(DeclaredValue::Value( + (PropertyDeclaration::Width(DeclaredValue::Value( LengthOrPercentageOrAuto::Percentage(Percentage(0.)))), + Importance::Normal), ]), }, Keyframe { selector: KeyframeSelector::new_for_unit_testing( vec![KeyframePercentage::new(1.)]), declarations: Arc::new(vec![ - PropertyDeclaration::Width(DeclaredValue::Value( + (PropertyDeclaration::Width(DeclaredValue::Value( LengthOrPercentageOrAuto::Percentage(Percentage(1.)))), - PropertyDeclaration::AnimationPlayState(DeclaredValue::Value( + Importance::Normal), + (PropertyDeclaration::AnimationPlayState(DeclaredValue::Value( animation_play_state::SpecifiedValue( vec![animation_play_state::SingleSpecifiedValue::running]))), + Importance::Normal), ]), }, ]