Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-04 13:10:20 +01:00
Code Issues Projects Releases Packages Wiki Activity

style: Avoid being so Arc-happy in the custom properties code.

Browse source
This commit is contained in:
Emilio Cobos Álvarez 2017-10-01 13:07:02 +02:00
parent 65d9b345bb
commit 2adfd84636
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C
7 changed files with 88 additions and 79 deletions
Download patch file Download diff file Expand all files Collapse all files

23
components/style/custom_properties.rs
View file

@ -448,7 +448,7 @@ fn parse_var_function<'i, 't>(
/// name was already there. /// name was already there.
pub fn cascade<'a>( pub fn cascade<'a>(
custom_properties: &mut Option<OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>>, custom_properties: &mut Option<OrderedMap<&'a Name, BorrowedSpecifiedValue<'a>>>,
inherited: &'a Option<Arc<CustomPropertiesMap>>, inherited: Option<&'a Arc<CustomPropertiesMap>>,
seen: &mut PrecomputedHashSet<&'a Name>, seen: &mut PrecomputedHashSet<&'a Name>,
name: &'a Name, name: &'a Name,
specified_value: DeclaredValue<'a, Box<SpecifiedValue>> specified_value: DeclaredValue<'a, Box<SpecifiedValue>>
@ -462,7 +462,7 @@ pub fn cascade<'a>(
Some(ref mut map) => map, Some(ref mut map) => map,
None => { None => {
let mut map = OrderedMap::new(); let mut map = OrderedMap::new();
if let Some(ref inherited) = *inherited { if let Some(inherited) = inherited {
for name in &inherited.index { for name in &inherited.index {
let inherited_value = inherited.get(name).unwrap(); let inherited_value = inherited.get(name).unwrap();
map.insert(name, BorrowedSpecifiedValue { map.insert(name, BorrowedSpecifiedValue {
@ -503,14 +503,15 @@ pub fn cascade<'a>(
/// to remove any potential cycles, and wrap it in an arc. /// to remove any potential cycles, and wrap it in an arc.
/// ///
/// Otherwise, just use the inherited custom properties map. /// Otherwise, just use the inherited custom properties map.
pub fn finish_cascade(specified_values_map: Option<OrderedMap<&Name, BorrowedSpecifiedValue>>, pub fn finish_cascade(
inherited: &Option<Arc<CustomPropertiesMap>>) specified_values_map: Option<OrderedMap<&Name, BorrowedSpecifiedValue>>,
-> Option<Arc<CustomPropertiesMap>> { inherited: Option<&Arc<CustomPropertiesMap>>,
) -> Option<Arc<CustomPropertiesMap>> {
if let Some(mut map) = specified_values_map { if let Some(mut map) = specified_values_map {
remove_cycles(&mut map); remove_cycles(&mut map);
Some(Arc::new(substitute_all(map))) Some(Arc::new(substitute_all(map)))
} else { } else {
inherited.clone() inherited.cloned()
} }
} }
@ -732,16 +733,18 @@ fn substitute_block<'i, 't, F>(input: &mut Parser<'i, 't>,
/// Replace `var()` functions for a non-custom property. /// Replace `var()` functions for a non-custom property.
/// Return `Err(())` for invalid at computed time. /// Return `Err(())` for invalid at computed time.
pub fn substitute<'i>(input: &'i str, first_token_type: TokenSerializationType, pub fn substitute<'i>(
computed_values_map: &Option<Arc<CustomPropertiesMap>>) input: &'i str,
-> Result<String, ParseError<'i>> { first_token_type: TokenSerializationType,
computed_values_map: Option<&Arc<CustomPropertiesMap>>,
) -> Result<String, ParseError<'i>> {
let mut substituted = ComputedValue::empty(); let mut substituted = ComputedValue::empty();
let mut input = ParserInput::new(input); let mut input = ParserInput::new(input);
let mut input = Parser::new(&mut input); let mut input = Parser::new(&mut input);
let mut position = (input.position(), first_token_type); let mut position = (input.position(), first_token_type);
let last_token_type = substitute_block( let last_token_type = substitute_block(
&mut input, &mut position, &mut substituted, &mut |name, substituted| { &mut input, &mut position, &mut substituted, &mut |name, substituted| {
if let Some(value) = computed_values_map.as_ref().and_then(|map| map.get(name)) { if let Some(value) = computed_values_map.and_then(|map| map.get(name)) {
substituted.push_variable(value); substituted.push_variable(value);
Ok(value.last_token_type) Ok(value.last_token_type)
} else { } else {

37
components/style/properties/declaration_block.rs
View file

@ -152,7 +152,7 @@ pub struct AnimationValueIterator<'a, 'cx, 'cx_a:'cx> {
context: &'cx mut Context<'cx_a>, context: &'cx mut Context<'cx_a>,
default_values: &'a ComputedValues, default_values: &'a ComputedValues,
/// Custom properties in a keyframe if exists. /// Custom properties in a keyframe if exists.
extra_custom_properties: &'a Option<Arc<::custom_properties::CustomPropertiesMap>>, extra_custom_properties: Option<&'a Arc<::custom_properties::CustomPropertiesMap>>,
} }
impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> { impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> {
@ -160,7 +160,7 @@ impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> {
declarations: &'a PropertyDeclarationBlock, declarations: &'a PropertyDeclarationBlock,
context: &'cx mut Context<'cx_a>, context: &'cx mut Context<'cx_a>,
default_values: &'a ComputedValues, default_values: &'a ComputedValues,
extra_custom_properties: &'a Option<Arc<::custom_properties::CustomPropertiesMap>>, extra_custom_properties: Option<&'a Arc<::custom_properties::CustomPropertiesMap>>,
) -> AnimationValueIterator<'a, 'cx, 'cx_a> { ) -> AnimationValueIterator<'a, 'cx, 'cx_a> {
AnimationValueIterator { AnimationValueIterator {
iter: declarations.declaration_importance_iter(), iter: declarations.declaration_importance_iter(),
@ -259,7 +259,7 @@ impl PropertyDeclarationBlock {
&'a self, &'a self,
context: &'cx mut Context<'cx_a>, context: &'cx mut Context<'cx_a>,
default_values: &'a ComputedValues, default_values: &'a ComputedValues,
extra_custom_properties: &'a Option<Arc<::custom_properties::CustomPropertiesMap>>, extra_custom_properties: Option<&'a Arc<::custom_properties::CustomPropertiesMap>>,
) -> AnimationValueIterator<'a, 'cx, 'cx_a> { ) -> AnimationValueIterator<'a, 'cx, 'cx_a> {
AnimationValueIterator::new(self, context, default_values, extra_custom_properties) AnimationValueIterator::new(self, context, default_values, extra_custom_properties)
} }
@ -592,12 +592,14 @@ impl PropertyDeclarationBlock {
if self.declarations.len() == 1 { if self.declarations.len() == 1 {
let declaration = &self.declarations[0]; let declaration = &self.declarations[0];
let custom_properties = if let Some(cv) = computed_values { let custom_properties = if let Some(cv) = computed_values {
// If there are extra custom properties for this declaration block, // If there are extra custom properties for this
// factor them in too. // declaration block, factor them in too.
if let Some(block) = custom_properties_block { if let Some(block) = custom_properties_block {
// FIXME(emilio): This is not super-efficient
// here...
block.cascade_custom_properties(cv.custom_properties()) block.cascade_custom_properties(cv.custom_properties())
} else { } else {
cv.custom_properties() cv.custom_properties().cloned()
} }
} else { } else {
None None
@ -610,13 +612,14 @@ impl PropertyDeclarationBlock {
// |computed_values| is supplied, we use it to expand such variable // |computed_values| is supplied, we use it to expand such variable
// declarations. This will be fixed properly in Gecko bug 1391537. // declarations. This will be fixed properly in Gecko bug 1391537.
(&PropertyDeclaration::WithVariables(id, ref unparsed), (&PropertyDeclaration::WithVariables(id, ref unparsed),
Some(ref _computed_values)) => unparsed Some(ref _computed_values)) => {
.substitute_variables( unparsed.substitute_variables(
id, id,
&custom_properties, custom_properties.as_ref(),
QuirksMode::NoQuirks, QuirksMode::NoQuirks,
) )
.to_css(dest), .to_css(dest)
},
(ref d, _) => d.to_css(dest), (ref d, _) => d.to_css(dest),
} }
} else { } else {
@ -690,7 +693,7 @@ impl PropertyDeclarationBlock {
/// properties. /// properties.
pub fn cascade_custom_properties( pub fn cascade_custom_properties(
&self, &self,
inherited_custom_properties: Option<Arc<::custom_properties::CustomPropertiesMap>>, inherited_custom_properties: Option<&Arc<::custom_properties::CustomPropertiesMap>>,
) -> Option<Arc<::custom_properties::CustomPropertiesMap>> { ) -> Option<Arc<::custom_properties::CustomPropertiesMap>> {
let mut custom_properties = None; let mut custom_properties = None;
let mut seen_custom = PrecomputedHashSet::default(); let mut seen_custom = PrecomputedHashSet::default();
@ -698,12 +701,18 @@ impl PropertyDeclarationBlock {
for declaration in self.normal_declaration_iter() { for declaration in self.normal_declaration_iter() {
if let PropertyDeclaration::Custom(ref name, ref value) = *declaration { if let PropertyDeclaration::Custom(ref name, ref value) = *declaration {
::custom_properties::cascade( ::custom_properties::cascade(
&mut custom_properties, &inherited_custom_properties, &mut custom_properties,
&mut seen_custom, name, value.borrow()); inherited_custom_properties,
&mut seen_custom,
name,
value.borrow(),
);
} }
} }
::custom_properties::finish_cascade( ::custom_properties::finish_cascade(
custom_properties, &inherited_custom_properties) custom_properties,
inherited_custom_properties,
)
} }
} }

9
components/style/properties/gecko.mako.rs
View file

@ -322,15 +322,6 @@ impl ComputedValuesInner {
self.visited_style.as_ref().map(|x| x.clone_arc()) self.visited_style.as_ref().map(|x| x.clone_arc())
} }
/// Gets a reference to the custom properties map (if one exists).
pub fn get_custom_properties(&self) -> Option<<&::custom_properties::CustomPropertiesMap> {
self.custom_properties.as_ref().map(|x| &**x)
}
pub fn custom_properties(&self) -> Option<Arc<CustomPropertiesMap>> {
self.custom_properties.clone()
}
#[allow(non_snake_case)] #[allow(non_snake_case)]
pub fn has_moz_binding(&self) -> bool { pub fn has_moz_binding(&self) -> bool {
!self.get_box().gecko.mBinding.mPtr.mRawPtr.is_null() !self.get_box().gecko.mBinding.mPtr.mRawPtr.is_null()

22
components/style/properties/helpers/animated_properties.mako.rs
View file

@ -402,7 +402,7 @@ impl AnimationValue {
pub fn from_declaration( pub fn from_declaration(
decl: &PropertyDeclaration, decl: &PropertyDeclaration,
context: &mut Context, context: &mut Context,
extra_custom_properties: &Option<Arc<::custom_properties::CustomPropertiesMap>>, extra_custom_properties: Option<<&Arc<::custom_properties::CustomPropertiesMap>>,
initial: &ComputedValues initial: &ComputedValues
) -> Option<Self> { ) -> Option<Self> {
use properties::LonghandId; use properties::LonghandId;
@ -476,14 +476,22 @@ impl AnimationValue {
} }
}, },
PropertyDeclaration::WithVariables(id, ref unparsed) => { PropertyDeclaration::WithVariables(id, ref unparsed) => {
let substituted = if extra_custom_properties.is_some() { let substituted = {
let custom_properties =
extra_custom_properties.or_else(|| context.style().custom_properties());
unparsed.substitute_variables( unparsed.substitute_variables(
id, &extra_custom_properties, context.quirks_mode) id,
} else { custom_properties,
unparsed.substitute_variables( context.quirks_mode
id, &context.style().custom_properties(), context.quirks_mode) )
}; };
AnimationValue::from_declaration(&substituted, context, extra_custom_properties, initial) AnimationValue::from_declaration(
&substituted,
context,
extra_custom_properties,
initial,
)
}, },
_ => None // non animatable properties will get included because of shorthands. ignore. _ => None // non animatable properties will get included because of shorthands. ignore.
} }

64
components/style/properties/properties.mako.rs
View file

@ -938,19 +938,23 @@ pub struct UnparsedValue {
} }
impl UnparsedValue { impl UnparsedValue {
fn substitute_variables(&self, longhand_id: LonghandId, fn substitute_variables(
custom_properties: &Option<Arc<::custom_properties::CustomPropertiesMap>>, &self,
quirks_mode: QuirksMode) longhand_id: LonghandId,
-> PropertyDeclaration { custom_properties: Option<<&Arc<::custom_properties::CustomPropertiesMap>>,
quirks_mode: QuirksMode,
) -> PropertyDeclaration {
::custom_properties::substitute(&self.css, self.first_token_type, custom_properties) ::custom_properties::substitute(&self.css, self.first_token_type, custom_properties)
.ok() .ok()
.and_then(|css| { .and_then(|css| {
// As of this writing, only the base URL is used for property values: // As of this writing, only the base URL is used for property values:
let context = ParserContext::new(Origin::Author, let context = ParserContext::new(
&self.url_data, Origin::Author,
None, &self.url_data,
PARSING_MODE_DEFAULT, None,
quirks_mode); PARSING_MODE_DEFAULT,
quirks_mode,
);
let mut input = ParserInput::new(&css); let mut input = ParserInput::new(&css);
Parser::new(&mut input).parse_entirely(|input| { Parser::new(&mut input).parse_entirely(|input| {
match self.from_shorthand { match self.from_shorthand {
@ -2097,6 +2101,11 @@ impl ComputedValues {
self.flags.contains(IS_IN_DISPLAY_NONE_SUBTREE) self.flags.contains(IS_IN_DISPLAY_NONE_SUBTREE)
} }
/// Gets a reference to the custom properties map (if one exists).
pub fn custom_properties(&self) -> Option<<&Arc<::custom_properties::CustomPropertiesMap>> {
self.custom_properties.as_ref()
}
} }
#[cfg(feature = "servo")] #[cfg(feature = "servo")]
@ -2228,21 +2237,6 @@ impl ComputedValuesInner {
self.visited_style.clone() self.visited_style.clone()
} }
// Aah! The << in the return type below is not valid syntax, but we must
// escape < that way for Mako.
/// Gets a reference to the custom properties map (if one exists).
pub fn get_custom_properties(&self) -> Option<<&::custom_properties::CustomPropertiesMap> {
self.custom_properties.as_ref().map(|x| &**x)
}
/// Get the custom properties map if necessary.
///
/// Cloning the Arc here is fine because it only happens in the case where
/// we have custom properties, and those are both rare and expensive.
pub fn custom_properties(&self) -> Option<Arc<::custom_properties::CustomPropertiesMap>> {
self.custom_properties.clone()
}
/// Whether this style has a -moz-binding value. This is always false for /// Whether this style has a -moz-binding value. This is always false for
/// Servo for obvious reasons. /// Servo for obvious reasons.
pub fn has_moz_binding(&self) -> bool { false } pub fn has_moz_binding(&self) -> bool { false }
@ -2769,7 +2763,7 @@ impl<'a> StyleBuilder<'a> {
pseudo, pseudo,
modified_reset: false, modified_reset: false,
rules: None, // FIXME(emilio): Dubious... rules: None, // FIXME(emilio): Dubious...
custom_properties: style_to_derive_from.custom_properties(), custom_properties: style_to_derive_from.custom_properties().cloned(),
writing_mode: style_to_derive_from.writing_mode, writing_mode: style_to_derive_from.writing_mode,
flags: style_to_derive_from.flags, flags: style_to_derive_from.flags,
visited_style: style_to_derive_from.clone_visited_style(), visited_style: style_to_derive_from.clone_visited_style(),
@ -2888,7 +2882,7 @@ impl<'a> StyleBuilder<'a> {
pseudo, pseudo,
CascadeFlags::empty(), CascadeFlags::empty(),
/* rules = */ None, /* rules = */ None,
parent.custom_properties(), parent.custom_properties().cloned(),
parent.writing_mode, parent.writing_mode,
parent.flags, parent.flags,
parent.clone_visited_style() parent.clone_visited_style()
@ -3009,8 +3003,8 @@ impl<'a> StyleBuilder<'a> {
/// ///
/// Cloning the Arc here is fine because it only happens in the case where /// Cloning the Arc here is fine because it only happens in the case where
/// we have custom properties, and those are both rare and expensive. /// we have custom properties, and those are both rare and expensive.
fn custom_properties(&self) -> Option<Arc<::custom_properties::CustomPropertiesMap>> { fn custom_properties(&self) -> Option<<&Arc<::custom_properties::CustomPropertiesMap>> {
self.custom_properties.clone() self.custom_properties.as_ref()
} }
/// Access to various information about our inherited styles. We don't /// Access to various information about our inherited styles. We don't
@ -3276,14 +3270,20 @@ where
for (declaration, _cascade_level) in iter_declarations() { for (declaration, _cascade_level) in iter_declarations() {
if let PropertyDeclaration::Custom(ref name, ref value) = *declaration { if let PropertyDeclaration::Custom(ref name, ref value) = *declaration {
::custom_properties::cascade( ::custom_properties::cascade(
&mut custom_properties, &inherited_custom_properties, &mut custom_properties,
&mut seen_custom, name, value.borrow()); inherited_custom_properties,
&mut seen_custom,
name,
value.borrow(),
);
} }
} }
let custom_properties = let custom_properties =
::custom_properties::finish_cascade( ::custom_properties::finish_cascade(
custom_properties, &inherited_custom_properties); custom_properties,
inherited_custom_properties,
);
let mut context = computed::Context { let mut context = computed::Context {
is_root_element: flags.contains(IS_ROOT_ELEMENT), is_root_element: flags.contains(IS_ROOT_ELEMENT),
@ -3348,7 +3348,7 @@ where
} }
Cow::Owned(unparsed.substitute_variables( Cow::Owned(unparsed.substitute_variables(
id, id,
&context.builder.custom_properties, context.builder.custom_properties.as_ref(),
context.quirks_mode context.quirks_mode
)) ))
} }

2
components/style/servo/restyle_damage.rs
View file

@ -281,7 +281,7 @@ fn compute_damage(old: &ComputedValues, new: &ComputedValues) -> ServoRestyleDam
// Paint worklets may depend on custom properties, // Paint worklets may depend on custom properties,
// so if they have changed we should repaint. // so if they have changed we should repaint.
if old.get_custom_properties() != new.get_custom_properties() { if old.custom_properties() != new.custom_properties() {
damage.insert(REPAINT); damage.insert(REPAINT);
} }

10
ports/geckolib/glue.rs
View file

@ -2044,7 +2044,7 @@ pub extern "C" fn Servo_ComputedValues_EqualCustomProperties(
first: ServoComputedDataBorrowed, first: ServoComputedDataBorrowed,
second: ServoComputedDataBorrowed second: ServoComputedDataBorrowed
) -> bool { ) -> bool {
first.get_custom_properties() == second.get_custom_properties() first.custom_properties == second.custom_properties
} }
#[no_mangle] #[no_mangle]
@ -3436,7 +3436,7 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeLis
let iter = guard.to_animation_value_iter( let iter = guard.to_animation_value_iter(
&mut context, &mut context,
&default_values, &default_values,
&custom_properties, custom_properties.as_ref(),
); );
for value in iter { for value in iter {
@ -3479,11 +3479,10 @@ pub extern "C" fn Servo_GetAnimationValues(declarations: RawServoDeclarationBloc
let declarations = Locked::<PropertyDeclarationBlock>::as_arc(&declarations); let declarations = Locked::<PropertyDeclarationBlock>::as_arc(&declarations);
let guard = declarations.read_with(&guard); let guard = declarations.read_with(&guard);
let no_extra_custom_properties = None; // SMIL has no extra custom properties.
let iter = guard.to_animation_value_iter( let iter = guard.to_animation_value_iter(
&mut context, &mut context,
&default_values, &default_values,
&no_extra_custom_properties, None, // SMIL has no extra custom properties.
); );
for (index, anim) in iter.enumerate() { for (index, anim) in iter.enumerate() {
unsafe { animation_values.set_len((index + 1) as u32) }; unsafe { animation_values.set_len((index + 1) as u32) };
@ -3524,11 +3523,10 @@ pub extern "C" fn Servo_AnimationValue_Compute(element: RawGeckoElementBorrowed,
// We only compute the first element in declarations. // We only compute the first element in declarations.
match declarations.read_with(&guard).declaration_importance_iter().next() { match declarations.read_with(&guard).declaration_importance_iter().next() {
Some((decl, imp)) if imp == Importance::Normal => { Some((decl, imp)) if imp == Importance::Normal => {
let no_extra_custom_properties = None; // No extra custom properties for devtools.
let animation = AnimationValue::from_declaration( let animation = AnimationValue::from_declaration(
decl, decl,
&mut context, &mut context,
&no_extra_custom_properties, None, // No extra custom properties for devtools.
default_values, default_values,
); );
animation.map_or(RawServoAnimationValueStrong::null(), |value| { animation.map_or(RawServoAnimationValueStrong::null(), |value| {