diff --git a/Cargo.lock b/Cargo.lock index 8631a7829a9..36328d939da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3234,6 +3234,7 @@ dependencies = [ "malloc_size_of_derive 0.0.1", "selectors 0.19.0", "serde 1.0.14 (registry+https://github.com/rust-lang/crates.io-index)", + "servo_arc 0.0.1", "servo_atoms 0.0.1", "webrender_api 0.52.0 (git+https://github.com/servo/webrender)", ] diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 650dca1829f..5507739a71f 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -10,7 +10,7 @@ use Atom; use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType}; use precomputed_hash::PrecomputedHash; use properties::{CSSWideKeyword, DeclaredValue}; -use selector_map::{PrecomputedHashSet, PrecomputedDiagnosticHashMap}; +use selector_map::{PrecomputedHashSet, PrecomputedHashMap, PrecomputedDiagnosticHashMap}; use selectors::parser::SelectorParseError; use servo_arc::Arc; use std::ascii::AsciiExt; @@ -35,14 +35,14 @@ pub fn parse_name(s: &str) -> Result<&str, ()> { } } -/// A specified value for a custom property is just a set of tokens. +/// A value for a custom property is just a set of tokens. /// /// We preserve the original CSS for serialization, and also the variable /// references to other custom property names. #[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "gecko", derive(MallocSizeOf))] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -pub struct SpecifiedValue { +pub struct VariableValue { css: String, first_token_type: TokenSerializationType, @@ -52,24 +52,6 @@ pub struct SpecifiedValue { references: PrecomputedHashSet, } -/// This struct is a cheap borrowed version of a `SpecifiedValue`. -pub struct BorrowedSpecifiedValue<'a> { - css: &'a str, - first_token_type: TokenSerializationType, - last_token_type: TokenSerializationType, - references: &'a PrecomputedHashSet, -} - -/// A computed value is just a set of tokens as well, until we resolve variables -/// properly. -#[derive(Clone, Debug, Eq, PartialEq)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] -pub struct ComputedValue { - css: String, - first_token_type: TokenSerializationType, - last_token_type: TokenSerializationType, -} - impl ToCss for SpecifiedValue { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write, @@ -78,14 +60,6 @@ impl ToCss for SpecifiedValue { } } -impl ToCss for ComputedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result - where W: fmt::Write, - { - dest.write_str(&self.css) - } -} - /// A map from CSS variable names to CSS variable computed values, used for /// resolving. /// @@ -93,7 +67,17 @@ impl ToCss for ComputedValue { /// DOM. CSSDeclarations expose property names as indexed properties, which /// need to be stable. So we keep an array of property names which order is /// determined on the order that they are added to the name-value map. -pub type CustomPropertiesMap = OrderedMap; +/// +/// The variable values are guaranteed to not have references to other +/// properties. +pub type CustomPropertiesMap = OrderedMap>; + +/// Both specified and computed values are VariableValues, the difference is +/// whether var() functions are expanded. +pub type SpecifiedValue = VariableValue; +/// Both specified and computed values are VariableValues, the difference is +/// whether var() functions are expanded. +pub type ComputedValue = VariableValue; /// A map that preserves order for the keys, and that is easily indexable. #[derive(Clone, Debug, Eq, PartialEq)] @@ -136,6 +120,11 @@ where value } + /// Get whether there's a value on the map for `key`. + pub fn contains_key(&self, key: &K) -> bool { + self.values.contains_key(key) + } + /// Get the key located at the given index. pub fn get_key_at(&self, index: u32) -> Option<&K> { self.index.get(index as usize) @@ -211,22 +200,30 @@ where } } -impl ComputedValue { - fn empty() -> ComputedValue { - ComputedValue { +impl VariableValue { + fn empty() -> Self { + Self { css: String::new(), last_token_type: TokenSerializationType::nothing(), first_token_type: TokenSerializationType::nothing(), + references: PrecomputedHashSet::default(), } } - fn push(&mut self, css: &str, css_first_token_type: TokenSerializationType, - css_last_token_type: TokenSerializationType) { - // This happens e.g. between to subsequent var() functions: `var(--a)var(--b)`. - // In that case, css_*_token_type is non-sensical. + fn push( + &mut self, + css: &str, + css_first_token_type: TokenSerializationType, + css_last_token_type: TokenSerializationType + ) { + // This happens e.g. between two subsequent var() functions: + // `var(--a)var(--b)`. + // + // In that case, css_*_token_type is nonsensical. if css.is_empty() { return } + self.first_token_type.set_if_nothing(css_first_token_type); // If self.first_token_type was nothing, // self.last_token_type is also nothing and this will be false: @@ -237,27 +234,32 @@ impl ComputedValue { self.last_token_type = css_last_token_type } - fn push_from(&mut self, position: (SourcePosition, TokenSerializationType), - input: &Parser, last_token_type: TokenSerializationType) { + fn push_from( + &mut self, + position: (SourcePosition, TokenSerializationType), + input: &Parser, + last_token_type: TokenSerializationType + ) { self.push(input.slice_from(position.0), position.1, last_token_type) } fn push_variable(&mut self, variable: &ComputedValue) { + debug_assert!(variable.references.is_empty()); self.push(&variable.css, variable.first_token_type, variable.last_token_type) } } -impl SpecifiedValue { - /// Parse a custom property SpecifiedValue. +impl VariableValue { + /// Parse a custom property value. pub fn parse<'i, 't>( input: &mut Parser<'i, 't>, - ) -> Result, ParseError<'i>> { + ) -> Result, ParseError<'i>> { let mut references = PrecomputedHashSet::default(); let (first_token_type, css, last_token_type) = parse_self_contained_declaration_value(input, Some(&mut references))?; - Ok(Box::new(SpecifiedValue { + Ok(Arc::new(VariableValue { css: css.into_owned(), first_token_type, last_token_type, @@ -467,7 +469,7 @@ fn parse_var_function<'i, 't>( pub struct CustomPropertiesBuilder<'a> { seen: PrecomputedHashSet<&'a Name>, may_have_cycles: bool, - specified_custom_properties: OrderedMap<&'a Name, Option>>, + custom_properties: Option, inherited: Option<&'a Arc>, } @@ -477,7 +479,7 @@ impl<'a> CustomPropertiesBuilder<'a> { Self { seen: PrecomputedHashSet::default(), may_have_cycles: false, - specified_custom_properties: OrderedMap::new(), + custom_properties: None, inherited, } } @@ -486,27 +488,30 @@ impl<'a> CustomPropertiesBuilder<'a> { pub fn cascade( &mut self, name: &'a Name, - specified_value: DeclaredValue<'a, Box>, + specified_value: DeclaredValue<'a, Arc>, ) { let was_already_present = !self.seen.insert(name); if was_already_present { return; } + if self.custom_properties.is_none() { + self.custom_properties = Some(match self.inherited { + Some(inherited) => (**inherited).clone(), + None => CustomPropertiesMap::new(), + }) + } + + let map = self.custom_properties.as_mut().unwrap(); match specified_value { DeclaredValue::Value(ref specified_value) => { self.may_have_cycles |= !specified_value.references.is_empty(); - self.specified_custom_properties.insert(name, Some(BorrowedSpecifiedValue { - css: &specified_value.css, - first_token_type: specified_value.first_token_type, - last_token_type: specified_value.last_token_type, - references: &specified_value.references, - })); + map.insert(name.clone(), (*specified_value).clone()); }, DeclaredValue::WithVariables(_) => unreachable!(), DeclaredValue::CSSWideKeyword(keyword) => match keyword { CSSWideKeyword::Initial => { - self.specified_custom_properties.insert(name, None); + map.remove(name); } CSSWideKeyword::Unset | // Custom properties are inherited by default. CSSWideKeyword::Inherit => {} // The inherited value is what we already have. @@ -521,14 +526,16 @@ impl<'a> CustomPropertiesBuilder<'a> { /// /// Otherwise, just use the inherited custom properties map. pub fn build(mut self) -> Option> { - if self.specified_custom_properties.is_empty() { - return self.inherited.cloned(); - } + let mut map = match self.custom_properties.take() { + Some(m) => m, + None => return self.inherited.cloned(), + }; if self.may_have_cycles { - remove_cycles(&mut self.specified_custom_properties); + remove_cycles(&mut map); + substitute_all(&mut map); } - Some(Arc::new(substitute_all(self.specified_custom_properties, self.inherited))) + Some(Arc::new(map)) } } @@ -536,7 +543,7 @@ impl<'a> CustomPropertiesBuilder<'a> { /// /// The initial value of a custom property is represented by this property not /// being in the map. -fn remove_cycles(map: &mut OrderedMap<&Name, Option>) { +fn remove_cycles(map: &mut CustomPropertiesMap) { let mut to_remove = PrecomputedHashSet::default(); { let mut visited = PrecomputedHashSet::default(); @@ -545,7 +552,7 @@ fn remove_cycles(map: &mut OrderedMap<&Name, Option>) { walk(map, name, &mut stack, &mut visited, &mut to_remove); fn walk<'a>( - map: &OrderedMap<&'a Name, Option>>, + map: &'a CustomPropertiesMap, name: &'a Name, stack: &mut Vec<&'a Name>, visited: &mut PrecomputedHashSet<&'a Name>, @@ -555,11 +562,11 @@ fn remove_cycles(map: &mut OrderedMap<&Name, Option>) { if already_visited_before { return } - if let Some(&Some(ref value)) = map.get(&name) { + if let Some(ref value) = map.get(name) { if !value.references.is_empty() { stack.push(name); - for next in value.references { - if let Some(position) = stack.iter().position(|&x| x == next) { + for next in value.references.iter() { + if let Some(position) = stack.iter().position(|x| *x == next) { // Found a cycle for &in_cycle in &stack[position..] { to_remove.insert(in_cycle.clone()); @@ -580,97 +587,116 @@ fn remove_cycles(map: &mut OrderedMap<&Name, Option>) { } /// Replace `var()` functions for all custom properties. -fn substitute_all( - specified_values_map: OrderedMap<&Name, Option>, - inherited: Option<&Arc>, -) -> CustomPropertiesMap { - let mut custom_properties_map = match inherited { - None => CustomPropertiesMap::new(), - Some(inherited) => (**inherited).clone(), - }; - +fn substitute_all(custom_properties_map: &mut CustomPropertiesMap) { + // FIXME(emilio): This stash is needed because we can't prove statically to + // rustc that we don't try to mutate the same variable from two recursive + // `substitute_one` calls. + // + // If this is really really hot, we may be able to cheat using `unsafe`, I + // guess... + let mut stash = PrecomputedHashMap::default(); let mut invalid = PrecomputedHashSet::default(); - for (name, value) in specified_values_map.iter() { - let value = match *value { - Some(ref v) => v, - None => { - custom_properties_map.remove(*name); - continue; - } - }; - - let _ = substitute_one( - name, - value, - &specified_values_map, - None, - &mut custom_properties_map, - &mut invalid, - ); + for (name, value) in custom_properties_map.iter() { + if !value.references.is_empty() && !stash.contains_key(name) { + let _ = substitute_one( + name, + value, + custom_properties_map, + None, + &mut stash, + &mut invalid, + ); + } } - custom_properties_map + for (name, value) in stash.drain() { + custom_properties_map.insert(name, value); + } + + for name in invalid.drain() { + custom_properties_map.remove(&name); + } + + debug_assert!(custom_properties_map.iter().all(|(_, v)| v.references.is_empty())); } -/// Replace `var()` functions for one custom property. -/// Also recursively record results for other custom properties referenced by `var()` functions. -/// Return `Err(())` for invalid at computed time. -/// or `Ok(last_token_type that was pushed to partial_computed_value)` otherwise. +/// Replace `var()` functions for one custom property, leaving the result in +/// `stash`. +/// +/// Also recursively record results for other custom properties referenced by +/// `var()` functions. +/// +/// Return `Err(())` for invalid at computed time. or `Ok(last_token_type that +/// was pushed to partial_computed_value)` otherwise. fn substitute_one( name: &Name, - specified_value: &BorrowedSpecifiedValue, - specified_values_map: &OrderedMap<&Name, Option>, - partial_computed_value: Option<&mut ComputedValue>, - custom_properties_map: &mut CustomPropertiesMap, + specified_value: &Arc, + custom_properties: &CustomPropertiesMap, + partial_computed_value: Option<&mut VariableValue>, + stash: &mut PrecomputedHashMap>, invalid: &mut PrecomputedHashSet, ) -> Result { + debug_assert!(!specified_value.references.is_empty()); + debug_assert!(!stash.contains_key(name)); + if invalid.contains(name) { return Err(()); } - let computed_value = if specified_value.references.is_empty() { - // The specified value contains no var() reference - ComputedValue { - css: specified_value.css.to_owned(), - first_token_type: specified_value.first_token_type, - last_token_type: specified_value.last_token_type, - } - } else { - let mut partial_computed_value = ComputedValue::empty(); - let mut input = ParserInput::new(&specified_value.css); - let mut input = Parser::new(&mut input); - let mut position = (input.position(), specified_value.first_token_type); - let result = substitute_block( - &mut input, &mut position, &mut partial_computed_value, - &mut |name, partial_computed_value| { - if let Some(other_specified_value) = specified_values_map.get(&name).and_then(|v| v.as_ref()) { - substitute_one( - name, - other_specified_value, - specified_values_map, - Some(partial_computed_value), - custom_properties_map, - invalid, - ) - } else { - Err(()) - } + let mut computed_value = ComputedValue::empty(); + let mut input = ParserInput::new(&specified_value.css); + let mut input = Parser::new(&mut input); + let mut position = (input.position(), specified_value.first_token_type); + + let result = substitute_block( + &mut input, + &mut position, + &mut computed_value, + &mut |name, partial_computed_value| { + if let Some(already_computed) = stash.get(name) { + partial_computed_value.push_variable(already_computed); + return Ok(already_computed.last_token_type); } - ); - if let Ok(last_token_type) = result { - partial_computed_value.push_from(position, &input, last_token_type); - partial_computed_value - } else { + + let other_specified_value = match custom_properties.get(name) { + Some(v) => v, + None => return Err(()), + }; + + if other_specified_value.references.is_empty() { + partial_computed_value.push_variable(other_specified_value); + return Ok(other_specified_value.last_token_type); + } + + substitute_one( + name, + other_specified_value, + custom_properties, + Some(partial_computed_value), + stash, + invalid + ) + } + ); + + match result { + Ok(last_token_type) => { + computed_value.push_from(position, &input, last_token_type); + } + Err(..) => { invalid.insert(name.clone()); return Err(()) } - }; + } + if let Some(partial_computed_value) = partial_computed_value { partial_computed_value.push_variable(&computed_value) } + let last_token_type = computed_value.last_token_type; - custom_properties_map.insert(name.clone(), computed_value); + stash.insert(name.clone(), Arc::new(computed_value)); + Ok(last_token_type) } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index d18bacbb631..128bb4fbf0a 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1398,7 +1398,12 @@ pub enum PropertyDeclaration { ), /// A custom property declaration, with the property name and the declared /// value. - Custom(::custom_properties::Name, DeclaredValueOwned>), + #[cfg_attr(feature = "gecko", ignore_malloc_size_of = "XXX: how to handle this?")] + Custom( + ::custom_properties::Name, + #[cfg_attr(feature = "gecko", ignore_malloc_size_of = "XXX: how to handle this?")] + DeclaredValueOwned> + ), } impl fmt::Debug for PropertyDeclaration { diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index 0d9cb81ebce..9569845d0a0 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -9,6 +9,7 @@ use Atom; use cssparser::serialize_identifier; use custom_properties; +use servo_arc::Arc; use std::fmt; use style_traits::ToCss; @@ -153,7 +154,8 @@ pub struct PaintWorklet { pub name: Atom, /// The arguments for the worklet. /// TODO: store a parsed representation of the arguments. - pub arguments: Vec, + #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] + pub arguments: Vec>, } trivial_to_computed_value!(PaintWorklet); diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index feb4e044c93..dea9fa14684 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -917,7 +917,7 @@ impl Parse for PaintWorklet { let name = Atom::from(&**input.expect_ident()?); let arguments = input.try(|input| { input.expect_comma()?; - input.parse_comma_separated(|input| Ok(*SpecifiedValue::parse(input)?)) + input.parse_comma_separated(|input| SpecifiedValue::parse(input)) }).unwrap_or(vec![]); Ok(PaintWorklet { name, arguments }) }) diff --git a/components/style_traits/Cargo.toml b/components/style_traits/Cargo.toml index 40a2ad8e573..db45195b9c1 100644 --- a/components/style_traits/Cargo.toml +++ b/components/style_traits/Cargo.toml @@ -26,3 +26,4 @@ selectors = { path = "../selectors" } serde = {version = "1.0", optional = true} webrender_api = {git = "https://github.com/servo/webrender", optional = true} servo_atoms = {path = "../atoms", optional = true} +servo_arc = {path = "../servo_arc" } diff --git a/components/style_traits/lib.rs b/components/style_traits/lib.rs index 669680bdcd0..724a2141fc3 100644 --- a/components/style_traits/lib.rs +++ b/components/style_traits/lib.rs @@ -24,6 +24,7 @@ extern crate euclid; extern crate selectors; #[cfg(feature = "servo")] #[macro_use] extern crate serde; #[cfg(feature = "servo")] extern crate webrender_api; +extern crate servo_arc; #[cfg(feature = "servo")] extern crate servo_atoms; #[cfg(feature = "servo")] pub use webrender_api::DevicePixel; diff --git a/components/style_traits/values.rs b/components/style_traits/values.rs index f0bef5c80ae..8c753b7d7db 100644 --- a/components/style_traits/values.rs +++ b/components/style_traits/values.rs @@ -7,6 +7,7 @@ use app_units::Au; use cssparser::{BasicParseError, ParseError, Parser, Token, UnicodeRange, serialize_string}; use cssparser::ToCss as CssparserToCss; +use servo_arc::Arc; use std::fmt::{self, Write}; /// Serialises a value according to its CSS representation. @@ -343,6 +344,14 @@ impl ToCss for Box where T: ?Sized + ToCss { } } +impl ToCss for Arc where T: ?Sized + ToCss { + fn to_css(&self, dest: &mut W) -> fmt::Result + where W: Write, + { + (**self).to_css(dest) + } +} + impl ToCss for Au { fn to_css(&self, dest: &mut W) -> fmt::Result where W: Write { self.to_f64_px().to_css(dest)?;