Bug 1208385 part 1 - Store a pointer to the owning animation on each KeyframeEffect; r=heycam
authorBrian Birtles <birtles@gmail.com>
Wed, 07 Oct 2015 14:30:27 +0900
changeset 299381 8770acc7fd9ced0c3ae5c1de72ee423cd094b2a5
parent 299380 c51389e092304761ccfa882400c214e86fbc3123
child 299382 ee3890805a7785d28254b3ba3fae679bd8598553
push id6207
push userjryans@gmail.com
push dateWed, 07 Oct 2015 19:27:34 +0000
reviewersheycam
bugs1208385, 1166500, 1190235
milestone44.0a1
Bug 1208385 part 1 - Store a pointer to the owning animation on each KeyframeEffect; r=heycam We need to do this so effects can query their owning animation for the current time and avoid falling out of sync. Furthermore, this pointer is needed for a number of other bugs (e.g. bug 1166500 comment 12, or bug 1190235) anyway.
dom/animation/Animation.cpp
dom/animation/Animation.h
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
layout/style/AnimationCommon.cpp
layout/style/nsAnimationManager.cpp
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -46,24 +46,28 @@ Animation::WrapObject(JSContext* aCx, JS
 //
 // Animation interface:
 //
 // ---------------------------------------------------------------------------
 
 void
 Animation::SetEffect(KeyframeEffectReadOnly* aEffect)
 {
+  nsRefPtr<Animation> kungFuDeathGrip(this);
+
   if (mEffect == aEffect) {
     return;
   }
   if (mEffect) {
+    mEffect->SetAnimation(nullptr);
     mEffect->SetParentTime(Nullable<TimeDuration>());
   }
   mEffect = aEffect;
   if (mEffect) {
+    mEffect->SetAnimation(this);
     mEffect->SetParentTime(GetCurrentTime());
   }
 
   UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 }
 
 void
 Animation::SetTimeline(AnimationTimeline* aTimeline)
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -233,21 +233,21 @@ public:
   bool IsPausedOrPausing() const
   {
     return PlayState() == AnimationPlayState::Paused ||
            mPendingState == PendingState::PausePending;
   }
 
   bool HasInPlayEffect() const
   {
-    return GetEffect() && GetEffect()->IsInPlay(*this);
+    return GetEffect() && GetEffect()->IsInPlay();
   }
   bool HasCurrentEffect() const
   {
-    return GetEffect() && GetEffect()->IsCurrent(*this);
+    return GetEffect() && GetEffect()->IsCurrent();
   }
   bool IsInEffect() const
   {
     return GetEffect() && GetEffect()->IsInEffect();
   }
 
   /**
    * "Playing" is different to "running". An animation in its delay phase is
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -104,17 +104,18 @@ ComputedTimingFunction::AppendToString(n
 // In the Web Animations model, the iteration progress can be outside the range
 // [0.0, 1.0] but it shouldn't be Infinity.
 const double ComputedTiming::kNullProgress = PositiveInfinity<double>();
 
 namespace dom {
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(KeyframeEffectReadOnly,
                                    AnimationEffectReadOnly,
-                                   mTarget)
+                                   mTarget,
+                                   mAnimation)
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(KeyframeEffectReadOnly,
                                                AnimationEffectReadOnly)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(KeyframeEffectReadOnly)
 NS_INTERFACE_MAP_END_INHERITING(AnimationEffectReadOnly)
 
@@ -144,24 +145,25 @@ KeyframeEffectReadOnly::WrapObject(JSCon
 
 void
 KeyframeEffectReadOnly::SetParentTime(Nullable<TimeDuration> aParentTime)
 {
   mParentTime = aParentTime;
 }
 
 void
-KeyframeEffectReadOnly::SetTiming(const AnimationTiming& aTiming,
-                                  Animation& aOwningAnimation)
+KeyframeEffectReadOnly::SetTiming(const AnimationTiming& aTiming)
 {
   if (mTiming == aTiming) {
     return;
   }
   mTiming = aTiming;
-  aOwningAnimation.NotifyEffectTimingUpdated();
+  if (mAnimation) {
+    mAnimation->NotifyEffectTimingUpdated();
+  }
 }
 
 ComputedTiming
 KeyframeEffectReadOnly::GetComputedTimingAt(
                           const Nullable<TimeDuration>& aLocalTime,
                           const AnimationTiming& aTiming)
 {
   const TimeDuration zeroDuration;
@@ -301,46 +303,52 @@ KeyframeEffectReadOnly::ActiveDuration(c
            : StickyTimeDuration::Forever();
   }
   return StickyTimeDuration(
     aTiming.mIterationDuration.MultDouble(aTiming.mIterationCount));
 }
 
 // https://w3c.github.io/web-animations/#in-play
 bool
-KeyframeEffectReadOnly::IsInPlay(const Animation& aAnimation) const
+KeyframeEffectReadOnly::IsInPlay() const
 {
-  if (aAnimation.PlayState() == AnimationPlayState::Finished) {
+  if (!mAnimation || mAnimation->PlayState() == AnimationPlayState::Finished) {
     return false;
   }
 
   return GetComputedTiming().mPhase == ComputedTiming::AnimationPhase_Active;
 }
 
 // https://w3c.github.io/web-animations/#current
 bool
-KeyframeEffectReadOnly::IsCurrent(const Animation& aAnimation) const
+KeyframeEffectReadOnly::IsCurrent() const
 {
-  if (aAnimation.PlayState() == AnimationPlayState::Finished) {
+  if (!mAnimation || mAnimation->PlayState() == AnimationPlayState::Finished) {
     return false;
   }
 
   ComputedTiming computedTiming = GetComputedTiming();
   return computedTiming.mPhase == ComputedTiming::AnimationPhase_Before ||
          computedTiming.mPhase == ComputedTiming::AnimationPhase_Active;
 }
 
 // https://w3c.github.io/web-animations/#in-effect
 bool
 KeyframeEffectReadOnly::IsInEffect() const
 {
   ComputedTiming computedTiming = GetComputedTiming();
   return computedTiming.mProgress != ComputedTiming::kNullProgress;
 }
 
+void
+KeyframeEffectReadOnly::SetAnimation(Animation* aAnimation)
+{
+  mAnimation = aAnimation;
+}
+
 const AnimationProperty*
 KeyframeEffectReadOnly::GetAnimationOfProperty(nsCSSProperty aProperty) const
 {
   for (size_t propIdx = 0, propEnd = mProperties.Length();
        propIdx != propEnd; ++propIdx) {
     if (aProperty == mProperties[propIdx].mProperty) {
       const AnimationProperty* result = &mProperties[propIdx];
       if (!result->mWinsInCascade) {
@@ -490,16 +498,22 @@ KeyframeEffectReadOnly::SetIsRunningOnCo
   for (size_t i = 0; i < ArrayLength(mIsPropertyRunningOnCompositor); i++) {
     if (info[i].mProperty == aProperty) {
       mIsPropertyRunningOnCompositor[i] = aIsRunning;
       return;
     }
   }
 }
 
+// We need to define this here since Animation is an incomplete type
+// (forward-declared) in the header.
+KeyframeEffectReadOnly::~KeyframeEffectReadOnly()
+{
+}
+
 void
 KeyframeEffectReadOnly::ResetIsRunningOnCompositor()
 {
   for (bool& isPropertyRunningOnCompositor : mIsPropertyRunningOnCompositor) {
     isPropertyRunningOnCompositor = false;
   }
 }
 
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -197,16 +197,18 @@ struct AnimationProperty
     return !(*this == aOther);
   }
 };
 
 struct ElementPropertyTransition;
 
 namespace dom {
 
+class Animation;
+
 class KeyframeEffectReadOnly : public AnimationEffectReadOnly
 {
 public:
   KeyframeEffectReadOnly(nsIDocument* aDocument,
                          Element* aTarget,
                          nsCSSPseudoElements::Type aPseudoType,
                          const AnimationTiming& aTiming);
 
@@ -248,20 +250,17 @@ public:
   void SetParentTime(Nullable<TimeDuration> aParentTime);
 
   const AnimationTiming& Timing() const {
     return mTiming;
   }
   AnimationTiming& Timing() {
     return mTiming;
   }
-
-  // FIXME: Drop |aOwningAnimation| once we make AnimationEffects track their
-  // owning animation.
-  void SetTiming(const AnimationTiming& aTiming, Animation& aOwningAnimtion);
+  void SetTiming(const AnimationTiming& aTiming);
 
   Nullable<TimeDuration> GetLocalTime() const {
     // Since the *animation* start time is currently always zero, the local
     // time is equal to the parent time.
     return mParentTime;
   }
 
   // This function takes as input the timing parameters of an animation and
@@ -284,48 +283,51 @@ public:
                                      = nullptr) const {
     return GetComputedTimingAt(GetLocalTime(), aTiming ? *aTiming : mTiming);
   }
 
   // Return the duration of the active interval for the given timing parameters.
   static StickyTimeDuration
   ActiveDuration(const AnimationTiming& aTiming);
 
-  bool IsInPlay(const Animation& aAnimation) const;
-  bool IsCurrent(const Animation& aAnimation) const;
+  bool IsInPlay() const;
+  bool IsCurrent() const;
   bool IsInEffect() const;
 
+  void SetAnimation(Animation* aAnimation);
+
   const AnimationProperty*
   GetAnimationOfProperty(nsCSSProperty aProperty) const;
   bool HasAnimationOfProperty(nsCSSProperty aProperty) const {
     return GetAnimationOfProperty(aProperty) != nullptr;
   }
   bool HasAnimationOfProperties(const nsCSSProperty* aProperties,
                                 size_t aPropertyCount) const;
   const InfallibleTArray<AnimationProperty>& Properties() const {
     return mProperties;
   }
   InfallibleTArray<AnimationProperty>& Properties() {
     return mProperties;
   }
 
   // Updates |aStyleRule| with the animation values produced by this
-  // Animation for the current time except any properties already contained
-  // in |aSetProperties|.
+  // AnimationEffect for the current time except any properties already
+  // contained in |aSetProperties|.
   // Any updated properties are added to |aSetProperties|.
   void ComposeStyle(nsRefPtr<AnimValuesStyleRule>& aStyleRule,
                     nsCSSPropertySet& aSetProperties);
   bool IsRunningOnCompositor() const;
   void SetIsRunningOnCompositor(nsCSSProperty aProperty, bool aIsRunning);
 
 protected:
-  virtual ~KeyframeEffectReadOnly() { }
+  virtual ~KeyframeEffectReadOnly();
   void ResetIsRunningOnCompositor();
 
   nsCOMPtr<Element> mTarget;
+  nsRefPtr<Animation> mAnimation;
   Nullable<TimeDuration> mParentTime;
 
   AnimationTiming mTiming;
   nsCSSPseudoElements::Type mPseudoType;
 
   InfallibleTArray<AnimationProperty> mProperties;
 
   // Parallel array corresponding to CommonAnimationManager::sLayerAnimationInfo
--- a/layout/style/AnimationCommon.cpp
+++ b/layout/style/AnimationCommon.cpp
@@ -910,17 +910,17 @@ bool
 AnimationCollection::HasCurrentAnimationsForProperties(
                               const nsCSSProperty* aProperties,
                               size_t aPropertyCount) const
 {
   for (size_t animIdx = mAnimations.Length(); animIdx-- != 0; ) {
     const Animation& anim = *mAnimations[animIdx];
     const KeyframeEffectReadOnly* effect = anim.GetEffect();
     if (effect &&
-        effect->IsCurrent(anim) &&
+        effect->IsCurrent() &&
         effect->HasAnimationOfProperties(aProperties, aPropertyCount)) {
       return true;
     }
   }
 
   return false;
 }
 
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -500,17 +500,17 @@ nsAnimationManager::CheckAnimationRule(n
         // Update the old from the new so we can keep the original object
         // identity (and any expando properties attached to it).
         if (oldAnim->GetEffect() && newAnim->GetEffect()) {
           KeyframeEffectReadOnly* oldEffect = oldAnim->GetEffect();
           KeyframeEffectReadOnly* newEffect = newAnim->GetEffect();
           animationChanged =
             oldEffect->Timing() != newEffect->Timing() ||
             oldEffect->Properties() != newEffect->Properties();
-          oldEffect->SetTiming(newEffect->Timing(), *oldAnim);
+          oldEffect->SetTiming(newEffect->Timing());
           oldEffect->Properties() = newEffect->Properties();
         }
 
         // Handle changes in play state. If the animation is idle, however,
         // changes to animation-play-state should *not* restart it.
         if (oldAnim->PlayState() != AnimationPlayState::Idle) {
           // CSSAnimation takes care of override behavior so that,
           // for example, if the author has called pause(), that will