Bug 1355349 - Treat properties that can't be animated by the Servo backend as unanimatable; r=hiro
authorBrian Birtles <birtles@gmail.com>
Fri, 02 Jun 2017 15:14:43 +0900
changeset 410396 992504b822d2044e57031bba733ffd2bfd9e5d49
parent 410395 ef01cc3b84b8103bfee7ad41021626656d4349de
child 410397 297d24c38ab3e2aaf83cf1c84be738a6abf055aa
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1355349
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
dom/smil/nsSMILCSSProperty.cpp
dom/smil/nsSMILCSSProperty.h
dom/smil/nsSMILCompositor.cpp
--- 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.