diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 2fefd0ba0fd..36e6ee4cf9b 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -10,11 +10,10 @@ use crate::dom::TElement; use crate::font_metrics::FontMetricsProvider; use crate::logical_geometry::WritingMode; use crate::media_queries::Device; -use crate::properties::{CSSWideKeyword, LonghandId, LonghandIdSet, PropertyFlags}; -use crate::properties::{ComputedValueFlags, CASCADE_PROPERTY}; -use crate::properties::{ComputedValues, Importance, StyleBuilder}; use crate::properties::{ - DeclarationImportanceIterator, PropertyDeclaration, PropertyDeclarationId, + CSSWideKeyword, ComputedValueFlags, ComputedValues, DeclarationImportanceIterator, Importance, + LonghandId, LonghandIdSet, PropertyDeclaration, PropertyDeclarationId, PropertyFlags, + ShorthandsWithPropertyReferencesCache, StyleBuilder, CASCADE_PROPERTY, }; use crate::rule_cache::{RuleCache, RuleCacheConditions}; use crate::rule_tree::StrongRuleNode; @@ -325,10 +324,12 @@ where let using_cached_reset_properties = { let mut cascade = Cascade::new(&mut context, cascade_mode); + let mut shorthand_cache = ShorthandsWithPropertyReferencesCache::default(); cascade.apply_properties::( ApplyResetProperties::Yes, declarations.iter().cloned(), + &mut shorthand_cache, ); cascade.compute_visited_style_if_needed( @@ -348,7 +349,11 @@ where ApplyResetProperties::Yes }; - cascade.apply_properties::(apply_reset, declarations.iter().cloned()); + cascade.apply_properties::( + apply_reset, + declarations.iter().cloned(), + &mut shorthand_cache, + ); using_cached_reset_properties }; @@ -501,10 +506,14 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { } } - fn substitute_variables_if_needed<'decl>( + fn substitute_variables_if_needed<'decl, 'cache>( &mut self, declaration: &'decl PropertyDeclaration, - ) -> Cow<'decl, PropertyDeclaration> { + cache: &'cache mut ShorthandsWithPropertyReferencesCache, + ) -> Cow<'decl, PropertyDeclaration> + where + 'cache: 'decl, + { let declaration = match *declaration { PropertyDeclaration::WithVariables(ref declaration) => declaration, ref d => return Cow::Borrowed(d), @@ -536,12 +545,13 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { } } - Cow::Owned(declaration.value.substitute_variables( + declaration.value.substitute_variables( declaration.id, self.context.builder.custom_properties.as_ref(), self.context.quirks_mode, self.context.device(), - )) + cache, + ) } #[inline(always)] @@ -559,6 +569,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { &mut self, apply_reset: ApplyResetProperties, declarations: I, + mut shorthand_cache: &mut ShorthandsWithPropertyReferencesCache, ) where Phase: CascadePhase, I: Iterator, @@ -619,7 +630,8 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { continue; } - let mut declaration = self.substitute_variables_if_needed(declaration); + let mut declaration = + self.substitute_variables_if_needed(declaration, &mut shorthand_cache); // When document colors are disabled, do special handling of // properties that are marked as ignored in that mode. diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 9170f6e650d..2cee5eb7322 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -837,6 +837,7 @@ impl PropertyDeclarationBlock { custom_properties.as_ref(), QuirksMode::NoQuirks, device, + &mut Default::default() ) .to_css(dest) }, diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index f2fc745c57b..8b91b35a139 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -367,6 +367,7 @@ impl AnimationValue { } }, PropertyDeclaration::WithVariables(ref declaration) => { + let mut cache = Default::default(); let substituted = { let custom_properties = extra_custom_properties.or_else(|| context.style().custom_properties()); @@ -376,6 +377,7 @@ impl AnimationValue { custom_properties, context.quirks_mode, context.device(), + &mut cache, ) }; return AnimationValue::from_declaration( diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 98276d36027..abbd7992dd9 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -29,11 +29,11 @@ use crate::context::QuirksMode; use crate::logical_geometry::WritingMode; use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use crate::computed_value_flags::*; +use crate::hash::FxHashMap; use crate::media_queries::Device; use crate::parser::ParserContext; use crate::properties::longhands::system_font::SystemFont; use crate::selector_parser::PseudoElement; -use selectors::parser::SelectorParseErrorKind; #[cfg(feature = "servo")] use servo_config::prefs; use style_traits::{CssWriter, KeywordsCollectFn, ParseError, ParsingMode}; use style_traits::{SpecifiedValueInfo, StyleParseErrorKind, ToCss}; @@ -1634,23 +1634,44 @@ impl ToCss for UnparsedValue { } } +/// A simple cache for properties that come from a shorthand and have variable +/// references. +/// +/// This cache works because of the fact that you can't have competing values +/// for a given longhand coming from the same shorthand (but note that this is +/// why the shorthand needs to be part of the cache key). +pub type ShorthandsWithPropertyReferencesCache = + FxHashMap<(ShorthandId, LonghandId), PropertyDeclaration>; + impl UnparsedValue { - fn substitute_variables( + fn substitute_variables<'cache>( &self, longhand_id: LonghandId, custom_properties: Option<<&Arc>, quirks_mode: QuirksMode, device: &Device, - ) -> PropertyDeclaration { + shorthand_cache: &'cache mut ShorthandsWithPropertyReferencesCache, + ) -> Cow<'cache, PropertyDeclaration> { let invalid_at_computed_value_time = || { let keyword = if longhand_id.inherited() { CSSWideKeyword::Inherit } else { CSSWideKeyword::Initial }; - PropertyDeclaration::css_wide_keyword(longhand_id, keyword) + Cow::Owned(PropertyDeclaration::css_wide_keyword(longhand_id, keyword)) }; + if let Some(shorthand_id) = self.from_shorthand { + let key = (shorthand_id, longhand_id); + if shorthand_cache.contains_key(&key) { + // FIXME: This double lookup should be avoidable, but rustc + // doesn't like that, see: + // + // https://github.com/rust-lang/rust/issues/82146 + return Cow::Borrowed(&shorthand_cache[&key]); + } + } + let css = match crate::custom_properties::substitute( &self.css, self.first_token_type, @@ -1685,53 +1706,59 @@ impl UnparsedValue { let mut input = Parser::new(&mut input); input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less. if let Ok(keyword) = input.try_parse(CSSWideKeyword::parse) { - return PropertyDeclaration::css_wide_keyword(longhand_id, keyword); + return Cow::Owned(PropertyDeclaration::css_wide_keyword(longhand_id, keyword)); } - let declaration = input.parse_entirely(|input| { - match self.from_shorthand { - None => longhand_id.parse_value(&context, input), - Some(ShorthandId::All) => { - // No need to parse the 'all' shorthand as anything other - // than a CSS-wide keyword, after variable substitution. - Err(input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent("all".into()))) + let shorthand = match self.from_shorthand { + None => { + return match input.parse_entirely(|input| longhand_id.parse_value(&context, input)) { + Ok(decl) => Cow::Owned(decl), + Err(..) => invalid_at_computed_value_time(), } - % for shorthand in data.shorthands_except_all(): - Some(ShorthandId::${shorthand.camel_case}) => { - shorthands::${shorthand.ident}::parse_value(&context, input) - .map(|longhands| { - match longhand_id { - <% seen = set() %> - % for property in shorthand.sub_properties: - // When animating logical properties, we end up - // physicalizing the value during the animation, but - // the value still comes from the logical shorthand. - // - // So we need to handle the physical properties too. - % for prop in [property] + property.all_physical_mapped_properties(data): - % if prop.camel_case not in seen: - LonghandId::${prop.camel_case} => { - PropertyDeclaration::${prop.camel_case}( - longhands.${property.ident} - ) - } - <% seen.add(prop.camel_case) %> + }, + Some(shorthand) => shorthand, + }; + + match shorthand { + ShorthandId::All => { + // No need to parse the 'all' shorthand as anything other + // than a CSS-wide keyword, after variable substitution. + return invalid_at_computed_value_time(); + } + % for shorthand in data.shorthands_except_all(): + ShorthandId::${shorthand.camel_case} => { + let longhands = match input.parse_entirely(|input| shorthands::${shorthand.ident}::parse_value(&context, input)) { + Ok(l) => l, + _ => return invalid_at_computed_value_time(), + }; + + <% seen = set() %> + % for property in shorthand.sub_properties: + // When animating logical properties, we end up + // physicalizing the value during the animation, but the + // value still comes from the logical shorthand. + // + // So we need to handle the physical properties too. + % for prop in property.all_physical_mapped_properties(data) + [property]: + % if prop.ident not in seen: + shorthand_cache.insert( + (shorthand, LonghandId::${prop.camel_case}), + PropertyDeclaration::${prop.camel_case}( + longhands.${property.ident} + % if prop.ident != property.ident: + .clone() % endif - % endfor - % endfor - <% del seen %> - _ => unreachable!() - } - }) - } + ) + ); + % endif + <% seen.add(prop.ident) %> + % endfor % endfor } - }); - - match declaration { - Ok(decl) => decl, - Err(..) => invalid_at_computed_value_time(), + % endfor } + + Cow::Borrowed(&shorthand_cache[&(shorthand, longhand_id)]) } }