Bug 1355349 - Treat properties that can't be animated by the Servo backend as unanimatable; r?hiro
If we attempt to call animation-related methods on the Servo backend (e.g.
Servo_ComputedValues_ExtractAnimationValue) and pass a property that is not
(yet) animatable by Servo, it will panic so we should avoid passing these
properties.
An alternative, is to make methods like
Servo_ComputedValues_ExtractAnimationValue fallible by having them first check
if the passed-in value is animatable and return null in that case. That too, is
probably worth doing (call sites like KeyframeEffectReadOnly::EnsureBaseStyle
that assume it is fallible could assert that the result is non-null since,
provided that property is animatable, the method should still be fallible) but
refusing to animate these properties from the start is cleaner so we just do
that for now.
MozReview-Commit-ID: ESYcbkTtfXG
--- a/dom/smil/nsSMILCSSProperty.cpp
+++ b/dom/smil/nsSMILCSSProperty.cpp
@@ -23,17 +23,18 @@ using namespace mozilla::dom;
// Class Methods
nsSMILCSSProperty::nsSMILCSSProperty(nsCSSPropertyID aPropID,
Element* aElement,
nsStyleContext* aBaseStyleContext)
: mPropID(aPropID)
, mElement(aElement)
, mBaseStyleContext(aBaseStyleContext)
{
- MOZ_ASSERT(IsPropertyAnimatable(mPropID),
+ MOZ_ASSERT(IsPropertyAnimatable(mPropID,
+ aElement->OwnerDoc()->GetStyleBackendType()),
"Creating a nsSMILCSSProperty for a property "
"that's not supported for animation");
}
nsSMILValue
nsSMILCSSProperty::GetBaseValue() const
{
// To benefit from Return Value Optimization and avoid copy constructor calls
@@ -88,17 +89,19 @@ nsSMILCSSProperty::GetBaseValue() const
}
nsresult
nsSMILCSSProperty::ValueFromString(const nsAString& aStr,
const SVGAnimationElement* aSrcElement,
nsSMILValue& aValue,
bool& aPreventCachingOfSandwich) const
{
- NS_ENSURE_TRUE(IsPropertyAnimatable(mPropID), NS_ERROR_FAILURE);
+ NS_ENSURE_TRUE(IsPropertyAnimatable(mPropID,
+ mElement->OwnerDoc()->GetStyleBackendType()),
+ NS_ERROR_FAILURE);
nsSMILCSSValueType::ValueFromString(mPropID, mElement, aStr, aValue,
&aPreventCachingOfSandwich);
if (aValue.IsNull()) {
return NS_ERROR_FAILURE;
}
@@ -109,17 +112,19 @@ nsSMILCSSProperty::ValueFromString(const
aPreventCachingOfSandwich = true;
}
return NS_OK;
}
nsresult
nsSMILCSSProperty::SetAnimValue(const nsSMILValue& aValue)
{
- NS_ENSURE_TRUE(IsPropertyAnimatable(mPropID), NS_ERROR_FAILURE);
+ NS_ENSURE_TRUE(IsPropertyAnimatable(mPropID,
+ mElement->OwnerDoc()->GetStyleBackendType()),
+ NS_ERROR_FAILURE);
// Convert nsSMILValue to string
nsAutoString valStr;
nsSMILCSSValueType::ValueToString(aValue, valStr);
// Use string value to style the target element
nsICSSDeclaration* overrideDecl = mElement->GetSMILOverrideStyle();
if (overrideDecl) {
@@ -141,18 +146,25 @@ nsSMILCSSProperty::ClearAnimValue()
if (overrideDecl) {
overrideDecl->SetPropertyValue(mPropID, EmptyString());
}
}
// Based on http://www.w3.org/TR/SVG/propidx.html
// static
bool
-nsSMILCSSProperty::IsPropertyAnimatable(nsCSSPropertyID aPropID)
+nsSMILCSSProperty::IsPropertyAnimatable(nsCSSPropertyID aPropID,
+ StyleBackendType aBackend)
{
+ // Bug 1353918: Drop this check
+ if (aBackend == StyleBackendType::Servo &&
+ !Servo_Property_IsAnimatable(aPropID)) {
+ return false;
+ }
+
// NOTE: Right now, Gecko doesn't recognize the following properties from
// the SVG Property Index:
// alignment-baseline
// baseline-shift
// color-profile
// color-rendering
// glyph-orientation-horizontal
// glyph-orientation-vertical
--- a/dom/smil/nsSMILCSSProperty.h
+++ b/dom/smil/nsSMILCSSProperty.h
@@ -5,16 +5,17 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
/* representation of a SMIL-animatable CSS property on an element */
#ifndef NS_SMILCSSPROPERTY_H_
#define NS_SMILCSSPROPERTY_H_
#include "mozilla/Attributes.h"
+#include "mozilla/StyleBackendType.h"
#include "nsISMILAttr.h"
#include "nsIAtom.h"
#include "nsCSSPropertyID.h"
#include "nsCSSValue.h"
class nsStyleContext;
namespace mozilla {
@@ -53,20 +54,24 @@ public:
virtual nsresult SetAnimValue(const nsSMILValue& aValue) override;
virtual void ClearAnimValue() override;
/**
* Utility method - returns true if the given property is supported for
* SMIL animation.
*
* @param aProperty The property to check for animation support.
+ * @param aBackend The style backend to check for animation support.
+ * This is a temporary measure until the Servo backend
+ * supports all animatable properties (bug 1353918).
* @return true if the given property is supported for SMIL animation, or
* false otherwise
*/
- static bool IsPropertyAnimatable(nsCSSPropertyID aPropID);
+ static bool IsPropertyAnimatable(nsCSSPropertyID aPropID,
+ mozilla::StyleBackendType aBackend);
protected:
nsCSSPropertyID mPropID;
// Using non-refcounted pointer for mElement -- we know mElement will stay
// alive for my lifetime because a nsISMILAttr (like me) only lives as long
// as the Compositing step, and DOM elements don't get a chance to die during
// that time.
mozilla::dom::Element* mElement;
--- a/dom/smil/nsSMILCompositor.cpp
+++ b/dom/smil/nsSMILCompositor.cpp
@@ -155,17 +155,18 @@ nsSMILCompositor::GetCSSPropertyToAnimat
if (mKey.mAttributeNamespaceID != kNameSpaceID_None) {
return eCSSProperty_UNKNOWN;
}
nsCSSPropertyID propID =
nsCSSProps::LookupProperty(nsDependentAtomString(mKey.mAttributeName),
CSSEnabledState::eForAllContent);
- if (!nsSMILCSSProperty::IsPropertyAnimatable(propID)) {
+ if (!nsSMILCSSProperty::IsPropertyAnimatable(propID,
+ mKey.mElement->OwnerDoc()->GetStyleBackendType())) {
return eCSSProperty_UNKNOWN;
}
// If we are animating the 'width' or 'height' of an outer SVG
// element we should animate it as a CSS property, but for other elements
// (e.g. <rect>) we should animate it as a length attribute.
// The easiest way to test for an outer SVG element, is to see if it is an
// SVG-namespace element mapping its width/height attribute to style.