diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index f658ed5375e..8af3b3975d2 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -62,15 +62,12 @@ impl CSSStyleOwner { let result = f(&mut pdb, &mut changed); result } else { - let mut pdb = PropertyDeclarationBlock { - important_count: 0, - declarations: vec![], - }; + let mut pdb = PropertyDeclarationBlock::new(); let result = f(&mut pdb, &mut changed); // Here `changed` is somewhat silly, because we know the // exact conditions under it changes. - changed = !pdb.declarations.is_empty(); + changed = !pdb.declarations().is_empty(); if changed { attr = Some(Arc::new(RwLock::new(pdb))); } @@ -116,10 +113,7 @@ impl CSSStyleOwner { match *el.style_attribute().borrow() { Some(ref pdb) => f(&pdb.read()), None => { - let pdb = PropertyDeclarationBlock { - important_count: 0, - declarations: vec![], - }; + let pdb = PropertyDeclarationBlock::new(); f(&pdb) } } @@ -250,14 +244,14 @@ impl CSSStyleDeclaration { // Step 6 let window = self.owner.window(); - let declarations = + let result = parse_one_declaration(id, &value, &self.owner.base_url(), window.css_error_reporter(), ParserContextExtraData::default()); // Step 7 - let declarations = match declarations { - Ok(declarations) => declarations, + let parsed = match result { + Ok(parsed) => parsed, Err(_) => { *changed = false; return Ok(()); @@ -267,9 +261,9 @@ impl CSSStyleDeclaration { // Step 8 // Step 9 *changed = false; - for declaration in declarations { - *changed |= pdb.set_parsed_declaration(declaration.0, importance); - } + parsed.expand(|declaration| { + *changed |= pdb.set_parsed_declaration(declaration, importance); + }); Ok(()) }) @@ -280,7 +274,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-length fn Length(&self) -> u32 { self.owner.with_block(|pdb| { - pdb.declarations.len() as u32 + pdb.declarations().len() as u32 }) } @@ -405,7 +399,7 @@ 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| { + pdb.declarations().get(index as usize).map(|entry| { let (ref declaration, importance) = *entry; let mut css = declaration.to_css_string(); if importance.important() { diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index eeff67ed318..a0fb1d0f179 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -385,10 +385,9 @@ impl LayoutElementHelpers for LayoutJS { #[inline] fn from_declaration(declaration: PropertyDeclaration) -> ApplicableDeclarationBlock { ApplicableDeclarationBlock::from_declarations( - Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![(declaration, Importance::Normal)], - important_count: 0, - })), + Arc::new(RwLock::new(PropertyDeclarationBlock::with_one( + declaration, Importance::Normal + ))), CascadeLevel::PresHints) } diff --git a/components/style/animation.rs b/components/style/animation.rs index aa115527d57..bebda0e6339 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -418,11 +418,11 @@ fn compute_style_for_animation_step(context: &SharedStyleContext, let guard = declarations.read(); // No !important in keyframes. - debug_assert!(guard.declarations.iter() + debug_assert!(guard.declarations().iter() .all(|&(_, importance)| importance == Importance::Normal)); let iter = || { - guard.declarations.iter().rev().map(|&(ref decl, _importance)| decl) + guard.declarations().iter().rev().map(|&(ref decl, _importance)| decl) }; let computed = diff --git a/components/style/font_face.rs b/components/style/font_face.rs index 4b3f6934f90..0d4bc4f8a09 100644 --- a/components/style/font_face.rs +++ b/components/style/font_face.rs @@ -21,7 +21,7 @@ use values::specified::url::SpecifiedUrl; /// A source for a font-face rule. #[derive(Clone, Debug, PartialEq, Eq)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))] +#[cfg_attr(feature = "servo", derive(Deserialize, Serialize))] pub enum Source { /// A `url()` source. Url(UrlSource), @@ -54,7 +54,7 @@ impl OneOrMoreCommaSeparated for Source {} /// /// https://drafts.csswg.org/css-fonts/#src-desc #[derive(Clone, Debug, PartialEq, Eq)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))] +#[cfg_attr(feature = "servo", derive(Deserialize, Serialize))] pub struct UrlSource { /// The specified url. pub url: SpecifiedUrl, @@ -184,7 +184,6 @@ macro_rules! font_face_descriptors { /// /// https://drafts.csswg.org/css-fonts/#font-face-rule #[derive(Debug, PartialEq, Eq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct FontFaceRule { $( #[$m_doc] diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index 87bebc308d2..348bc028ee6 100644 --- a/components/style/keyframes.rs +++ b/components/style/keyframes.rs @@ -11,9 +11,8 @@ use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule}; use parking_lot::RwLock; use parser::{ParserContext, ParserContextExtraData, log_css_error}; use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock, PropertyId}; -use properties::{PropertyDeclarationId, LonghandId, DeclaredValue}; +use properties::{PropertyDeclarationId, LonghandId, DeclaredValue, ParsedDeclaration}; use properties::LonghandIdSet; -use properties::PropertyDeclarationParseResult; use properties::animated_properties::TransitionProperty; use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction; use std::fmt; @@ -71,8 +70,7 @@ impl KeyframePercentage { /// A keyframes selector is a list of percentages or from/to symbols, which are /// converted at parse time to percentages. -#[derive(Debug, Clone, PartialEq)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +#[derive(Debug, PartialEq)] pub struct KeyframeSelector(Vec); impl KeyframeSelector { /// Return the list of percentages this selector contains. @@ -94,8 +92,7 @@ impl KeyframeSelector { } /// A keyframe. -#[derive(Debug, Clone)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +#[derive(Debug)] pub struct Keyframe { /// The selector this keyframe was specified from. pub selector: KeyframeSelector, @@ -104,7 +101,6 @@ pub struct Keyframe { /// /// Note that `!important` rules in keyframes don't apply, but we keep this /// `Arc` just for convenience. - #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] pub block: Arc>, } @@ -149,7 +145,7 @@ impl Keyframe { /// declarations to apply. /// /// TODO: Find a better name for this? -#[derive(Debug, Clone)] +#[derive(Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum KeyframesStepValue { /// A step formed by a declaration block specified by the CSS. @@ -164,7 +160,7 @@ pub enum KeyframesStepValue { } /// A single step from a keyframe animation. -#[derive(Debug, Clone)] +#[derive(Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct KeyframesStep { /// The percentage of the animation duration when this step starts. @@ -185,7 +181,7 @@ impl KeyframesStep { value: KeyframesStepValue) -> Self { let declared_timing_function = match value { KeyframesStepValue::Declarations { ref block } => { - block.read().declarations.iter().any(|&(ref prop_decl, _)| { + block.read().declarations().iter().any(|&(ref prop_decl, _)| { match *prop_decl { PropertyDeclaration::AnimationTimingFunction(..) => true, _ => false, @@ -236,7 +232,7 @@ impl KeyframesStep { /// of keyframes, in order. /// /// It only takes into account animable properties. -#[derive(Debug, Clone)] +#[derive(Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct KeyframesAnimation { /// The difference steps of the animation. @@ -253,7 +249,7 @@ fn get_animated_properties(keyframes: &[Arc>]) -> Vec QualifiedRuleParser for KeyframeListParser<'a> { -> Result { let parser = KeyframeDeclarationParser { context: self.context, - declarations: vec![], }; let mut iter = DeclarationListParser::new(input, parser); + let mut block = PropertyDeclarationBlock::new(); while let Some(declaration) = iter.next() { match declaration { - Ok(_) => (), + Ok(parsed) => parsed.expand(|d| block.push(d, Importance::Normal)), Err(range) => { let pos = range.start; let message = format!("Unsupported keyframe property declaration: '{}'", @@ -381,45 +377,36 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { } Ok(Arc::new(RwLock::new(Keyframe { selector: prelude, - block: Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: iter.parser.declarations, - important_count: 0, - })), + block: Arc::new(RwLock::new(block)), }))) } } struct KeyframeDeclarationParser<'a, 'b: 'a> { context: &'a ParserContext<'b>, - declarations: Vec<(PropertyDeclaration, Importance)> } /// Default methods reject all at rules. impl<'a, 'b> AtRuleParser for KeyframeDeclarationParser<'a, 'b> { type Prelude = (); - type AtRule = (); + type AtRule = ParsedDeclaration; } impl<'a, 'b> DeclarationParser for KeyframeDeclarationParser<'a, 'b> { - /// We parse rules directly into the declarations object - type Declaration = (); + type Declaration = ParsedDeclaration; - fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<(), ()> { + fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result { let id = try!(PropertyId::parse(name.into())); - let old_len = self.declarations.len(); - match PropertyDeclaration::parse(id, self.context, input, &mut self.declarations, true) { - PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => {} - _ => { - self.declarations.truncate(old_len); - return Err(()); + match ParsedDeclaration::parse(id, self.context, input, true) { + Ok(parsed) => { + // In case there is still unparsed text in the declaration, we should roll back. + if !input.is_exhausted() { + Err(()) + } else { + Ok(parsed) + } } - } - // In case there is still unparsed text in the declaration, we should roll back. - if !input.is_exhausted() { - self.declarations.truncate(old_len); - Err(()) - } else { - Ok(()) + Err(_) => Err(()) } } } diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index ac618dcbf72..2cb5cba4874 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -40,28 +40,54 @@ impl Importance { } } -impl Default for Importance { - #[inline] - fn default() -> Self { - Importance::Normal - } -} - /// Overridden declarations are skipped. -#[derive(Debug, PartialEq, Clone)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +#[derive(Clone)] pub struct PropertyDeclarationBlock { /// The group of declarations, along with their importance. /// /// Only deduplicated declarations appear here. - #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")] - pub declarations: Vec<(PropertyDeclaration, Importance)>, + declarations: Vec<(PropertyDeclaration, Importance)>, /// The number of entries in `self.declaration` with `Importance::Important` - pub important_count: u32, + important_count: usize, + + longhands: LonghandIdSet, +} + +impl fmt::Debug for PropertyDeclarationBlock { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.declarations.fmt(f) + } } impl PropertyDeclarationBlock { + /// Create an empty block + pub fn new() -> Self { + PropertyDeclarationBlock { + declarations: Vec::new(), + important_count: 0, + longhands: LonghandIdSet::new(), + } + } + + /// Create a block with a single declaration + pub fn with_one(declaration: PropertyDeclaration, importance: Importance) -> Self { + let mut longhands = LonghandIdSet::new(); + if let PropertyDeclarationId::Longhand(id) = declaration.id() { + longhands.insert(id); + } + PropertyDeclarationBlock { + declarations: vec![(declaration, importance)], + important_count: if importance.important() { 1 } else { 0 }, + longhands: longhands, + } + } + + /// The declarations in this block + pub fn declarations(&self) -> &[(PropertyDeclaration, Importance)] { + &self.declarations + } + /// Returns wheather this block contains any declaration with `!important`. /// /// This is based on the `important_count` counter, @@ -77,7 +103,7 @@ impl PropertyDeclarationBlock { /// 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 as usize + self.declarations.len() > self.important_count } /// Get a declaration for a given property. @@ -171,29 +197,54 @@ impl PropertyDeclarationBlock { } /// Adds or overrides the declaration for a given property in this block, - /// without taking into account any kind of priority. Returns whether the - /// declaration block is actually changed. + /// except if an existing declaration for the same property is more important. + pub fn push(&mut self, declaration: PropertyDeclaration, importance: Importance) { + self.push_common(declaration, importance, false); + } + + /// Adds or overrides the declaration for a given property in this block, + /// Returns whether the declaration block is actually changed. pub fn set_parsed_declaration(&mut self, declaration: PropertyDeclaration, importance: Importance) -> bool { - for slot in &mut *self.declarations { - if slot.0.id() == declaration.id() { - match (slot.1, importance) { - (Importance::Normal, Importance::Important) => { - self.important_count += 1; - } - (Importance::Important, Importance::Normal) => { - self.important_count -= 1; - } - _ => if slot.0 == declaration { - return false; + self.push_common(declaration, importance, true) + } + + fn push_common(&mut self, declaration: PropertyDeclaration, importance: Importance, + overwrite_more_important: bool) -> bool { + let definitely_new = if let PropertyDeclarationId::Longhand(id) = declaration.id() { + !self.longhands.contains(id) + } else { + false // For custom properties, always scan + }; + + if !definitely_new { + for slot in &mut *self.declarations { + 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 { + self.important_count -= 1; + } else { + return false + } + } + _ => if slot.0 == declaration { + return false; + } } + *slot = (declaration, importance); + return true } - *slot = (declaration, importance); - return true; } } + if let PropertyDeclarationId::Longhand(id) = declaration.id() { + self.longhands.insert(id); + } self.declarations.push((declaration, importance)); if importance.important() { self.important_count += 1; @@ -230,6 +281,11 @@ impl PropertyDeclarationBlock { /// /// Returns whether any declaration was actually removed. pub fn remove_property(&mut self, property: &PropertyId) -> bool { + if let PropertyId::Longhand(id) = *property { + if !self.longhands.contains(id) { + return false + } + } let important_count = &mut self.important_count; let mut removed_at_least_one = false; self.declarations.retain(|&(ref declaration, importance)| { @@ -243,6 +299,10 @@ impl PropertyDeclarationBlock { !remove }); + if let PropertyId::Longhand(id) = *property { + debug_assert!(removed_at_least_one); + self.longhands.remove(id); + } removed_at_least_one } @@ -278,39 +338,6 @@ impl PropertyDeclarationBlock { } } } - - /// Only keep the "winning" declaration for any given property, by importance then source order. - pub fn deduplicate(&mut self) { - let mut deduplicated = Vec::with_capacity(self.declarations.len()); - let mut seen_normal = PropertyDeclarationIdSet::new(); - let mut seen_important = PropertyDeclarationIdSet::new(); - - for (declaration, importance) in self.declarations.drain(..).rev() { - if importance.important() { - let id = declaration.id(); - if seen_important.contains(id) { - self.important_count -= 1; - continue - } - if seen_normal.contains(id) { - let previous_len = deduplicated.len(); - deduplicated.retain(|&(ref d, _)| PropertyDeclaration::id(d) != id); - debug_assert_eq!(deduplicated.len(), previous_len - 1); - } - seen_important.insert(id); - } else { - let id = declaration.id(); - if seen_normal.contains(id) || - seen_important.contains(id) { - continue - } - seen_normal.insert(id) - } - deduplicated.push((declaration, importance)) - } - deduplicated.reverse(); - self.declarations = deduplicated; - } } impl ToCss for PropertyDeclarationBlock { @@ -556,65 +583,46 @@ pub fn parse_one_declaration(id: PropertyId, base_url: &ServoUrl, error_reporter: StdBox, extra_data: ParserContextExtraData) - -> Result, ()> { + -> Result { let context = ParserContext::new_with_extra_data(Origin::Author, base_url, error_reporter, extra_data); Parser::new(input).parse_entirely(|parser| { - let mut results = vec![]; - match PropertyDeclaration::parse(id, &context, parser, &mut results, false) { - PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => Ok(results), - _ => Err(()) - } + ParsedDeclaration::parse(id, &context, parser, false) + .map_err(|_| ()) }) } /// A struct to parse property declarations. struct PropertyDeclarationParser<'a, 'b: 'a> { context: &'a ParserContext<'b>, - declarations: Vec<(PropertyDeclaration, Importance)> } /// Default methods reject all at rules. impl<'a, 'b> AtRuleParser for PropertyDeclarationParser<'a, 'b> { type Prelude = (); - type AtRule = (u32, Importance); + type AtRule = (ParsedDeclaration, Importance); } impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> { - /// Declarations are pushed to the internal vector. This will only - /// let you know if the decl was !important and how many longhands - /// it expanded to - type Declaration = (u32, Importance); + type Declaration = (ParsedDeclaration, Importance); fn parse_value(&mut self, name: &str, input: &mut Parser) - -> Result<(u32, Importance), ()> { + -> Result<(ParsedDeclaration, Importance), ()> { let id = try!(PropertyId::parse(name.into())); - let old_len = self.declarations.len(); - let parse_result = input.parse_until_before(Delimiter::Bang, |input| { - match PropertyDeclaration::parse(id, self.context, input, &mut self.declarations, false) { - PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => Ok(()), - _ => Err(()) - } - }); - if let Err(_) = parse_result { - // rollback - self.declarations.truncate(old_len); - return Err(()) - } + let parsed = input.parse_until_before(Delimiter::Bang, |input| { + ParsedDeclaration::parse(id, self.context, input, false) + .map_err(|_| ()) + })?; let importance = match input.try(parse_important) { Ok(()) => Importance::Important, Err(()) => Importance::Normal, }; // In case there is still unparsed text in the declaration, we should roll back. if !input.is_exhausted() { - self.declarations.truncate(old_len); return Err(()) } - for decl in &mut self.declarations[old_len..] { - decl.1 = importance - } - Ok(((self.declarations.len() - old_len) as u32, importance)) + Ok((parsed, importance)) } } @@ -624,19 +632,14 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> { pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Parser) -> PropertyDeclarationBlock { - let mut important_count = 0; + let mut block = PropertyDeclarationBlock::new(); let parser = PropertyDeclarationParser { context: context, - declarations: vec![], }; let mut iter = DeclarationListParser::new(input, parser); while let Some(declaration) = iter.next() { match declaration { - Ok((count, importance)) => { - if importance.important() { - important_count += count; - } - } + Ok((parsed, importance)) => parsed.expand(|d| block.push(d, importance)), Err(range) => { let pos = range.start; let message = format!("Unsupported property declaration: '{}'", @@ -645,10 +648,5 @@ pub fn parse_property_declaration_list(context: &ParserContext, } } } - let mut block = PropertyDeclarationBlock { - declarations: iter.parser.declarations, - important_count: important_count, - }; - block.deduplicate(); block } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 69a1cca08bb..8e52d9d739c 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -168,23 +168,19 @@ impl ComputedValues { % for prop in data.longhands: % if prop.animatable: PropertyDeclarationId::Longhand(LonghandId::${prop.camel_case}) => { - PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::${prop.camel_case}(DeclaredValue::Value( - % if prop.boxed: - Box::new( - % endif - longhands::${prop.ident}::SpecifiedValue::from_computed_value( - &self.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}()) - % if prop.boxed: - ) - % endif - - )), - Importance::Normal) - ], - important_count: 0 - } + PropertyDeclarationBlock::with_one( + PropertyDeclaration::${prop.camel_case}(DeclaredValue::Value( + % if prop.boxed: + Box::new( + % endif + longhands::${prop.ident}::SpecifiedValue::from_computed_value( + &self.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}()) + % if prop.boxed: + ) + % endif + )), + Importance::Normal + ) }, % endif % endfor diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index b85f9de75db..e2f182e3fcb 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -339,7 +339,7 @@ input.reset(start); let (first_token_type, css) = try!( ::custom_properties::parse_non_custom_with_var(input)); - return Ok(DeclaredValue::WithVariables(Box::new(UnparsedValue { + return Ok(DeclaredValue::WithVariables(Arc::new(UnparsedValue { css: css.into_owned(), first_token_type: first_token_type, base_url: context.base_url.clone(), @@ -483,10 +483,10 @@ #[allow(unused_imports)] use cssparser::Parser; use parser::ParserContext; - use properties::{DeclaredValue, PropertyDeclaration}; + use properties::{DeclaredValue, PropertyDeclaration, ParsedDeclaration}; use properties::{ShorthandId, UnparsedValue, longhands}; - use properties::declaration_block::Importance; use std::fmt; + use std::sync::Arc; use style_traits::ToCss; pub struct Longhands { @@ -551,10 +551,7 @@ /// Parse the given shorthand and fill the result into the /// `declarations` vector. - pub fn parse(context: &ParserContext, - input: &mut Parser, - declarations: &mut Vec<(PropertyDeclaration, Importance)>) - -> Result<(), ()> { + pub fn parse(context: &ParserContext, input: &mut Parser) -> Result { input.look_for_var_functions(); let start = input.position(); let value = input.parse_entirely(|input| parse_value(context, input)); @@ -563,31 +560,17 @@ } let var = input.seen_var_functions(); if let Ok(value) = value { - % for sub_property in shorthand.sub_properties: - declarations.push((PropertyDeclaration::${sub_property.camel_case}( - % if sub_property.boxed: - DeclaredValue::Value(Box::new(value.${sub_property.ident})) - % else: - DeclaredValue::Value(value.${sub_property.ident}) - % endif - ), Importance::Normal)); - % endfor - Ok(()) + Ok(ParsedDeclaration::${shorthand.camel_case}(value)) } else if var { input.reset(start); let (first_token_type, css) = try!( ::custom_properties::parse_non_custom_with_var(input)); - % for sub_property in shorthand.sub_properties: - declarations.push((PropertyDeclaration::${sub_property.camel_case}( - DeclaredValue::WithVariables(Box::new(UnparsedValue { - css: css.clone().into_owned(), - first_token_type: first_token_type, - base_url: context.base_url.clone(), - from_shorthand: Some(ShorthandId::${shorthand.camel_case}), - })) - ), Importance::Normal)); - % endfor - Ok(()) + Ok(ParsedDeclaration::${shorthand.camel_case}WithVariables(Arc::new(UnparsedValue { + css: css.into_owned(), + first_token_type: first_token_type, + base_url: context.base_url.clone(), + from_shorthand: Some(ShorthandId::${shorthand.camel_case}), + }))) } else { Err(()) } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index e0a2b24b6cc..af4943e8d33 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -177,6 +177,7 @@ pub mod animated_properties { } /// A set of longhand properties +#[derive(Clone)] pub struct LonghandIdSet { storage: [u32; (${len(data.longhands)} - 1 + 32) / 32] } @@ -202,6 +203,13 @@ impl LonghandIdSet { self.storage[bit / 32] |= 1 << (bit % 32); } + /// Remove the given property from the set + #[inline] + pub fn remove(&mut self, id: LonghandId) { + let bit = id as usize; + self.storage[bit / 32] &= !(1 << (bit % 32)); + } + /// Set the corresponding bit of TransitionProperty. /// This function will panic if TransitionProperty::All is given. pub fn set_transition_property_bit(&mut self, property: &TransitionProperty) { @@ -563,19 +571,17 @@ impl ShorthandId { /// Servo's representation of a declared value for a given `T`, which is the /// declared value for that property. #[derive(Clone, PartialEq, Eq, Debug)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum DeclaredValue { /// A known specified value from the stylesheet. Value(T), /// An unparsed value that contains `var()` functions. - WithVariables(Box), + WithVariables(Arc), /// An CSS-wide keyword. CSSWideKeyword(CSSWideKeyword), } /// An unparsed property value that contains `var()` functions. -#[derive(Clone, PartialEq, Eq, Debug)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +#[derive(PartialEq, Eq, Debug)] pub struct UnparsedValue { /// The css serialization for this value. css: String, @@ -797,9 +803,158 @@ impl PropertyId { } } +/// Includes shorthands before expansion +pub enum ParsedDeclaration { + % for shorthand in data.shorthands: + /// ${shorthand.name} + ${shorthand.camel_case}(shorthands::${shorthand.ident}::Longhands), + + /// ${shorthand.name} with a CSS-wide keyword + ${shorthand.camel_case}CSSWideKeyword(CSSWideKeyword), + + /// ${shorthand.name} with var() functions + ${shorthand.camel_case}WithVariables(Arc), + % endfor + + /// Not a shorthand + LonghandOrCustom(PropertyDeclaration), +} + +impl ParsedDeclaration { + /// Transform this ParsedDeclaration into a sequence of PropertyDeclaration + /// by expanding shorthand declarations into their corresponding longhands + pub fn expand(self, mut push: F) where F: FnMut(PropertyDeclaration) { + match self { + % for shorthand in data.shorthands: + ParsedDeclaration::${shorthand.camel_case}( + shorthands::${shorthand.ident}::Longhands { + % for sub_property in shorthand.sub_properties: + ${sub_property.ident}, + % endfor + } + ) => { + % for sub_property in shorthand.sub_properties: + push(PropertyDeclaration::${sub_property.camel_case}( + % if sub_property.boxed: + DeclaredValue::Value(Box::new(${sub_property.ident})) + % else: + DeclaredValue::Value(${sub_property.ident}) + % endif + )); + % endfor + } + ParsedDeclaration::${shorthand.camel_case}CSSWideKeyword(keyword) => { + % for sub_property in shorthand.sub_properties: + push(PropertyDeclaration::${sub_property.camel_case}( + DeclaredValue::CSSWideKeyword(keyword) + )); + % endfor + } + ParsedDeclaration::${shorthand.camel_case}WithVariables(value) => { + debug_assert_eq!( + value.from_shorthand, + Some(ShorthandId::${shorthand.camel_case}) + ); + % for sub_property in shorthand.sub_properties: + push(PropertyDeclaration::${sub_property.camel_case}( + DeclaredValue::WithVariables(value.clone()) + )); + % endfor + } + % endfor + ParsedDeclaration::LonghandOrCustom(declaration) => push(declaration), + } + } + + /// The `in_keyframe_block` parameter controls this: + /// + /// https://drafts.csswg.org/css-animations/#keyframes + /// > The inside of accepts any CSS property + /// > except those defined in this specification, + /// > but does accept the `animation-play-state` property and interprets it specially. + /// + /// This will not actually parse Importance values, and will always set things + /// to Importance::Normal. Parsing Importance values is the job of PropertyDeclarationParser, + /// we only set them here so that we don't have to reallocate + pub fn parse(id: PropertyId, context: &ParserContext, input: &mut Parser, + in_keyframe_block: bool) + -> Result { + match id { + PropertyId::Custom(name) => { + let value = match input.try(|i| CSSWideKeyword::parse(context, i)) { + Ok(keyword) => DeclaredValue::CSSWideKeyword(keyword), + Err(()) => match ::custom_properties::SpecifiedValue::parse(context, input) { + Ok(value) => DeclaredValue::Value(value), + Err(()) => return Err(PropertyDeclarationParseError::InvalidValue), + } + }; + Ok(ParsedDeclaration::LonghandOrCustom(PropertyDeclaration::Custom(name, value))) + } + PropertyId::Longhand(id) => match id { + % for property in data.longhands: + LonghandId::${property.camel_case} => { + % if not property.derived_from: + % if not property.allowed_in_keyframe_block: + if in_keyframe_block { + return Err(PropertyDeclarationParseError::AnimationPropertyInKeyframeBlock) + } + % endif + % if property.internal: + if context.stylesheet_origin != Origin::UserAgent { + return Err(PropertyDeclarationParseError::UnknownProperty) + } + % endif + + ${property_pref_check(property)} + + match longhands::${property.ident}::parse_declared(context, input) { + Ok(value) => { + Ok(ParsedDeclaration::LonghandOrCustom( + PropertyDeclaration::${property.camel_case}(value) + )) + }, + Err(()) => Err(PropertyDeclarationParseError::InvalidValue), + } + % else: + Err(PropertyDeclarationParseError::UnknownProperty) + % endif + } + % endfor + }, + PropertyId::Shorthand(id) => match id { + % for shorthand in data.shorthands: + ShorthandId::${shorthand.camel_case} => { + % if not shorthand.allowed_in_keyframe_block: + if in_keyframe_block { + return Err(PropertyDeclarationParseError::AnimationPropertyInKeyframeBlock) + } + % endif + % if shorthand.internal: + if context.stylesheet_origin != Origin::UserAgent { + return Err(PropertyDeclarationParseError::UnknownProperty) + } + % endif + + ${property_pref_check(shorthand)} + + match input.try(|i| CSSWideKeyword::parse(context, i)) { + Ok(keyword) => { + Ok(ParsedDeclaration::${shorthand.camel_case}CSSWideKeyword(keyword)) + }, + Err(()) => { + shorthands::${shorthand.ident}::parse(context, input) + .map_err(|()| PropertyDeclarationParseError::InvalidValue) + } + } + } + % endfor + } + } + } +} + /// Servo's representation for a property declaration. #[derive(PartialEq, Clone)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum PropertyDeclaration { % for property in data.longhands: /// ${property.name} @@ -831,7 +986,7 @@ impl HasViewportPercentage for PropertyDeclaration { /// The result of parsing a property declaration. #[derive(Eq, PartialEq, Copy, Clone)] -pub enum PropertyDeclarationParseResult { +pub enum PropertyDeclarationParseError { /// The property declaration was for an unknown property. UnknownProperty, /// The property declaration was for a disabled experimental property. @@ -843,8 +998,6 @@ pub enum PropertyDeclarationParseResult { /// /// See: https://drafts.csswg.org/css-animations/#keyframes AnimationPropertyInKeyframeBlock, - /// The declaration was either valid or ignored. - ValidOrIgnoredDeclaration, } impl fmt::Debug for PropertyDeclaration { @@ -878,7 +1031,7 @@ impl ToCss for PropertyDeclaration { % if property.experimental and product == "servo": if !PREFS.get("${property.experimental}") .as_boolean().unwrap_or(false) { - return PropertyDeclarationParseResult::ExperimentalProperty + return Err(PropertyDeclarationParseError::ExperimentalProperty) } % endif % if product == "gecko": @@ -895,7 +1048,7 @@ impl ToCss for PropertyDeclaration { let id = structs::${helpers.to_nscsspropertyid(property.ident)}; let enabled = unsafe { bindings::Gecko_PropertyId_IsPrefEnabled(id) }; if !enabled { - return PropertyDeclarationParseResult::ExperimentalProperty + return Err(PropertyDeclarationParseError::ExperimentalProperty) } } % endif @@ -992,100 +1145,6 @@ impl PropertyDeclaration { } } - /// The `in_keyframe_block` parameter controls this: - /// - /// https://drafts.csswg.org/css-animations/#keyframes - /// > The inside of accepts any CSS property - /// > except those defined in this specification, - /// > but does accept the `animation-play-state` property and interprets it specially. - /// - /// This will not actually parse Importance values, and will always set things - /// to Importance::Normal. Parsing Importance values is the job of PropertyDeclarationParser, - /// we only set them here so that we don't have to reallocate - pub fn parse(id: PropertyId, context: &ParserContext, input: &mut Parser, - result_list: &mut Vec<(PropertyDeclaration, Importance)>, - in_keyframe_block: bool) - -> PropertyDeclarationParseResult { - match id { - PropertyId::Custom(name) => { - let value = match input.try(|i| CSSWideKeyword::parse(context, i)) { - Ok(keyword) => DeclaredValue::CSSWideKeyword(keyword), - Err(()) => match ::custom_properties::SpecifiedValue::parse(context, input) { - Ok(value) => DeclaredValue::Value(value), - Err(()) => return PropertyDeclarationParseResult::InvalidValue, - } - }; - result_list.push((PropertyDeclaration::Custom(name, value), - Importance::Normal)); - return PropertyDeclarationParseResult::ValidOrIgnoredDeclaration; - } - PropertyId::Longhand(id) => match id { - % for property in data.longhands: - LonghandId::${property.camel_case} => { - % if not property.derived_from: - % if not property.allowed_in_keyframe_block: - if in_keyframe_block { - return PropertyDeclarationParseResult::AnimationPropertyInKeyframeBlock - } - % endif - % if property.internal: - if context.stylesheet_origin != Origin::UserAgent { - return PropertyDeclarationParseResult::UnknownProperty - } - % endif - - ${property_pref_check(property)} - - match longhands::${property.ident}::parse_declared(context, input) { - Ok(value) => { - result_list.push((PropertyDeclaration::${property.camel_case}(value), - Importance::Normal)); - PropertyDeclarationParseResult::ValidOrIgnoredDeclaration - }, - Err(()) => PropertyDeclarationParseResult::InvalidValue, - } - % else: - PropertyDeclarationParseResult::UnknownProperty - % endif - } - % endfor - }, - PropertyId::Shorthand(id) => match id { - % for shorthand in data.shorthands: - ShorthandId::${shorthand.camel_case} => { - % if not shorthand.allowed_in_keyframe_block: - if in_keyframe_block { - return PropertyDeclarationParseResult::AnimationPropertyInKeyframeBlock - } - % endif - % if shorthand.internal: - if context.stylesheet_origin != Origin::UserAgent { - return PropertyDeclarationParseResult::UnknownProperty - } - % endif - - ${property_pref_check(shorthand)} - - match input.try(|i| CSSWideKeyword::parse(context, i)) { - Ok(keyword) => { - % for sub_property in shorthand.sub_properties: - result_list.push(( - PropertyDeclaration::${sub_property.camel_case}( - DeclaredValue::CSSWideKeyword(keyword)), Importance::Normal)); - % endfor - PropertyDeclarationParseResult::ValidOrIgnoredDeclaration - }, - Err(()) => match shorthands::${shorthand.ident}::parse(context, input, result_list) { - Ok(()) => PropertyDeclarationParseResult::ValidOrIgnoredDeclaration, - Err(()) => PropertyDeclarationParseResult::InvalidValue, - } - } - } - % endfor - } - } - } - /// The shorthands that this longhand is part of. pub fn shorthands(&self) -> &'static [ShorthandId] { // first generate longhand to shorthands lookup map @@ -1766,7 +1825,7 @@ pub fn cascade(viewport_size: Size2D, }).collect::>(); let iter_declarations = || { lock_guards.iter().flat_map(|&(ref source, source_importance)| { - source.declarations.iter() + source.declarations().iter() // Yield declarations later in source order (with more precedence) first. .rev() .filter_map(move |&(ref declaration, declaration_importance)| { diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 1ecd0a0cb14..9a725a19286 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -99,7 +99,7 @@ impl StyleSource { let _ = write!(writer, "{:?}", rule.read().selectors); } - let _ = write!(writer, " -> {:?}", self.read().declarations); + let _ = write!(writer, " -> {:?}", self.read().declarations()); } /// Read the style source guard, and obtain thus read access to the diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 6ee7b6843a0..cc72acd4c15 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -52,7 +52,6 @@ pub enum Origin { /// A set of namespaces applying to a given stylesheet. #[derive(Default, Debug)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] #[allow(missing_docs)] pub struct Namespaces { pub default: Option, @@ -389,7 +388,6 @@ impl ToCss for CssRule { } #[derive(Debug, PartialEq)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] #[allow(missing_docs)] pub struct NamespaceRule { /// `None` for the default Namespace @@ -536,7 +534,7 @@ impl ToCss for StyleRule { let declaration_block = self.block.read(); try!(declaration_block.to_css(dest)); // Step 4 - if declaration_block.declarations.len() > 0 { + if declaration_block.declarations().len() > 0 { try!(write!(dest, " ")); } // Step 5 @@ -954,7 +952,7 @@ impl<'a> QualifiedRuleParser for TopLevelRuleParser<'a> { } } -#[derive(Clone)] // shallow, relatively cheap clone +#[derive(Clone)] // shallow, relatively cheap .clone struct NestedRuleParser<'a, 'b: 'a> { stylesheet_origin: Origin, context: &'a ParserContext<'b>, diff --git a/components/style/supports.rs b/components/style/supports.rs index 0451204eb1e..96c4bd3b5dc 100644 --- a/components/style/supports.rs +++ b/components/style/supports.rs @@ -6,7 +6,7 @@ use cssparser::{parse_important, Parser, Token}; use parser::ParserContext; -use properties::{PropertyDeclaration, PropertyId}; +use properties::{PropertyId, ParsedDeclaration}; use std::fmt; use style_traits::ToCss; @@ -205,27 +205,14 @@ impl Declaration { /// /// https://drafts.csswg.org/css-conditional-3/#support-definition pub fn eval(&self, cx: &ParserContext) -> bool { - use properties::PropertyDeclarationParseResult::*; let id = if let Ok(id) = PropertyId::parse((&*self.prop).into()) { id } else { return false }; let mut input = Parser::new(&self.val); - let mut list = Vec::new(); - let res = PropertyDeclaration::parse(id, cx, &mut input, - &mut list, /* in_keyframe */ false); + let res = ParsedDeclaration::parse(id, cx, &mut input, /* in_keyframe */ false); let _ = input.try(parse_important); - if !input.is_exhausted() { - return false; - } - match res { - UnknownProperty => false, - ExperimentalProperty => false, // only happens for experimental props - // that haven't been enabled - InvalidValue => false, - AnimationPropertyInKeyframeBlock => unreachable!(), - ValidOrIgnoredDeclaration => true, - } + res.is_ok() && input.is_exhausted() } } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 4dccb916c72..06f3c0f3de8 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -67,8 +67,8 @@ use style::gecko_properties::{self, style_structs}; use style::keyframes::KeyframesStepValue; use style::parallel; use style::parser::{ParserContext, ParserContextExtraData}; -use style::properties::{ComputedValues, Importance, PropertyDeclaration}; -use style::properties::{PropertyDeclarationParseResult, PropertyDeclarationBlock, PropertyId}; +use style::properties::{ComputedValues, Importance, ParsedDeclaration}; +use style::properties::{PropertyDeclarationBlock, PropertyId}; use style::properties::animated_properties::{AnimationValue, Interpolate, TransitionProperty}; use style::properties::parse_one_declaration; use style::restyle_hints::{self, RestyleHint}; @@ -251,18 +251,13 @@ pub extern "C" fn Servo_AnimationValues_Uncompute(value: RawServoAnimationValueB -> RawServoDeclarationBlockStrong { let value = unsafe { value.as_ref().unwrap() }; - let uncomputed_values = value.into_iter() - .map(|v| { - let raw_anim = unsafe { v.as_ref().unwrap() }; - let anim = AnimationValue::as_arc(&raw_anim); - (anim.uncompute(), Importance::Normal) - }) - .collect(); - - Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: uncomputed_values, - important_count: 0, - })).into_strong() + let mut block = PropertyDeclarationBlock::new(); + for v in value.iter() { + let raw_anim = unsafe { v.as_ref().unwrap() }; + let anim = AnimationValue::as_arc(&raw_anim); + block.push(anim.uncompute(), Importance::Normal); + } + Arc::new(RwLock::new(block)).into_strong() } macro_rules! get_property_id_from_nscsspropertyid { @@ -281,10 +276,8 @@ pub extern "C" fn Servo_AnimationValue_Serialize(value: RawServoAnimationValueBo { let uncomputed_value = AnimationValue::as_arc(&value).uncompute(); let mut string = String::new(); - let rv = PropertyDeclarationBlock { - declarations: vec![(uncomputed_value, Importance::Normal)], - important_count: 0 - }.single_value_to_css(&get_property_id_from_nscsspropertyid!(property, ()), &mut string); + let rv = PropertyDeclarationBlock::with_one(uncomputed_value, Importance::Normal) + .single_value_to_css(&get_property_id_from_nscsspropertyid!(property, ()), &mut string); debug_assert!(rv.is_ok()); write!(unsafe { &mut *buffer }, "{}", string).expect("Failed to copy string"); @@ -713,17 +706,14 @@ pub extern "C" fn Servo_ParseProperty(property: *const nsACString, value: *const Box::new(StdoutErrorReporter), extra_data); - let mut results = vec![]; - match PropertyDeclaration::parse(id, &context, &mut Parser::new(value), - &mut results, false) { - PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => {}, - _ => return RawServoDeclarationBlockStrong::null(), + match ParsedDeclaration::parse(id, &context, &mut Parser::new(value), false) { + Ok(parsed) => { + let mut block = PropertyDeclarationBlock::new(); + parsed.expand(|d| block.push(d, Importance::Normal)); + Arc::new(RwLock::new(block)).into_strong() + } + Err(_) => RawServoDeclarationBlockStrong::null() } - - Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: results, - important_count: 0, - })).into_strong() } #[no_mangle] @@ -734,7 +724,7 @@ pub extern "C" fn Servo_ParseStyleAttribute(data: *const nsACString) -> RawServo #[no_mangle] pub extern "C" fn Servo_DeclarationBlock_CreateEmpty() -> RawServoDeclarationBlockStrong { - Arc::new(RwLock::new(PropertyDeclarationBlock { declarations: vec![], important_count: 0 })).into_strong() + Arc::new(RwLock::new(PropertyDeclarationBlock::new())).into_strong() } #[no_mangle] @@ -748,7 +738,8 @@ pub extern "C" fn Servo_DeclarationBlock_Clone(declarations: RawServoDeclaration pub extern "C" fn Servo_DeclarationBlock_Equals(a: RawServoDeclarationBlockBorrowed, b: RawServoDeclarationBlockBorrowed) -> bool { - *RwLock::::as_arc(&a).read() == *RwLock::::as_arc(&b).read() + *RwLock::::as_arc(&a).read().declarations() == + *RwLock::::as_arc(&b).read().declarations() } #[no_mangle] @@ -775,14 +766,14 @@ pub extern "C" fn Servo_DeclarationBlock_SerializeOneValue( #[no_mangle] pub extern "C" fn Servo_DeclarationBlock_Count(declarations: RawServoDeclarationBlockBorrowed) -> u32 { let declarations = RwLock::::as_arc(&declarations); - declarations.read().declarations.len() as u32 + declarations.read().declarations().len() as u32 } #[no_mangle] pub extern "C" fn Servo_DeclarationBlock_GetNthProperty(declarations: RawServoDeclarationBlockBorrowed, index: u32, result: *mut nsAString) -> bool { let declarations = RwLock::::as_arc(&declarations); - if let Some(&(ref decl, _)) = declarations.read().declarations.get(index as usize) { + if let Some(&(ref decl, _)) = declarations.read().declarations().get(index as usize) { let result = unsafe { result.as_mut().unwrap() }; decl.id().to_css(result).unwrap(); true @@ -833,14 +824,14 @@ fn set_property(declarations: RawServoDeclarationBlockBorrowed, property_id: Pro // FIXME Needs real URL and ParserContextExtraData. let base_url = &*DUMMY_BASE_URL; let extra_data = ParserContextExtraData::default(); - if let Ok(decls) = parse_one_declaration(property_id, value, &base_url, - Box::new(StdoutErrorReporter), extra_data) { + if let Ok(parsed) = parse_one_declaration(property_id, value, &base_url, + Box::new(StdoutErrorReporter), extra_data) { let mut declarations = RwLock::::as_arc(&declarations).write(); let importance = if is_important { Importance::Important } else { Importance::Normal }; let mut changed = false; - for decl in decls.into_iter() { - changed |= declarations.set_parsed_declaration(decl.0, importance); - } + parsed.expand(|decl| { + changed |= declarations.set_parsed_declaration(decl, importance); + }); changed } else { false @@ -933,7 +924,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetIdentStringValue(declarations: let prop = match_wrap_declared! { long, XLang => Lang(Atom::from(value)), }; - declarations.write().declarations.push((prop, Default::default())); + declarations.write().push(prop, Importance::Normal); } #[no_mangle] @@ -970,7 +961,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetKeywordValue(declarations: BorderBottomStyle => BorderStyle::from_gecko_keyword(value), BorderLeftStyle => BorderStyle::from_gecko_keyword(value), }; - declarations.write().declarations.push((prop, Default::default())); + declarations.write().push(prop, Importance::Normal); } #[no_mangle] @@ -985,7 +976,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetIntValue(declarations: RawServoDecla let prop = match_wrap_declared! { long, XSpan => Span(value), }; - declarations.write().declarations.push((prop, Default::default())); + declarations.write().push(prop, Importance::Normal); } #[no_mangle] @@ -1024,7 +1015,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetPixelValue(declarations: } ), }; - declarations.write().declarations.push((prop, Default::default())); + declarations.write().push(prop, Importance::Normal); } #[no_mangle] @@ -1047,7 +1038,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetPercentValue(declarations: MarginBottom => pc.into(), MarginLeft => pc.into(), }; - declarations.write().declarations.push((prop, Default::default())); + declarations.write().push(prop, Importance::Normal); } #[no_mangle] @@ -1069,7 +1060,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetAutoValue(declarations: MarginBottom => auto, MarginLeft => auto, }; - declarations.write().declarations.push((prop, Default::default())); + declarations.write().push(prop, Importance::Normal); } #[no_mangle] @@ -1090,7 +1081,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetCurrentColor(declarations: BorderBottomColor => cc, BorderLeftColor => cc, }; - declarations.write().declarations.push((prop, Default::default())); + declarations.write().push(prop, Importance::Normal); } #[no_mangle] @@ -1117,7 +1108,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetColorValue(declarations: Color => longhands::color::SpecifiedValue(color), BackgroundColor => color, }; - declarations.write().declarations.push((prop, Default::default())); + declarations.write().push(prop, Importance::Normal); } #[no_mangle] @@ -1134,7 +1125,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetFontFamily(declarations: if let Ok(family) = FontFamily::parse(&mut parser) { if parser.is_exhausted() { let decl = PropertyDeclaration::FontFamily(DeclaredValue::Value(family)); - declarations.write().declarations.push((decl, Default::default())); + declarations.write().push(decl, Importance::Normal); } } } @@ -1149,7 +1140,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetTextDecorationColorOverride(declarat let mut decoration = text_decoration_line::computed_value::none; decoration |= text_decoration_line::COLOR_OVERRIDE; let decl = PropertyDeclaration::TextDecorationLine(DeclaredValue::Value(decoration)); - declarations.write().declarations.push((decl, Default::default())); + declarations.write().push(decl, Importance::Normal); } #[no_mangle] @@ -1165,10 +1156,7 @@ pub extern "C" fn Servo_CSSSupports2(property: *const nsACString, value: *const let base_url = &*DUMMY_BASE_URL; let extra_data = ParserContextExtraData::default(); - match parse_one_declaration(id, &value, &base_url, Box::new(StdoutErrorReporter), extra_data) { - Ok(decls) => !decls.is_empty(), - Err(()) => false, - } + parse_one_declaration(id, &value, &base_url, Box::new(StdoutErrorReporter), extra_data).is_ok() } #[no_mangle] @@ -1369,7 +1357,7 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeLis let declarations = RwLock::::as_arc(&declarations); let guard = declarations.read(); - let anim_iter = guard.declarations + let anim_iter = guard.declarations() .iter() .filter_map(|&(ref decl, imp)| { if imp == Importance::Normal { @@ -1476,7 +1464,7 @@ pub extern "C" fn Servo_StyleSet_FillKeyframesForName(raw_data: RawServoStyleSet let guard = block.read(); // Filter out non-animatable properties. let animatable = - guard.declarations + guard.declarations() .iter() .filter(|&&(ref declaration, _)| { declaration.is_animatable() @@ -1490,10 +1478,9 @@ pub extern "C" fn Servo_StyleSet_FillKeyframesForName(raw_data: RawServoStyleSet (*keyframe).mPropertyValues.set_len((index + 1) as u32); (*keyframe).mPropertyValues[index].mProperty = property.into(); (*keyframe).mPropertyValues[index].mServoDeclarationBlock.set_arc_leaky( - Arc::new(RwLock::new( - PropertyDeclarationBlock { declarations: vec![ (declaration.clone(), - Importance::Normal) ], - important_count: 0 }))); + Arc::new(RwLock::new(PropertyDeclarationBlock::with_one( + declaration.clone(), Importance::Normal + )))); if step.start_percentage.0 == 0. || step.start_percentage.0 == 1. { seen.set_transition_property_bit(&property); diff --git a/tests/unit/style/keyframes.rs b/tests/unit/style/keyframes.rs index d01dbcab2d0..a1861754419 100644 --- a/tests/unit/style/keyframes.rs +++ b/tests/unit/style/keyframes.rs @@ -27,10 +27,7 @@ fn test_no_property_in_keyframe() { let keyframes = vec![ Arc::new(RwLock::new(Keyframe { selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]), - block: Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![], - important_count: 0, - })) + block: Arc::new(RwLock::new(PropertyDeclarationBlock::new())) })), ]; let animation = KeyframesAnimation::from_keyframes(&keyframes); @@ -45,27 +42,26 @@ fn test_no_property_in_keyframe() { #[test] fn test_missing_property_in_initial_keyframe() { let declarations_on_initial_keyframe = - Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::Width( - DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), - Importance::Normal), - ], - important_count: 0, - })); + Arc::new(RwLock::new(PropertyDeclarationBlock::with_one( + PropertyDeclaration::Width( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal + ))); let declarations_on_final_keyframe = - Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::Width( + Arc::new(RwLock::new({ + let mut block = PropertyDeclarationBlock::new(); + block.push( + PropertyDeclaration::Width( DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), - Importance::Normal), - - (PropertyDeclaration::Height( + Importance::Normal + ); + block.push( + PropertyDeclaration::Height( DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), - Importance::Normal), - ], - important_count: 0, + Importance::Normal + ); + block })); let keyframes = vec![ @@ -102,28 +98,27 @@ fn test_missing_property_in_initial_keyframe() { #[test] fn test_missing_property_in_final_keyframe() { let declarations_on_initial_keyframe = - Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::Width( + Arc::new(RwLock::new({ + let mut block = PropertyDeclarationBlock::new(); + block.push( + PropertyDeclaration::Width( DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), - Importance::Normal), - - (PropertyDeclaration::Height( + Importance::Normal + ); + block.push( + PropertyDeclaration::Height( DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), - Importance::Normal), - ], - important_count: 0, + Importance::Normal + ); + block })); let declarations_on_final_keyframe = - Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::Height( - DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), - Importance::Normal), - ], - important_count: 0, - })); + Arc::new(RwLock::new(PropertyDeclarationBlock::with_one( + PropertyDeclaration::Height( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal, + ))); let keyframes = vec![ Arc::new(RwLock::new(Keyframe { @@ -159,26 +154,25 @@ fn test_missing_property_in_final_keyframe() { #[test] fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() { let declarations = - Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::Width( - DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), - Importance::Normal), - - (PropertyDeclaration::Height( - DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), - Importance::Normal), - ], - important_count: 0, + Arc::new(RwLock::new({ + let mut block = PropertyDeclarationBlock::new(); + block.push( + PropertyDeclaration::Width( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal + ); + block.push( + PropertyDeclaration::Height( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal + ); + block })); let keyframes = vec![ Arc::new(RwLock::new(Keyframe { selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]), - block: Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![], - important_count: 0, - })) + block: Arc::new(RwLock::new(PropertyDeclarationBlock::new())) })), Arc::new(RwLock::new(Keyframe { selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.5)]), @@ -191,11 +185,10 @@ fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() { KeyframesStep { start_percentage: KeyframePercentage(0.), value: KeyframesStepValue::Declarations { - block: Arc::new(RwLock::new(PropertyDeclarationBlock { + block: Arc::new(RwLock::new( // XXX: Should we use ComputedValues in this case? - declarations: vec![], - important_count: 0, - })) + PropertyDeclarationBlock::new() + )) }, declared_timing_function: false, }, diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index 9433d97ed3d..f69e75ed571 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -17,6 +17,7 @@ use style::values::specified::{BorderStyle, BorderWidth, CSSColor, Length, NoCal use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent}; use style::values::specified::url::SpecifiedUrl; use style_traits::ToCss; +use stylesheets::block_from; fn parse_declaration_block(css_properties: &str) -> PropertyDeclarationBlock { let url = ServoUrl::parse("http://localhost").unwrap(); @@ -56,10 +57,7 @@ fn property_declaration_block_should_serialize_correctly() { Importance::Normal), ]; - let block = PropertyDeclarationBlock { - declarations: declarations, - important_count: 0, - }; + let block = block_from(declarations); let css_string = block.to_css_string(); @@ -73,10 +71,7 @@ mod shorthand_serialization { pub use super::*; pub fn shorthand_properties_to_string(properties: Vec) -> String { - let block = PropertyDeclarationBlock { - declarations: properties.into_iter().map(|d| (d, Importance::Normal)).collect(), - important_count: 0, - }; + let block = block_from(properties.into_iter().map(|d| (d, Importance::Normal))); block.to_css_string() } @@ -882,10 +877,7 @@ mod shorthand_serialization { Importance::Normal) ]; - let block = PropertyDeclarationBlock { - declarations: declarations, - important_count: 0 - }; + let block = block_from(declarations); let mut s = String::new(); @@ -905,10 +897,7 @@ mod shorthand_serialization { Importance::Normal) ]; - let block = PropertyDeclarationBlock { - declarations: declarations, - important_count: 0 - }; + let block = block_from(declarations); let mut s = String::new(); diff --git a/tests/unit/style/rule_tree/bench.rs b/tests/unit/style/rule_tree/bench.rs index fccab1252f9..74242967766 100644 --- a/tests/unit/style/rule_tree/bench.rs +++ b/tests/unit/style/rule_tree/bench.rs @@ -17,7 +17,7 @@ use test::{self, Bencher}; struct ErrorringErrorReporter; impl ParseErrorReporter for ErrorringErrorReporter { - fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str, + fn report_error(&self, _input: &mut Parser, position: SourcePosition, message: &str, url: &ServoUrl) { panic!("CSS error: {}\t\n{:?} {}", url.as_str(), position, message); } @@ -68,14 +68,11 @@ fn test_insertion(rule_tree: &RuleTree, rules: Vec<(StyleSource, CascadeLevel)>) 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![ - (PropertyDeclaration::Display(DeclaredValue::Value( - longhands::display::SpecifiedValue::block)), - Importance::Normal), - ], - important_count: 0, - }))), CascadeLevel::UserNormal)); + rules.push((StyleSource::Declarations(Arc::new(RwLock::new(PropertyDeclarationBlock::with_one( + PropertyDeclaration::Display(DeclaredValue::Value( + longhands::display::SpecifiedValue::block)), + Importance::Normal + )))), CascadeLevel::UserNormal)); test_insertion(rule_tree, rules) } diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 586f2616ecf..a85e5e36fb0 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -24,6 +24,15 @@ use style::stylesheets::{Origin, Namespaces}; use style::stylesheets::{Stylesheet, NamespaceRule, CssRule, CssRules, StyleRule, KeyframesRule}; use style::values::specified::{LengthOrPercentageOrAuto, Percentage}; +pub fn block_from(iterable: I) -> PropertyDeclarationBlock +where I: IntoIterator { + let mut block = PropertyDeclarationBlock::new(); + for (d, i) in iterable { + block.push(d, i) + } + block +} + #[test] fn test_parse_stylesheet() { let css = r" @@ -98,17 +107,14 @@ fn test_parse_stylesheet() { specificity: (0 << 20) + (1 << 10) + (1 << 0), }, ]), - block: Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::Display(DeclaredValue::Value( - longhands::display::SpecifiedValue::none)), - Importance::Important), - (PropertyDeclaration::Custom(Atom::from("a"), - DeclaredValue::CSSWideKeyword(CSSWideKeyword::Inherit)), - Importance::Important), - ], - important_count: 2, - })), + block: Arc::new(RwLock::new(block_from(vec![ + (PropertyDeclaration::Display(DeclaredValue::Value( + longhands::display::SpecifiedValue::none)), + Importance::Important), + (PropertyDeclaration::Custom(Atom::from("a"), + DeclaredValue::CSSWideKeyword(CSSWideKeyword::Inherit)), + Importance::Important), + ]))), }))), CssRule::Style(Arc::new(RwLock::new(StyleRule { selectors: SelectorList(vec![ @@ -147,14 +153,11 @@ fn test_parse_stylesheet() { specificity: (0 << 20) + (0 << 10) + (1 << 0), }, ]), - block: Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::Display(DeclaredValue::Value( - longhands::display::SpecifiedValue::block)), - Importance::Normal), - ], - important_count: 0, - })), + block: Arc::new(RwLock::new(block_from(vec![ + (PropertyDeclaration::Display(DeclaredValue::Value( + longhands::display::SpecifiedValue::block)), + Importance::Normal), + ]))), }))), CssRule::Style(Arc::new(RwLock::new(StyleRule { selectors: SelectorList(vec![ @@ -182,58 +185,55 @@ fn test_parse_stylesheet() { specificity: (1 << 20) + (1 << 10) + (0 << 0), }, ]), - block: Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::BackgroundColor(DeclaredValue::Value( - longhands::background_color::SpecifiedValue { - authored: Some("blue".to_owned().into_boxed_str()), - parsed: cssparser::Color::RGBA(cssparser::RGBA::new(0, 0, 255, 255)), - } - )), - Importance::Normal), - (PropertyDeclaration::BackgroundPositionX(DeclaredValue::Value( - longhands::background_position_x::SpecifiedValue( - vec![longhands::background_position_x::single_value - ::get_initial_position_value()]))), - Importance::Normal), - (PropertyDeclaration::BackgroundPositionY(DeclaredValue::Value( - longhands::background_position_y::SpecifiedValue( - vec![longhands::background_position_y::single_value - ::get_initial_position_value()]))), - Importance::Normal), - (PropertyDeclaration::BackgroundRepeat(DeclaredValue::Value( - longhands::background_repeat::SpecifiedValue( - vec![longhands::background_repeat::single_value - ::get_initial_specified_value()]))), - Importance::Normal), - (PropertyDeclaration::BackgroundAttachment(DeclaredValue::Value( - longhands::background_attachment::SpecifiedValue( - vec![longhands::background_attachment::single_value - ::get_initial_specified_value()]))), - Importance::Normal), - (PropertyDeclaration::BackgroundImage(DeclaredValue::Value( - longhands::background_image::SpecifiedValue( - vec![longhands::background_image::single_value - ::get_initial_specified_value()]))), - Importance::Normal), - (PropertyDeclaration::BackgroundSize(DeclaredValue::Value( - longhands::background_size::SpecifiedValue( - vec![longhands::background_size::single_value - ::get_initial_specified_value()]))), - Importance::Normal), - (PropertyDeclaration::BackgroundOrigin(DeclaredValue::Value( - longhands::background_origin::SpecifiedValue( - vec![longhands::background_origin::single_value - ::get_initial_specified_value()]))), - Importance::Normal), - (PropertyDeclaration::BackgroundClip(DeclaredValue::Value( - longhands::background_clip::SpecifiedValue( - vec![longhands::background_clip::single_value - ::get_initial_specified_value()]))), - Importance::Normal), - ], - important_count: 0, - })), + block: Arc::new(RwLock::new(block_from(vec![ + (PropertyDeclaration::BackgroundColor(DeclaredValue::Value( + longhands::background_color::SpecifiedValue { + authored: Some("blue".to_owned().into_boxed_str()), + parsed: cssparser::Color::RGBA(cssparser::RGBA::new(0, 0, 255, 255)), + } + )), + Importance::Normal), + (PropertyDeclaration::BackgroundPositionX(DeclaredValue::Value( + longhands::background_position_x::SpecifiedValue( + vec![longhands::background_position_x::single_value + ::get_initial_position_value()]))), + Importance::Normal), + (PropertyDeclaration::BackgroundPositionY(DeclaredValue::Value( + longhands::background_position_y::SpecifiedValue( + vec![longhands::background_position_y::single_value + ::get_initial_position_value()]))), + Importance::Normal), + (PropertyDeclaration::BackgroundRepeat(DeclaredValue::Value( + longhands::background_repeat::SpecifiedValue( + vec![longhands::background_repeat::single_value + ::get_initial_specified_value()]))), + Importance::Normal), + (PropertyDeclaration::BackgroundAttachment(DeclaredValue::Value( + longhands::background_attachment::SpecifiedValue( + vec![longhands::background_attachment::single_value + ::get_initial_specified_value()]))), + Importance::Normal), + (PropertyDeclaration::BackgroundImage(DeclaredValue::Value( + longhands::background_image::SpecifiedValue( + vec![longhands::background_image::single_value + ::get_initial_specified_value()]))), + Importance::Normal), + (PropertyDeclaration::BackgroundSize(DeclaredValue::Value( + longhands::background_size::SpecifiedValue( + vec![longhands::background_size::single_value + ::get_initial_specified_value()]))), + Importance::Normal), + (PropertyDeclaration::BackgroundOrigin(DeclaredValue::Value( + longhands::background_origin::SpecifiedValue( + vec![longhands::background_origin::single_value + ::get_initial_specified_value()]))), + Importance::Normal), + (PropertyDeclaration::BackgroundClip(DeclaredValue::Value( + longhands::background_clip::SpecifiedValue( + vec![longhands::background_clip::single_value + ::get_initial_specified_value()]))), + Importance::Normal), + ]))), }))), CssRule::Keyframes(Arc::new(RwLock::new(KeyframesRule { name: "foo".into(), @@ -241,30 +241,24 @@ fn test_parse_stylesheet() { Arc::new(RwLock::new(Keyframe { selector: KeyframeSelector::new_for_unit_testing( vec![KeyframePercentage::new(0.)]), - block: Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::Width(DeclaredValue::Value( - LengthOrPercentageOrAuto::Percentage(Percentage(0.)))), - Importance::Normal), - ], - important_count: 0, - })) + block: Arc::new(RwLock::new(block_from(vec![ + (PropertyDeclaration::Width(DeclaredValue::Value( + LengthOrPercentageOrAuto::Percentage(Percentage(0.)))), + Importance::Normal), + ]))) })), Arc::new(RwLock::new(Keyframe { selector: KeyframeSelector::new_for_unit_testing( vec![KeyframePercentage::new(1.)]), - block: Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::Width(DeclaredValue::Value( - LengthOrPercentageOrAuto::Percentage(Percentage(1.)))), - Importance::Normal), - (PropertyDeclaration::AnimationPlayState(DeclaredValue::Value( - animation_play_state::SpecifiedValue( - vec![animation_play_state::SingleSpecifiedValue::running]))), - Importance::Normal), - ], - important_count: 0, - })), + block: Arc::new(RwLock::new(block_from(vec![ + (PropertyDeclaration::Width(DeclaredValue::Value( + LengthOrPercentageOrAuto::Percentage(Percentage(1.)))), + Importance::Normal), + (PropertyDeclaration::AnimationPlayState(DeclaredValue::Value( + animation_play_state::SpecifiedValue( + vec![animation_play_state::SingleSpecifiedValue::running]))), + Importance::Normal), + ]))), })), ] }))) diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index abb8f459211..46fb3eef164 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -23,14 +23,11 @@ fn get_mock_rules(css_selectors: &[&str]) -> Vec> { let rule = Arc::new(RwLock::new(StyleRule { selectors: selectors, - block: Arc::new(RwLock::new(PropertyDeclarationBlock { - declarations: vec![ - (PropertyDeclaration::Display(DeclaredValue::Value( - longhands::display::SpecifiedValue::block)), - Importance::Normal), - ], - important_count: 0, - })), + block: Arc::new(RwLock::new(PropertyDeclarationBlock::with_one( + PropertyDeclaration::Display(DeclaredValue::Value( + longhands::display::SpecifiedValue::block)), + Importance::Normal + ))), })); let guard = rule.read();