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

Make all PropertyDeclaration variants have only one field

Browse source
This commit is contained in:
Anthony Ramine 2018-02-05 02:22:04 +01:00
parent 3918c983e1
commit 5195eb38f2
6 changed files with 138 additions and 81 deletions
Download patch file Download diff file Expand all files Collapse all files

2
components/style/properties/data.py
View file

@ -149,6 +149,7 @@ class Longhand(object):
predefined_type=None, servo_pref=None, gecko_pref=None, predefined_type=None, servo_pref=None, gecko_pref=None,
enabled_in="content", need_index=False, enabled_in="content", need_index=False,
gecko_ffi_name=None, gecko_ffi_name=None,
is_gecko_size_type_hack=False,
allowed_in_keyframe_block=True, cast_type='u8', allowed_in_keyframe_block=True, cast_type='u8',
logical=False, alias=None, extra_prefixes=None, boxed=False, logical=False, alias=None, extra_prefixes=None, boxed=False,
flags=None, allowed_in_page_rule=False, allow_quirks=False, ignored_when_colors_disabled=False, flags=None, allowed_in_page_rule=False, allow_quirks=False, ignored_when_colors_disabled=False,
@ -185,6 +186,7 @@ class Longhand(object):
self.allow_quirks = allow_quirks self.allow_quirks = allow_quirks
self.ignored_when_colors_disabled = ignored_when_colors_disabled self.ignored_when_colors_disabled = ignored_when_colors_disabled
self.is_vector = vector self.is_vector = vector
self.is_gecko_size_type_hack = is_gecko_size_type_hack
# https://drafts.csswg.org/css-animations/#keyframes # https://drafts.csswg.org/css-animations/#keyframes
# > The <declaration-list> inside of <keyframe-block> accepts any CSS property # > The <declaration-list> inside of <keyframe-block> accepts any CSS property

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

@ -430,7 +430,9 @@ impl PropertyDeclarationBlock {
AllShorthand::NotSet => {} AllShorthand::NotSet => {}
AllShorthand::CSSWideKeyword(keyword) => { AllShorthand::CSSWideKeyword(keyword) => {
for &id in ShorthandId::All.longhands() { for &id in ShorthandId::All.longhands() {
let decl = PropertyDeclaration::CSSWideKeyword(id, keyword); let decl = PropertyDeclaration::CSSWideKeyword(
WideKeywordDeclaration { id, keyword },
);
changed |= self.push( changed |= self.push(
decl, decl,
importance, importance,
@ -440,7 +442,9 @@ impl PropertyDeclarationBlock {
} }
AllShorthand::WithVariables(unparsed) => { AllShorthand::WithVariables(unparsed) => {
for &id in ShorthandId::All.longhands() { for &id in ShorthandId::All.longhands() {
let decl = PropertyDeclaration::WithVariables(id, unparsed.clone()); let decl = PropertyDeclaration::WithVariables(
VariableDeclaration { id, value: unparsed.clone() },
);
changed |= self.push( changed |= self.push(
decl, decl,
importance, importance,
@ -639,14 +643,15 @@ impl PropertyDeclarationBlock {
// in Gecko's getKeyframes() implementation for CSS animations, if // in Gecko's getKeyframes() implementation for CSS animations, if
// |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), (
Some(ref _computed_values)) => { &PropertyDeclaration::WithVariables(ref declaration),
unparsed.substitute_variables( Some(ref _computed_values),
id, ) => {
declaration.value.substitute_variables(
declaration.id,
custom_properties.as_ref(), 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),
} }
@ -726,8 +731,8 @@ impl PropertyDeclarationBlock {
let mut builder = CustomPropertiesBuilder::new(inherited_custom_properties); let mut builder = CustomPropertiesBuilder::new(inherited_custom_properties);
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 declaration) = *declaration {
builder.cascade(name, value.borrow()); builder.cascade(&declaration.name, declaration.value.borrow());
} }
} }

7
components/style/properties/helpers.mako.rs
View file

@ -302,9 +302,9 @@
PropertyDeclaration::${property.camel_case}(ref value) => { PropertyDeclaration::${property.camel_case}(ref value) => {
DeclaredValue::Value(value) DeclaredValue::Value(value)
}, },
PropertyDeclaration::CSSWideKeyword(id, value) => { PropertyDeclaration::CSSWideKeyword(ref declaration) => {
debug_assert!(id == LonghandId::${property.camel_case}); debug_assert!(declaration.id == LonghandId::${property.camel_case});
DeclaredValue::CSSWideKeyword(value) DeclaredValue::CSSWideKeyword(declaration.keyword)
}, },
PropertyDeclaration::WithVariables(..) => { PropertyDeclaration::WithVariables(..) => {
panic!("variables should already have been substituted") panic!("variables should already have been substituted")
@ -903,6 +903,7 @@
<%call expr="longhand(name, <%call expr="longhand(name,
predefined_type=length_type, predefined_type=length_type,
logical=logical, logical=logical,
is_gecko_size_type_hack=True,
**kwargs)"> **kwargs)">
% if not logical: % if not logical:
use values::specified::AllowQuirks; use values::specified::AllowQuirks;

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

@ -435,14 +435,14 @@ impl AnimationValue {
}, },
% endif % endif
% endfor % endfor
PropertyDeclaration::CSSWideKeyword(id, keyword) => { PropertyDeclaration::CSSWideKeyword(ref declaration) => {
match id { match declaration.id {
// We put all the animatable properties first in the hopes // We put all the animatable properties first in the hopes
// that it might increase match locality. // that it might increase match locality.
% for prop in data.longhands: % for prop in data.longhands:
% if prop.animatable: % if prop.animatable:
LonghandId::${prop.camel_case} => { LonghandId::${prop.camel_case} => {
let style_struct = match keyword { let style_struct = match declaration.keyword {
% if not prop.style_struct.inherited: % if not prop.style_struct.inherited:
CSSWideKeyword::Unset | CSSWideKeyword::Unset |
% endif % endif
@ -472,15 +472,15 @@ impl AnimationValue {
% endfor % endfor
} }
}, },
PropertyDeclaration::WithVariables(id, ref unparsed) => { PropertyDeclaration::WithVariables(ref declaration) => {
let substituted = { let substituted = {
let custom_properties = let custom_properties =
extra_custom_properties.or_else(|| context.style().custom_properties()); extra_custom_properties.or_else(|| context.style().custom_properties());
unparsed.substitute_variables( declaration.value.substitute_variables(
id, declaration.id,
custom_properties, custom_properties,
context.quirks_mode context.quirks_mode,
) )
}; };
return AnimationValue::from_declaration( return AnimationValue::from_declaration(

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

@ -1074,7 +1074,10 @@ impl UnparsedValue {
} else { } else {
CSSWideKeyword::Initial CSSWideKeyword::Initial
}; };
PropertyDeclaration::CSSWideKeyword(longhand_id, keyword) PropertyDeclaration::CSSWideKeyword(WideKeywordDeclaration {
id: longhand_id,
keyword,
})
}) })
} }
} }
@ -1443,33 +1446,51 @@ impl PropertyId {
#[derive(Clone, PartialEq)] #[derive(Clone, PartialEq)]
pub enum PropertyDeclaration { pub enum PropertyDeclaration {
% for property in data.longhands: % for property in data.longhands:
/// ${property.name}
<% <%
if property.predefined_type and not property.is_vector: if property.predefined_type and not property.is_vector and not property.is_gecko_size_type_hack:
ty = "::values::specified::{}".format(property.predefined_type) ty = "::values::specified::{}".format(property.predefined_type)
else: else:
ty = "longhands::{}::SpecifiedValue".format(property.ident) ty = "longhands::{}::SpecifiedValue".format(property.ident)
if property.boxed: if property.boxed:
ty = "Box<{}>".format(ty) ty = "Box<{}>".format(ty)
%> %>
/// ${property.name}
${property.camel_case}(${ty}), ${property.camel_case}(${ty}),
% endfor % endfor
/// A css-wide keyword. /// A css-wide keyword.
CSSWideKeyword(LonghandId, CSSWideKeyword), CSSWideKeyword(WideKeywordDeclaration),
/// An unparsed value that contains `var()` functions. /// An unparsed declaration.
WithVariables( WithVariables(VariableDeclaration),
LonghandId, /// A custom property declaration.
#[cfg_attr(feature = "gecko", ignore_malloc_size_of = "XXX: how to handle this?")] Custom(CustomDeclaration),
Arc<UnparsedValue> }
),
/// A custom property declaration, with the property name and the declared /// A declaration using a CSS-wide keyword.
/// value. #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
#[derive(Clone, PartialEq)]
pub struct WideKeywordDeclaration {
id: LonghandId,
keyword: CSSWideKeyword,
}
/// An unparsed declaration that contains `var()` functions.
#[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
#[derive(Clone, PartialEq)]
pub struct VariableDeclaration {
id: LonghandId,
#[cfg_attr(feature = "gecko", ignore_malloc_size_of = "XXX: how to handle this?")] #[cfg_attr(feature = "gecko", ignore_malloc_size_of = "XXX: how to handle this?")]
Custom( value: Arc<UnparsedValue>,
::custom_properties::Name, }
#[cfg_attr(feature = "gecko", ignore_malloc_size_of = "XXX: how to handle this?")]
DeclaredValueOwned<Arc<::custom_properties::SpecifiedValue>> /// A custom property declaration with the property name and the declared value.
), #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
#[derive(Clone, PartialEq)]
pub struct CustomDeclaration {
/// The name of the custom property.
pub name: ::custom_properties::Name,
/// The value of the custom property.
#[cfg_attr(feature = "gecko", ignore_malloc_size_of = "XXX: how to handle this?")]
pub value: DeclaredValueOwned<Arc<::custom_properties::SpecifiedValue>>,
} }
impl fmt::Debug for PropertyDeclaration { impl fmt::Debug for PropertyDeclaration {
@ -1497,24 +1518,24 @@ impl PropertyDeclaration {
value.to_css(&mut CssWriter::new(dest)) value.to_css(&mut CssWriter::new(dest))
} }
% endfor % endfor
PropertyDeclaration::CSSWideKeyword(_, keyword) => { PropertyDeclaration::CSSWideKeyword(ref declaration) => {
keyword.to_css(&mut CssWriter::new(dest)) declaration.keyword.to_css(&mut CssWriter::new(dest))
}, },
PropertyDeclaration::WithVariables(_, ref with_variables) => { PropertyDeclaration::WithVariables(ref declaration) => {
// https://drafts.csswg.org/css-variables/#variables-in-shorthands // https://drafts.csswg.org/css-variables/#variables-in-shorthands
match with_variables.from_shorthand { match declaration.value.from_shorthand {
// Normally, we shouldn't be printing variables here if they came from // Normally, we shouldn't be printing variables here if they came from
// shorthands. But we should allow properties that came from shorthand // shorthands. But we should allow properties that came from shorthand
// aliases. That also matches with the Gecko behavior. // aliases. That also matches with the Gecko behavior.
Some(shorthand) if shorthand.flags().contains(PropertyFlags::SHORTHAND_ALIAS_PROPERTY) => Some(shorthand) if shorthand.flags().contains(PropertyFlags::SHORTHAND_ALIAS_PROPERTY) =>
dest.write_str(&*with_variables.css)?, dest.write_str(&*declaration.value.css)?,
None => dest.write_str(&*with_variables.css)?, None => dest.write_str(&*declaration.value.css)?,
_ => {}, _ => {},
} }
Ok(()) Ok(())
}, },
PropertyDeclaration::Custom(_, ref value) => { PropertyDeclaration::Custom(ref declaration) => {
value.borrow().to_css(&mut CssWriter::new(dest)) declaration.value.borrow().to_css(&mut CssWriter::new(dest))
}, },
} }
} }
@ -1524,12 +1545,14 @@ impl PropertyDeclaration {
/// Given a property declaration, return the property declaration id. /// Given a property declaration, return the property declaration id.
pub fn id(&self) -> PropertyDeclarationId { pub fn id(&self) -> PropertyDeclarationId {
match *self { match *self {
PropertyDeclaration::Custom(ref name, _) => { PropertyDeclaration::Custom(ref declaration) => {
return PropertyDeclarationId::Custom(name) return PropertyDeclarationId::Custom(&declaration.name)
} }
PropertyDeclaration::CSSWideKeyword(id, _) | PropertyDeclaration::CSSWideKeyword(ref declaration) => {
PropertyDeclaration::WithVariables(id, _) => { return PropertyDeclarationId::Longhand(declaration.id);
return PropertyDeclarationId::Longhand(id) }
PropertyDeclaration::WithVariables(ref declaration) => {
return PropertyDeclarationId::Longhand(declaration.id);
} }
_ => {} _ => {}
} }
@ -1554,10 +1577,10 @@ impl PropertyDeclaration {
fn with_variables_from_shorthand(&self, shorthand: ShorthandId) -> Option< &str> { fn with_variables_from_shorthand(&self, shorthand: ShorthandId) -> Option< &str> {
match *self { match *self {
PropertyDeclaration::WithVariables(_, ref with_variables) => { PropertyDeclaration::WithVariables(ref declaration) => {
if let Some(s) = with_variables.from_shorthand { if let Some(s) = declaration.value.from_shorthand {
if s == shorthand { if s == shorthand {
Some(&*with_variables.css) Some(&*declaration.value.css)
} else { None } } else { None }
} else { } else {
// Normally, longhand property that doesn't come from a shorthand // Normally, longhand property that doesn't come from a shorthand
@ -1565,7 +1588,7 @@ impl PropertyDeclaration {
// came from a shorthand alias. Because for example, we should be able to // came from a shorthand alias. Because for example, we should be able to
// get -moz-transform's value from transform. // get -moz-transform's value from transform.
if shorthand.flags().contains(PropertyFlags::SHORTHAND_ALIAS_PROPERTY) { if shorthand.flags().contains(PropertyFlags::SHORTHAND_ALIAS_PROPERTY) {
return Some(&*with_variables.css); return Some(&*declaration.value.css);
} }
None None
} }
@ -1577,7 +1600,9 @@ impl PropertyDeclaration {
/// Returns a CSS-wide keyword if the declaration's value is one. /// Returns a CSS-wide keyword if the declaration's value is one.
pub fn get_css_wide_keyword(&self) -> Option<CSSWideKeyword> { pub fn get_css_wide_keyword(&self) -> Option<CSSWideKeyword> {
match *self { match *self {
PropertyDeclaration::CSSWideKeyword(_, keyword) => Some(keyword), PropertyDeclaration::CSSWideKeyword(ref declaration) => {
Some(declaration.keyword)
},
_ => None, _ => None,
} }
} }
@ -1640,8 +1665,8 @@ impl PropertyDeclaration {
pub fn value_is_unparsed(&self) -> bool { pub fn value_is_unparsed(&self) -> bool {
match *self { match *self {
PropertyDeclaration::WithVariables(..) => true, PropertyDeclaration::WithVariables(..) => true,
PropertyDeclaration::Custom(_, ref value) => { PropertyDeclaration::Custom(ref declaration) => {
!matches!(value.borrow(), DeclaredValue::CSSWideKeyword(..)) !matches!(declaration.value.borrow(), DeclaredValue::CSSWideKeyword(..))
} }
_ => false, _ => false,
} }
@ -1667,7 +1692,7 @@ impl PropertyDeclaration {
/// Returns true if this property is a custom property, false /// Returns true if this property is a custom property, false
/// otherwise. /// otherwise.
pub fn is_custom(&self) -> bool { pub fn is_custom(&self) -> bool {
matches!(*self, PropertyDeclaration::Custom(_, _)) matches!(*self, PropertyDeclaration::Custom(..))
} }
/// The `context` parameter controls this: /// The `context` parameter controls this:
@ -1708,14 +1733,19 @@ impl PropertyDeclaration {
Err(e) => return Err(StyleParseErrorKind::new_invalid(name, e)), Err(e) => return Err(StyleParseErrorKind::new_invalid(name, e)),
} }
}; };
declarations.push(PropertyDeclaration::Custom(property_name, value)); declarations.push(PropertyDeclaration::Custom(CustomDeclaration {
name: property_name,
value,
}));
Ok(()) Ok(())
} }
PropertyId::LonghandAlias(id, _) | PropertyId::LonghandAlias(id, _) |
PropertyId::Longhand(id) => { PropertyId::Longhand(id) => {
input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less. input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less.
input.try(|i| CSSWideKeyword::parse(i)).map(|keyword| { input.try(|i| CSSWideKeyword::parse(i)).map(|keyword| {
PropertyDeclaration::CSSWideKeyword(id, keyword) PropertyDeclaration::CSSWideKeyword(
WideKeywordDeclaration { id, keyword },
)
}).or_else(|()| { }).or_else(|()| {
input.look_for_var_functions(); input.look_for_var_functions();
input.parse_entirely(|input| id.parse_value(context, input)) input.parse_entirely(|input| id.parse_value(context, input))
@ -1727,12 +1757,15 @@ impl PropertyDeclaration {
::custom_properties::parse_non_custom_with_var(input).map_err(|e| { ::custom_properties::parse_non_custom_with_var(input).map_err(|e| {
StyleParseErrorKind::new_invalid(name, e) StyleParseErrorKind::new_invalid(name, e)
})?; })?;
Ok(PropertyDeclaration::WithVariables(id, Arc::new(UnparsedValue { Ok(PropertyDeclaration::WithVariables(VariableDeclaration {
id,
value: Arc::new(UnparsedValue {
css: css.into_owned(), css: css.into_owned(),
first_token_type: first_token_type, first_token_type: first_token_type,
url_data: context.url_data.clone(), url_data: context.url_data.clone(),
from_shorthand: None, from_shorthand: None,
}))) }),
}))
} else { } else {
Err(StyleParseErrorKind::new_invalid(name, err)) Err(StyleParseErrorKind::new_invalid(name, err))
} }
@ -1749,7 +1782,12 @@ impl PropertyDeclaration {
declarations.all_shorthand = AllShorthand::CSSWideKeyword(keyword) declarations.all_shorthand = AllShorthand::CSSWideKeyword(keyword)
} else { } else {
for &longhand in id.longhands() { for &longhand in id.longhands() {
declarations.push(PropertyDeclaration::CSSWideKeyword(longhand, keyword)) declarations.push(PropertyDeclaration::CSSWideKeyword(
WideKeywordDeclaration {
id: longhand,
keyword,
},
))
} }
} }
Ok(()) Ok(())
@ -1774,9 +1812,12 @@ impl PropertyDeclaration {
if id == ShorthandId::All { if id == ShorthandId::All {
declarations.all_shorthand = AllShorthand::WithVariables(unparsed) declarations.all_shorthand = AllShorthand::WithVariables(unparsed)
} else { } else {
for &longhand in id.longhands() { for &id in id.longhands() {
declarations.push( declarations.push(
PropertyDeclaration::WithVariables(longhand, unparsed.clone()) PropertyDeclaration::WithVariables(VariableDeclaration {
id,
value: unparsed.clone(),
})
) )
} }
} }
@ -3283,8 +3324,8 @@ where
CustomPropertiesBuilder::new(inherited_style.custom_properties()); CustomPropertiesBuilder::new(inherited_style.custom_properties());
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 declaration) = *declaration {
builder.cascade(name, value.borrow()); builder.cascade(&declaration.name, declaration.value.borrow());
} }
} }
@ -3372,13 +3413,13 @@ where
} }
let mut declaration = match *declaration { let mut declaration = match *declaration {
PropertyDeclaration::WithVariables(id, ref unparsed) => { PropertyDeclaration::WithVariables(ref declaration) => {
if !id.inherited() { if !declaration.id.inherited() {
context.rule_cache_conditions.borrow_mut() context.rule_cache_conditions.borrow_mut()
.set_uncacheable(); .set_uncacheable();
} }
Cow::Owned(unparsed.substitute_variables( Cow::Owned(declaration.value.substitute_variables(
id, declaration.id,
context.builder.custom_properties.as_ref(), context.builder.custom_properties.as_ref(),
context.quirks_mode context.quirks_mode
)) ))
@ -3537,8 +3578,10 @@ where
seen.contains(LonghandId::MozMinFontSizeRatio) || seen.contains(LonghandId::MozMinFontSizeRatio) ||
font_family.is_some() { font_family.is_some() {
let discriminant = LonghandId::FontSize as usize; let discriminant = LonghandId::FontSize as usize;
let size = PropertyDeclaration::CSSWideKeyword( let size = PropertyDeclaration::CSSWideKeyword(WideKeywordDeclaration {
LonghandId::FontSize, CSSWideKeyword::Inherit); id: LonghandId::FontSize,
keyword: CSSWideKeyword::Inherit,
});
(CASCADE_PROPERTY[discriminant])(&size, &mut context); (CASCADE_PROPERTY[discriminant])(&size, &mut context);
% endif % endif

22
tests/unit/style/stylesheets.rs
View file

@ -18,9 +18,9 @@ use std::sync::atomic::AtomicBool;
use style::context::QuirksMode; use style::context::QuirksMode;
use style::error_reporting::{ParseErrorReporter, ContextualParseError}; use style::error_reporting::{ParseErrorReporter, ContextualParseError};
use style::media_queries::MediaList; use style::media_queries::MediaList;
use style::properties::Importance; use style::properties::{CSSWideKeyword, CustomDeclaration, DeclarationSource};
use style::properties::{CSSWideKeyword, DeclaredValueOwned, PropertyDeclaration, PropertyDeclarationBlock}; use style::properties::{DeclaredValueOwned, Importance};
use style::properties::DeclarationSource; use style::properties::{PropertyDeclaration, PropertyDeclarationBlock};
use style::properties::longhands::{self, animation_timing_function}; use style::properties::longhands::{self, animation_timing_function};
use style::shared_lock::SharedRwLock; use style::shared_lock::SharedRwLock;
use style::stylesheets::{Origin, Namespaces}; use style::stylesheets::{Origin, Namespaces};
@ -108,11 +108,17 @@ fn test_parse_stylesheet() {
), (0 << 20) + (1 << 10) + (1 << 0)) ), (0 << 20) + (1 << 10) + (1 << 0))
)), )),
block: Arc::new(stylesheet.shared_lock.wrap(block_from(vec![ block: Arc::new(stylesheet.shared_lock.wrap(block_from(vec![
(PropertyDeclaration::Display(longhands::display::SpecifiedValue::None), (
Importance::Important), PropertyDeclaration::Display(longhands::display::SpecifiedValue::None),
(PropertyDeclaration::Custom(Atom::from("a"), Importance::Important,
DeclaredValueOwned::CSSWideKeyword(CSSWideKeyword::Inherit)), ),
Importance::Important), (
PropertyDeclaration::Custom(CustomDeclaration {
name: Atom::from("a"),
value: DeclaredValueOwned::CSSWideKeyword(CSSWideKeyword::Inherit),
}),
Importance::Important,
),
]))), ]))),
source_location: SourceLocation { source_location: SourceLocation {
line: 3, line: 3,