Bug 1246320 part 5 - Simplify KeyframeEffect(ReadOnly) Constructor overloads further; r=hiro
authorBrian Birtles <birtles@gmail.com>
Sat, 12 Mar 2016 22:14:10 +0900
changeset 288545 5b2891e4650c9b2a1bd5dc83703b19d4a4f681f5
parent 288544 e56bc90e163def00e9703a886bec10b200465687
child 288546 a811164e762da26f4d1cdcc7221b9a2e914f0609
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 5 - Simplify KeyframeEffect(ReadOnly) Constructor overloads further; r=hiro As well as generally simplifying the different KeyframeEffect(ReadOnly) constructor methods, this patch also means we will use the realm document for parsing timing functions in all cases. Although this currently doesn't have any impact (the current set of timing functions are expected to be parsed identically regardless of the document used) it may become significant if, in future, it becomes possible to register hooks with certain documents for parsing CSS properties as part of the houdini efforts. MozReview-Commit-ID: 4gAZi1G1uAD
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
dom/base/Element.cpp
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/dom/KeyframeEffect.h"
 
+#include "mozilla/dom/AnimatableBinding.h"
 #include "mozilla/dom/KeyframeEffectBinding.h"
 #include "mozilla/dom/PropertyIndexedKeyframesBinding.h"
 #include "mozilla/AnimationUtils.h"
 #include "mozilla/EffectCompositor.h"
 #include "mozilla/FloatingPoint.h"
 #include "mozilla/LookAndFeel.h" // For LookAndFeel::GetInt
 #include "mozilla/StyleAnimationValue.h"
 #include "Layers.h" // For Layer
@@ -668,29 +669,16 @@ KeyframeEffectReadOnly::ConstructKeyfram
   }
 
   TimingParams timingParams =
     TimingParams::FromOptionsUnion(aOptions, doc, 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)
-{
   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();
   MOZ_ASSERT(target.IsElement() || target.IsCSSPseudoElement(),
@@ -715,17 +703,17 @@ KeyframeEffectReadOnly::ConstructKeyfram
                              aFrames, animationProperties, aRv);
 
   if (aRv.Failed()) {
     return nullptr;
   }
 
   RefPtr<KeyframeEffectType> effect =
     new KeyframeEffectType(targetElement->OwnerDoc(), targetElement,
-                           pseudoType, aTiming);
+                           pseudoType, timingParams);
   effect->mProperties = Move(animationProperties);
   return effect.forget();
 }
 
 void
 KeyframeEffectReadOnly::ResetIsRunningOnCompositor()
 {
   for (AnimationProperty& property : mProperties) {
@@ -2245,21 +2233,21 @@ KeyframeEffect::Constructor(
                                                  aOptions, aRv);
 }
 
 /* static */ already_AddRefed<KeyframeEffect>
 KeyframeEffect::Constructor(
     const GlobalObject& aGlobal,
     const Nullable<ElementOrCSSPseudoElement>& aTarget,
     JS::Handle<JSObject*> aFrames,
-    const TimingParams& aTiming,
+    const UnrestrictedDoubleOrKeyframeAnimationOptions& aOptions,
     ErrorResult& aRv)
 {
   return ConstructKeyframeEffect<KeyframeEffect>(aGlobal, aTarget, aFrames,
-                                                 aTiming, aRv);
+                                                 aOptions, aRv);
 }
 
 void KeyframeEffect::NotifySpecifiedTimingUpdated()
 {
   nsIDocument* doc = nullptr;
   // Bug 1249219:
   // We don't support animation mutation observers on pseudo-elements yet.
   if (mTarget &&
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -37,16 +37,17 @@ class nsPresContext;
 namespace mozilla {
 
 class AnimValuesStyleRule;
 enum class CSSPseudoElementType : uint8_t;
 
 namespace dom {
 class ElementOrCSSPseudoElement;
 class OwningElementOrCSSPseudoElement;
+class UnrestrictedDoubleOrKeyframeAnimationOptions;
 class UnrestrictedDoubleOrKeyframeEffectOptions;
 enum class IterationCompositeOperation : uint32_t;
 enum class CompositeOperation : uint32_t;
 struct AnimationPropertyState;
 }
 
 /**
  * Stores the results of calculating the timing properties of an animation
@@ -347,23 +348,16 @@ protected:
 
   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();
 
   // This effect is registered with its target element so long as:
   //
   // (a) It has a target element, and
   // (b) It is "relevant" (i.e. yet to finish but not idle, or finished but
   //     filling forwards)
@@ -428,23 +422,24 @@ public:
 
   static already_AddRefed<KeyframeEffect>
   Constructor(const GlobalObject& aGlobal,
               const Nullable<ElementOrCSSPseudoElement>& aTarget,
               JS::Handle<JSObject*> aFrames,
               const UnrestrictedDoubleOrKeyframeEffectOptions& aOptions,
               ErrorResult& aRv);
 
-  // More generalized version of Constructor for Animatable.animate.
+  // Variant of Constructor that accepts a KeyframeAnimationOptions object
+  // for use with for Animatable.animate.
   // Not exposed to content.
   static already_AddRefed<KeyframeEffect>
   Constructor(const GlobalObject& aGlobal,
               const Nullable<ElementOrCSSPseudoElement>& aTarget,
               JS::Handle<JSObject*> aFrames,
-              const TimingParams& aTiming,
+              const UnrestrictedDoubleOrKeyframeAnimationOptions& aOptions,
               ErrorResult& aRv);
 
   void NotifySpecifiedTimingUpdated();
 
 protected:
   ~KeyframeEffect() override;
 };
 
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -3348,25 +3348,18 @@ Element::Animate(const Nullable<ElementO
   if (js::GetContextCompartment(aContext) !=
       js::GetObjectCompartment(ownerGlobal->GetGlobalJSObject())) {
     ac.emplace(aContext, ownerGlobal->GetGlobalJSObject());
     if (!JS_WrapObject(aContext, &frames)) {
       return nullptr;
     }
   }
 
-  TimingParams timingParams =
-    TimingParams::FromOptionsUnion(aOptions, referenceElement->OwnerDoc(),
-                                   aError);
-  if (aError.Failed()) {
-    return nullptr;
-  }
-
   RefPtr<KeyframeEffect> effect =
-    KeyframeEffect::Constructor(global, aTarget, frames, timingParams, aError);
+    KeyframeEffect::Constructor(global, aTarget, frames, aOptions, aError);
   if (aError.Failed()) {
     return nullptr;
   }
 
   RefPtr<Animation> animation =
     Animation::Constructor(global, effect,
                            referenceElement->OwnerDoc()->Timeline(), aError);
   if (aError.Failed()) {