Bug 1371518 - Make KeyframeUtils::IsAnimatable consult the Servo backend; r?hiro
When styling with the Servo backend, we should also use the Servo backend to
determine if a property is animatable or not. However, if we do this,
Servo_Property_IsAnimatable will start returning true for the 'display' property
once we mark that as animatable in Servo (for SMIL).
Even if we later fail to actually animate 'display' (due checks added to Servo
in the next patch) we still need to treat 'display' as un-animatable in
KeyframeUtils so that we don't *read* the 'display' property of Keyframe objects
passed to the Animations API, since that is observable.
This patch makes us consult Servo_Property_IsAnimatable when using the Servo
backend and also explicitly treat the 'display' property as not animatable.
MozReview-Commit-ID: 1JllbeJisAS
--- a/dom/animation/KeyframeEffectParams.cpp
+++ b/dom/animation/KeyframeEffectParams.cpp
@@ -147,17 +147,21 @@ KeyframeEffectParams::ParseSpacing(const
aRv.ThrowTypeError<dom::MSG_INVALID_SPACING_MODE_ERROR>(aSpacing);
return;
}
aPacedProperty =
nsCSSProps::LookupProperty(identToken, CSSEnabledState::eForAllContent);
if (aPacedProperty == eCSSProperty_UNKNOWN ||
aPacedProperty == eCSSPropertyExtra_variable ||
- !KeyframeUtils::IsAnimatableProperty(aPacedProperty)) {
+ // We just unconditionally pass Gecko as the backend type here since
+ // Servo doesn't support paced timing and this feature will soon be
+ // removed (bug 1339690).
+ !KeyframeUtils::IsAnimatableProperty(aPacedProperty,
+ StyleBackendType::Gecko)) {
aPacedProperty = eCSSProperty_UNKNOWN;
aInvalidPacedProperty = identToken;
}
if (end - iter.get() != 1 || *iter != ')') {
aRv.ThrowTypeError<dom::MSG_INVALID_SPACING_MODE_ERROR>(aSpacing);
return;
}
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -363,16 +363,17 @@ ConvertKeyframeSequence(JSContext* aCx,
nsIDocument* aDocument,
JS::ForOfIterator& aIterator,
nsTArray<Keyframe>& aResult);
static bool
GetPropertyValuesPairs(JSContext* aCx,
JS::Handle<JSObject*> aObject,
ListAllowance aAllowLists,
+ StyleBackendType aBackend,
nsTArray<PropertyValuesPair>& aResult);
static bool
AppendStringOrStringSequenceToArray(JSContext* aCx,
JS::Handle<JS::Value> aValue,
ListAllowance aAllowLists,
nsTArray<nsString>& aValues);
@@ -488,17 +489,20 @@ KeyframeUtils::ApplySpacing(nsTArray<Key
nsStyleContext* aStyleContext)
{
if (aKeyframes.IsEmpty()) {
return;
}
nsTArray<double> cumulativeDistances;
if (aSpacingMode == SpacingMode::paced) {
- MOZ_ASSERT(IsAnimatableProperty(aProperty),
+ // We just unconditionally pass Gecko as the backend type here since
+ // Servo doesn't support paced timing and this feature will soon be removed
+ // (bug 1339690).
+ MOZ_ASSERT(IsAnimatableProperty(aProperty, StyleBackendType::Gecko),
"Paced property should be animatable");
cumulativeDistances = GetCumulativeDistances(aComputedValues, aProperty,
aStyleContext);
// Reset the computed offsets if using paced spacing.
for (Keyframe& keyframe : aKeyframes) {
keyframe.mComputedOffset = Keyframe::kComputedOffsetNotSet;
}
@@ -712,18 +716,30 @@ KeyframeUtils::GetAnimationPropertiesFro
}
nsTArray<AnimationProperty> result;
BuildSegmentsFromValueEntries(entries, result);
return result;
}
/* static */ bool
-KeyframeUtils::IsAnimatableProperty(nsCSSPropertyID aProperty)
+KeyframeUtils::IsAnimatableProperty(nsCSSPropertyID aProperty,
+ StyleBackendType aBackend)
{
+ // Regardless of the backend type, treat the 'display' property as not
+ // animatable. (The Servo backend will report it as being animatable, since
+ // it is in fact animatable by SMIL.)
+ if (aProperty == eCSSProperty_display) {
+ return false;
+ }
+
+ if (aBackend == StyleBackendType::Servo) {
+ return Servo_Property_IsAnimatable(aProperty);
+ }
+
if (aProperty == eCSSProperty_UNKNOWN) {
return false;
}
if (!nsCSSProps::IsShorthand(aProperty)) {
return nsCSSProps::kAnimTypeTable[aProperty] != eStyleAnimType_None;
}
@@ -864,16 +880,17 @@ ConvertKeyframeSequence(JSContext* aCx,
}
// Look for additional property-values pairs on the object.
nsTArray<PropertyValuesPair> propertyValuePairs;
if (value.isObject()) {
JS::Rooted<JSObject*> object(aCx, &value.toObject());
if (!GetPropertyValuesPairs(aCx, object,
ListAllowance::eDisallow,
+ aDocument->GetStyleBackendType(),
propertyValuePairs)) {
return false;
}
}
for (PropertyValuesPair& pair : propertyValuePairs) {
MOZ_ASSERT(pair.mValues.Length() == 1);
keyframe->mPropertyValues.AppendElement(
@@ -900,25 +917,28 @@ ConvertKeyframeSequence(JSContext* aCx,
/**
* Reads the property-values pairs from the specified JS object.
*
* @param aObject The JS object to look at.
* @param aAllowLists If eAllow, values will be converted to
* (DOMString or sequence<DOMString); if eDisallow, values
* will be converted to DOMString.
+ * @param aBackend The style backend in use. Used to determine which properties
+ * are animatable since only animatable properties are read.
* @param aResult The array into which the enumerated property-values
* pairs will be stored.
* @return false on failure or JS exception thrown while interacting
* with aObject; true otherwise.
*/
static bool
GetPropertyValuesPairs(JSContext* aCx,
JS::Handle<JSObject*> aObject,
ListAllowance aAllowLists,
+ StyleBackendType aBackend,
nsTArray<PropertyValuesPair>& aResult)
{
nsTArray<AdditionalProperty> properties;
// Iterate over all the properties on aObject and append an
// entry to properties for them.
//
// We don't compare the jsids that we encounter with those for
@@ -931,17 +951,17 @@ GetPropertyValuesPairs(JSContext* aCx,
for (size_t i = 0, n = ids.length(); i < n; i++) {
nsAutoJSString propName;
if (!propName.init(aCx, ids[i])) {
return false;
}
nsCSSPropertyID property =
nsCSSProps::LookupPropertyByIDLName(propName,
CSSEnabledState::eForAllContent);
- if (KeyframeUtils::IsAnimatableProperty(property)) {
+ if (KeyframeUtils::IsAnimatableProperty(property, aBackend)) {
AdditionalProperty* p = properties.AppendElement();
p->mProperty = property;
p->mJsidIndex = i;
}
}
// Sort the entries by IDL name and then get each value and
// convert it either to a DOMString or to a
@@ -1445,16 +1465,17 @@ GetKeyframeListFromPropertyIndexedKeyfra
if (aRv.Failed()) {
return;
}
// Get all the property--value-list pairs off the object.
JS::Rooted<JSObject*> object(aCx, &aValue.toObject());
nsTArray<PropertyValuesPair> propertyValuesPairs;
if (!GetPropertyValuesPairs(aCx, object, ListAllowance::eAllow,
+ aDocument->GetStyleBackendType(),
propertyValuesPairs)) {
aRv.Throw(NS_ERROR_FAILURE);
return;
}
// Create a set of keyframes for each property.
nsCSSParser parser(aDocument->CSSLoader());
nsClassHashtable<nsFloatHashKey, Keyframe> processedKeyframes;
--- a/dom/animation/KeyframeUtils.h
+++ b/dom/animation/KeyframeUtils.h
@@ -153,19 +153,22 @@ public:
const nsTArray<ComputedKeyframeValues>& aComputedValues,
dom::CompositeOperation aEffectComposite);
/**
* Check if the property or, for shorthands, one or more of
* its subproperties, is animatable.
*
* @param aProperty The property to check.
+ * @param aBackend The style backend, Servo or Gecko, that should determine
+ * if the property is animatable or not.
* @return true if |aProperty| is animatable.
*/
- static bool IsAnimatableProperty(nsCSSPropertyID aProperty);
+ static bool IsAnimatableProperty(nsCSSPropertyID aProperty,
+ StyleBackendType aBackend);
/**
* Parse a string representing a CSS property value into a
* RawServoDeclarationBlock.
*
* @param aProperty The property to be parsed.
* @param aValue The specified value.
* @param aDocument The current document.