diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index cf03ff45d88..5aea41c1b36 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -410,10 +410,10 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // https://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface fn IndexedGetter(&self, index: u32) -> Option { self.owner.with_block(|pdb| { - pdb.declarations().get(index as usize).map(|entry| { - let (ref declaration, importance) = *entry; + pdb.declarations().get(index as usize).map(|declaration| { + let important = pdb.declarations_importance().get(index); let mut css = declaration.to_css_string(); - if importance.important() { + if important { css += " !important"; } DOMString::from(css) diff --git a/components/style/animation.rs b/components/style/animation.rs index c2e3f33b794..528aab830e8 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -10,7 +10,7 @@ use bezier::Bezier; use context::SharedStyleContext; use dom::OpaqueNode; use font_metrics::FontMetricsProvider; -use properties::{self, CascadeFlags, ComputedValues, Importance}; +use properties::{self, CascadeFlags, ComputedValues}; use properties::animated_properties::{AnimatableLonghand, AnimatedProperty, TransitionProperty}; use properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection; use properties::longhands::animation_iteration_count::single_value::computed_value::T as AnimationIterationCount; @@ -484,12 +484,11 @@ fn compute_style_for_animation_step(context: &SharedStyleContext, let guard = declarations.read_with(context.guards.author); // No !important in keyframes. - debug_assert!(guard.declarations().iter() - .all(|&(_, importance)| importance == Importance::Normal)); + debug_assert!(!guard.any_important()); let iter = || { guard.declarations().iter().rev() - .map(|&(ref decl, _importance)| (decl, CascadeLevel::Animations)) + .map(|decl| (decl, CascadeLevel::Animations)) }; // This currently ignores visited styles, which seems acceptable, diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 60849029527..0a158fcca55 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -14,8 +14,10 @@ use parser::{ParserContext, ParserErrorContext}; use properties::animated_properties::AnimationValue; use selectors::parser::SelectorParseError; use shared_lock::Locked; +use smallbitvec::{self, SmallBitVec}; use smallvec::SmallVec; use std::fmt; +use std::iter::{DoubleEndedIterator, Zip}; use std::slice::Iter; use style_traits::{PARSING_MODE_DEFAULT, ToCss, ParseError, ParsingMode, StyleParseError}; use stylesheets::{CssRuleType, Origin, UrlExtraData}; @@ -73,10 +75,10 @@ pub struct PropertyDeclarationBlock { /// The group of declarations, along with their importance. /// /// Only deduplicated declarations appear here. - declarations: Vec<(PropertyDeclaration, Importance)>, + declarations: Vec, - /// The number of entries in `self.declaration` with `Importance::Important` - important_count: usize, + /// The "important" flag for each declaration in `declarations`. + declarations_importance: SmallBitVec, longhands: LonghandIdSet, } @@ -87,23 +89,39 @@ impl MallocSizeOf for PropertyDeclarationBlock { } } -/// Iterator for PropertyDeclaration to be generated from PropertyDeclarationBlock. -#[derive(Clone)] -pub struct PropertyDeclarationIterator<'a> { - iter: Iter<'a, (PropertyDeclaration, Importance)>, +/// Iterator over `(PropertyDeclaration, Importance)` pairs. +pub struct DeclarationImportanceIterator<'a> { + iter: Zip, smallbitvec::Iter<'a>>, } -impl<'a> Iterator for PropertyDeclarationIterator<'a> { - type Item = &'a PropertyDeclaration; - #[inline] - fn next(&mut self) -> Option<&'a PropertyDeclaration> { - self.iter.next().map(|&(ref decl, _)| decl) +impl<'a> DeclarationImportanceIterator<'a> { + /// Constructor + pub fn new(declarations: &'a [PropertyDeclaration], important: &'a SmallBitVec) -> Self { + DeclarationImportanceIterator { + iter: declarations.iter().zip(important.iter()), + } + } +} + +impl<'a> Iterator for DeclarationImportanceIterator<'a> { + type Item = (&'a PropertyDeclaration, Importance); + + fn next(&mut self) -> Option { + self.iter.next().map(|(decl, important)| + (decl, if important { Importance::Important } else { Importance::Normal })) + } +} + +impl<'a> DoubleEndedIterator for DeclarationImportanceIterator<'a> { + fn next_back(&mut self) -> Option { + self.iter.next_back().map(|(decl, important)| + (decl, if important { Importance::Important } else { Importance::Normal })) } } /// Iterator for AnimationValue to be generated from PropertyDeclarationBlock. pub struct AnimationValueIterator<'a, 'cx, 'cx_a:'cx> { - iter: Iter<'a, (PropertyDeclaration, Importance)>, + iter: DeclarationImportanceIterator<'a>, context: &'cx mut Context<'cx_a>, default_values: &'a ComputedValues, } @@ -113,7 +131,7 @@ impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> { context: &'cx mut Context<'cx_a>, default_values: &'a ComputedValues) -> AnimationValueIterator<'a, 'cx, 'cx_a> { AnimationValueIterator { - iter: declarations.declarations().iter(), + iter: declarations.declaration_importance_iter(), context: context, default_values: default_values, } @@ -124,12 +142,10 @@ impl<'a, 'cx, 'cx_a:'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> { type Item = (AnimatableLonghand, AnimationValue); #[inline] fn next(&mut self) -> Option { - use properties::Importance; - loop { let next = self.iter.next(); match next { - Some(&(ref decl, importance)) => { + Some((decl, importance)) => { if importance == Importance::Normal { let property = AnimatableLonghand::from_declaration(decl); let animation = AnimationValue::from_declaration(decl, &mut self.context, @@ -166,7 +182,7 @@ impl PropertyDeclarationBlock { pub fn new() -> Self { PropertyDeclarationBlock { declarations: Vec::new(), - important_count: 0, + declarations_importance: SmallBitVec::new(), longhands: LonghandIdSet::new(), } } @@ -178,22 +194,25 @@ impl PropertyDeclarationBlock { longhands.insert(id); } PropertyDeclarationBlock { - declarations: vec![(declaration, importance)], - important_count: if importance.important() { 1 } else { 0 }, + declarations: vec![declaration], + declarations_importance: SmallBitVec::from_elem(1, importance.important()), longhands: longhands, } } /// The declarations in this block - pub fn declarations(&self) -> &[(PropertyDeclaration, Importance)] { + pub fn declarations(&self) -> &[PropertyDeclaration] { &self.declarations } - /// Iterate over only PropertyDeclaration. - pub fn declarations_iter(&self) -> PropertyDeclarationIterator { - PropertyDeclarationIterator { - iter: self.declarations.iter(), - } + /// The `important` flags for declarations in this block + pub fn declarations_importance(&self) -> &SmallBitVec { + &self.declarations_importance + } + + /// Iterate over `(PropertyDeclaration, Importance)` pairs + pub fn declaration_importance_iter(&self) -> DeclarationImportanceIterator { + DeclarationImportanceIterator::new(&self.declarations, &self.declarations_importance) } /// Return an iterator of (AnimatableLonghand, AnimationValue). @@ -206,27 +225,32 @@ impl PropertyDeclarationBlock { /// Returns whether this block contains any declaration with `!important`. /// - /// This is based on the `important_count` counter, + /// This is based on the `declarations_importance` bit-vector, /// which should be maintained whenever `declarations` is changed. - // FIXME: make fields private and maintain it here in methods? pub fn any_important(&self) -> bool { - self.important_count > 0 + !self.declarations_importance.all_false() } /// Returns whether this block contains any declaration without `!important`. /// - /// This is based on the `important_count` counter, + /// This is based on the `declarations_importance` bit-vector, /// which should be maintained whenever `declarations` is changed. - // FIXME: make fields private and maintain it here in methods? pub fn any_normal(&self) -> bool { - self.declarations.len() > self.important_count + !self.declarations_importance.all_true() } /// Get a declaration for a given property. /// /// NOTE: This is linear time. - pub fn get(&self, property: PropertyDeclarationId) -> Option< &(PropertyDeclaration, Importance)> { - self.declarations.iter().find(|&&(ref decl, _)| decl.id() == property) + pub fn get(&self, property: PropertyDeclarationId) -> Option<(&PropertyDeclaration, Importance)> { + self.declarations.iter().enumerate().find(|&(_, decl)| decl.id() == property).map(|(i, decl)| { + let importance = if self.declarations_importance.get(i as u32) { + Importance::Important + } else { + Importance::Normal + }; + (decl, importance) + }) } /// Find the value of the given property in this block and serialize it @@ -251,7 +275,7 @@ impl PropertyDeclarationBlock { // Step 1.2.2.2 & 1.2.2.3 match declaration { - Some(&(ref declaration, importance)) => { + Some((declaration, importance)) => { list.push(declaration); if importance.important() { important_count += 1; @@ -277,7 +301,7 @@ impl PropertyDeclarationBlock { } } Err(longhand_or_custom) => { - if let Some(&(ref value, _importance)) = self.get(longhand_or_custom) { + if let Some((value, _importance)) = self.get(longhand_or_custom) { // Step 2 value.to_css(dest) } else { @@ -298,7 +322,7 @@ impl PropertyDeclarationBlock { // Step 2.1 & 2.2 & 2.3 if shorthand.longhands().iter().all(|&l| { self.get(PropertyDeclarationId::Longhand(l)) - .map_or(false, |&(_, importance)| importance.important()) + .map_or(false, |(_, importance)| importance.important()) }) { Importance::Important } else { @@ -307,7 +331,7 @@ impl PropertyDeclarationBlock { } Err(longhand_or_custom) => { // Step 3 - self.get(longhand_or_custom).map_or(Importance::Normal, |&(_, importance)| importance) + self.get(longhand_or_custom).map_or(Importance::Normal, |(_, importance)| importance) } } } @@ -409,25 +433,24 @@ impl PropertyDeclarationBlock { if !definitely_new { let mut index_to_remove = None; for (i, slot) in self.declarations.iter_mut().enumerate() { - if slot.0.id() == declaration.id() { - match (slot.1, importance) { - (Importance::Normal, Importance::Important) => { - self.important_count += 1; - } - (Importance::Important, Importance::Normal) => { - if overwrite_more_important_and_reuse_slot { - self.important_count -= 1; - } else { + if slot.id() == declaration.id() { + let important = self.declarations_importance.get(i as u32); + match (important, importance.important()) { + (false, true) => {} + + (true, false) => { + if !overwrite_more_important_and_reuse_slot { return false } } - _ => if slot.0 == declaration { + _ => if *slot == declaration { return false; } } if overwrite_more_important_and_reuse_slot { - *slot = (declaration, importance); + *slot = declaration; + self.declarations_importance.set(i as u32, importance.important()); return true; } @@ -441,7 +464,9 @@ impl PropertyDeclarationBlock { if let Some(index) = index_to_remove { self.declarations.remove(index); - self.declarations.push((declaration, importance)); + self.declarations_importance.remove(index as u32); + self.declarations.push(declaration); + self.declarations_importance.push(importance.important()); return true; } } @@ -449,10 +474,8 @@ impl PropertyDeclarationBlock { if let Some(id) = longhand_id { self.longhands.insert(id); } - self.declarations.push((declaration, importance)); - if importance.important() { - self.important_count += 1; - } + self.declarations.push(declaration); + self.declarations_importance.push(importance.important()); true } @@ -461,21 +484,13 @@ impl PropertyDeclarationBlock { /// Returns whether any declaration was updated. pub fn set_importance(&mut self, property: &PropertyId, new_importance: Importance) -> bool { let mut updated_at_least_one = false; - for &mut (ref declaration, ref mut importance) in &mut self.declarations { + for (i, declaration) in self.declarations.iter().enumerate() { if declaration.id().is_or_is_longhand_of(property) { - match (*importance, new_importance) { - (Importance::Normal, Importance::Important) => { - self.important_count += 1; - } - (Importance::Important, Importance::Normal) => { - self.important_count -= 1; - } - _ => { - continue; - } + let is_important = new_importance.important(); + if self.declarations_importance.get(i as u32) != is_important { + self.declarations_importance.set(i as u32, is_important); + updated_at_least_one = true; } - updated_at_least_one = true; - *importance = new_importance; } } updated_at_least_one @@ -490,20 +505,21 @@ impl PropertyDeclarationBlock { return false } } - let important_count = &mut self.important_count; let mut removed_at_least_one = false; let longhands = &mut self.longhands; - self.declarations.retain(|&(ref declaration, importance)| { + let declarations_importance = &mut self.declarations_importance; + let mut i = 0; + self.declarations.retain(|declaration| { let id = declaration.id(); let remove = id.is_or_is_longhand_of(property); if remove { removed_at_least_one = true; if let PropertyDeclarationId::Longhand(id) = id { - longhands.remove(id) - } - if importance.important() { - *important_count -= 1 + longhands.remove(id); } + declarations_importance.remove(i); + } else { + i += 1; } !remove }); @@ -527,7 +543,7 @@ impl PropertyDeclarationBlock { match property.as_shorthand() { Err(_longhand_or_custom) => { if self.declarations.len() == 1 { - let declaration = &self.declarations[0].0; + let declaration = &self.declarations[0]; // If we have a longhand declaration with variables, those variables will be // stored as unparsed values. As a temporary measure to produce sensible results // in Gecko's getKeyframes() implementation for CSS animations, if @@ -549,10 +565,10 @@ impl PropertyDeclarationBlock { } } Ok(shorthand) => { - if !self.declarations.iter().all(|decl| decl.0.shorthands().contains(&shorthand)) { + if !self.declarations.iter().all(|decl| decl.shorthands().contains(&shorthand)) { return Err(fmt::Error) } - let iter = self.declarations_iter(); + let iter = self.declarations.iter(); match shorthand.get_shorthand_appendable_value(iter) { Some(AppendableValue::Css { css, .. }) => { dest.write_str(css) @@ -569,17 +585,18 @@ impl PropertyDeclarationBlock { /// Convert AnimationValueMap to PropertyDeclarationBlock. #[cfg(feature = "gecko")] pub fn from_animation_value_map(animation_value_map: &AnimationValueMap) -> Self { - let mut declarations = vec![]; + let len = animation_value_map.len(); + let mut declarations = Vec::with_capacity(len); let mut longhands = LonghandIdSet::new(); for (property, animation_value) in animation_value_map.iter() { longhands.set_animatable_longhand_bit(property); - declarations.push((animation_value.uncompute(), Importance::Normal)); + declarations.push(animation_value.uncompute()); } PropertyDeclarationBlock { declarations: declarations, - important_count: 0, + declarations_importance: SmallBitVec::from_elem(len as u32, false), longhands: longhands, } } @@ -593,7 +610,7 @@ impl PropertyDeclarationBlock { return false } } - self.declarations.iter().any(|&(ref decl, _)| + self.declarations.iter().any(|decl| decl.id().is_or_is_longhand_of(property) && decl.get_css_wide_keyword().is_some() ) @@ -613,7 +630,7 @@ impl ToCss for PropertyDeclarationBlock { let mut already_serialized = PropertyDeclarationIdSet::new(); // Step 3 - for &(ref declaration, importance) in &*self.declarations { + for (declaration, importance) in self.declaration_importance_iter() { // Step 3.1 let property = declaration.id(); @@ -639,13 +656,13 @@ impl ToCss for PropertyDeclarationBlock { let is_system_font = shorthand == ShorthandId::Font && - self.declarations.iter().any(|&(ref l, _)| { + self.declarations.iter().any(|l| { !already_serialized.contains(l.id()) && l.get_system().is_some() }); if is_system_font { - for &(ref longhand, longhand_importance) in &self.declarations { + for (longhand, importance) in self.declaration_importance_iter() { if already_serialized.contains(longhand.id()) { continue; } @@ -655,20 +672,20 @@ impl ToCss for PropertyDeclarationBlock { if found_system.is_none() { found_system = longhand.get_system(); } - if longhand_importance.important() { + if importance.important() { important_count += 1; } } } } else { - for &(ref longhand, longhand_importance) in &self.declarations { + for (longhand, importance) in self.declaration_importance_iter() { if already_serialized.contains(longhand.id()) { continue; } if longhand.id().is_longhand_of(shorthand) { current_longhands.push(longhand); - if longhand_importance.important() { + if importance.important() { important_count += 1; } } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index cfb7e04c1f3..919a5b8c11d 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -12,6 +12,7 @@ #[cfg(feature = "servo")] use app_units::Au; use servo_arc::{Arc, UniqueArc}; +use smallbitvec::SmallBitVec; use std::borrow::Cow; use hash::HashSet; use std::{fmt, mem, ops}; @@ -3007,25 +3008,26 @@ pub fn cascade( ptr::eq(parent_style.unwrap(), parent_style_ignoring_first_line.unwrap()) || parent_style.unwrap().pseudo() == Some(PseudoElement::FirstLine)); + let empty = SmallBitVec::new(); let iter_declarations = || { rule_node.self_and_ancestors().flat_map(|node| { let cascade_level = node.cascade_level(); let source = node.style_source(); + let declarations = if source.is_some() { - source.read(cascade_level.guard(guards)).declarations() + source.read(cascade_level.guard(guards)).declaration_importance_iter() } else { // The root node has no style source. - &[] + DeclarationImportanceIterator::new(&[], &empty) }; let node_importance = node.importance(); let property_restriction = pseudo.and_then(|p| p.property_restriction()); declarations - .iter() // Yield declarations later in source order (with more precedence) first. .rev() - .filter_map(move |&(ref declaration, declaration_importance)| { + .filter_map(move |(declaration, declaration_importance)| { if let Some(property_restriction) = property_restriction { // declaration.id() is either a longhand or a custom // property. Custom properties are always allowed, but diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 6f677fdbe79..cf0994d7a9f 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -1211,15 +1211,15 @@ impl StrongRuleNode { for node in element_rule_node.self_and_ancestors() { let source = node.style_source(); let declarations = if source.is_some() { - source.read(node.cascade_level().guard(guards)).declarations() + source.read(node.cascade_level().guard(guards)).declaration_importance_iter() } else { continue }; // Iterate over declarations of the longhands we care about. let node_importance = node.importance(); - let longhands = declarations.iter().rev() - .filter_map(|&(ref declaration, importance)| { + let longhands = declarations.rev() + .filter_map(|(declaration, importance)| { if importance != node_importance { return None } match declaration.id() { PropertyDeclarationId::Longhand(id) => { @@ -1329,9 +1329,8 @@ impl StrongRuleNode { let mut result = (LonghandIdSet::new(), false); for node in iter { let style = node.style_source(); - for &(ref decl, important) in style.read(node.cascade_level().guard(guards)) - .declarations() - .iter() { + for (decl, important) in style.read(node.cascade_level().guard(guards)) + .declaration_importance_iter() { // Although we are only iterating over cascade levels that // override animations, in a given property declaration block we // can have a mixture of !important and non-!important diff --git a/components/style/stylesheets/keyframes_rule.rs b/components/style/stylesheets/keyframes_rule.rs index e9fffab9209..be577a0d3d8 100644 --- a/components/style/stylesheets/keyframes_rule.rs +++ b/components/style/stylesheets/keyframes_rule.rs @@ -299,7 +299,7 @@ impl KeyframesStep { guard: &SharedRwLockReadGuard) -> Self { let declared_timing_function = match value { KeyframesStepValue::Declarations { ref block } => { - block.read_with(guard).declarations().iter().any(|&(ref prop_decl, _)| { + block.read_with(guard).declarations().iter().any(|prop_decl| { match *prop_decl { PropertyDeclaration::AnimationTimingFunction(..) => true, _ => false, @@ -325,7 +325,7 @@ impl KeyframesStep { match self.value { KeyframesStepValue::Declarations { ref block } => { let guard = block.read_with(guard); - let &(ref declaration, _) = + let (declaration, _) = guard.get(PropertyDeclarationId::Longhand(LonghandId::AnimationTimingFunction)).unwrap(); match *declaration { PropertyDeclaration::AnimationTimingFunction(ref value) => { @@ -369,7 +369,7 @@ fn get_animated_properties(keyframes: &[Arc>], guard: &SharedRw for keyframe in keyframes { let keyframe = keyframe.read_with(&guard); let block = keyframe.block.read_with(guard); - for &(ref declaration, importance) in block.declarations().iter() { + for (declaration, importance) in block.declaration_importance_iter() { assert!(!importance.important()); if let Some(property) = AnimatableLonghand::from_declaration(declaration) { diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 0f908675228..575f30ec253 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2212,7 +2212,7 @@ pub extern "C" fn Servo_SerializeFontValueForCanvas( use style::properties::shorthands::font; read_locked_arc(declarations, |decls: &PropertyDeclarationBlock| { - let longhands = match font::LonghandsToSerialize::from_iter(decls.declarations_iter()) { + let longhands = match font::LonghandsToSerialize::from_iter(decls.declarations().iter()) { Ok(l) => l, Err(()) => { warn!("Unexpected property!"); @@ -2240,7 +2240,7 @@ pub extern "C" fn Servo_DeclarationBlock_Count(declarations: RawServoDeclaration pub extern "C" fn Servo_DeclarationBlock_GetNthProperty(declarations: RawServoDeclarationBlockBorrowed, index: u32, result: *mut nsAString) -> bool { read_locked_arc(declarations, |decls: &PropertyDeclarationBlock| { - if let Some(&(ref decl, _)) = decls.declarations().get(index as usize) { + if let Some(decl) = decls.declarations().get(index as usize) { let result = unsafe { result.as_mut().unwrap() }; result.assign_utf8(&decl.id().name()); true @@ -3312,8 +3312,8 @@ pub extern "C" fn Servo_AnimationValue_Compute(element: RawGeckoElementBorrowed, let guard = global_style_data.shared_lock.read(); let declarations = Locked::::as_arc(&declarations); // We only compute the first element in declarations. - match declarations.read_with(&guard).declarations().first() { - Some(&(ref decl, imp)) if imp == Importance::Normal => { + match declarations.read_with(&guard).declaration_importance_iter().next() { + Some((decl, imp)) if imp == Importance::Normal => { let animation = AnimationValue::from_declaration(decl, &mut context, default_values); animation.map_or(RawServoAnimationValueStrong::null(), |value| { Arc::new(value).into_strong() @@ -3459,11 +3459,9 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName(raw_data: RawServoStyleSetB let animatable = guard.declarations() .iter() - .filter(|&&(ref declaration, _)| { - declaration.is_animatable() - }); + .filter(|declaration| declaration.is_animatable()); - for &(ref declaration, _) in animatable { + for declaration in animatable { let property = AnimatableLonghand::from_declaration(declaration).unwrap(); // Skip the 'display' property because although it is animatable from SMIL, // it should not be animatable from CSS Animations.