Bug 1846516 - More fixes on top of bug 1870009. r=zrhoffman

This fixes some leaks, and fixes IsAnimatable/IsTransitionable to look
at PropertyId.

Differential Revision: https://phabricator.services.mozilla.com/D196419

dom/animation/Keyframe.h

@@ -24,6 +24,7 @@ struct StyleLockedDeclarationBlock;
 struct PropertyValuePair {
   explicit PropertyValuePair(const AnimatedPropertyID& aProperty)
       : mProperty(aProperty) {}
+
   PropertyValuePair(const AnimatedPropertyID& aProperty,
                     RefPtr<StyleLockedDeclarationBlock>&& aValue)
       : mProperty(aProperty), mServoDeclarationBlock(std::move(aValue)) {

dom/animation/KeyframeEffect.cpp

@@ -1253,8 +1253,8 @@ void KeyframeEffect::GetKeyframes(JSContext* aCx, nsTArray<JSObject*>& aResult,
   // know what custom property values to provide.
   RefPtr<const ComputedStyle> 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<JSObject*>& 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<JSObject*>& 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:

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);
-    }
+    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});
     }
   }
 

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<StyleAnimationValue> 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;

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

servo/components/style/custom_properties.rs

@@ -863,23 +863,26 @@ 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 {
+            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()
@@ -887,10 +890,12 @@ impl<'a, 'b: 'a> CustomPropertiesBuilder<'a, 'b> {
                 inherited.inherited.clone()
             },
             non_inherited: initial_values.non_inherited.clone(),
-            },
-            stylist,
-            computed_context,
-        }
+        };
+
+        // 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.

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.

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<Self> {
         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,
                 )
             },

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<Self, ()> {
-        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<Self> {
+        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

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<Self, ()> {
+    pub fn from_gecko_animated_property_id(property: &AnimatedPropertyID) -> Option<Self> {
         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,
         }
     }
 

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<structs::ComputedKeyframeValues>,
 ) {
+    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<structs::Keyframe>,
 ) {
     // 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)
                     {

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

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

testing/web-platform/meta/css/css-contain/container-queries/container-longhand-animation-type.html.ini

@@ -1,5 +0,0 @@
-[container-longhand-animation-type.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [Reference variable is applied]
-    expected: FAIL

testing/web-platform/meta/css/css-variables/variable-animation-from-to.html.ini

@@ -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

testing/web-platform/meta/css/css-variables/variable-animation-over-transition.html.ini

@@ -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

testing/web-platform/meta/css/css-variables/variable-animation-to-only.html.ini

@@ -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

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

testing/web-platform/meta/web-animations/interfaces/Animation/commitStyles.html.ini

@@ -1,4 +0,0 @@
-[commitStyles.html]
-  [Commits custom variables]
-    expected: FAIL
-

testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/constructor.html.ini

@@ -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
-

testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/setKeyframes.html.ini

@@ -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