Bug 1246320 part 3 - Rework KeyframeEffect(ReadOnly) constructor helpers; r=hiro
authorBrian Birtles <birtles@gmail.com>
Fri, 11 Mar 2016 17:27:16 +0900
changeset 288543 f7d44886eff3de4a8dc35ff1878c5c53dc3a6100
parent 288542 4ef42bdeeabb29139d828ef93af56a0a5ffa8c08
child 288544 e56bc90e163def00e9703a886bec10b200465687
push id30084
push userkwierso@gmail.com
push dateTue, 15 Mar 2016 00:39:07 +0000
treeherdermozilla-central@422077f61bcb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1246320
milestone48.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 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
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -647,23 +647,43 @@ KeyframeEffectReadOnly::SetIsRunningOnCo
     }
   }
 }
 
 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.
     aRv.Throw(NS_ERROR_DOM_ANIM_NO_TARGET_ERR);
     return nullptr;
   }
 
   const ElementOrCSSPseudoElement& target = aTarget.Value();
@@ -694,34 +714,16 @@ KeyframeEffectReadOnly::ConstructKeyfram
 
   RefPtr<KeyframeEffectType> effect =
     new KeyframeEffectType(targetElement->OwnerDoc(), targetElement,
                            pseudoType, aTiming);
   effect->mProperties = Move(animationProperties);
   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()
 {
   for (AnimationProperty& property : mProperties) {
     property.mIsRunningOnCompositor = false;
   }
 }
 
@@ -1749,16 +1751,29 @@ KeyframeEffectReadOnly::BuildAnimationPr
   } else {
     BuildAnimationPropertyListFromPropertyIndexedKeyframes(aCx, aTarget,
                                                            aPseudoType,
                                                            objectValue, aResult,
                                                            aRv);
   }
 }
 
+/* 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
 {
   if (!mTarget) {
     aRv.SetNull();
     return;
   }
@@ -2207,16 +2222,40 @@ KeyframeEffect::KeyframeEffect(nsIDocume
 
 JSObject*
 KeyframeEffect::WrapObject(JSContext* aCx,
                            JS::Handle<JSObject*> aGivenProto)
 {
   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;
   // Bug 1249219:
   // We don't support animation mutation observers on pseudo-elements yet.
   if (mTarget &&
       mPseudoType == CSSPseudoElementType::NotPseudo) {
     doc = mTarget->OwnerDoc();
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -197,26 +197,17 @@ public:
   }
 
   // KeyframeEffectReadOnly interface
   static already_AddRefed<KeyframeEffectReadOnly>
   Constructor(const GlobalObject& aGlobal,
               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,
                  nsTArray<JSObject*>& aResult,
                  ErrorResult& aRv);
 
   // Temporary workaround to return both the target element and pseudo-type
   // until we implement PseudoElement (bug 1174575).
@@ -349,17 +340,24 @@ public:
 protected:
   KeyframeEffectReadOnly(nsIDocument* aDocument,
                          Element* aTarget,
                          CSSPseudoElementType aPseudoType,
                          AnimationEffectTimingReadOnly* aTiming);
 
   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,
                           JS::Handle<JSObject*> aFrames,
                           const TimingParams& aTiming,
                           ErrorResult& aRv);
 
   void ResetIsRunningOnCompositor();
@@ -428,39 +426,26 @@ public:
   JSObject* WrapObject(JSContext* aCx,
                        JS::Handle<JSObject*> aGivenProto) override;
 
   static already_AddRefed<KeyframeEffect>
   Constructor(const GlobalObject& aGlobal,
               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();
 
 protected:
   ~KeyframeEffect() override;
 };
 
 } // namespace dom