Bug 1390384 - Apply SMIL's calcMode=discrete handling to discretely animatable properties in Stylo; r=hiro

In SMIL if a discrete calcMode is used, the keyTimes attribute may be used to
specify the times at which the various animation values are used, overriding the
regular 50% flip behavior (see nsSMILAnimationFunction::ScaleSimpleProgress).

However, this behavior is only applied when the calcMode is known to be
discrete. If the author specifies calcMode="linear" but
attributeType="stroke-linecap" then SMIL should fall back to discrete calcMode
including applying the special keyTimes behavior.

For the Servo backend, Servo_AnimationValues_Interpolate does not return an
error for discretely animated properties so SMIL does not recognize when we fall
back to discrete calcMode. This patch adds a check before performing
interpolation that tests if the property is a discretely interpolable one and,
if it is, returns an error so that we run SMIL's special discrete calcMode
handling.

MozReview-Commit-ID: FOp8XcNbvdu

--HG--
extra : rebase_source : 57ddb333855de111aa585fe894e99937681e5cd2

dom/smil/nsSMILCSSValueType.cpp

@@ -526,18 +526,78 @@ InterpolateForGecko(const ValueWrapper* aStartWrapper,
   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)) {
+    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 "
-             "The start value exists");
+             "the start value exists");
   for (size_t i = 0; i < len; i++) {
     const RefPtr<RawServoAnimationValue>*
       startValue = aStartWrapper

dom/smil/test/mochitest.ini

@@ -42,7 +42,6 @@ skip-if = toolkit == 'android'
 [test_smilInvalidValues.html]
 [test_smilKeySplines.xhtml]
 [test_smilKeyTimes.xhtml]
-skip-if = stylo
 [test_smilKeyTimesPacedMode.xhtml]
 [test_smilMappedAttrFromBy.xhtml]
 skip-if = stylo