style: Make the transition-property code make more sense.

We were working around the lack of alias support during parsing in
TransitionProperty by doing a Gecko lookup. That's a hack and is now gone.

Bug: 1419695
Reviewed-by: xidorn
MozReview-Commit-ID: EptUvJNTrZr
This commit is contained in:
Emilio Cobos Álvarez 2018-06-01 16:34:08 +02:00
parent 35a1a30f6b
commit 6940787916
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C
6 changed files with 86 additions and 43 deletions

View file

@ -303,7 +303,8 @@ impl PropertyAnimation {
let duration = box_style.transition_duration_mod(transition_index);
match transition_property {
TransitionProperty::Unsupported(_) => result,
TransitionProperty::Custom(..) |
TransitionProperty::Unsupported(..) => result,
TransitionProperty::Shorthand(ref shorthand_id) => shorthand_id
.longhands()
.filter_map(|longhand| {

View file

@ -1629,6 +1629,7 @@ impl<'le> TElement for GeckoElement<'le> {
};
match transition_property {
TransitionProperty::Custom(..) |
TransitionProperty::Unsupported(..) => {},
TransitionProperty::Shorthand(ref shorthand) => {
if shorthand.longhands().any(property_check_helper) {

View file

@ -28,7 +28,6 @@ use gecko_bindings::bindings::Gecko_CopyListStyleImageFrom;
use gecko_bindings::bindings::Gecko_EnsureImageLayersLength;
use gecko_bindings::bindings::Gecko_SetCursorArrayLength;
use gecko_bindings::bindings::Gecko_SetCursorImageValue;
use gecko_bindings::bindings::Gecko_StyleTransition_SetUnsupportedProperty;
use gecko_bindings::bindings::Gecko_NewCSSShadowArray;
use gecko_bindings::bindings::Gecko_nsStyleFont_SetLang;
use gecko_bindings::bindings::Gecko_nsStyleFont_CopyLangFrom;
@ -3267,6 +3266,8 @@ fn static_assert() {
I::IntoIter: ExactSizeIterator
{
use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_no_properties;
use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_variable;
use gecko_bindings::structs::nsCSSPropertyID::eCSSProperty_UNKNOWN;
let v = v.into_iter();
@ -3274,10 +3275,20 @@ fn static_assert() {
self.gecko.mTransitions.ensure_len(v.len());
self.gecko.mTransitionPropertyCount = v.len() as u32;
for (servo, gecko) in v.zip(self.gecko.mTransitions.iter_mut()) {
if !gecko.mUnknownProperty.mRawPtr.is_null() {
unsafe { Atom::from_addrefed(gecko.mUnknownProperty.mRawPtr) };
gecko.mUnknownProperty.mRawPtr = ptr::null_mut();
}
match servo {
TransitionProperty::Unsupported(ref ident) => unsafe {
Gecko_StyleTransition_SetUnsupportedProperty(gecko, ident.0.as_ptr())
TransitionProperty::Unsupported(ident) => {
gecko.mProperty = eCSSProperty_UNKNOWN;
gecko.mUnknownProperty.mRawPtr = ident.0.into_addrefed();
},
TransitionProperty::Custom(name) => {
gecko.mProperty = eCSSPropertyExtra_variable;
gecko.mUnknownProperty.mRawPtr = name.into_addrefed();
}
_ => gecko.mProperty = servo.to_nscsspropertyid().unwrap(),
}
}
@ -3307,15 +3318,24 @@ fn static_assert() {
use gecko_bindings::structs::nsCSSPropertyID::eCSSProperty_UNKNOWN;
let property = self.gecko.mTransitions[index].mProperty;
if property == eCSSProperty_UNKNOWN || property == eCSSPropertyExtra_variable {
if property == eCSSProperty_UNKNOWN {
let atom = self.gecko.mTransitions[index].mUnknownProperty.mRawPtr;
debug_assert!(!atom.is_null());
TransitionProperty::Unsupported(CustomIdent(unsafe{
Atom::from_raw(atom)
}))
} else if property == eCSSPropertyExtra_variable {
let atom = self.gecko.mTransitions[index].mUnknownProperty.mRawPtr;
debug_assert!(!atom.is_null());
TransitionProperty::Custom(unsafe{
Atom::from_raw(atom)
})
} else if property == eCSSPropertyExtra_no_properties {
// Actually, we don't expect TransitionProperty::Unsupported also represents "none",
// but if the caller wants to convert it, it is fine. Please use it carefully.
// Actually, we don't expect TransitionProperty::Unsupported also
// represents "none", but if the caller wants to convert it, it is
// fine. Please use it carefully.
//
// FIXME(emilio): This is a hack, is this reachable?
TransitionProperty::Unsupported(CustomIdent(atom!("none")))
} else {
property.into()
@ -3336,11 +3356,15 @@ fn static_assert() {
for (index, transition) in self.gecko.mTransitions.iter_mut().enumerate().take(count as usize) {
transition.mProperty = other.gecko.mTransitions[index].mProperty;
if !transition.mUnknownProperty.mRawPtr.is_null() {
unsafe { Atom::from_addrefed(transition.mUnknownProperty.mRawPtr) };
transition.mUnknownProperty.mRawPtr = ptr::null_mut();
}
if transition.mProperty == eCSSProperty_UNKNOWN ||
transition.mProperty == eCSSPropertyExtra_variable {
let atom = other.gecko.mTransitions[index].mUnknownProperty.mRawPtr;
debug_assert!(!atom.is_null());
unsafe { Gecko_StyleTransition_SetUnsupportedProperty(transition, atom) };
transition.mUnknownProperty.mRawPtr = unsafe { Atom::from_raw(atom) }.into_addrefed();
}
}
}

View file

@ -9,6 +9,7 @@
from itertools import groupby
%>
use Atom;
use cssparser::Parser;
#[cfg(feature = "gecko")] use gecko_bindings::bindings::RawServoAnimationValueMap;
#[cfg(feature = "gecko")] use gecko_bindings::structs::RawGeckoGfxMatrix4x4;
@ -16,7 +17,8 @@ use cssparser::Parser;
#[cfg(feature = "gecko")] use gecko_bindings::sugar::ownership::{HasFFI, HasSimpleFFI};
use itertools::{EitherOrBoth, Itertools};
use num_traits::Zero;
use properties::{CSSWideKeyword, PropertyDeclaration};
use parser::ParserContext;
use properties::{CSSWideKeyword, PropertyDeclaration, PropertyDeclarationId};
use properties::longhands;
use properties::longhands::font_weight::computed_value::T as FontWeight;
use properties::longhands::visibility::computed_value::T as Visibility;
@ -25,9 +27,10 @@ use properties::{LonghandId, ShorthandId};
use servo_arc::Arc;
use smallvec::SmallVec;
use std::{cmp, ptr};
use std::fmt::{self, Write};
use std::mem::{self, ManuallyDrop};
#[cfg(feature = "gecko")] use hash::FnvHashMap;
use style_traits::{KeywordsCollectFn, ParseError, SpecifiedValueInfo};
use style_traits::{KeywordsCollectFn, ParseError, SpecifiedValueInfo, ToCss, CssWriter};
use super::ComputedValues;
use values::{CSSFloat, CustomIdent};
use values::animated::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero};
@ -74,17 +77,37 @@ pub fn nscsspropertyid_is_animatable(property: nsCSSPropertyID) -> bool {
/// a shorthand with at least one transitionable longhand component, or an unsupported property.
// NB: This needs to be here because it needs all the longhands generated
// beforehand.
#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue, ToCss)]
#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue)]
pub enum TransitionProperty {
/// A shorthand.
Shorthand(ShorthandId),
/// A longhand transitionable property.
Longhand(LonghandId),
/// A custom property.
Custom(Atom),
/// Unrecognized property which could be any non-transitionable, custom property, or
/// unknown property.
Unsupported(CustomIdent),
}
impl ToCss for TransitionProperty {
fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
where
W: Write,
{
use values::serialize_atom_identifier;
match *self {
TransitionProperty::Shorthand(ref s) => s.to_css(dest),
TransitionProperty::Longhand(ref l) => l.to_css(dest),
TransitionProperty::Custom(ref name) => {
dest.write_str("--")?;
serialize_atom_identifier(name, dest)
}
TransitionProperty::Unsupported(ref i) => i.to_css(dest),
}
}
}
impl TransitionProperty {
/// Returns `all`.
#[inline]
@ -93,38 +116,30 @@ impl TransitionProperty {
}
/// Parse a transition-property value.
pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
// FIXME(https://github.com/rust-lang/rust/issues/33156): remove this
// enum and use PropertyId when stable Rust allows destructors in
// statics.
//
// FIXME: This should handle aliases too.
pub enum StaticId {
Longhand(LonghandId),
Shorthand(ShorthandId),
}
ascii_case_insensitive_phf_map! {
static_id -> StaticId = {
% for prop in data.shorthands:
"${prop.name}" => StaticId::Shorthand(ShorthandId::${prop.camel_case}),
% endfor
% for prop in data.longhands:
"${prop.name}" => StaticId::Longhand(LonghandId::${prop.camel_case}),
% endfor
}
}
pub fn parse<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
let location = input.current_source_location();
let ident = input.expect_ident()?;
Ok(match static_id(&ident) {
Some(&StaticId::Longhand(id)) => TransitionProperty::Longhand(id),
Some(&StaticId::Shorthand(id)) => TransitionProperty::Shorthand(id),
None => {
TransitionProperty::Unsupported(
CustomIdent::from_ident(location, ident, &["none"])?,
)
},
let id = match PropertyId::parse(&ident, context) {
Ok(id) => id,
Err(..) => return Ok(TransitionProperty::Unsupported(
CustomIdent::from_ident(location, ident, &["none"])?,
)),
};
Ok(match id.as_shorthand() {
Ok(s) => TransitionProperty::Shorthand(s),
Err(longhand_or_custom) => {
match longhand_or_custom {
PropertyDeclarationId::Longhand(id) => TransitionProperty::Longhand(id),
PropertyDeclarationId::Custom(custom) => {
TransitionProperty::Custom(custom.clone())
}
}
}
})
}
@ -137,6 +152,7 @@ impl TransitionProperty {
}
TransitionProperty::Shorthand(ref id) => id.to_nscsspropertyid(),
TransitionProperty::Longhand(ref id) => id.to_nscsspropertyid(),
TransitionProperty::Custom(..) |
TransitionProperty::Unsupported(..) => return Err(()),
})
}

View file

@ -257,7 +257,6 @@ ${helpers.predefined_type(
vector=True,
allow_empty="NotInitial",
need_index=True,
needs_context=False,
animation_value_type="none",
extra_prefixes=transition_extra_prefixes,
spec="https://drafts.csswg.org/css-transitions/#propdef-transition-property",

View file

@ -158,10 +158,12 @@ macro_rules! try_parse_one {
// Must check 'transition-property' after 'transition-timing-function' since
// 'transition-property' accepts any keyword.
if property.is_none() {
if let Ok(value) = input.try(|i| transition_property::SingleSpecifiedValue::parse(i)) {
if let Ok(value) = input.try(|i| transition_property::SingleSpecifiedValue::parse(context, i)) {
property = Some(Some(value));
continue;
} else if input.try(|i| i.expect_ident_matching("none")).is_ok() {
}
if input.try(|i| i.expect_ident_matching("none")).is_ok() {
// 'none' is not a valid value for <single-transition-property>,
// so it's not acceptable in the function above.
property = Some(None);