diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2017-05-19 18:37:14 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-05-19 18:37:14 -0500 |
commit | 60682cf81fe19a82c73dd98ba4c1eebc1dbbfcac (patch) | |
tree | 9004384638efc214b00b9024c26cfa454e764c46 | |
parent | d1e31f7aa46b3b230194de805fff38afcaf9eb68 (diff) | |
parent | 341fc54313aa35980119868cb90854a91c1c6e63 (diff) | |
download | servo-60682cf81fe19a82c73dd98ba4c1eebc1dbbfcac.tar.gz servo-60682cf81fe19a82c73dd98ba4c1eebc1dbbfcac.zip |
Auto merge of #16954 - servo:arrayvec, r=emilio
Avoid returning / passing around a huge ParsedDeclaration type
This enum type used to contain the result of parsing one CSS source declaration (`name: value;`) and expanding shorthands. Enum types are as big as the biggest of their variant (plus discriminant), which was quite big because some shorthands expand to many longhand properties. This type was returned through many functions and methods, wrapped and rewrapped in `Result` with different error types. This presumably caused significant `memmove` traffic.
Instead, we now allocate an `ArrayVec` on the stack and pass `&mut` references to it for various functions to push into it. This type is also very big, but we never move it.
We still use an intermediate data structure because we sometimes decide after shorthand expansion that a declaration is invalid after all and that we’re gonna drop it. Only later do we push to a `PropertyDeclarationBlock`, with an entire `ArrayVec` or nothing.
In future work we can try to avoid a large stack-allocated array, and instead writing directly to the heap allocation of the `Vec` inside `PropertyDeclarationBlock`. However this is tricky: we need to preserve this "all or nothing" aspect of parsing one source declaration, and at the same time we want to make it as little error-prone as possible for the various call sites. `PropertyDeclarationBlock` curently does property deduplication incrementally: as each `PropertyDeclaration` is pushed, we check if an existing declaration of the same property exists and if so overwrite it. To get rid of the stack allocated array we’d need to somehow deduplicate separately after pushing multiple `PropertyDeclaration`.
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16954)
<!-- Reviewable:end -->
26 files changed, 488 insertions, 386 deletions
diff --git a/Cargo.lock b/Cargo.lock index ce3bd10234a..2ef4d663720 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2824,6 +2824,7 @@ name = "style" version = "0.0.1" dependencies = [ "app_units 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "arrayvec 0.3.20 (registry+https://github.com/rust-lang/crates.io-index)", "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "bindgen 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)", "bit-vec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index 4d7d8feaac5..ecf4b35e61c 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -19,7 +19,7 @@ use std::ascii::AsciiExt; use style::attr::AttrValue; use style::parser::PARSING_MODE_DEFAULT; use style::properties::{Importance, PropertyDeclarationBlock, PropertyId, LonghandId, ShorthandId}; -use style::properties::{parse_one_declaration, parse_style_attribute}; +use style::properties::{parse_one_declaration_into, parse_style_attribute, SourcePropertyDeclaration}; use style::selector_parser::PseudoElement; use style::shared_lock::Locked; use style::stylearc::Arc; @@ -239,12 +239,12 @@ impl CSSStyleDeclaration { self.owner.mutate_associated_block(|ref mut pdb, mut changed| { if value.is_empty() { - // Step 4 + // Step 3 *changed = pdb.remove_property(&id); return Ok(()); } - // Step 5 + // Step 4 let importance = match &*priority { "" => Importance::Normal, p if p.eq_ignore_ascii_case("important") => Importance::Important, @@ -254,27 +254,26 @@ impl CSSStyleDeclaration { } }; - // Step 6 + // Step 5 let window = self.owner.window(); let quirks_mode = window.Document().quirks_mode(); - let result = - parse_one_declaration(id, &value, &self.owner.base_url(), - window.css_error_reporter(), - PARSING_MODE_DEFAULT, - quirks_mode); + let mut declarations = SourcePropertyDeclaration::new(); + let result = parse_one_declaration_into( + &mut declarations, id, &value, &self.owner.base_url(), + window.css_error_reporter(), PARSING_MODE_DEFAULT, quirks_mode); - // Step 7 - let parsed = match result { - Ok(parsed) => parsed, + // Step 6 + match result { + Ok(()) => {}, Err(_) => { *changed = false; return Ok(()); } - }; + } + // Step 7 // Step 8 - // Step 9 - *changed = parsed.expand_set_into(pdb, importance); + *changed = pdb.extend_reset(declarations.drain(), importance); Ok(()) }) diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index 8cc372343e8..ef60be1ab18 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -21,12 +21,17 @@ use_bindgen = ["bindgen", "regex", "toml"] servo = ["serde", "serde_derive", "heapsize", "heapsize_derive", "style_traits/servo", "servo_atoms", "servo_config", "html5ever", "cssparser/heapsize", "cssparser/serde", "encoding", "smallvec/heapsizeof", + + # FIXME: Uncomment when https://github.com/servo/servo/pull/16953 has landed: + #"arrayvec/use_union" + "rayon/unstable", "servo_url"] testing = [] gecko_debug = ["nsstring_vendor/gecko_debug"] [dependencies] app_units = "0.4" +arrayvec = "0.3.20" atomic_refcell = "0.1" bitflags = "0.7" bit-vec = "0.4.3" diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index ab797b191f3..db60c86aa9e 100644 --- a/components/style/keyframes.rs +++ b/components/style/keyframes.rs @@ -11,7 +11,7 @@ use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule}; use error_reporting::NullReporter; use parser::{PARSING_MODE_DEFAULT, ParserContext, log_css_error}; use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock, PropertyId}; -use properties::{PropertyDeclarationId, LonghandId, ParsedDeclaration}; +use properties::{PropertyDeclarationId, LonghandId, SourcePropertyDeclaration}; use properties::LonghandIdSet; use properties::animated_properties::TransitionProperty; use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction; @@ -142,9 +142,11 @@ impl Keyframe { parent_stylesheet.quirks_mode); let mut input = Parser::new(css); + let mut declarations = SourcePropertyDeclaration::new(); let mut rule_parser = KeyframeListParser { context: &context, shared_lock: &parent_stylesheet.shared_lock, + declarations: &mut declarations, }; parse_one_rule(&mut input, &mut rule_parser) } @@ -345,14 +347,17 @@ impl KeyframesAnimation { struct KeyframeListParser<'a> { context: &'a ParserContext<'a>, shared_lock: &'a SharedRwLock, + declarations: &'a mut SourcePropertyDeclaration, } /// Parses a keyframe list from CSS input. pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser, shared_lock: &SharedRwLock) -> Vec<Arc<Locked<Keyframe>>> { + let mut declarations = SourcePropertyDeclaration::new(); RuleListParser::new_for_nested_rule(input, KeyframeListParser { context: context, shared_lock: shared_lock, + declarations: &mut declarations, }).filter_map(Result::ok).collect() } @@ -383,13 +388,17 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { let context = ParserContext::new_with_rule_type(self.context, Some(CssRuleType::Keyframe)); let parser = KeyframeDeclarationParser { context: &context, + declarations: self.declarations, }; let mut iter = DeclarationListParser::new(input, parser); let mut block = PropertyDeclarationBlock::new(); while let Some(declaration) = iter.next() { match declaration { - Ok(parsed) => parsed.expand_push_into(&mut block, Importance::Normal), + Ok(()) => { + block.extend(iter.parser.declarations.drain(), Importance::Normal); + } Err(range) => { + iter.parser.declarations.clear(); let pos = range.start; let message = format!("Unsupported keyframe property declaration: '{}'", iter.input.slice(range)); @@ -407,27 +416,24 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { struct KeyframeDeclarationParser<'a, 'b: 'a> { context: &'a ParserContext<'b>, + declarations: &'a mut SourcePropertyDeclaration, } /// Default methods reject all at rules. impl<'a, 'b> AtRuleParser for KeyframeDeclarationParser<'a, 'b> { type Prelude = (); - type AtRule = ParsedDeclaration; + type AtRule = (); } impl<'a, 'b> DeclarationParser for KeyframeDeclarationParser<'a, 'b> { - type Declaration = ParsedDeclaration; + type Declaration = (); - fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<ParsedDeclaration, ()> { + fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<(), ()> { let id = try!(PropertyId::parse(name.into())); - match ParsedDeclaration::parse(id, self.context, input) { - Ok(parsed) => { + match PropertyDeclaration::parse_into(self.declarations, id, self.context, input) { + Ok(()) => { // In case there is still unparsed text in the declaration, we should roll back. - if !input.is_exhausted() { - Err(()) - } else { - Ok(parsed) - } + input.expect_exhausted() } Err(_) => Err(()) } diff --git a/components/style/lib.rs b/components/style/lib.rs index 7eb74bd6d42..8e0e45d2bda 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -38,6 +38,7 @@ #![recursion_limit = "500"] // For define_css_keyword_enum! in -moz-appearance extern crate app_units; +extern crate arrayvec; extern crate atomic_refcell; extern crate bit_vec; #[macro_use] diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 201ad8ea33d..74a138721e7 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -202,14 +202,62 @@ impl PropertyDeclarationBlock { } /// Adds or overrides the declaration for a given property in this block, - /// except if an existing declaration for the same property is more important. + /// **except** if an existing declaration for the same property is more important. + pub fn extend(&mut self, drain: SourcePropertyDeclarationDrain, importance: Importance) { + self.extend_common(drain, importance, false); + } + + /// Adds or overrides the declaration for a given property in this block, + /// **even** if an existing declaration for the same property is more important. + /// + /// Return whether anything changed. + pub fn extend_reset(&mut self, drain: SourcePropertyDeclarationDrain, + importance: Importance) -> bool { + self.extend_common(drain, importance, true) + } + + fn extend_common(&mut self, mut drain: SourcePropertyDeclarationDrain, + importance: Importance, overwrite_more_important: bool) -> bool { + let all_shorthand_len = match drain.all_shorthand { + AllShorthand::NotSet => 0, + AllShorthand::CSSWideKeyword(_) | + AllShorthand::WithVariables(_) => ShorthandId::All.longhands().len() + }; + let push_calls_count = drain.declarations.len() + all_shorthand_len; + + // With deduplication the actual length increase may be less than this. + self.declarations.reserve(push_calls_count); + + let mut changed = false; + for decl in &mut drain.declarations { + changed |= self.push_common(decl, importance, overwrite_more_important); + } + match drain.all_shorthand { + AllShorthand::NotSet => {} + AllShorthand::CSSWideKeyword(keyword) => { + for &id in ShorthandId::All.longhands() { + let decl = PropertyDeclaration::CSSWideKeyword(id, keyword); + changed |= self.push_common(decl, importance, overwrite_more_important); + } + } + AllShorthand::WithVariables(unparsed) => { + for &id in ShorthandId::All.longhands() { + let decl = PropertyDeclaration::WithVariables(id, unparsed.clone()); + changed |= self.push_common(decl, importance, overwrite_more_important); + } + } + } + changed + } + + /// Adds or overrides the declaration for a given property in this block, + /// **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); } - /// Implementation detail of push and ParsedDeclaration::expand* - pub fn push_common(&mut self, declaration: PropertyDeclaration, importance: Importance, - overwrite_more_important: bool) -> bool { + 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 { @@ -657,15 +705,15 @@ pub fn parse_style_attribute(input: &str, /// Parse a given property declaration. Can result in multiple /// `PropertyDeclaration`s when expanding a shorthand, for example. /// -/// The vector returned will not have the importance set; -/// this does not attempt to parse !important at all -pub fn parse_one_declaration(id: PropertyId, - input: &str, - url_data: &UrlExtraData, - error_reporter: &ParseErrorReporter, - parsing_mode: ParsingMode, - quirks_mode: QuirksMode) - -> Result<ParsedDeclaration, ()> { +/// This does not attempt to parse !important at all. +pub fn parse_one_declaration_into(declarations: &mut SourcePropertyDeclaration, + id: PropertyId, + input: &str, + url_data: &UrlExtraData, + error_reporter: &ParseErrorReporter, + parsing_mode: ParsingMode, + quirks_mode: QuirksMode) + -> Result<(), ()> { let context = ParserContext::new(Origin::Author, url_data, error_reporter, @@ -673,7 +721,7 @@ pub fn parse_one_declaration(id: PropertyId, parsing_mode, quirks_mode); Parser::new(input).parse_entirely(|parser| { - ParsedDeclaration::parse(id, &context, parser) + PropertyDeclaration::parse_into(declarations, id, &context, parser) .map_err(|_| ()) }) } @@ -681,24 +729,23 @@ pub fn parse_one_declaration(id: PropertyId, /// A struct to parse property declarations. struct PropertyDeclarationParser<'a, 'b: 'a> { context: &'a ParserContext<'b>, + declarations: &'a mut SourcePropertyDeclaration, } /// Default methods reject all at rules. impl<'a, 'b> AtRuleParser for PropertyDeclarationParser<'a, 'b> { type Prelude = (); - type AtRule = (ParsedDeclaration, Importance); + type AtRule = Importance; } - impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> { - type Declaration = (ParsedDeclaration, Importance); + type Declaration = Importance; - fn parse_value(&mut self, name: &str, input: &mut Parser) - -> Result<(ParsedDeclaration, Importance), ()> { + fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<Importance, ()> { let id = try!(PropertyId::parse(name.into())); - let parsed = input.parse_until_before(Delimiter::Bang, |input| { - ParsedDeclaration::parse(id, self.context, input) + input.parse_until_before(Delimiter::Bang, |input| { + PropertyDeclaration::parse_into(self.declarations, id, self.context, input) .map_err(|_| ()) })?; let importance = match input.try(parse_important) { @@ -706,10 +753,8 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> { Err(()) => Importance::Normal, }; // In case there is still unparsed text in the declaration, we should roll back. - if !input.is_exhausted() { - return Err(()) - } - Ok((parsed, importance)) + input.expect_exhausted()?; + Ok(importance) } } @@ -719,15 +764,20 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> { pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Parser) -> PropertyDeclarationBlock { + let mut declarations = SourcePropertyDeclaration::new(); let mut block = PropertyDeclarationBlock::new(); let parser = PropertyDeclarationParser { context: context, + declarations: &mut declarations, }; let mut iter = DeclarationListParser::new(input, parser); while let Some(declaration) = iter.next() { match declaration { - Ok((parsed, importance)) => parsed.expand_push_into(&mut block, importance), + Ok(importance) => { + block.extend(iter.parser.declarations.drain(), importance); + } Err(range) => { + iter.parser.declarations.clear(); let pos = range.start; let message = format!("Unsupported property declaration: '{}'", iter.input.slice(range)); diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 16236150cd9..9c0807ae33a 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -748,15 +748,23 @@ #[allow(unused_imports)] use cssparser::Parser; use parser::ParserContext; - use properties::{PropertyDeclaration, ParsedDeclaration}; - use properties::{ShorthandId, UnparsedValue, longhands}; + use properties::{PropertyDeclaration, SourcePropertyDeclaration, MaybeBoxed}; + use properties::{ShorthandId, LonghandId, UnparsedValue, longhands}; use std::fmt; use stylearc::Arc; use style_traits::ToCss; pub struct Longhands { % for sub_property in shorthand.sub_properties: - pub ${sub_property.ident}: longhands::${sub_property.ident}::SpecifiedValue, + pub ${sub_property.ident}: + % if sub_property.boxed: + Box< + % endif + longhands::${sub_property.ident}::SpecifiedValue + % if sub_property.boxed: + > + % endif + , % endfor } @@ -816,7 +824,8 @@ /// Parse the given shorthand and fill the result into the /// `declarations` vector. - pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<ParsedDeclaration, ()> { + pub fn parse_into(declarations: &mut SourcePropertyDeclaration, + 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)); @@ -825,17 +834,29 @@ } let var = input.seen_var_functions(); if let Ok(value) = value { - Ok(ParsedDeclaration::${shorthand.camel_case}(value)) + % for sub_property in shorthand.sub_properties: + declarations.push(PropertyDeclaration::${sub_property.camel_case}( + value.${sub_property.ident} + )); + % endfor + Ok(()) } else if var { input.reset(start); let (first_token_type, css) = try!( ::custom_properties::parse_non_custom_with_var(input)); - Ok(ParsedDeclaration::${shorthand.camel_case}WithVariables(Arc::new(UnparsedValue { + let unparsed = Arc::new(UnparsedValue { css: css.into_owned(), first_token_type: first_token_type, url_data: context.url_data.clone(), from_shorthand: Some(ShorthandId::${shorthand.camel_case}), - }))) + }); + % for sub_property in shorthand.sub_properties: + declarations.push(PropertyDeclaration::WithVariables( + LonghandId::${sub_property.camel_case}, + unparsed.clone() + )); + % endfor + Ok(()) } else { Err(()) } @@ -865,7 +886,7 @@ try!(parse_four_sides(input, ${parser_function})); let _unused = context; % endif - Ok(Longhands { + Ok(expanded! { % for side in ["top", "right", "bottom", "left"]: ${to_rust_ident(sub_property_pattern % side)}: ${side}, % endfor diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index cc89cf1b648..01db4868011 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -13,6 +13,7 @@ use std::borrow::Cow; use std::collections::HashSet; use std::fmt; +use std::mem; use std::ops::Deref; use stylearc::{Arc, UniqueArc}; @@ -58,6 +59,35 @@ macro_rules! property_name { #[path="${repr(os.path.join(os.path.dirname(__file__), 'declaration_block.rs'))[1:-1]}"] pub mod declaration_block; +/// Conversion with fewer impls than From/Into +pub trait MaybeBoxed<Out> { + /// Convert + fn maybe_boxed(self) -> Out; +} + +impl<T> MaybeBoxed<T> for T { + #[inline] + fn maybe_boxed(self) -> T { self } +} + +impl<T> MaybeBoxed<Box<T>> for T { + #[inline] + fn maybe_boxed(self) -> Box<T> { Box::new(self) } +} + +macro_rules! expanded { + ( $( $name: ident: $value: expr ),+ ) => { + expanded!( $( $name: $value, )+ ) + }; + ( $( $name: ident: $value: expr, )+ ) => { + Longhands { + $( + $name: MaybeBoxed::maybe_boxed($value), + )+ + } + } +} + /// A module with all the code for longhand properties. #[allow(missing_docs)] pub mod longhands { @@ -177,10 +207,11 @@ pub mod shorthands { pub mod all { use cssparser::Parser; use parser::ParserContext; - use properties::{ParsedDeclaration, ShorthandId, UnparsedValue}; + use properties::{SourcePropertyDeclaration, AllShorthand, ShorthandId, UnparsedValue}; use stylearc::Arc; - pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<ParsedDeclaration, ()> { + pub fn parse_into(declarations: &mut SourcePropertyDeclaration, + context: &ParserContext, input: &mut Parser) -> Result<(), ()> { // This function is like the parse() that is generated by // helpers:shorthand, but since the only values for the 'all' // shorthand when not just a single CSS-wide keyword is one @@ -196,12 +227,13 @@ pub mod shorthands { input.reset(start); let (first_token_type, css) = try!( ::custom_properties::parse_non_custom_with_var(input)); - Ok(ParsedDeclaration::AllWithVariables(Arc::new(UnparsedValue { + declarations.all_shorthand = AllShorthand::WithVariables(Arc::new(UnparsedValue { css: css.into_owned(), first_token_type: first_token_type, url_data: context.url_data.clone(), from_shorthand: Some(ShorthandId::All), - }))) + })); + Ok(()) } else { Err(()) } @@ -405,11 +437,7 @@ impl PropertyDeclarationIdSet { Some(ShorthandId::${shorthand.camel_case}) => { shorthands::${shorthand.ident}::parse_value(&context, input) .map(|result| { - % if property.boxed: - DeclaredValueOwned::Value(Box::new(result.${property.ident})) - % else: - DeclaredValueOwned::Value(result.${property.ident}) - % endif + DeclaredValueOwned::Value(result.${property.ident}) }) } % endif @@ -976,220 +1004,6 @@ impl PropertyId { } } -/// Includes shorthands before expansion -pub enum ParsedDeclaration { - % for shorthand in data.shorthands: - % if shorthand.name == "all": - // No need for an All(shorthands::all::Longhands) case, since we can - // never have any values for 'all' other than the CSS-wide keywords - // and values with variable references. - % else: - /// ${shorthand.name} - ${shorthand.camel_case}(shorthands::${shorthand.ident}::Longhands), - % endif - - /// ${shorthand.name} with a CSS-wide keyword - ${shorthand.camel_case}CSSWideKeyword(CSSWideKeyword), - - /// ${shorthand.name} with var() functions - ${shorthand.camel_case}WithVariables(Arc<UnparsedValue>), - % endfor - - /// Not a shorthand - LonghandOrCustom(PropertyDeclaration), -} - -impl ParsedDeclaration { - /// Transform this ParsedDeclaration into a sequence of PropertyDeclaration - /// by expanding shorthand declarations into their corresponding longhands - /// - /// Adds or overrides exsting declarations in the given block, - /// except if existing declarations are more important. - #[inline] - pub fn expand_push_into(self, block: &mut PropertyDeclarationBlock, - importance: Importance) { - self.expand_into(block, importance, false); - } - - /// Transform this ParsedDeclaration into a sequence of PropertyDeclaration - /// by expanding shorthand declarations into their corresponding longhands - /// - /// Add or override existing declarations in the given block. - /// Return whether anything changed. - #[inline] - pub fn expand_set_into(self, block: &mut PropertyDeclarationBlock, - importance: Importance) -> bool { - self.expand_into(block, importance, true) - } - - fn expand_into(self, block: &mut PropertyDeclarationBlock, - importance: Importance, - overwrite_more_important: bool) -> bool { - match self { - % for shorthand in data.shorthands: - % if shorthand.name != "all": - ParsedDeclaration::${shorthand.camel_case}( - shorthands::${shorthand.ident}::Longhands { - % for sub_property in shorthand.sub_properties: - ${sub_property.ident}, - % endfor - } - ) => { - let mut changed = false; - % for sub_property in shorthand.sub_properties: - changed |= block.push_common( - PropertyDeclaration::${sub_property.camel_case}( - % if sub_property.boxed: - Box::new(${sub_property.ident}) - % else: - ${sub_property.ident} - % endif - ), - importance, - overwrite_more_important, - ); - % endfor - changed - }, - % endif - ParsedDeclaration::${shorthand.camel_case}CSSWideKeyword(keyword) => { - let mut changed = false; - % for sub_property in shorthand.sub_properties: - changed |= block.push_common( - PropertyDeclaration::CSSWideKeyword( - LonghandId::${sub_property.camel_case}, - keyword, - ), - importance, - overwrite_more_important, - ); - % endfor - changed - }, - ParsedDeclaration::${shorthand.camel_case}WithVariables(value) => { - debug_assert_eq!( - value.from_shorthand, - Some(ShorthandId::${shorthand.camel_case}) - ); - let mut changed = false; - % for sub_property in shorthand.sub_properties: - changed |= block.push_common( - PropertyDeclaration::WithVariables( - LonghandId::${sub_property.camel_case}, - value.clone() - ), - importance, - overwrite_more_important, - ); - % endfor - changed - } - % endfor - ParsedDeclaration::LonghandOrCustom(declaration) => { - block.push_common(declaration, importance, overwrite_more_important) - } - } - } - - /// The `in_keyframe_block` parameter controls this: - /// - /// https://drafts.csswg.org/css-animations/#keyframes - /// > The <declaration-list> inside of <keyframe-block> 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<ParsedDeclaration, PropertyDeclarationParseError> { - let rule_type = context.rule_type(); - debug_assert!(rule_type == CssRuleType::Keyframe || - rule_type == CssRuleType::Page || - rule_type == CssRuleType::Style, - "Declarations are only expected inside a keyframe, page, or style rule."); - match id { - PropertyId::Custom(name) => { - let value = match input.try(|i| CSSWideKeyword::parse(context, i)) { - Ok(keyword) => DeclaredValueOwned::CSSWideKeyword(keyword), - Err(()) => match ::custom_properties::SpecifiedValue::parse(context, input) { - Ok(value) => DeclaredValueOwned::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 rule_type == CssRuleType::Keyframe { - return Err(PropertyDeclarationParseError::AnimationPropertyInKeyframeBlock) - } - % endif - % if property.internal: - if context.stylesheet_origin != Origin::UserAgent { - return Err(PropertyDeclarationParseError::UnknownProperty) - } - % endif - % if not property.allowed_in_page_rule: - if rule_type == CssRuleType::Page { - return Err(PropertyDeclarationParseError::NotAllowedInPageRule) - } - % endif - - ${property_pref_check(property)} - - match longhands::${property.ident}::parse_declared(context, input) { - Ok(value) => { - Ok(ParsedDeclaration::LonghandOrCustom(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 rule_type == CssRuleType::Keyframe { - return Err(PropertyDeclarationParseError::AnimationPropertyInKeyframeBlock) - } - % endif - % if shorthand.internal: - if context.stylesheet_origin != Origin::UserAgent { - return Err(PropertyDeclarationParseError::UnknownProperty) - } - % endif - % if not shorthand.allowed_in_page_rule: - if rule_type == CssRuleType::Page { - return Err(PropertyDeclarationParseError::NotAllowedInPageRule) - } - % 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)] pub enum PropertyDeclaration { @@ -1483,6 +1297,180 @@ impl PropertyDeclaration { PropertyDeclaration::Custom(..) => false, } } + + /// The `context` parameter controls this: + /// + /// https://drafts.csswg.org/css-animations/#keyframes + /// > The <declaration-list> inside of <keyframe-block> 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_into(declarations: &mut SourcePropertyDeclaration, + id: PropertyId, context: &ParserContext, input: &mut Parser) + -> Result<(), PropertyDeclarationParseError> { + assert!(declarations.is_empty()); + let rule_type = context.rule_type(); + debug_assert!(rule_type == CssRuleType::Keyframe || + rule_type == CssRuleType::Page || + rule_type == CssRuleType::Style, + "Declarations are only expected inside a keyframe, page, or style rule."); + match id { + PropertyId::Custom(name) => { + let value = match input.try(|i| CSSWideKeyword::parse(context, i)) { + Ok(keyword) => DeclaredValueOwned::CSSWideKeyword(keyword), + Err(()) => match ::custom_properties::SpecifiedValue::parse(context, input) { + Ok(value) => DeclaredValueOwned::Value(value), + Err(()) => return Err(PropertyDeclarationParseError::InvalidValue), + } + }; + declarations.push(PropertyDeclaration::Custom(name, value)); + Ok(()) + } + 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 rule_type == CssRuleType::Keyframe { + return Err(PropertyDeclarationParseError::AnimationPropertyInKeyframeBlock) + } + % endif + % if property.internal: + if context.stylesheet_origin != Origin::UserAgent { + return Err(PropertyDeclarationParseError::UnknownProperty) + } + % endif + % if not property.allowed_in_page_rule: + if rule_type == CssRuleType::Page { + return Err(PropertyDeclarationParseError::NotAllowedInPageRule) + } + % endif + + ${property_pref_check(property)} + + match longhands::${property.ident}::parse_declared(context, input) { + Ok(value) => { + declarations.push(value); + Ok(()) + }, + 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 rule_type == CssRuleType::Keyframe { + return Err(PropertyDeclarationParseError::AnimationPropertyInKeyframeBlock) + } + % endif + % if shorthand.internal: + if context.stylesheet_origin != Origin::UserAgent { + return Err(PropertyDeclarationParseError::UnknownProperty) + } + % endif + % if not shorthand.allowed_in_page_rule: + if rule_type == CssRuleType::Page { + return Err(PropertyDeclarationParseError::NotAllowedInPageRule) + } + % endif + + ${property_pref_check(shorthand)} + + match input.try(|i| CSSWideKeyword::parse(context, i)) { + Ok(keyword) => { + % if shorthand.name == "all": + declarations.all_shorthand = AllShorthand::CSSWideKeyword(keyword); + % else: + % for sub_property in shorthand.sub_properties: + declarations.push(PropertyDeclaration::CSSWideKeyword( + LonghandId::${sub_property.camel_case}, + keyword, + )); + % endfor + % endif + Ok(()) + }, + Err(()) => { + shorthands::${shorthand.ident}::parse_into(declarations, context, input) + .map_err(|()| PropertyDeclarationParseError::InvalidValue) + } + } + } + % endfor + } + } + } +} + +const MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL: usize = + ${max(len(s.sub_properties) for s in data.shorthands_except_all())}; + +type SourcePropertyDeclarationArray = + [PropertyDeclaration; MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL]; + +/// A stack-allocated vector of `PropertyDeclaration` +/// large enough to parse one CSS `key: value` declaration. +/// (Shorthands expand to multiple `PropertyDeclaration`s.) +pub struct SourcePropertyDeclaration { + declarations: ::arrayvec::ArrayVec<SourcePropertyDeclarationArray>, + + /// Stored separately to keep MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL smaller. + all_shorthand: AllShorthand, +} + +impl SourcePropertyDeclaration { + /// Create one. It’s big, try not to move it around. + #[inline] + pub fn new() -> Self { + SourcePropertyDeclaration { + declarations: ::arrayvec::ArrayVec::new(), + all_shorthand: AllShorthand::NotSet, + } + } + + /// Similar to Vec::drain: leaves this empty when the return value is dropped. + pub fn drain(&mut self) -> SourcePropertyDeclarationDrain { + SourcePropertyDeclarationDrain { + declarations: self.declarations.drain(..), + all_shorthand: mem::replace(&mut self.all_shorthand, AllShorthand::NotSet), + } + } + + /// Reset to initial state + pub fn clear(&mut self) { + self.declarations.clear(); + self.all_shorthand = AllShorthand::NotSet; + } + + fn is_empty(&self) -> bool { + self.declarations.is_empty() && matches!(self.all_shorthand, AllShorthand::NotSet) + } + + fn push(&mut self, declaration: PropertyDeclaration) { + let over_capacity = self.declarations.push(declaration).is_some(); + debug_assert!(!over_capacity); + } +} + +/// Return type of SourcePropertyDeclaration::drain +pub struct SourcePropertyDeclarationDrain<'a> { + declarations: ::arrayvec::Drain<'a, SourcePropertyDeclarationArray>, + all_shorthand: AllShorthand, +} + +enum AllShorthand { + NotSet, + CSSWideKeyword(CSSWideKeyword), + WithVariables(Arc<UnparsedValue>) } #[cfg(feature = "gecko")] diff --git a/components/style/properties/shorthand/background.mako.rs b/components/style/properties/shorthand/background.mako.rs index 89874f82084..b4f3f9fe079 100644 --- a/components/style/properties/shorthand/background.mako.rs +++ b/components/style/properties/shorthand/background.mako.rs @@ -108,7 +108,7 @@ } })); - Ok(Longhands { + Ok(expanded! { background_color: background_color.unwrap_or(CSSColor::transparent()), background_image: background_image, background_position_x: background_position_x, @@ -207,7 +207,7 @@ return Err(()); } - Ok(Longhands { + Ok(expanded! { background_position_x: position_x, background_position_y: position_y, }) diff --git a/components/style/properties/shorthand/border.mako.rs b/components/style/properties/shorthand/border.mako.rs index 25ef00190f1..772105fae25 100644 --- a/components/style/properties/shorthand/border.mako.rs +++ b/components/style/properties/shorthand/border.mako.rs @@ -24,7 +24,7 @@ ${helpers.four_sides_shorthand("border-style", "border-%s-style", let (top, right, bottom, left) = try!(parse_four_sides(input, |i| { BorderWidth::parse_quirky(context, i, AllowQuirks::Yes) })); - Ok(Longhands { + Ok(expanded! { % for side in PHYSICAL_SIDES: ${to_rust_ident('border-%s-width' % side)}: ${side}, % endfor @@ -103,7 +103,7 @@ pub fn parse_border(context: &ParserContext, input: &mut Parser) pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> { let (color, style, width) = try!(super::parse_border(context, input)); - Ok(Longhands { + Ok(expanded! { border_${to_rust_ident(side)}_color: color, border_${to_rust_ident(side)}_style: style, border_${to_rust_ident(side)}_width: width @@ -144,7 +144,7 @@ pub fn parse_border(context: &ParserContext, input: &mut Parser) use properties::longhands::{border_image_source, border_image_width}; let (color, style, width) = try!(super::parse_border(context, input)); - Ok(Longhands { + Ok(expanded! { % for side in PHYSICAL_SIDES: border_${side}_color: color.clone(), border_${side}_style: style, @@ -211,7 +211,7 @@ pub fn parse_border(context: &ParserContext, input: &mut Parser) pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> { let radii = try!(BorderRadius::parse(context, input)); - Ok(Longhands { + Ok(expanded! { border_top_left_radius: radii.top_left, border_top_right_radius: radii.top_right, border_bottom_right_radius: radii.bottom_right, @@ -303,7 +303,7 @@ pub fn parse_border(context: &ParserContext, input: &mut Parser) } })); - Ok(Longhands { + Ok(expanded! { % for name in "outset repeat slice source width".split(): border_image_${name}: border_image_${name}, % endfor diff --git a/components/style/properties/shorthand/box.mako.rs b/components/style/properties/shorthand/box.mako.rs index 9b9ccfe78d7..e05818e3bcc 100644 --- a/components/style/properties/shorthand/box.mako.rs +++ b/components/style/properties/shorthand/box.mako.rs @@ -16,19 +16,19 @@ let moz_kw_found = input.try(|i| match_ignore_ascii_case! { &i.expect_ident()?, "-moz-scrollbars-horizontal" => { - Ok(Longhands { + Ok(expanded! { overflow_x: SpecifiedValue::scroll, overflow_y: SpecifiedValue::hidden, }) } "-moz-scrollbars-vertical" => { - Ok(Longhands { + Ok(expanded! { overflow_x: SpecifiedValue::hidden, overflow_y: SpecifiedValue::scroll, }) } "-moz-scrollbars-none" => { - Ok(Longhands { + Ok(expanded! { overflow_x: SpecifiedValue::hidden, overflow_y: SpecifiedValue::hidden, }) @@ -40,7 +40,7 @@ } % endif let overflow = try!(parse_overflow(context, input)); - Ok(Longhands { + Ok(expanded! { overflow_x: overflow, overflow_y: overflow, }) @@ -148,7 +148,7 @@ macro_rules! try_parse_one { % endfor } - Ok(Longhands { + Ok(expanded! { % for prop in "property duration timing_function delay".split(): transition_${prop}: transition_${prop}::SpecifiedValue(${prop}s), % endfor @@ -259,7 +259,7 @@ macro_rules! try_parse_one { % endfor } - Ok(Longhands { + Ok(expanded! { % for prop in props: animation_${prop}: animation_${prop}::SpecifiedValue(${prop}s), % endfor @@ -305,7 +305,7 @@ macro_rules! try_parse_one { pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> { let result = try!(scroll_snap_type_x::parse(context, input)); - Ok(Longhands { + Ok(expanded! { scroll_snap_type_x: result, scroll_snap_type_y: result, }) @@ -332,7 +332,7 @@ macro_rules! try_parse_one { use properties::longhands::transform; pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> { - Ok(Longhands { + Ok(expanded! { transform: transform::parse_prefixed(context, input)?, }) } diff --git a/components/style/properties/shorthand/column.mako.rs b/components/style/properties/shorthand/column.mako.rs index 3e8648d38ba..336cad51356 100644 --- a/components/style/properties/shorthand/column.mako.rs +++ b/components/style/properties/shorthand/column.mako.rs @@ -42,7 +42,7 @@ if values == 0 || values > 2 { Err(()) } else { - Ok(Longhands { + Ok(expanded! { column_count: unwrap_or_initial!(column_count), column_width: unwrap_or_initial!(column_width), }) @@ -86,7 +86,7 @@ break } if any { - Ok(Longhands { + Ok(expanded! { column_rule_width: unwrap_or_initial!(column_rule_width), column_rule_style: unwrap_or_initial!(column_rule_style), column_rule_color: unwrap_or_initial!(column_rule_color), diff --git a/components/style/properties/shorthand/font.mako.rs b/components/style/properties/shorthand/font.mako.rs index 409e0717013..18e2b97d387 100644 --- a/components/style/properties/shorthand/font.mako.rs +++ b/components/style/properties/shorthand/font.mako.rs @@ -44,7 +44,7 @@ let size; % if product == "gecko": if let Ok(sys) = input.try(SystemFont::parse) { - return Ok(Longhands { + return Ok(expanded! { % for name in SYSTEM_FONT_LONGHANDS: ${name}: ${name}::SpecifiedValue::system_font(sys), % endfor @@ -102,7 +102,7 @@ None }; let family = FontFamily::parse(input)?; - Ok(Longhands { + Ok(expanded! { % for name in "style weight stretch size variant_caps".split(): font_${name}: unwrap_or_initial!(font_${name}, ${name}), % endfor @@ -236,7 +236,7 @@ if count == 0 || count > 1 { return Err(()) } - Ok(Longhands { + Ok(expanded! { font_variant_caps: unwrap_or_initial!(font_variant_caps, caps), // FIXME: Bug 1356134 - parse all sub properties. % if product == "gecko" or data.testing: diff --git a/components/style/properties/shorthand/inherited_svg.mako.rs b/components/style/properties/shorthand/inherited_svg.mako.rs index e33aa3bac16..b47e3021fd7 100644 --- a/components/style/properties/shorthand/inherited_svg.mako.rs +++ b/components/style/properties/shorthand/inherited_svg.mako.rs @@ -13,7 +13,7 @@ use parser::Parse; let url = UrlOrNone::parse(context, input)?; - Ok(Longhands { + Ok(expanded! { marker_start: url.clone(), marker_mid: url.clone(), marker_end: url, diff --git a/components/style/properties/shorthand/inherited_text.mako.rs b/components/style/properties/shorthand/inherited_text.mako.rs index af94643bc3c..30ded7c2a7a 100644 --- a/components/style/properties/shorthand/inherited_text.mako.rs +++ b/components/style/properties/shorthand/inherited_text.mako.rs @@ -29,7 +29,7 @@ break } if color.is_some() || style.is_some() { - Ok(Longhands { + Ok(expanded! { text_emphasis_color: unwrap_or_initial!(text_emphasis_color, color), text_emphasis_style: unwrap_or_initial!(text_emphasis_style, style), }) @@ -77,7 +77,7 @@ } if color.is_some() || width.is_some() { - Ok(Longhands { + Ok(expanded! { _webkit_text_stroke_color: unwrap_or_initial!(_webkit_text_stroke_color, color), _webkit_text_stroke_width: unwrap_or_initial!(_webkit_text_stroke_width, width), }) diff --git a/components/style/properties/shorthand/list.mako.rs b/components/style/properties/shorthand/list.mako.rs index 11a9e170bd1..2aa0f2a6c82 100644 --- a/components/style/properties/shorthand/list.mako.rs +++ b/components/style/properties/shorthand/list.mako.rs @@ -70,35 +70,35 @@ // long as we parsed something. match (any, nones, list_style_type, image) { (true, 2, None, None) => { - Ok(Longhands { + Ok(expanded! { list_style_position: position, list_style_image: list_style_image::SpecifiedValue(Either::Second(None_)), list_style_type: list_style_type_none(), }) } (true, 1, None, Some(image)) => { - Ok(Longhands { + Ok(expanded! { list_style_position: position, list_style_image: image, list_style_type: list_style_type_none(), }) } (true, 1, Some(list_style_type), None) => { - Ok(Longhands { + Ok(expanded! { list_style_position: position, list_style_image: list_style_image::SpecifiedValue(Either::Second(None_)), list_style_type: list_style_type, }) } (true, 1, None, None) => { - Ok(Longhands { + Ok(expanded! { list_style_position: position, list_style_image: list_style_image::SpecifiedValue(Either::Second(None_)), list_style_type: list_style_type_none(), }) } (true, 0, list_style_type, image) => { - Ok(Longhands { + Ok(expanded! { list_style_position: position, list_style_image: unwrap_or_initial!(list_style_image, image), list_style_type: unwrap_or_initial!(list_style_type), diff --git a/components/style/properties/shorthand/mask.mako.rs b/components/style/properties/shorthand/mask.mako.rs index e9d03035964..f51720d1e00 100644 --- a/components/style/properties/shorthand/mask.mako.rs +++ b/components/style/properties/shorthand/mask.mako.rs @@ -107,7 +107,7 @@ } })); - Ok(Longhands { + Ok(expanded! { % for name in "image mode position_x position_y size repeat origin clip composite".split(): mask_${name}: mask_${name}, % endfor @@ -196,7 +196,7 @@ return Err(()); } - Ok(Longhands { + Ok(expanded! { mask_position_x: position_x, mask_position_y: position_y, }) diff --git a/components/style/properties/shorthand/outline.mako.rs b/components/style/properties/shorthand/outline.mako.rs index 2a1e7e54486..c609bd8a463 100644 --- a/components/style/properties/shorthand/outline.mako.rs +++ b/components/style/properties/shorthand/outline.mako.rs @@ -41,7 +41,7 @@ break } if any { - Ok(Longhands { + Ok(expanded! { outline_color: unwrap_or_initial!(outline_color, color), outline_style: unwrap_or_initial!(outline_style, style), outline_width: unwrap_or_initial!(outline_width, width), @@ -73,7 +73,7 @@ pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> { // Re-use border-radius parsing. shorthands::border_radius::parse_value(context, input).map(|longhands| { - Longhands { + expanded! { % for corner in ["top_left", "top_right", "bottom_right", "bottom_left"]: _moz_outline_radius_${corner.replace("_", "")}: longhands.border_${corner}_radius, % endfor diff --git a/components/style/properties/shorthand/position.mako.rs b/components/style/properties/shorthand/position.mako.rs index 58206bb67f4..423e1657d9b 100644 --- a/components/style/properties/shorthand/position.mako.rs +++ b/components/style/properties/shorthand/position.mako.rs @@ -30,7 +30,7 @@ if direction.is_none() && wrap.is_none() { return Err(()) } - Ok(Longhands { + Ok(expanded! { flex_direction: unwrap_or_initial!(flex_direction, direction), flex_wrap: unwrap_or_initial!(flex_wrap, wrap), }) @@ -63,7 +63,7 @@ let mut basis = None; if input.try(|input| input.expect_ident_matching("none")).is_ok() { - return Ok(Longhands { + return Ok(expanded! { flex_grow: Number::new(0.0), flex_shrink: Number::new(0.0), flex_basis: longhands::flex_basis::SpecifiedValue::auto(), @@ -89,7 +89,7 @@ if grow.is_none() && basis.is_none() { return Err(()) } - Ok(Longhands { + Ok(expanded! { flex_grow: grow.unwrap_or(Number::new(1.0)), flex_shrink: shrink.unwrap_or(Number::new(1.0)), flex_basis: basis.unwrap_or(longhands::flex_basis::SpecifiedValue::zero()), @@ -118,7 +118,7 @@ let row_gap = grid_row_gap::parse(context, input)?; let column_gap = input.try(|input| grid_column_gap::parse(context, input)).unwrap_or(row_gap.clone()); - Ok(Longhands { + Ok(expanded! { grid_row_gap: row_gap, grid_column_gap: column_gap, }) @@ -161,7 +161,7 @@ line }; - Ok(Longhands { + Ok(expanded! { grid_${kind}_start: start, grid_${kind}_end: end, }) @@ -219,7 +219,7 @@ (line.clone(), line.clone(), line) }; - Ok(Longhands { + Ok(expanded! { grid_row_start: row_start, grid_row_end: row_end, grid_column_start: column_start, @@ -258,7 +258,7 @@ return Err(()); } - Ok(Longhands { + Ok(expanded! { align_content: align, justify_content: justify, }) @@ -292,7 +292,7 @@ return Err(()); } - Ok(Longhands { + Ok(expanded! { align_self: align, justify_self: justify, }) @@ -334,7 +334,7 @@ return Err(()); } - Ok(Longhands { + Ok(expanded! { align_items: align, justify_items: justify, }) diff --git a/components/style/properties/shorthand/text.mako.rs b/components/style/properties/shorthand/text.mako.rs index c035158a511..c5e7ba12c5e 100644 --- a/components/style/properties/shorthand/text.mako.rs +++ b/components/style/properties/shorthand/text.mako.rs @@ -50,7 +50,7 @@ return Err(()); } - Ok(Longhands { + Ok(expanded! { text_decoration_line: unwrap_or_initial!(text_decoration_line, line), % if product == "gecko" or data.testing: diff --git a/components/style/supports.rs b/components/style/supports.rs index 932b8bc2880..3b651ba9264 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::{PropertyId, ParsedDeclaration}; +use properties::{PropertyId, PropertyDeclaration, SourcePropertyDeclaration}; use std::fmt; use style_traits::ToCss; use stylesheets::CssRuleType; @@ -213,7 +213,8 @@ impl Declaration { }; let mut input = Parser::new(&self.val); let context = ParserContext::new_with_rule_type(cx, Some(CssRuleType::Style)); - let res = ParsedDeclaration::parse(id, &context, &mut input); + let mut declarations = SourcePropertyDeclaration::new(); + let res = PropertyDeclaration::parse_into(&mut declarations, id, &context, &mut input); let _ = input.try(parse_important); res.is_ok() && input.is_exhausted() } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index c56d9ebb39f..58075cbd063 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -80,11 +80,11 @@ use style::keyframes::{Keyframe, KeyframeSelector, KeyframesStepValue}; use style::media_queries::{MediaList, parse_media_query_list}; use style::parallel; use style::parser::{PARSING_MODE_DEFAULT, ParserContext}; -use style::properties::{CascadeFlags, ComputedValues, Importance, ParsedDeclaration, StyleBuilder}; -use style::properties::{LonghandIdSet, PropertyDeclarationBlock, PropertyId}; +use style::properties::{CascadeFlags, ComputedValues, Importance, SourcePropertyDeclaration}; +use style::properties::{LonghandIdSet, PropertyDeclarationBlock, PropertyId, StyleBuilder}; use style::properties::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP; use style::properties::animated_properties::{Animatable, AnimationValue, TransitionProperty}; -use style::properties::parse_one_declaration; +use style::properties::parse_one_declaration_into; use style::restyle_hints::{self, RestyleHint}; use style::rule_tree::StyleSource; use style::selector_parser::PseudoElementCascadeType; @@ -1252,22 +1252,25 @@ pub extern "C" fn Servo_StyleSet_Drop(data: RawServoStyleSetOwned) { let _ = data.into_box::<PerDocumentStyleData>(); } -fn parse_property(property_id: PropertyId, - value: *const nsACString, - data: *mut URLExtraData, - parsing_mode: structs::ParsingMode, - quirks_mode: QuirksMode) -> Result<ParsedDeclaration, ()> { +fn parse_property_into(declarations: &mut SourcePropertyDeclaration, + property_id: PropertyId, + value: *const nsACString, + data: *mut URLExtraData, + parsing_mode: structs::ParsingMode, + quirks_mode: QuirksMode) -> Result<(), ()> { use style::parser::ParsingMode; let value = unsafe { value.as_ref().unwrap().as_str_unchecked() }; let url_data = unsafe { RefPtr::from_ptr_ref(&data) }; let parsing_mode = ParsingMode::from_bits_truncate(parsing_mode); - parse_one_declaration(property_id, - value, - url_data, - &RustLogReporter, - parsing_mode, - quirks_mode) + parse_one_declaration_into( + declarations, + property_id, + value, + url_data, + &RustLogReporter, + parsing_mode, + quirks_mode) } #[no_mangle] @@ -1278,11 +1281,13 @@ pub extern "C" fn Servo_ParseProperty(property: nsCSSPropertyID, value: *const n -> RawServoDeclarationBlockStrong { let id = get_property_id_from_nscsspropertyid!(property, RawServoDeclarationBlockStrong::null()); - match parse_property(id, value, data, parsing_mode, quirks_mode.into()) { - Ok(parsed) => { + let mut declarations = SourcePropertyDeclaration::new(); + match parse_property_into(&mut declarations, id, value, data, + parsing_mode, quirks_mode.into()) { + Ok(()) => { let global_style_data = &*GLOBAL_STYLE_DATA; let mut block = PropertyDeclarationBlock::new(); - parsed.expand_push_into(&mut block, Importance::Normal); + block.extend(declarations.drain(), Importance::Normal); Arc::new(global_style_data.shared_lock.wrap(block)).into_strong() } Err(_) => RawServoDeclarationBlockStrong::null() @@ -1439,11 +1444,13 @@ fn set_property(declarations: RawServoDeclarationBlockBorrowed, property_id: Pro value: *const nsACString, is_important: bool, data: *mut URLExtraData, parsing_mode: structs::ParsingMode, quirks_mode: QuirksMode) -> bool { - match parse_property(property_id, value, data, parsing_mode, quirks_mode) { - Ok(parsed) => { + let mut source_declarations = SourcePropertyDeclaration::new(); + match parse_property_into(&mut source_declarations, property_id, value, data, + parsing_mode, quirks_mode) { + Ok(()) => { let importance = if is_important { Importance::Important } else { Importance::Normal }; write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| { - parsed.expand_set_into(decls, importance) + decls.extend_reset(source_declarations.drain(), importance) }) }, Err(_) => false, @@ -1968,11 +1975,15 @@ pub extern "C" fn Servo_CSSSupports2(property: *const nsACString, value: *const nsACString) -> bool { let id = get_property_id_from_property!(property, false); - parse_property(id, - value, - unsafe { DUMMY_URL_DATA }, - structs::ParsingMode_Default, - QuirksMode::NoQuirks).is_ok() + let mut declarations = SourcePropertyDeclaration::new(); + parse_property_into( + &mut declarations, + id, + value, + unsafe { DUMMY_URL_DATA }, + structs::ParsingMode_Default, + QuirksMode::NoQuirks + ).is_ok() } #[no_mangle] diff --git a/python/servo/testing_commands.py b/python/servo/testing_commands.py index a36d50c2658..427f0fad38f 100644 --- a/python/servo/testing_commands.py +++ b/python/servo/testing_commands.py @@ -288,9 +288,11 @@ class MachCommands(CommandBase): @Command('test-stylo', description='Run stylo unit tests', category='testing') + @CommandArgument('test_name', nargs=argparse.REMAINDER, + help="Only run tests that match this pattern or file path") @CommandArgument('--release', default=False, action="store_true", help="Run with a release build of servo") - def test_stylo(self, release=False): + def test_stylo(self, release=False, test_name=None): self.set_use_stable_rust() self.ensure_bootstrapped() @@ -298,9 +300,10 @@ class MachCommands(CommandBase): env["RUST_BACKTRACE"] = "1" env["CARGO_TARGET_DIR"] = path.join(self.context.topdir, "target", "geckolib").encode("UTF-8") - release = ["--release"] if release else [] + args = (["cargo", "test", "-p", "stylo_tests", "--features", "testing"] + + (["--release"] if release else []) + (test_name or [])) with cd(path.join("ports", "geckolib")): - return call(["cargo", "test", "-p", "stylo_tests", "--features", "testing"] + release, env=env) + return call(args, env=env) @Command('test-compiletest', description='Run compiletests', diff --git a/tests/unit/style/parsing/border.rs b/tests/unit/style/parsing/border.rs index e26a4edf4e2..d9588f116db 100644 --- a/tests/unit/style/parsing/border.rs +++ b/tests/unit/style/parsing/border.rs @@ -4,22 +4,34 @@ use parsing::parse; use style::parser::Parse; +use style::properties::MaybeBoxed; use style::properties::longhands::{border_image_outset, border_image_repeat, border_image_slice}; use style::properties::longhands::{border_image_source, border_image_width}; use style::properties::shorthands::border_image; use style_traits::ToCss; +macro_rules! assert_longhand { + ($parsed_shorthand: expr, $prop: ident, $value_string: expr) => { + assert_eq!($parsed_shorthand.$prop, parse_longhand!($prop, $value_string).maybe_boxed()) + } +} + +macro_rules! assert_initial { + ($parsed_shorthand: expr, $prop: ident) => { + assert_eq!($parsed_shorthand.$prop, $prop::get_initial_specified_value().maybe_boxed()) + } +} + #[test] fn border_image_shorthand_should_parse_when_all_properties_specified() { let input = "linear-gradient(red, blue) 30 30% 45 fill / 20px 40px / 10px round stretch"; let result = parse(border_image::parse_value, input).unwrap(); - assert_eq!(result.border_image_source, - parse_longhand!(border_image_source, "linear-gradient(red, blue)")); - assert_eq!(result.border_image_slice, parse_longhand!(border_image_slice, "30 30% 45 fill")); - assert_eq!(result.border_image_width, parse_longhand!(border_image_width, "20px 40px")); - assert_eq!(result.border_image_outset, parse_longhand!(border_image_outset, "10px")); - assert_eq!(result.border_image_repeat, parse_longhand!(border_image_repeat, "round stretch")); + assert_longhand!(result, border_image_source, "linear-gradient(red, blue)"); + assert_longhand!(result, border_image_slice, "30 30% 45 fill"); + assert_longhand!(result, border_image_width, "20px 40px"); + assert_longhand!(result, border_image_outset, "10px"); + assert_longhand!(result, border_image_repeat, "round stretch"); } #[test] @@ -27,12 +39,11 @@ fn border_image_shorthand_should_parse_without_width() { let input = "linear-gradient(red, blue) 30 30% 45 fill / / 10px round stretch"; let result = parse(border_image::parse_value, input).unwrap(); - assert_eq!(result.border_image_source, - parse_longhand!(border_image_source, "linear-gradient(red, blue)")); - assert_eq!(result.border_image_slice, parse_longhand!(border_image_slice, "30 30% 45 fill")); - assert_eq!(result.border_image_outset, parse_longhand!(border_image_outset, "10px")); - assert_eq!(result.border_image_repeat, parse_longhand!(border_image_repeat, "round stretch")); - assert_eq!(result.border_image_width, border_image_width::get_initial_specified_value()); + assert_longhand!(result, border_image_source, "linear-gradient(red, blue)"); + assert_longhand!(result, border_image_slice, "30 30% 45 fill"); + assert_longhand!(result, border_image_outset, "10px"); + assert_longhand!(result, border_image_repeat, "round stretch"); + assert_initial!(result, border_image_width); } #[test] @@ -40,12 +51,11 @@ fn border_image_shorthand_should_parse_without_outset() { let input = "linear-gradient(red, blue) 30 30% 45 fill / 20px 40px round"; let result = parse(border_image::parse_value, input).unwrap(); - assert_eq!(result.border_image_source, - parse_longhand!(border_image_source, "linear-gradient(red, blue)")); - assert_eq!(result.border_image_slice, parse_longhand!(border_image_slice, "30 30% 45 fill")); - assert_eq!(result.border_image_width, parse_longhand!(border_image_width, "20px 40px")); - assert_eq!(result.border_image_repeat, parse_longhand!(border_image_repeat, "round")); - assert_eq!(result.border_image_outset, border_image_outset::get_initial_specified_value()); + assert_longhand!(result, border_image_source, "linear-gradient(red, blue)"); + assert_longhand!(result, border_image_slice, "30 30% 45 fill"); + assert_longhand!(result, border_image_width, "20px 40px"); + assert_longhand!(result, border_image_repeat, "round"); + assert_initial!(result, border_image_outset); } #[test] @@ -53,24 +63,22 @@ fn border_image_shorthand_should_parse_without_width_or_outset() { let input = "linear-gradient(red, blue) 30 30% 45 fill round"; let result = parse(border_image::parse_value, input).unwrap(); - assert_eq!(result.border_image_source, - parse_longhand!(border_image_source, "linear-gradient(red, blue)")); - assert_eq!(result.border_image_slice, parse_longhand!(border_image_slice, "30 30% 45 fill")); - assert_eq!(result.border_image_repeat, parse_longhand!(border_image_repeat, "round")); - assert_eq!(result.border_image_width, border_image_width::get_initial_specified_value()); - assert_eq!(result.border_image_outset, border_image_outset::get_initial_specified_value()); + assert_longhand!(result, border_image_source, "linear-gradient(red, blue)"); + assert_longhand!(result, border_image_slice, "30 30% 45 fill"); + assert_longhand!(result, border_image_repeat, "round"); + assert_initial!(result, border_image_width); + assert_initial!(result, border_image_outset); } #[test] fn border_image_shorthand_should_parse_with_just_source() { let result = parse(border_image::parse_value, "linear-gradient(red, blue)").unwrap(); - assert_eq!(result.border_image_source, - parse_longhand!(border_image_source, "linear-gradient(red, blue)")); - assert_eq!(result.border_image_slice, border_image_slice::get_initial_specified_value()); - assert_eq!(result.border_image_width, border_image_width::get_initial_specified_value()); - assert_eq!(result.border_image_outset, border_image_outset::get_initial_specified_value()); - assert_eq!(result.border_image_repeat, border_image_repeat::get_initial_specified_value()); + assert_longhand!(result, border_image_source, "linear-gradient(red, blue)"); + assert_initial!(result, border_image_slice); + assert_initial!(result, border_image_width); + assert_initial!(result, border_image_outset); + assert_initial!(result, border_image_repeat); } #[test] diff --git a/tests/unit/style/size_of.rs b/tests/unit/style/size_of.rs index c0b14276c24..60d8c7b4ac0 100644 --- a/tests/unit/style/size_of.rs +++ b/tests/unit/style/size_of.rs @@ -6,6 +6,10 @@ use style::properties; size_of_test!(test_size_of_property_declaration, properties::PropertyDeclaration, 32); +// This is huge, but we allocate it on the stack and then never move it, +// we only pass `&mut SourcePropertyDeclaration` references around. +size_of_test!(test_size_of_parsed_declaration, properties::SourcePropertyDeclaration, 576); + #[test] fn size_of_specified_values() { ::style::properties::test_size_of_specified_values(); diff --git a/tests/unit/stylo/size_of.rs b/tests/unit/stylo/size_of.rs index ce2e076ba86..7909a69680e 100644 --- a/tests/unit/stylo/size_of.rs +++ b/tests/unit/stylo/size_of.rs @@ -21,6 +21,10 @@ fn size_of_selectors_dummy_types() { size_of_test!(test_size_of_property_declaration, style::properties::PropertyDeclaration, 32); +// This is huge, but we allocate it on the stack and then never move it, +// we only pass `&mut SourcePropertyDeclaration` references around. +size_of_test!(test_size_of_parsed_declaration, style::properties::SourcePropertyDeclaration, 704); + #[test] fn size_of_specified_values() { ::style::properties::test_size_of_specified_values(); |