Bug 1246320 part 3 - Rework KeyframeEffect(ReadOnly) constructor helpers; r=hiro

Once we update TimingParams to take a document, we will need to get an
appropriate document within the various constructor methods. This complicates
these methods and suggests they should be pushed into the .cpp file where
we can hide the complexity more easily and templatize the type of the options
argument so that we can share the document-fetching code.

By moving all uses of the declared template methods to the .cpp file we
can drop the explicit instantiations.

(We still need to declare the templated methods in the header file since
these methods need to be protected methods of KeyframeEffectReadOnly in
order to construct a KeyframeEffectReadOnly since its constructor is
protected.)

MozReview-Commit-ID: 8KrCWrWIb7X

--HG--
extra : rebase_source : c5b550b271cc68ceeb60a25243268a17b3ab7f65

dom/animation/KeyframeEffect.cpp

@@ -652,13 +652,33 @@ KeyframeEffectReadOnly::~KeyframeEffectReadOnly()
 {
 }
 
+template <class KeyframeEffectType, class OptionsType>
+/* static */ already_AddRefed<KeyframeEffectType>
+KeyframeEffectReadOnly::ConstructKeyframeEffect(
+    const GlobalObject& aGlobal,
+    const Nullable<ElementOrCSSPseudoElement>& aTarget,
+    JS::Handle<JSObject*> aFrames,
+    const OptionsType& aOptions,
+    ErrorResult& aRv)
+{
+  TimingParams timingParams =
+    TimingParams::FromOptionsUnion(aOptions, aTarget, aRv);
+  if (aRv.Failed()) {
+    return nullptr;
+  }
+
+  return ConstructKeyframeEffect<KeyframeEffectType>(
+    aGlobal, aTarget, aFrames, timingParams, aRv);
+}
+
 template <class KeyframeEffectType>
 /* static */ already_AddRefed<KeyframeEffectType>
-KeyframeEffectReadOnly::ConstructKeyframeEffect(const GlobalObject& aGlobal,
-                                                const Nullable<ElementOrCSSPseudoElement>& aTarget,
-                                                JS::Handle<JSObject*> aFrames,
-                                                const TimingParams& aTiming,
-                                                ErrorResult& aRv)
+KeyframeEffectReadOnly::ConstructKeyframeEffect(
+    const GlobalObject& aGlobal,
+    const Nullable<ElementOrCSSPseudoElement>& aTarget,
+    JS::Handle<JSObject*> aFrames,
+    const TimingParams& aTiming,
+    ErrorResult& aRv)
 {
   if (aTarget.IsNull()) {
     // We don't support null targets yet.
@@ -699,24 +719,6 @@ KeyframeEffectReadOnly::ConstructKeyframeEffect(const GlobalObject& aGlobal,
   return effect.forget();
 }
 
-// Explicit instantiations to avoid linker errors.
-
-template
-already_AddRefed<KeyframeEffectReadOnly>
-KeyframeEffectReadOnly::ConstructKeyframeEffect<>(const GlobalObject& aGlobal,
-                                                  const Nullable<ElementOrCSSPseudoElement>& aTarget,
-                                                  JS::Handle<JSObject*> aFrames,
-                                                  const TimingParams& aTiming,
-                                                  ErrorResult& aRv);
-
-template
-already_AddRefed<KeyframeEffect>
-KeyframeEffectReadOnly::ConstructKeyframeEffect<>(const GlobalObject& aGlobal,
-                                                  const Nullable<ElementOrCSSPseudoElement>& aTarget,
-                                                  JS::Handle<JSObject*> aFrames,
-                                                  const TimingParams& aTiming,
-                                                  ErrorResult& aRv);
-
 void
 KeyframeEffectReadOnly::ResetIsRunningOnCompositor()
 {
@@ -1754,6 +1756,19 @@ KeyframeEffectReadOnly::BuildAnimationPropertyList(
   }
 }
 
+/* static */ already_AddRefed<KeyframeEffectReadOnly>
+KeyframeEffectReadOnly::Constructor(
+    const GlobalObject& aGlobal,
+    const Nullable<ElementOrCSSPseudoElement>& aTarget,
+    JS::Handle<JSObject*> aFrames,
+    const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions,
+    ErrorResult& aRv)
+{
+  return ConstructKeyframeEffect<KeyframeEffectReadOnly>(aGlobal, aTarget,
+                                                         aFrames, aOptions,
+                                                         aRv);
+}
+
 void
 KeyframeEffectReadOnly::GetTarget(
     Nullable<OwningElementOrCSSPseudoElement>& aRv) const
@@ -2212,6 +2227,30 @@ KeyframeEffect::WrapObject(JSContext* aCx,
   return KeyframeEffectBinding::Wrap(aCx, this, aGivenProto);
 }
 
+/* static */ already_AddRefed<KeyframeEffect>
+KeyframeEffect::Constructor(
+    const GlobalObject& aGlobal,
+    const Nullable<ElementOrCSSPseudoElement>& aTarget,
+    JS::Handle<JSObject*> aFrames,
+    const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions,
+    ErrorResult& aRv)
+{
+  return ConstructKeyframeEffect<KeyframeEffect>(aGlobal, aTarget, aFrames,
+                                                 aOptions, aRv);
+}
+
+/* static */ already_AddRefed<KeyframeEffect>
+KeyframeEffect::Constructor(
+    const GlobalObject& aGlobal,
+    const Nullable<ElementOrCSSPseudoElement>& aTarget,
+    JS::Handle<JSObject*> aFrames,
+    const TimingParams& aTiming,
+    ErrorResult& aRv)
+{
+  return ConstructKeyframeEffect<KeyframeEffect>(aGlobal, aTarget, aFrames,
+                                                 aTiming, aRv);
+}
+
 void KeyframeEffect::NotifySpecifiedTimingUpdated()
 {
   nsIDocument* doc = nullptr;

dom/animation/KeyframeEffect.h

@@ -202,16 +202,7 @@ public:
               const Nullable<ElementOrCSSPseudoElement>& aTarget,
               JS::Handle<JSObject*> aFrames,
               const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions,
-              ErrorResult& aRv)
-  {
-    TimingParams timingParams =
-      TimingParams::FromOptionsUnion(aOptions, aTarget, aRv);
-    if (aRv.Failed()) {
-      return nullptr;
-    }
-    return ConstructKeyframeEffect<KeyframeEffectReadOnly>(
-             aGlobal, aTarget, aFrames, timingParams, aRv);
-  }
+              ErrorResult& aRv);
 
   void GetTarget(Nullable<OwningElementOrCSSPseudoElement>& aRv) const;
   void GetFrames(JSContext*& aCx,
@@ -354,7 +345,14 @@ protected:
 
   virtual ~KeyframeEffectReadOnly();
 
-  template<typename KeyframeEffectType>
+  template<class KeyframeEffectType, class OptionsType>
+  static already_AddRefed<KeyframeEffectType>
+  ConstructKeyframeEffect(const GlobalObject& aGlobal,
+                          const Nullable<ElementOrCSSPseudoElement>& aTarget,
+                          JS::Handle<JSObject*> aFrames,
+                          const OptionsType& aOptions,
+                          ErrorResult& aRv);
+  template<class KeyframeEffectType>
   static already_AddRefed<KeyframeEffectType>
   ConstructKeyframeEffect(const GlobalObject& aGlobal,
                           const Nullable<ElementOrCSSPseudoElement>& aTarget,
@@ -433,29 +431,16 @@ public:
               const Nullable<ElementOrCSSPseudoElement>& aTarget,
               JS::Handle<JSObject*> aFrames,
               const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions,
-              ErrorResult& aRv)
-  {
-    TimingParams timingParams =
-      TimingParams::FromOptionsUnion(aOptions, aTarget, aRv);
-    if (aRv.Failed()) {
-      return nullptr;
-    }
-    return ConstructKeyframeEffect<KeyframeEffect>(
-      aGlobal, aTarget, aFrames, timingParams, aRv);
-  }
+              ErrorResult& aRv);
 
-  // More generalized version for Animatable.animate.
+  // More generalized version of Constructor for Animatable.animate.
   // Not exposed to content.
   static already_AddRefed<KeyframeEffect>
-  inline Constructor(const GlobalObject& aGlobal,
-                     const Nullable<ElementOrCSSPseudoElement>& aTarget,
-                     JS::Handle<JSObject*> aFrames,
-                     const TimingParams& aTiming,
-                     ErrorResult& aRv)
-  {
-    return ConstructKeyframeEffect<KeyframeEffect>(aGlobal, aTarget, aFrames,
-                                                   aTiming, aRv);
-  }
+  Constructor(const GlobalObject& aGlobal,
+              const Nullable<ElementOrCSSPseudoElement>& aTarget,
+              JS::Handle<JSObject*> aFrames,
+              const TimingParams& aTiming,
+              ErrorResult& aRv);
 
   void NotifySpecifiedTimingUpdated();