Bug 1474766 - Merge the three-argument form of KeyframeEffect::Constructor and ConstructKeyframeEffect; r?hiro draft
authorBrian Birtles <birtles@gmail.com>
Wed, 11 Jul 2018 10:27:56 +0900
changeset 816360 f33d654b4f1b1114bdafb15f710ded6076592182
parent 816359 e5e31bd1321e1653479b1568c712ef897bffe690
child 817233 154aae5387ba78a5bec4eba53af1deec1dbffd2b
push id115813
push userbmo:bbirtles@mozilla.com
push dateWed, 11 Jul 2018 02:02:41 +0000
reviewershiro
bugs1474766
milestone63.0a1
Bug 1474766 - Merge the three-argument form of KeyframeEffect::Constructor and ConstructKeyframeEffect; r?hiro There doesn't appear to be any need to separate these anymore. MozReview-Commit-ID: GHR259JJHJV
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -674,50 +674,16 @@ KeyframeEffect::ConstructKeyframeEffect(
   effect->SetKeyframes(aGlobal.Context(), aKeyframes, aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   return effect.forget();
 }
 
-/* static */ already_AddRefed<KeyframeEffect>
-KeyframeEffect::ConstructKeyframeEffect(const GlobalObject& aGlobal,
-                                        KeyframeEffect& aSource,
-                                        ErrorResult& aRv)
-{
-  nsIDocument* doc = AnimationUtils::GetCurrentRealmDocument(aGlobal.Context());
-  if (!doc) {
-    aRv.Throw(NS_ERROR_FAILURE);
-    return nullptr;
-  }
-
-  // Create a new KeyframeEffect object with aSource's target,
-  // iteration composite operation, composite operation, and spacing mode.
-  // The constructor creates a new AnimationEffect object by
-  // aSource's TimingParams.
-  // Note: we don't need to re-throw exceptions since the value specified on
-  //       aSource's timing object can be assumed valid.
-  RefPtr<KeyframeEffect> effect =
-    new KeyframeEffect(doc,
-                       aSource.mTarget,
-                       aSource.SpecifiedTiming(),
-                       aSource.mEffectOptions);
-  // Copy cumulative change hint. mCumulativeChangeHint should be the same as
-  // the source one because both of targets are the same.
-  effect->mCumulativeChangeHint = aSource.mCumulativeChangeHint;
-
-  // Copy aSource's keyframes and animation properties.
-  // Note: We don't call SetKeyframes directly, which might revise the
-  //       computed offsets and rebuild the animation properties.
-  effect->mKeyframes = aSource.mKeyframes;
-  effect->mProperties = aSource.mProperties;
-  return effect.forget();
-}
-
 nsTArray<AnimationProperty>
 KeyframeEffect::BuildProperties(const ComputedStyle* aStyle)
 {
 
   MOZ_ASSERT(aStyle);
 
   nsTArray<AnimationProperty> result;
   // If mTarget is null, return an empty property array.
@@ -880,34 +846,60 @@ KeyframeEffect::Constructor(
     JS::Handle<JSObject*> aKeyframes,
     const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions,
     ErrorResult& aRv)
 {
   return ConstructKeyframeEffect(aGlobal, aTarget, aKeyframes, aOptions, aRv);
 }
 
 /* static */ already_AddRefed<KeyframeEffect>
-KeyframeEffect::Constructor(const GlobalObject& aGlobal,
-                            KeyframeEffect& aSource,
-                            ErrorResult& aRv)
-{
-  return ConstructKeyframeEffect(aGlobal, aSource, aRv);
-}
-
-/* static */ already_AddRefed<KeyframeEffect>
 KeyframeEffect::Constructor(
     const GlobalObject& aGlobal,
     const Nullable<ElementOrCSSPseudoElement>& aTarget,
     JS::Handle<JSObject*> aKeyframes,
     const UnrestrictedDoubleOrKeyframeAnimationOptions& aOptions,
     ErrorResult& aRv)
 {
   return ConstructKeyframeEffect(aGlobal, aTarget, aKeyframes, aOptions, aRv);
 }
 
+/* static */ already_AddRefed<KeyframeEffect>
+KeyframeEffect::Constructor(const GlobalObject& aGlobal,
+                            KeyframeEffect& aSource,
+                            ErrorResult& aRv)
+{
+  nsIDocument* doc = AnimationUtils::GetCurrentRealmDocument(aGlobal.Context());
+  if (!doc) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return nullptr;
+  }
+
+  // Create a new KeyframeEffect object with aSource's target,
+  // iteration composite operation, composite operation, and spacing mode.
+  // The constructor creates a new AnimationEffect object by
+  // aSource's TimingParams.
+  // Note: we don't need to re-throw exceptions since the value specified on
+  //       aSource's timing object can be assumed valid.
+  RefPtr<KeyframeEffect> effect =
+    new KeyframeEffect(doc,
+                       aSource.mTarget,
+                       aSource.SpecifiedTiming(),
+                       aSource.mEffectOptions);
+  // Copy cumulative change hint. mCumulativeChangeHint should be the same as
+  // the source one because both of targets are the same.
+  effect->mCumulativeChangeHint = aSource.mCumulativeChangeHint;
+
+  // Copy aSource's keyframes and animation properties.
+  // Note: We don't call SetKeyframes directly, which might revise the
+  //       computed offsets and rebuild the animation properties.
+  effect->mKeyframes = aSource.mKeyframes;
+  effect->mProperties = aSource.mProperties;
+  return effect.forget();
+}
+
 void
 KeyframeEffect::GetTarget(Nullable<OwningElementOrCSSPseudoElement>& aRv) const
 {
   if (!mTarget) {
     aRv.SetNull();
     return;
   }
 
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -308,21 +308,16 @@ protected:
   template<class OptionsType>
   static already_AddRefed<KeyframeEffect>
   ConstructKeyframeEffect(const GlobalObject& aGlobal,
                           const Nullable<ElementOrCSSPseudoElement>& aTarget,
                           JS::Handle<JSObject*> aKeyframes,
                           const OptionsType& aOptions,
                           ErrorResult& aRv);
 
-  static already_AddRefed<KeyframeEffect>
-  ConstructKeyframeEffect(const GlobalObject& aGlobal,
-                          KeyframeEffect& aSource,
-                          ErrorResult& aRv);
-
   // Build properties by recalculating from |mKeyframes| using |aComputedStyle|
   // to resolve specified values. This function also applies paced spacing if
   // needed.
   nsTArray<AnimationProperty> BuildProperties(const ComputedStyle* aStyle);
 
   // This effect is registered with its target element so long as:
   //
   // (a) It has a target element, and