diff --git a/dom/animation/Keyframe.h b/dom/animation/Keyframe.h index 7a9500250218..8a2fd7c2698f 100644 --- a/dom/animation/Keyframe.h +++ b/dom/animation/Keyframe.h @@ -24,6 +24,7 @@ struct StyleLockedDeclarationBlock; struct PropertyValuePair { explicit PropertyValuePair(const AnimatedPropertyID& aProperty) : mProperty(aProperty) {} + PropertyValuePair(const AnimatedPropertyID& aProperty, RefPtr&& aValue) : mProperty(aProperty), mServoDeclarationBlock(std::move(aValue)) { diff --git a/dom/animation/KeyframeEffect.cpp b/dom/animation/KeyframeEffect.cpp index ff7dd36e6891..29c390f88144 100644 --- a/dom/animation/KeyframeEffect.cpp +++ b/dom/animation/KeyframeEffect.cpp @@ -1253,8 +1253,8 @@ void KeyframeEffect::GetKeyframes(JSContext* aCx, nsTArray& aResult, // know what custom property values to provide. RefPtr computedStyle; if (isCSSAnimation) { - // The following will flush style but that's ok since if you update - // a variable's computed value, you expect to see that updated value in the + // The following will flush style but that's ok since if you update a + // variable's computed value, you expect to see that updated value in the // result of getKeyframes(). // // If we don't have a target, the following will return null. In that case @@ -1297,12 +1297,10 @@ void KeyframeEffect::GetKeyframes(JSContext* aCx, nsTArray& aResult, if (propertyValue.mServoDeclarationBlock) { Servo_DeclarationBlock_SerializeOneValue( propertyValue.mServoDeclarationBlock, &propertyValue.mProperty, - &stringValue, computedStyle, nullptr, rawData); - } else { - if (auto* value = mBaseValues.GetWeak(propertyValue.mProperty)) { - Servo_AnimationValue_Serialize(value, &propertyValue.mProperty, - rawData, &stringValue); - } + &stringValue, computedStyle, rawData); + } else if (auto* value = mBaseValues.GetWeak(propertyValue.mProperty)) { + Servo_AnimationValue_Serialize(value, &propertyValue.mProperty, rawData, + &stringValue); } // Basically, we need to do the mapping: @@ -1314,12 +1312,10 @@ void KeyframeEffect::GetKeyframes(JSContext* aCx, nsTArray& aResult, // https://drafts.csswg.org/web-animations/#property-name-conversion const char* name = nullptr; nsAutoCString customName; - nsAutoCString identifier; switch (propertyValue.mProperty.mID) { case nsCSSPropertyID::eCSSPropertyExtra_variable: customName.Append("--"); - propertyValue.mProperty.mCustomName->ToUTF8String(identifier); - customName.Append(identifier); + customName.Append(nsAtomCString(propertyValue.mProperty.mCustomName)); name = customName.get(); break; case nsCSSPropertyID::eCSSProperty_offset: diff --git a/dom/animation/KeyframeUtils.cpp b/dom/animation/KeyframeUtils.cpp index fc8eb0638ac1..329273fd2244 100644 --- a/dom/animation/KeyframeUtils.cpp +++ b/dom/animation/KeyframeUtils.cpp @@ -72,10 +72,8 @@ struct PropertyValuesPair { * BaseKeyframe or BasePropertyIndexedKeyframe object. */ struct AdditionalProperty { - AdditionalProperty() : mProperty(eCSSProperty_UNKNOWN), mJsidIndex() {} - AnimatedPropertyID mProperty; - size_t mJsidIndex; // Index into |ids| in GetPropertyValuesPairs. + size_t mJsidIndex = 0; // Index into |ids| in GetPropertyValuesPairs. struct PropertyComparator { bool Equals(const AdditionalProperty& aLhs, @@ -572,20 +570,16 @@ static bool GetPropertyValuesPairs(JSContext* aCx, propName, CSSEnabledState::ForAllContent); } - AnimatedPropertyID* property; - if (propertyID == eCSSPropertyExtra_variable) { - // TODO(zrhoffman, bug 1811897) Add test coverage for removing the `--` - // prefix here. - property = new AnimatedPropertyID( - NS_Atomize(Substring(propName, 2, propName.Length() - 2))); - } else { - property = new AnimatedPropertyID(propertyID); - } + // TODO(zrhoffman, bug 1811897) Add test coverage for removing the `--` + // prefix here. + AnimatedPropertyID property = + propertyID == eCSSPropertyExtra_variable + ? AnimatedPropertyID( + NS_Atomize(Substring(propName, 2, propName.Length() - 2))) + : AnimatedPropertyID(propertyID); - if (KeyframeUtils::IsAnimatableProperty(*property)) { - AdditionalProperty* p = properties.AppendElement(); - p->mProperty = *property; - p->mJsidIndex = i; + if (KeyframeUtils::IsAnimatableProperty(property)) { + properties.AppendElement(AdditionalProperty{std::move(property), i}); } } diff --git a/dom/base/nsDOMWindowUtils.cpp b/dom/base/nsDOMWindowUtils.cpp index 2f596db1fddd..0ec7ba24cf7d 100644 --- a/dom/base/nsDOMWindowUtils.cpp +++ b/dom/base/nsDOMWindowUtils.cpp @@ -3207,12 +3207,9 @@ nsDOMWindowUtils::GetUnanimatedComputedStyle(Element* aElement, nsCSSProps::IsShorthand(propertyID)) { return NS_ERROR_INVALID_ARG; } - AnimatedPropertyID* property; - if (propertyID == eCSSPropertyExtra_variable) { - property = new AnimatedPropertyID(NS_Atomize(aProperty)); - } else { - property = new AnimatedPropertyID(propertyID); - } + AnimatedPropertyID property = propertyID == eCSSPropertyExtra_variable + ? AnimatedPropertyID(NS_Atomize(aProperty)) + : AnimatedPropertyID(propertyID); switch (aFlushType) { case FLUSH_NONE: @@ -3244,7 +3241,7 @@ nsDOMWindowUtils::GetUnanimatedComputedStyle(Element* aElement, } RefPtr value = - Servo_ComputedValues_ExtractAnimationValue(computedStyle, property) + Servo_ComputedValues_ExtractAnimationValue(computedStyle, &property) .Consume(); if (!value) { return NS_ERROR_FAILURE; @@ -3253,7 +3250,7 @@ nsDOMWindowUtils::GetUnanimatedComputedStyle(Element* aElement, return NS_ERROR_FAILURE; } nsAutoCString result; - Servo_AnimationValue_Serialize(value, property, + Servo_AnimationValue_Serialize(value, &property, presShell->StyleSet()->RawData(), &result); CopyUTF8toUTF16(result, aResult); return NS_OK; diff --git a/servo/components/style/applicable_declarations.rs b/servo/components/style/applicable_declarations.rs index a0dbb60da8e1..96049b76e33c 100644 --- a/servo/components/style/applicable_declarations.rs +++ b/servo/components/style/applicable_declarations.rs @@ -125,6 +125,11 @@ impl CascadePriority { pub fn important(&self) -> Self { Self::new(self.cascade_level().important(), self.layer_order()) } + + /// The same tree, in author origin, at the root layer. + pub fn same_tree_author_normal_at_root_layer() -> Self { + Self::new(CascadeLevel::same_tree_author_normal(), LayerOrder::root()) + } } /// A property declaration together with its precedence among rules of equal diff --git a/servo/components/style/custom_properties.rs b/servo/components/style/custom_properties.rs index 2524d695e15e..360113ae33e5 100644 --- a/servo/components/style/custom_properties.rs +++ b/servo/components/style/custom_properties.rs @@ -863,36 +863,41 @@ pub struct CustomPropertiesBuilder<'a, 'b: 'a> { impl<'a, 'b: 'a> CustomPropertiesBuilder<'a, 'b> { /// Create a new builder, inheriting from a given custom properties map. - pub fn new(stylist: &'a Stylist, computed_context: &'a computed::Context<'b>) -> Self { - let is_root_element = computed_context.is_root_element(); - - let inherited = computed_context.inherited_custom_properties(); - let initial_values = stylist.get_custom_property_initial_values(); - - // Reuse flags from computing registered custom properties initial values, such as whether - // they depend on viewport units. - computed_context - .style() - .add_flags(stylist.get_custom_property_initial_values_flags()); - + /// + /// We expose this publicly mostly for @keyframe blocks. + pub fn new_with_properties(stylist: &'a Stylist, custom_properties: ComputedCustomProperties, computed_context: &'a computed::Context<'b>) -> Self { Self { seen: PrecomputedHashSet::default(), reverted: Default::default(), may_have_cycles: false, - custom_properties: ComputedCustomProperties { - inherited: if is_root_element { - debug_assert!(inherited.is_empty()); - initial_values.inherited.clone() - } else { - inherited.inherited.clone() - }, - non_inherited: initial_values.non_inherited.clone(), - }, + custom_properties, stylist, computed_context, } } + /// Create a new builder, inheriting from the right style given context. + pub fn new(stylist: &'a Stylist, context: &'a computed::Context<'b>) -> Self { + let is_root_element = context.is_root_element(); + + let inherited = context.inherited_custom_properties(); + let initial_values = stylist.get_custom_property_initial_values(); + let properties = ComputedCustomProperties { + inherited: if is_root_element { + debug_assert!(inherited.is_empty()); + initial_values.inherited.clone() + } else { + inherited.inherited.clone() + }, + non_inherited: initial_values.non_inherited.clone(), + }; + + // Reuse flags from computing registered custom properties initial values, such as + // whether they depend on viewport units. + context.style().add_flags(stylist.get_custom_property_initial_values_flags()); + Self::new_with_properties(stylist, properties, context) + } + /// Cascade a given custom property declaration. pub fn cascade(&mut self, declaration: &'a CustomDeclaration, priority: CascadePriority) { let CustomDeclaration { diff --git a/servo/components/style/properties/declaration_block.rs b/servo/components/style/properties/declaration_block.rs index f893f2b0fda5..a4122e6dcf1d 100644 --- a/servo/components/style/properties/declaration_block.rs +++ b/servo/components/style/properties/declaration_block.rs @@ -13,9 +13,8 @@ use super::generated::{ }; use super::property_declaration::PropertyDeclarationId; use super::LonghandIdSetIterator; -use crate::applicable_declarations::CascadePriority; use crate::context::QuirksMode; -use crate::custom_properties::{self, ComputedCustomProperties, CustomPropertiesBuilder}; +use crate::custom_properties; use crate::error_reporting::{ContextualParseError, ParseErrorReporter}; use crate::parser::ParserContext; use crate::properties::{ @@ -23,13 +22,12 @@ use crate::properties::{ StyleBuilder, }; use crate::rule_cache::RuleCacheConditions; -use crate::rule_tree::CascadeLevel; use crate::selector_map::PrecomputedHashSet; use crate::selector_parser::SelectorImpl; use crate::shared_lock::Locked; use crate::str::{CssString, CssStringWriter}; use crate::stylesheets::container_rule::ContainerSizeQuery; -use crate::stylesheets::{layer_rule::LayerOrder, CssRuleType, Origin, UrlExtraData}; +use crate::stylesheets::{CssRuleType, Origin, UrlExtraData}; use crate::stylist::Stylist; use crate::values::computed::Context; use cssparser::{ @@ -307,8 +305,6 @@ pub struct AnimationValueIterator<'a, 'cx, 'cx_a: 'cx> { iter: DeclarationImportanceIterator<'a>, context: &'cx mut Context<'cx_a>, default_values: &'a ComputedValues, - /// Custom properties in a keyframe if exists. - extra_custom_properties: Option<&'a crate::custom_properties::ComputedCustomProperties>, } impl<'a, 'cx, 'cx_a: 'cx> AnimationValueIterator<'a, 'cx, 'cx_a> { @@ -316,13 +312,11 @@ impl<'a, 'cx, 'cx_a: 'cx> AnimationValueIterator<'a, 'cx, 'cx_a> { declarations: &'a PropertyDeclarationBlock, context: &'cx mut Context<'cx_a>, default_values: &'a ComputedValues, - extra_custom_properties: Option<&'a crate::custom_properties::ComputedCustomProperties>, ) -> AnimationValueIterator<'a, 'cx, 'cx_a> { AnimationValueIterator { iter: declarations.declaration_importance_iter(), context, default_values, - extra_custom_properties, } } } @@ -341,7 +335,6 @@ impl<'a, 'cx, 'cx_a: 'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> { let animation = AnimationValue::from_declaration( decl, &mut self.context, - self.extra_custom_properties, self.default_values, ); @@ -428,9 +421,8 @@ impl PropertyDeclarationBlock { &'a self, context: &'cx mut Context<'cx_a>, default_values: &'a ComputedValues, - extra_custom_properties: Option<&'a crate::custom_properties::ComputedCustomProperties>, ) -> AnimationValueIterator<'a, 'cx, 'cx_a> { - AnimationValueIterator::new(self, context, default_values, extra_custom_properties) + AnimationValueIterator::new(self, context, default_values) } /// Returns whether this block contains any declaration with `!important`. @@ -919,7 +911,6 @@ impl PropertyDeclarationBlock { property: &PropertyId, dest: &mut CssStringWriter, computed_values: Option<&ComputedValues>, - custom_properties_block: Option<&PropertyDeclarationBlock>, stylist: &Stylist, ) -> fmt::Result { if let Ok(shorthand) = property.as_shorthand() { @@ -950,15 +941,6 @@ impl PropertyDeclarationBlock { if let Some(cv) = computed_values { context.builder.custom_properties = cv.custom_properties.clone(); - - // If there are extra custom properties for this declaration block, - // factor them in too. - if let Some(block) = custom_properties_block { - // FIXME(emilio): This is not super-efficient here, and all this - // feels like a hack anyway... - context.builder.custom_properties = - block.cascade_custom_properties(stylist, &context); - } }; match (declaration, computed_values) { @@ -1014,31 +996,6 @@ impl PropertyDeclarationBlock { }) } - /// Returns a custom properties map which is the result of cascading custom - /// properties in this declaration block along with context's custom - /// properties. - pub fn cascade_custom_properties( - &self, - stylist: &Stylist, - context: &Context, - ) -> ComputedCustomProperties { - let mut builder = CustomPropertiesBuilder::new(stylist, context); - - for declaration in self.normal_declaration_iter() { - if let PropertyDeclaration::Custom(ref declaration) = *declaration { - builder.cascade( - declaration, - CascadePriority::new( - CascadeLevel::same_tree_author_normal(), - LayerOrder::root(), - ), - ); - } - } - - builder.build() - } - /// Like the method on ToCss, but without the type parameter to avoid /// accidentally monomorphizing this large function multiple times for /// different writers. diff --git a/servo/components/style/properties/helpers/animated_properties.mako.rs b/servo/components/style/properties/helpers/animated_properties.mako.rs index 16bc0ceaab38..0f527f5552cb 100644 --- a/servo/components/style/properties/helpers/animated_properties.mako.rs +++ b/servo/components/style/properties/helpers/animated_properties.mako.rs @@ -275,7 +275,6 @@ impl AnimationValue { pub fn from_declaration( decl: &PropertyDeclaration, context: &mut Context, - extra_custom_properties: Option< &crate::custom_properties::ComputedCustomProperties>, initial: &ComputedValues, ) -> Option { use super::PropertyDeclarationVariantRepr; @@ -385,7 +384,7 @@ impl AnimationValue { PropertyDeclaration::WithVariables(ref declaration) => { let mut cache = Default::default(); let substituted = { - let custom_properties = extra_custom_properties.unwrap_or(&context.style().custom_properties()); + let custom_properties = &context.style().custom_properties(); debug_assert!( context.builder.stylist.is_some(), @@ -402,7 +401,6 @@ impl AnimationValue { return AnimationValue::from_declaration( &substituted, context, - extra_custom_properties, initial, ) }, diff --git a/servo/components/style/properties/properties.mako.rs b/servo/components/style/properties/properties.mako.rs index ce70ae1c7e41..98ae9e889e7b 100644 --- a/servo/components/style/properties/properties.mako.rs +++ b/servo/components/style/properties/properties.mako.rs @@ -1038,12 +1038,6 @@ impl LonghandIdSet { &RESET } - #[inline] - fn animatable() -> &'static Self { - ${static_longhand_id_set("ANIMATABLE", lambda p: p.animatable)} - &ANIMATABLE - } - #[inline] fn discrete_animatable() -> &'static Self { ${static_longhand_id_set("DISCRETE_ANIMATABLE", lambda p: p.animation_value_type == "discrete")} @@ -1448,7 +1442,7 @@ impl LonghandId { /// Returns whether this property is animatable. #[inline] pub fn is_animatable(self) -> bool { - LonghandIdSet::animatable().contains(self) + NonCustomPropertyId::from(self).is_animatable() } /// Returns whether this property is animatable in a discrete way. @@ -1986,6 +1980,22 @@ impl PropertyId { self.non_custom_non_alias_id()?.as_longhand() } + /// Returns true if this property is one of the animatable properties. + pub fn is_animatable(&self) -> bool { + match self { + Self::NonCustom(id) => id.is_animatable(), + Self::Custom(..) => true, + } + } + + /// Returns true if this property is one of the transitionable properties. + pub fn is_transitionable(&self) -> bool { + match self { + Self::NonCustom(id) => id.is_transitionable(), + Self::Custom(..) => true, + } + } + /// Returns a given property from the given name, _regardless of whether it /// is enabled or not_, or Err(()) for unknown properties. /// @@ -2092,18 +2102,13 @@ impl PropertyId { /// Returns a property id from Gecko's AnimatedPropertyID. #[cfg(feature = "gecko")] - #[allow(non_upper_case_globals)] #[inline] - pub fn from_gecko_animated_property_id(property: &structs::AnimatedPropertyID) -> Result { - Ok(if property.mID == nsCSSPropertyID::eCSSPropertyExtra_variable { - if property.mCustomName.mRawPtr.is_null() { - return Err(()); - } - PropertyId::Custom(unsafe { - Atom::from_raw(property.mCustomName.mRawPtr) - }) + pub fn from_gecko_animated_property_id(property: &structs::AnimatedPropertyID) -> Option { + Some(if property.mID == nsCSSPropertyID::eCSSPropertyExtra_variable { + debug_assert!(!property.mCustomName.mRawPtr.is_null()); + Self::Custom(unsafe { Atom::from_raw(property.mCustomName.mRawPtr) }) } else { - NonCustomPropertyId::from_nscsspropertyid(property.mID)?.to_property_id() + Self::NonCustom(NonCustomPropertyId::from_nscsspropertyid(property.mID)?) }) } @@ -2425,10 +2430,7 @@ impl PropertyDeclaration { /// Returns true if this property declaration is for one of the animatable properties. pub fn is_animatable(&self) -> bool { - match self.id() { - PropertyDeclarationId::Longhand(id) => id.is_animatable(), - PropertyDeclarationId::Custom(..) => false, - } + self.id().is_animatable() } /// Returns true if this property is a custom property, false diff --git a/servo/components/style/properties/property_declaration.rs b/servo/components/style/properties/property_declaration.rs index 4db87b5cbe1f..51f88a44edd0 100644 --- a/servo/components/style/properties/property_declaration.rs +++ b/servo/components/style/properties/property_declaration.rs @@ -4,7 +4,7 @@ //! Structs used for property declarations. -use super::{LonghandId, NonCustomPropertyId, PropertyFlags, PropertyId, ShorthandId}; +use super::{LonghandId, PropertyFlags, PropertyId, ShorthandId}; use crate::custom_properties::Name; #[cfg(feature = "gecko")] use crate::gecko_bindings::structs::{nsCSSPropertyID, AnimatedPropertyID, RefPtr}; @@ -34,15 +34,6 @@ impl OwnedPropertyDeclarationId { self.as_borrowed().is_logical() } - /// Returns whether this property is animatable. - #[inline] - pub fn is_animatable(&self) -> bool { - match self { - Self::Longhand(id) => id.is_animatable(), - Self::Custom(_) => true, - } - } - /// Returns the corresponding PropertyDeclarationId. #[inline] pub fn as_borrowed(&self) -> PropertyDeclarationId { @@ -52,33 +43,15 @@ impl OwnedPropertyDeclarationId { } } - /// Returns whether this property is transitionable. - #[inline] - pub fn is_transitionable(&self) -> bool { - match self { - Self::Longhand(longhand) => NonCustomPropertyId::from(*longhand).is_transitionable(), - // TODO(bug 1846516): Implement this for custom properties. - Self::Custom(_) => false, - } - } - /// Convert an `AnimatedPropertyID` into an `OwnedPropertyDeclarationId`. #[cfg(feature = "gecko")] #[inline] - pub fn from_gecko_animated_property_id(property: &AnimatedPropertyID) -> Result { + pub fn from_gecko_animated_property_id(property: &AnimatedPropertyID) -> Option { if property.mID == nsCSSPropertyID::eCSSPropertyExtra_variable { - if property.mCustomName.mRawPtr.is_null() { - Err(()) - } else { - Ok(Self::Custom(unsafe { - Atom::from_raw(property.mCustomName.mRawPtr) - })) - } - } else if let Ok(longhand) = LonghandId::from_nscsspropertyid(property.mID) { - Ok(Self::Longhand(longhand)) - } else { - Err(()) + debug_assert!(!property.mCustomName.mRawPtr.is_null()); + return Some(Self::Custom(unsafe { Atom::from_raw(property.mCustomName.mRawPtr) })); } + LonghandId::from_nscsspropertyid(property.mID).map(Self::Longhand) } } @@ -188,10 +161,8 @@ impl<'a> PropertyDeclarationId<'a> { #[inline] pub fn to_physical(&self, wm: WritingMode) -> Self { match self { - PropertyDeclarationId::Longhand(id) => { - PropertyDeclarationId::Longhand(id.to_physical(wm)) - }, - PropertyDeclarationId::Custom(_) => self.clone(), + Self::Longhand(id) => Self::Longhand(id.to_physical(wm)), + Self::Custom(_) => self.clone(), } } @@ -199,8 +170,8 @@ impl<'a> PropertyDeclarationId<'a> { #[inline] pub fn is_animatable(&self) -> bool { match self { - PropertyDeclarationId::Longhand(id) => id.is_animatable(), - PropertyDeclarationId::Custom(_) => true, + Self::Longhand(id) => id.is_animatable(), + Self::Custom(_) => true, } } diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 811370972580..0f9146be628c 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -632,7 +632,7 @@ pub extern "C" fn Servo_AnimationCompose( use style::gecko_bindings::bindings::Gecko_GetProgressFromComputedTiming; let property = match OwnedPropertyDeclarationId::from_gecko_animated_property_id(css_property) { - Some(property) if property.is_animatable() => property, + Some(property) if property.as_borrowed().is_animatable() => property, _ => return, }; @@ -700,8 +700,8 @@ macro_rules! get_property_id_from_nscsspropertyid { macro_rules! get_property_id_from_animatedpropertyid { ($property_id: ident, $ret: expr) => {{ match PropertyId::from_gecko_animated_property_id($property_id) { - Ok(property_id) => property_id, - Err(()) => { + Some(property_id) => property_id, + None => { return $ret; }, } @@ -722,7 +722,6 @@ pub extern "C" fn Servo_AnimationValue_Serialize( &get_property_id_from_animatedpropertyid!(property, ()), buffer, None, - None, /* No extra custom properties */ &data.stylist, ); debug_assert!(rv.is_ok()); @@ -1420,14 +1419,12 @@ pub unsafe extern "C" fn Servo_Property_GetCSSValuesForProperty( #[no_mangle] pub extern "C" fn Servo_Property_IsAnimatable(prop: &structs::AnimatedPropertyID) -> bool { - OwnedPropertyDeclarationId::from_gecko_animated_property_id(prop) - .map_or(false, |p| p.is_animatable()) + PropertyId::from_gecko_animated_property_id(prop).map_or(false, |p| p.is_animatable()) } #[no_mangle] pub extern "C" fn Servo_Property_IsTransitionable(prop: &structs::AnimatedPropertyID) -> bool { - OwnedPropertyDeclarationId::from_gecko_animated_property_id(prop) - .map_or(false, |p| p.is_transitionable()) + PropertyId::from_gecko_animated_property_id(prop).map_or(false, |p| p.is_transitionable()) } #[no_mangle] @@ -4723,20 +4720,17 @@ pub extern "C" fn Servo_DeclarationBlock_SerializeOneValue( property_id: &structs::AnimatedPropertyID, buffer: &mut nsACString, computed_values: Option<&ComputedValues>, - custom_properties: Option<&LockedDeclarationBlock>, data: &PerDocumentStyleData, ) { let property_id = get_property_id_from_animatedpropertyid!(property_id, ()); let global_style_data = &*GLOBAL_STYLE_DATA; let guard = global_style_data.shared_lock.read(); - let custom_properties = custom_properties.map(|block| block.read_with(&guard)); let data = data.borrow(); let rv = decls.read_with(&guard).single_value_to_css( &property_id, buffer, computed_values, - custom_properties, &data.stylist, ); debug_assert!(rv.is_ok()); @@ -6130,7 +6124,7 @@ impl<'a> PrioritizedPropertyIter<'a> { .enumerate() .map(|(index, pair)| { let property = PropertyId::from_gecko_animated_property_id(&pair.mProperty) - .unwrap_or(PropertyId::Shorthand(ShorthandId::All)); + .unwrap_or(PropertyId::NonCustom(ShorthandId::All.into())); PropertyAndIndex { property, index } }) .collect(); @@ -6142,6 +6136,10 @@ impl<'a> PrioritizedPropertyIter<'a> { curr: 0, } } + + fn reset(&mut self) { + self.curr = 0; + } } impl<'a> Iterator for PrioritizedPropertyIter<'a> { @@ -6165,6 +6163,9 @@ pub extern "C" fn Servo_GetComputedKeyframeValues( raw_data: &PerDocumentStyleData, computed_keyframes: &mut nsTArray, ) { + use style::properties::PropertyDeclaration; + use style::custom_properties::CustomPropertiesBuilder; + use style::applicable_declarations::CascadePriority; let data = raw_data.borrow(); let element = GeckoElement(element); let pseudo = PseudoElement::from_pseudo_type(pseudo_type, None); @@ -6201,9 +6202,42 @@ pub extern "C" fn Servo_GetComputedKeyframeValues( let ref mut animation_values = computed_keyframes[index]; let mut seen = PropertyDeclarationIdSet::default(); + let mut iter = PrioritizedPropertyIter::new(&keyframe.mPropertyValues); + + // FIXME: This is pretty much a hack. Instead, the AnimatedValue should be better + // integrated in the cascade. This would allow us to fix revert() too. + context.builder.custom_properties = { + let mut builder = CustomPropertiesBuilder::new_with_properties( + &data.stylist, + style.custom_properties().clone(), + &context, + ); + let priority = CascadePriority::same_tree_author_normal_at_root_layer(); + for property in &mut iter { + let is_custom = match PropertyId::from_gecko_animated_property_id(&property.mProperty) { + Some(PropertyId::Custom(..)) => true, + _ => false, + }; + if !is_custom { + break; // Custom props are guaranteed to sort earlier. + } + if property.mServoDeclarationBlock.mRawPtr.is_null() { + continue; + } + let declarations = unsafe { &*property.mServoDeclarationBlock.mRawPtr }; + let guard = declarations.read_with(&guard); + for decl in guard.normal_declaration_iter() { + if let PropertyDeclaration::Custom(ref declaration) = *decl { + builder.cascade(declaration, priority); + } + } + } + iter.reset(); + builder.build() + }; let mut property_index = 0; - for property in PrioritizedPropertyIter::new(&keyframe.mPropertyValues) { + for property in iter { if simulate_compute_values_failure(property) { continue; } @@ -6247,7 +6281,7 @@ pub extern "C" fn Servo_GetComputedKeyframeValues( }; if property.mServoDeclarationBlock.mRawPtr.is_null() { - if let Some(ref prop) = + if let Some(prop) = OwnedPropertyDeclarationId::from_gecko_animated_property_id(&property.mProperty) { maybe_append_animation_value(prop.as_borrowed(), None); @@ -6257,7 +6291,7 @@ pub extern "C" fn Servo_GetComputedKeyframeValues( let declarations = unsafe { &*property.mServoDeclarationBlock.mRawPtr }; let guard = declarations.read_with(&guard); - let iter = guard.to_animation_value_iter(&mut context, &default_values, None); + let iter = guard.to_animation_value_iter(&mut context, &default_values); for value in iter { maybe_append_animation_value(value.id(), Some(value.clone())); @@ -6303,7 +6337,6 @@ pub extern "C" fn Servo_GetAnimationValues( let iter = guard.to_animation_value_iter( &mut context, &default_values, - None, // SMIL has no extra custom properties. ); animation_values.extend(iter.map(|v| structs::RefPtr::from_arc(Arc::new(v)))); } @@ -6358,7 +6391,6 @@ pub extern "C" fn Servo_AnimationValue_Compute( let animation = AnimationValue::from_declaration( decl, &mut context, - None, // No extra custom properties for devtools. default_values, ); animation.map_or(Strong::null(), |value| Arc::new(value).into()) @@ -6408,12 +6440,12 @@ enum Offset { fn fill_in_missing_keyframe_values( all_properties: &PropertyDeclarationIdSet, timing_function: &ComputedTimingFunction, - longhands_at_offset: &PropertyDeclarationIdSet, + properties_at_offset: &PropertyDeclarationIdSet, offset: Offset, keyframes: &mut nsTArray, ) { // Return early if all animated properties are already set. - if longhands_at_offset.contains_all(all_properties) { + if properties_at_offset.contains_all(all_properties) { return; } @@ -6434,7 +6466,7 @@ fn fill_in_missing_keyframe_values( // Append properties that have not been set at this offset. for property in all_properties.iter() { - if !longhands_at_offset.contains(property) { + if !properties_at_offset.contains(property) { unsafe { Gecko_AppendPropertyValuePair( &mut *(*keyframe).mPropertyValues, @@ -6550,10 +6582,9 @@ pub unsafe extern "C" fn Servo_StyleSet_GetKeyframesForName( for declaration in guard.normal_declaration_iter().rev() { let id = declaration.id().to_physical(writing_mode); - // Skip non-animatable properties, including - // the 'display' property because although it - // is animatable from SMIL, it should not be - // animatable from CSS Animations. + // Skip non-animatable properties, including the 'display' property because + // although it is animatable from SMIL, it should not be animatable from CSS + // Animations. if !id.is_animatable() || id == PropertyDeclarationId::Longhand(LonghandId::Display) { diff --git a/testing/web-platform/meta/css/css-animations/KeyframeEffect-getKeyframes.tentative.html.ini b/testing/web-platform/meta/css/css-animations/KeyframeEffect-getKeyframes.tentative.html.ini index 8c0e5ecb1439..3ed7d4e5d544 100644 --- a/testing/web-platform/meta/css/css-animations/KeyframeEffect-getKeyframes.tentative.html.ini +++ b/testing/web-platform/meta/css/css-animations/KeyframeEffect-getKeyframes.tentative.html.ini @@ -35,3 +35,6 @@ [KeyframeEffect.getKeyframes() returns expected values for animations with a CSS variable which is overriden by the value in keyframe] expected: FAIL + + [KeyframeEffect.getKeyframes() returns expected values for animations with only custom property in a keyframe] + expected: FAIL diff --git a/testing/web-platform/meta/css/css-contain/container-queries/container-inner-at-rules.html.ini b/testing/web-platform/meta/css/css-contain/container-queries/container-inner-at-rules.html.ini index e53bfecef47f..1cc85f2a8810 100644 --- a/testing/web-platform/meta/css/css-contain/container-queries/container-inner-at-rules.html.ini +++ b/testing/web-platform/meta/css/css-contain/container-queries/container-inner-at-rules.html.ini @@ -1,8 +1,3 @@ [container-inner-at-rules.html] - expected: - if (os == "android") and fission: [OK, TIMEOUT] - [@keyframes is defined regardless of evaluation] - expected: FAIL - [@property is defined regardless of evaluation] expected: FAIL diff --git a/testing/web-platform/meta/css/css-contain/container-queries/container-longhand-animation-type.html.ini b/testing/web-platform/meta/css/css-contain/container-queries/container-longhand-animation-type.html.ini deleted file mode 100644 index 5f2b395b8f0a..000000000000 --- a/testing/web-platform/meta/css/css-contain/container-queries/container-longhand-animation-type.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[container-longhand-animation-type.html] - expected: - if (os == "android") and fission: [OK, TIMEOUT] - [Reference variable is applied] - expected: FAIL diff --git a/testing/web-platform/meta/css/css-variables/variable-animation-from-to.html.ini b/testing/web-platform/meta/css/css-variables/variable-animation-from-to.html.ini deleted file mode 100644 index 267063b51715..000000000000 --- a/testing/web-platform/meta/css/css-variables/variable-animation-from-to.html.ini +++ /dev/null @@ -1,14 +0,0 @@ -[variable-animation-from-to.html] - expected: - if (os == "android") and fission: [OK, TIMEOUT] - [Verify CSS variable value before animation] - expected: FAIL - - [Verify substituted color value before animation] - expected: FAIL - - [Verify CSS variable value after animation] - expected: FAIL - - [Verify substituted color value after animation] - expected: FAIL diff --git a/testing/web-platform/meta/css/css-variables/variable-animation-over-transition.html.ini b/testing/web-platform/meta/css/css-variables/variable-animation-over-transition.html.ini deleted file mode 100644 index 0d6f98fd093f..000000000000 --- a/testing/web-platform/meta/css/css-variables/variable-animation-over-transition.html.ini +++ /dev/null @@ -1,14 +0,0 @@ -[variable-animation-over-transition.html] - expected: - if (os == "android") and fission: [OK, TIMEOUT] - [Verify CSS variable value before animation] - expected: FAIL - - [Verify substituted color value before animation] - expected: FAIL - - [Verify CSS variable value after animation] - expected: FAIL - - [Verify substituted color value after animation] - expected: FAIL diff --git a/testing/web-platform/meta/css/css-variables/variable-animation-to-only.html.ini b/testing/web-platform/meta/css/css-variables/variable-animation-to-only.html.ini deleted file mode 100644 index fdf91e897159..000000000000 --- a/testing/web-platform/meta/css/css-variables/variable-animation-to-only.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[variable-animation-to-only.html] - expected: - if (os == "android") and fission: [OK, TIMEOUT] - [Verify CSS variable value after animation] - expected: FAIL diff --git a/testing/web-platform/meta/web-animations/interfaces/Animatable/animate.html.ini b/testing/web-platform/meta/web-animations/interfaces/Animatable/animate.html.ini index 25611167597c..1828655ebdee 100644 --- a/testing/web-platform/meta/web-animations/interfaces/Animatable/animate.html.ini +++ b/testing/web-platform/meta/web-animations/interfaces/Animatable/animate.html.ini @@ -1,10 +1,4 @@ [animate.html] - [Element.animate() accepts a keyframe sequence with a CSS variable as its property] - expected: FAIL - - [Element.animate() accepts a property-indexed keyframes specification with a CSS variable as the property] - expected: FAIL - [animate() with pseudoElement parameter creates an Animation object for ::first-line] bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1615469 expected: FAIL diff --git a/testing/web-platform/meta/web-animations/interfaces/Animation/commitStyles.html.ini b/testing/web-platform/meta/web-animations/interfaces/Animation/commitStyles.html.ini deleted file mode 100644 index 5233077a1b42..000000000000 --- a/testing/web-platform/meta/web-animations/interfaces/Animation/commitStyles.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[commitStyles.html] - [Commits custom variables] - expected: FAIL - diff --git a/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/constructor.html.ini b/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/constructor.html.ini deleted file mode 100644 index ce09c622abc8..000000000000 --- a/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/constructor.html.ini +++ /dev/null @@ -1,7 +0,0 @@ -[constructor.html] - [A KeyframeEffect can be constructed with a property-indexed keyframes specification with a CSS variable as the property] - expected: FAIL - - [A KeyframeEffect can be constructed with a keyframe sequence with a CSS variable as its property] - expected: FAIL - diff --git a/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/setKeyframes.html.ini b/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/setKeyframes.html.ini deleted file mode 100644 index 0d0189582e58..000000000000 --- a/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/setKeyframes.html.ini +++ /dev/null @@ -1,8 +0,0 @@ -[setKeyframes.html] - expected: - if (os == "android") and fission: [OK, TIMEOUT] - [Keyframes can be replaced with a keyframe sequence with a CSS variable as its property] - expected: FAIL - - [Keyframes can be replaced with a property-indexed keyframes specification with a CSS variable as the property] - expected: FAIL