Bug 1096773 part 2 - Add a KeyframeEffectReadOnly constructor that takes a TimingParams argument; r=boris

This will allow us to re-use the constructor from Animatable.animate() since the
existing type, UnrestrictedDoubleOrKeyframeEffectOptions, is not compatible with
UnrestrictedDoubleOrKeyframeAnimationOptions (introduced in the next patch in
this series), as used by Animatable.animate()

dom/animation/AnimationEffectTimingReadOnly.cpp

@@ -5,20 +5,36 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/dom/AnimationEffectTimingReadOnly.h"
+
 #include "mozilla/dom/AnimationEffectTimingReadOnlyBinding.h"
+#include "mozilla/dom/KeyframeEffectBinding.h"
 
 namespace mozilla {
 
-TimingParams&
-TimingParams::operator=(const dom::AnimationEffectTimingProperties& aRhs)
+TimingParams::TimingParams(const dom::AnimationEffectTimingProperties& aRhs)
+  : mDuration(aRhs.mDuration)
+  , mDelay(TimeDuration::FromMilliseconds(aRhs.mDelay))
+  , mIterations(aRhs.mIterations)
+  , mDirection(aRhs.mDirection)
+  , mFill(aRhs.mFill)
 {
-  mDuration = aRhs.mDuration;
-  mDelay = TimeDuration::FromMilliseconds(aRhs.mDelay);
-  mIterations = aRhs.mIterations;
-  mDirection = aRhs.mDirection;
-  mFill = aRhs.mFill;
+}
 
-  return *this;
+TimingParams::TimingParams(double aDuration)
+{
+  mDuration.SetAsUnrestrictedDouble() = aDuration;
+}
+
+/* static */ TimingParams
+TimingParams::FromOptionsUnion(
+  const dom::UnrestrictedDoubleOrKeyframeEffectOptions& aOptions)
+{
+  if (aOptions.IsUnrestrictedDouble()) {
+    return TimingParams(aOptions.GetAsUnrestrictedDouble());
+  } else {
+    MOZ_ASSERT(aOptions.IsKeyframeEffectOptions());
+    return TimingParams(aOptions.GetAsKeyframeEffectOptions());
+  }
 }
 
 bool

dom/animation/AnimationEffectTimingReadOnly.h

@@ -24,8 +24,21 @@
 
 namespace mozilla {
 
+namespace dom {
+struct AnimationEffectTimingProperties;
+class UnrestrictedDoubleOrKeyframeEffectOptions;
+}
+
 struct TimingParams
 {
+  TimingParams() = default;
+  explicit TimingParams(
+    const dom::AnimationEffectTimingProperties& aTimingProperties);
+  explicit TimingParams(double aDuration);
+
+  static TimingParams FromOptionsUnion(
+    const dom::UnrestrictedDoubleOrKeyframeEffectOptions& aOptions);
+
   // The unitialized state of mDuration represents "auto".
   // Bug 1237173: We will replace this with Maybe<TimeDuration>.
   dom::OwningUnrestrictedDoubleOrString mDuration;
@@ -34,8 +47,6 @@ struct TimingParams
   dom::PlaybackDirection mDirection = dom::PlaybackDirection::Normal;
   dom::FillMode mFill = dom::FillMode::Auto;
 
-  TimingParams& operator=(const dom::AnimationEffectTimingProperties& aRhs);
-
   bool operator==(const TimingParams& aOther) const;
   bool operator!=(const TimingParams& aOther) const
   {

dom/animation/KeyframeEffect.cpp

@@ -650,21 +650,6 @@ DumpAnimationProperties(nsTArray<AnimationProperty>& aAnimationProperties)
 }
 #endif
 
-/* static */ TimingParams
-KeyframeEffectReadOnly::ConvertKeyframeEffectOptions(
-    const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions)
-{
-  TimingParams timing;
-
-  if (aOptions.IsKeyframeEffectOptions()) {
-    timing = aOptions.GetAsKeyframeEffectOptions();
-  } else {
-    timing.mDuration.SetAsUnrestrictedDouble() =
-      aOptions.GetAsUnrestrictedDouble();
-  }
-  return timing;
-}
-
 /**
  * A property and StyleAnimationValue pair.
  */
@@ -1688,7 +1673,7 @@ KeyframeEffectReadOnly::Constructor(
     const GlobalObject& aGlobal,
     Element* aTarget,
     JS::Handle<JSObject*> aFrames,
-    const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions,
+    const TimingParams& aTiming,
     ErrorResult& aRv)
 {
   if (!aTarget) {
@@ -1697,8 +1682,6 @@ KeyframeEffectReadOnly::Constructor(
     return nullptr;
   }
 
-  TimingParams timing = ConvertKeyframeEffectOptions(aOptions);
-
   InfallibleTArray<AnimationProperty> animationProperties;
   BuildAnimationPropertyList(aGlobal.Context(), aTarget, aFrames,
                              animationProperties, aRv);
@@ -1710,7 +1693,7 @@ KeyframeEffectReadOnly::Constructor(
   RefPtr<KeyframeEffectReadOnly> effect =
     new KeyframeEffectReadOnly(aTarget->OwnerDoc(), aTarget,
                                nsCSSPseudoElements::ePseudo_NotPseudoElement,
-                               timing);
+                               aTiming);
   effect->mProperties = Move(animationProperties);
   return effect.forget();
 }

dom/animation/KeyframeEffect.h

@@ -191,7 +191,21 @@ public:
               Element* aTarget,
               JS::Handle<JSObject*> aFrames,
               const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions,
+              ErrorResult& aRv)
+  {
+    return Constructor(aGlobal, aTarget, aFrames,
+                       TimingParams::FromOptionsUnion(aOptions), aRv);
+  }
+
+  // More generalized version for Animatable.animate.
+  // Not exposed to content.
+  static already_AddRefed<KeyframeEffectReadOnly>
+  Constructor(const GlobalObject& aGlobal,
+              Element* aTarget,
+              JS::Handle<JSObject*> aFrames,
+              const TimingParams& aTiming,
               ErrorResult& aRv);
+
   Element* GetTarget() const {
     // Currently we never return animations from the API whose effect
     // targets a pseudo-element so this should never be called when
@@ -333,9 +347,6 @@ protected:
   // owning Animation's timing.
   void UpdateTargetRegistration();
 
-  static TimingParams ConvertKeyframeEffectOptions(
-    const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions);
-
   static void BuildAnimationPropertyList(
     JSContext* aCx,
     Element* aTarget,