style: Only expose longhands to rust via iterators.

The key here is that we only filter longhands if the shorthand is accessible to
content and vice-versa. This prevents the bug that prevented me to land this
patch before, which was us not expanding properly chrome-only shorthands.

Again, this is incomplete, and I need to teach LonghandsToSerialize to get a
potentially incomplete list of properties, and all that.
This commit is contained in:
Emilio Cobos Álvarez 2018-02-20 12:00:31 +01:00
parent 6fbf2c1e3c
commit 8c4b401dd3
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C
8 changed files with 95 additions and 59 deletions

View file

@ -285,9 +285,9 @@ impl PropertyAnimation {
match transition_property { match transition_property {
TransitionProperty::Unsupported(_) => result, TransitionProperty::Unsupported(_) => result,
TransitionProperty::Shorthand(ref shorthand_id) => { TransitionProperty::Shorthand(ref shorthand_id) => {
shorthand_id.longhands().iter().filter_map(|longhand| { shorthand_id.longhands().filter_map(|longhand| {
PropertyAnimation::from_longhand( PropertyAnimation::from_longhand(
&longhand, longhand,
timing_function, timing_function,
duration, duration,
old_style, old_style,
@ -295,7 +295,7 @@ impl PropertyAnimation {
) )
}).collect() }).collect()
} }
TransitionProperty::Longhand(ref longhand_id) => { TransitionProperty::Longhand(longhand_id) => {
let animation = PropertyAnimation::from_longhand( let animation = PropertyAnimation::from_longhand(
longhand_id, longhand_id,
timing_function, timing_function,
@ -313,7 +313,7 @@ impl PropertyAnimation {
} }
fn from_longhand( fn from_longhand(
longhand: &LonghandId, longhand: LonghandId,
timing_function: TimingFunction, timing_function: TimingFunction,
duration: Time, duration: Time,
old_style: &ComputedValues, old_style: &ComputedValues,
@ -751,7 +751,7 @@ where
debug!("update_style_for_animation: scanning prop {:?} for animation \"{}\"", debug!("update_style_for_animation: scanning prop {:?} for animation \"{}\"",
property, name); property, name);
let animation = PropertyAnimation::from_longhand( let animation = PropertyAnimation::from_longhand(
&property, property,
timing_function, timing_function,
Time::from_seconds(relative_duration as f32), Time::from_seconds(relative_duration as f32),
&from_style, &from_style,

View file

@ -773,7 +773,7 @@ impl<'le> GeckoElement<'le> {
fn needs_transitions_update_per_property( fn needs_transitions_update_per_property(
&self, &self,
longhand_id: &LonghandId, longhand_id: LonghandId,
combined_duration: f32, combined_duration: f32,
before_change_style: &ComputedValues, before_change_style: &ComputedValues,
after_change_style: &ComputedValues, after_change_style: &ComputedValues,
@ -787,7 +787,7 @@ impl<'le> GeckoElement<'le> {
// If the end value has not changed, we should leave the currently // If the end value has not changed, we should leave the currently
// running transition as-is since we don't want to interrupt its timing // running transition as-is since we don't want to interrupt its timing
// function. // function.
if let Some(ref existing) = existing_transitions.get(longhand_id) { if let Some(ref existing) = existing_transitions.get(&longhand_id) {
let after_value = let after_value =
AnimationValue::from_computed_values( AnimationValue::from_computed_values(
longhand_id, longhand_id,
@ -798,11 +798,11 @@ impl<'le> GeckoElement<'le> {
} }
let from = AnimationValue::from_computed_values( let from = AnimationValue::from_computed_values(
&longhand_id, longhand_id,
before_change_style, before_change_style,
); );
let to = AnimationValue::from_computed_values( let to = AnimationValue::from_computed_values(
&longhand_id, longhand_id,
after_change_style, after_change_style,
); );
@ -1531,8 +1531,8 @@ impl<'le> TElement for GeckoElement<'le> {
let transition_property: TransitionProperty = property.into(); let transition_property: TransitionProperty = property.into();
let mut property_check_helper = |property: &LonghandId| -> bool { let mut property_check_helper = |property: LonghandId| -> bool {
transitions_to_keep.insert(*property); transitions_to_keep.insert(property);
self.needs_transitions_update_per_property( self.needs_transitions_update_per_property(
property, property,
combined_duration, combined_duration,
@ -1545,11 +1545,11 @@ impl<'le> TElement for GeckoElement<'le> {
match transition_property { match transition_property {
TransitionProperty::Unsupported(..) => {}, TransitionProperty::Unsupported(..) => {},
TransitionProperty::Shorthand(ref shorthand) => { TransitionProperty::Shorthand(ref shorthand) => {
if shorthand.longhands().iter().any(property_check_helper) { if shorthand.longhands().any(property_check_helper) {
return true; return true;
} }
}, },
TransitionProperty::Longhand(ref longhand_id) => { TransitionProperty::Longhand(longhand_id) => {
if property_check_helper(longhand_id) { if property_check_helper(longhand_id) {
return true; return true;
} }

View file

@ -333,7 +333,7 @@ impl PropertyDeclarationBlock {
let mut important_count = 0; let mut important_count = 0;
// Step 1.2.2 // Step 1.2.2
for &longhand in shorthand.longhands() { for longhand in shorthand.longhands() {
// Step 1.2.2.1 // Step 1.2.2.1
let declaration = self.get(PropertyDeclarationId::Longhand(longhand)); let declaration = self.get(PropertyDeclarationId::Longhand(longhand));
@ -395,7 +395,7 @@ impl PropertyDeclarationBlock {
match property.as_shorthand() { match property.as_shorthand() {
Ok(shorthand) => { Ok(shorthand) => {
// Step 2.1 & 2.2 & 2.3 // Step 2.1 & 2.2 & 2.3
if shorthand.longhands().iter().all(|&l| { if shorthand.longhands().all(|l| {
self.get(PropertyDeclarationId::Longhand(l)) self.get(PropertyDeclarationId::Longhand(l))
.map_or(false, |(_, importance)| importance.important()) .map_or(false, |(_, importance)| importance.important())
}) { }) {
@ -455,7 +455,7 @@ impl PropertyDeclarationBlock {
match drain.all_shorthand { match drain.all_shorthand {
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( let decl = PropertyDeclaration::CSSWideKeyword(
WideKeywordDeclaration { id, keyword }, WideKeywordDeclaration { id, keyword },
); );
@ -467,7 +467,7 @@ impl PropertyDeclarationBlock {
} }
} }
AllShorthand::WithVariables(unparsed) => { AllShorthand::WithVariables(unparsed) => {
for &id in ShorthandId::All.longhands() { for id in ShorthandId::All.longhands() {
let decl = PropertyDeclaration::WithVariables( let decl = PropertyDeclaration::WithVariables(
VariableDeclaration { id, value: unparsed.clone() }, VariableDeclaration { id, value: unparsed.clone() },
); );
@ -809,7 +809,7 @@ impl PropertyDeclarationBlock {
// iterating below. // iterating below.
// Step 3.3.2 // Step 3.3.2
for &shorthand in longhand_id.shorthands() { for shorthand in longhand_id.shorthands() {
// We already attempted to serialize this shorthand before. // We already attempted to serialize this shorthand before.
if already_serialized.contains(shorthand.into()) { if already_serialized.contains(shorthand.into()) {
continue; continue;
@ -853,7 +853,7 @@ impl PropertyDeclarationBlock {
} }
} else { } else {
let mut contains_all_longhands = true; let mut contains_all_longhands = true;
for &longhand in shorthand.longhands() { for longhand in shorthand.longhands() {
match self.get(PropertyDeclarationId::Longhand(longhand)) { match self.get(PropertyDeclarationId::Longhand(longhand)) {
Some((declaration, importance)) => { Some((declaration, importance)) => {
current_longhands.push(declaration); current_longhands.push(declaration);

View file

@ -3472,7 +3472,7 @@ fn static_assert() {
use properties::PropertyId; use properties::PropertyId;
use properties::longhands::will_change::computed_value::T; use properties::longhands::will_change::computed_value::T;
fn will_change_bitfield_from_prop_flags(prop: &LonghandId) -> u8 { fn will_change_bitfield_from_prop_flags(prop: LonghandId) -> u8 {
use properties::PropertyFlags; use properties::PropertyFlags;
use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_ABSPOS_CB; use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_ABSPOS_CB;
use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_FIXPOS_CB; use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_FIXPOS_CB;
@ -3526,7 +3526,7 @@ fn static_assert() {
if let PropertyDeclarationId::Longhand(longhand) if let PropertyDeclarationId::Longhand(longhand)
= longhand_or_custom { = longhand_or_custom {
self.gecko.mWillChangeBitField |= self.gecko.mWillChangeBitField |=
will_change_bitfield_from_prop_flags(&longhand); will_change_bitfield_from_prop_flags(longhand);
} }
}, },
} }

View file

@ -35,9 +35,10 @@
% endif % endif
#[allow(unused_variables)] #[allow(unused_variables)]
#[inline] #[inline]
pub fn parse<'i, 't>(context: &ParserContext, pub fn parse<'i, 't>(
input: &mut Parser<'i, 't>) context: &ParserContext,
-> Result<SpecifiedValue, ParseError<'i>> { input: &mut Parser<'i, 't>,
) -> Result<SpecifiedValue, ParseError<'i>> {
% if allow_quirks: % if allow_quirks:
specified::${type}::${parse_method}_quirky(context, input, AllowQuirks::Yes) specified::${type}::${parse_method}_quirky(context, input, AllowQuirks::Yes)
% elif needs_context: % elif needs_context:
@ -695,7 +696,8 @@
/// Tries to get a serializable set of longhands given a set of /// Tries to get a serializable set of longhands given a set of
/// property declarations. /// property declarations.
pub fn from_iter<I>(iter: I) -> Result<Self, ()> pub fn from_iter<I>(iter: I) -> Result<Self, ()>
where I: Iterator<Item=&'a PropertyDeclaration>, where
I: Iterator<Item=&'a PropertyDeclaration>,
{ {
// Define all of the expected variables that correspond to the shorthand // Define all of the expected variables that correspond to the shorthand
% for sub_property in shorthand.sub_properties: % for sub_property in shorthand.sub_properties:
@ -738,9 +740,11 @@
/// Parse the given shorthand and fill the result into the /// Parse the given shorthand and fill the result into the
/// `declarations` vector. /// `declarations` vector.
pub fn parse_into<'i, 't>(declarations: &mut SourcePropertyDeclaration, pub fn parse_into<'i, 't>(
context: &ParserContext, input: &mut Parser<'i, 't>) declarations: &mut SourcePropertyDeclaration,
-> Result<(), ParseError<'i>> { context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<(), ParseError<'i>> {
input.parse_entirely(|input| parse_value(context, input)).map(|longhands| { input.parse_entirely(|input| parse_value(context, input)).map(|longhands| {
% for sub_property in shorthand.sub_properties: % for sub_property in shorthand.sub_properties:
declarations.push(PropertyDeclaration::${sub_property.camel_case}( declarations.push(PropertyDeclaration::${sub_property.camel_case}(

View file

@ -21,7 +21,6 @@ use properties::longhands;
use properties::longhands::font_weight::computed_value::T as FontWeight; use properties::longhands::font_weight::computed_value::T as FontWeight;
use properties::longhands::font_stretch::computed_value::T as FontStretch; use properties::longhands::font_stretch::computed_value::T as FontStretch;
use properties::longhands::visibility::computed_value::T as Visibility; use properties::longhands::visibility::computed_value::T as Visibility;
#[cfg(feature = "gecko")]
use properties::PropertyId; use properties::PropertyId;
use properties::{LonghandId, ShorthandId}; use properties::{LonghandId, ShorthandId};
use servo_arc::Arc; use servo_arc::Arc;
@ -114,10 +113,10 @@ impl TransitionProperty {
} }
/// Iterates over each longhand property. /// Iterates over each longhand property.
pub fn each<F: FnMut(&LonghandId) -> ()>(mut cb: F) { pub fn each<F: FnMut(LonghandId) -> ()>(mut cb: F) {
% for prop in data.longhands: % for prop in data.longhands:
% if prop.transitionable: % if prop.transitionable:
cb(&LonghandId::${prop.camel_case}); cb(LonghandId::${prop.camel_case});
% endif % endif
% endfor % endfor
} }
@ -300,11 +299,11 @@ impl AnimatedProperty {
/// Get an animatable value from a transition-property, an old style, and a /// Get an animatable value from a transition-property, an old style, and a
/// new style. /// new style.
pub fn from_longhand( pub fn from_longhand(
property: &LonghandId, property: LonghandId,
old_style: &ComputedValues, old_style: &ComputedValues,
new_style: &ComputedValues, new_style: &ComputedValues,
) -> Option<AnimatedProperty> { ) -> Option<AnimatedProperty> {
Some(match *property { Some(match property {
% for prop in data.longhands: % for prop in data.longhands:
% if prop.animatable: % if prop.animatable:
LonghandId::${prop.camel_case} => { LonghandId::${prop.camel_case} => {
@ -646,10 +645,10 @@ impl AnimationValue {
/// Get an AnimationValue for an AnimatableLonghand from a given computed values. /// Get an AnimationValue for an AnimatableLonghand from a given computed values.
pub fn from_computed_values( pub fn from_computed_values(
property: &LonghandId, property: LonghandId,
computed_values: &ComputedValues computed_values: &ComputedValues
) -> Option<Self> { ) -> Option<Self> {
Some(match *property { Some(match property {
% for prop in data.longhands: % for prop in data.longhands:
% if prop.animatable: % if prop.animatable:
LonghandId::${prop.camel_case} => { LonghandId::${prop.camel_case} => {
@ -3088,15 +3087,17 @@ impl ComputeSquaredDistance for AnimatedFilterList {
/// border-top-color, border-color, border-top, border /// border-top-color, border-color, border-top, border
/// ///
/// [property-order] https://drafts.csswg.org/web-animations/#calculating-computed-keyframes /// [property-order] https://drafts.csswg.org/web-animations/#calculating-computed-keyframes
#[cfg(feature = "gecko")]
pub fn compare_property_priority(a: &PropertyId, b: &PropertyId) -> cmp::Ordering { pub fn compare_property_priority(a: &PropertyId, b: &PropertyId) -> cmp::Ordering {
match (a.as_shorthand(), b.as_shorthand()) { match (a.as_shorthand(), b.as_shorthand()) {
// Within shorthands, sort by the number of subproperties, then by IDL name. // Within shorthands, sort by the number of subproperties, then by IDL name.
(Ok(a), Ok(b)) => { (Ok(a), Ok(b)) => {
let subprop_count_a = a.longhands().len(); let subprop_count_a = a.longhands().count();
let subprop_count_b = b.longhands().len(); let subprop_count_b = b.longhands().count();
subprop_count_a.cmp(&subprop_count_b).then_with( subprop_count_a
|| get_idl_name_sort_order(&a).cmp(&get_idl_name_sort_order(&b))) .cmp(&subprop_count_b)
.then_with(|| {
get_idl_name_sort_order(a).cmp(&get_idl_name_sort_order(b))
})
}, },
// Longhands go before shorthands. // Longhands go before shorthands.
@ -3109,8 +3110,7 @@ pub fn compare_property_priority(a: &PropertyId, b: &PropertyId) -> cmp::Orderin
} }
} }
#[cfg(feature = "gecko")] fn get_idl_name_sort_order(shorthand: ShorthandId) -> u32 {
fn get_idl_name_sort_order(shorthand: &ShorthandId) -> u32 {
<% <%
# Sort by IDL name. # Sort by IDL name.
sorted_shorthands = sorted(data.shorthands, key=lambda p: to_idl_name(p.ident)) sorted_shorthands = sorted(data.shorthands, key=lambda p: to_idl_name(p.ident))
@ -3118,7 +3118,7 @@ sorted_shorthands = sorted(data.shorthands, key=lambda p: to_idl_name(p.ident))
# Annotate with sorted position # Annotate with sorted position
sorted_shorthands = [(p, position) for position, p in enumerate(sorted_shorthands)] sorted_shorthands = [(p, position) for position, p in enumerate(sorted_shorthands)]
%> %>
match *shorthand { match shorthand {
% for property, position in sorted_shorthands: % for property, position in sorted_shorthands:
ShorthandId::${property.camel_case} => ${position}, ShorthandId::${property.camel_case} => ${position},
% endfor % endfor

View file

@ -821,7 +821,7 @@ impl LonghandId {
INHERITED.contains(*self) INHERITED.contains(*self)
} }
fn shorthands(&self) -> &'static [ShorthandId] { fn shorthands(&self) -> NonCustomPropertyIterator<ShorthandId> {
// first generate longhand to shorthands lookup map // first generate longhand to shorthands lookup map
// //
// NOTE(emilio): This currently doesn't exclude the "all" shorthand. It // NOTE(emilio): This currently doesn't exclude the "all" shorthand. It
@ -862,15 +862,21 @@ impl LonghandId {
]; ];
% endfor % endfor
match *self { NonCustomPropertyIterator {
filter: NonCustomPropertyId::from(*self).enabled_for_all_content(),
iter: match *self {
% for property in data.longhands: % for property in data.longhands:
LonghandId::${property.camel_case} => ${property.ident.upper()}, LonghandId::${property.camel_case} => ${property.ident.upper()},
% endfor % endfor
}.iter(),
} }
} }
fn parse_value<'i, 't>(&self, context: &ParserContext, input: &mut Parser<'i, 't>) fn parse_value<'i, 't>(
-> Result<PropertyDeclaration, ParseError<'i>> { &self,
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<PropertyDeclaration, ParseError<'i>> {
match *self { match *self {
% for property in data.longhands: % for property in data.longhands:
LonghandId::${property.camel_case} => { LonghandId::${property.camel_case} => {
@ -1100,6 +1106,29 @@ impl LonghandId {
} }
} }
/// An iterator over all the property ids that are enabled for a given
/// shorthand, if that shorthand is enabled for all content too.
pub struct NonCustomPropertyIterator<Item: 'static> {
filter: bool,
iter: ::std::slice::Iter<'static, Item>,
}
impl<Item> Iterator for NonCustomPropertyIterator<Item>
where
Item: 'static + Copy + Into<NonCustomPropertyId>,
{
type Item = Item;
fn next(&mut self) -> Option<Self::Item> {
loop {
let id = *self.iter.next()?;
if !self.filter || id.into().enabled_for_all_content() {
return Some(id)
}
}
}
}
/// An identifier for a given shorthand property. /// An identifier for a given shorthand property.
#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToCss)] #[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToCss)]
pub enum ShorthandId { pub enum ShorthandId {
@ -1127,7 +1156,7 @@ impl ShorthandId {
} }
/// Get the longhand ids that form this shorthand. /// Get the longhand ids that form this shorthand.
pub fn longhands(&self) -> &'static [LonghandId] { pub fn longhands(&self) -> NonCustomPropertyIterator<LonghandId> {
% for property in data.shorthands: % for property in data.shorthands:
static ${property.ident.upper()}: &'static [LonghandId] = &[ static ${property.ident.upper()}: &'static [LonghandId] = &[
% for sub in property.sub_properties: % for sub in property.sub_properties:
@ -1135,10 +1164,13 @@ impl ShorthandId {
% endfor % endfor
]; ];
% endfor % endfor
match *self { NonCustomPropertyIterator {
filter: NonCustomPropertyId::from(*self).enabled_for_all_content(),
iter: match *self {
% for property in data.shorthands: % for property in data.shorthands:
ShorthandId::${property.camel_case} => ${property.ident.upper()}, ShorthandId::${property.camel_case} => ${property.ident.upper()},
% endfor % endfor
}.iter()
} }
} }
@ -1441,7 +1473,7 @@ impl<'a> PropertyDeclarationId<'a> {
/// shorthand. /// shorthand.
pub fn is_longhand_of(&self, shorthand: ShorthandId) -> bool { pub fn is_longhand_of(&self, shorthand: ShorthandId) -> bool {
match *self { match *self {
PropertyDeclarationId::Longhand(ref id) => id.shorthands().contains(&shorthand), PropertyDeclarationId::Longhand(ref id) => id.shorthands().any(|s| s == shorthand),
_ => false, _ => false,
} }
} }
@ -1952,7 +1984,7 @@ impl PropertyDeclaration {
if id == ShorthandId::All { if id == ShorthandId::All {
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( declarations.push(PropertyDeclaration::CSSWideKeyword(
WideKeywordDeclaration { WideKeywordDeclaration {
id: longhand, id: longhand,
@ -1983,7 +2015,7 @@ 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 &id in id.longhands() { for id in id.longhands() {
declarations.push( declarations.push(
PropertyDeclaration::WithVariables(VariableDeclaration { PropertyDeclaration::WithVariables(VariableDeclaration {
id, id,

View file

@ -927,7 +927,7 @@ pub extern "C" fn Servo_ComputedValues_ExtractAnimationValue(
Err(()) => return Strong::null(), Err(()) => return Strong::null(),
}; };
match AnimationValue::from_computed_values(&property, &computed_values) { match AnimationValue::from_computed_values(property, &computed_values) {
Some(v) => Arc::new(v).into_strong(), Some(v) => Arc::new(v).into_strong(),
None => Strong::null(), None => Strong::null(),
} }