Bug 1390766 - Drop complex handling of shorthand properties when determining if they are discrete or not; r?hiro draft
authorBrian Birtles <birtles@gmail.com>
Wed, 16 Aug 2017 16:09:40 +0900
changeset 647350 297b235b99934cf5cd1818d9911bf85715f41ac2
parent 647200 4e93516e92e58d166ad37b8544c3230024afb587
child 726462 5b770e991450450a0a734eabc78117bf5079497b
push id74360
push userbmo:bbirtles@mozilla.com
push dateWed, 16 Aug 2017 07:11:21 +0000
reviewershiro
bugs1390766
milestone57.0a1
Bug 1390766 - Drop complex handling of shorthand properties when determining if they are discrete or not; r?hiro MozReview-Commit-ID: 2OzjTXGE7DU
dom/smil/nsSMILCSSValueType.cpp
--- a/dom/smil/nsSMILCSSValueType.cpp
+++ b/dom/smil/nsSMILCSSValueType.cpp
@@ -521,79 +521,34 @@ InterpolateForGecko(const ValueWrapper* 
                                        *endCSSValue,
                                        aUnitDistance, resultValue)) {
     aResult.mU.mPtr = new ValueWrapper(aEndWrapper.mPropID, resultValue);
     return NS_OK;
   }
   return NS_ERROR_FAILURE;
 }
 
-static bool
-IsPropertyDiscretelyAnimatable(nsCSSPropertyID aProperty)
-{
-  // For shorthands, Servo_Property_IsDiscreteAnimatable will always return
-  // false. That makes sense for shorthands like 'font' which have smoothly
-  // interpolable longhand components. However, for shorthands where all the
-  // components are discretely animatable, like 'font-variant', we should
-  // treat the shorthand as discretely animatable so that we apply SMIL's
-  // special discrete handling to it and all its longhand components.
-  if (nsCSSProps::IsShorthand(aProperty)) {
-    // We could iterate over all the longhand components and call
-    // Servo_Property_IsDiscreteAnimatable on each of them, but that's a lot of
-    // FFI calls do be doing each time we interpolate. Instead, since there are
-    // only about six shorthands in the set of properties that can be animated
-    // by SMIL, we just hard code the discrete ones below then add a debug-mode
-    // check that this list matches what the result would be if we performed
-    // all the FFI calls.
-    bool result;
-    switch (aProperty) {
-      case eCSSProperty_font_variant:
-      case eCSSProperty_marker:
-      case eCSSProperty_overflow:
-        result = true;
-        break;
-      default:
-        result = false;
-        break;
-    }
-
-#ifdef DEBUG
-    bool resultAccordingToServo = true;
-    CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(p, aProperty,
-                                         CSSEnabledState::eForAllContent) {
-      // If the shorthand has one or more non-discrete components, it should not
-      // be treated as discrete.
-      if (!Servo_Property_IsDiscreteAnimatable(*p)) {
-        resultAccordingToServo = false;
-      }
-    }
-    MOZ_ASSERT(result == resultAccordingToServo,
-               "Gecko and Servo should agree on which shorthands should be"
-               " treated as discretely animatable");
-#endif
-
-    return result;
-  }
-
-  return Servo_Property_IsDiscreteAnimatable(aProperty);
-}
-
 static nsresult
 InterpolateForServo(const ValueWrapper* aStartWrapper,
                     const ValueWrapper& aEndWrapper,
                     double aUnitDistance,
                     nsSMILValue& aResult)
 {
   // For discretely-animated properties Servo_AnimationValues_Interpolate will
   // perform the discrete animation (i.e. 50% flip) and return a success result.
   // However, SMIL has its own special discrete animation behavior that it uses
   // when keyTimes are specified, but we won't run that unless that this method
   // returns a failure to indicate that the property cannot be smoothly
   // interpolated, i.e. that we need to use a discrete calcMode.
-  if (IsPropertyDiscretelyAnimatable(aEndWrapper.mPropID)) {
+  //
+  // For shorthands, Servo_Property_IsDiscreteAnimatable will always return
+  // false. That's fine since most shorthands (like 'font' and
+  // 'text-decoration') include non-discrete components. If authors want to
+  // treat all components as discrete then they should use calcMode="discrete".
+  if (Servo_Property_IsDiscreteAnimatable(aEndWrapper.mPropID)) {
     return NS_ERROR_FAILURE;
   }
 
   ServoAnimationValues results;
   size_t len = aEndWrapper.mServoValues.Length();
   results.SetCapacity(len);
   MOZ_ASSERT(!aStartWrapper || aStartWrapper->mServoValues.Length() == len,
              "Start and end values length should be the same if "