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

@@ -51,14 +51,18 @@ Animation::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 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());
   }
 

dom/animation/Animation.h

@@ -238,11 +238,11 @@ public:
 
   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
   {

dom/animation/KeyframeEffect.cpp

@@ -109,7 +109,8 @@ namespace dom {
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(KeyframeEffectReadOnly,
                                    AnimationEffectReadOnly,
-                                   mTarget)
+                                   mTarget,
+                                   mAnimation)
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(KeyframeEffectReadOnly,
                                                AnimationEffectReadOnly)
@@ -149,14 +150,15 @@ KeyframeEffectReadOnly::SetParentTime(Nullable<TimeDuration> 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
@@ -306,9 +308,9 @@ KeyframeEffectReadOnly::ActiveDuration(const AnimationTiming& aTiming)
 
 // 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;
   }
 
@@ -317,9 +319,9 @@ KeyframeEffectReadOnly::IsInPlay(const Animation& aAnimation) const
 
 // 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;
   }
 
@@ -336,6 +338,12 @@ KeyframeEffectReadOnly::IsInEffect() const
   return computedTiming.mProgress != ComputedTiming::kNullProgress;
 }
 
+void
+KeyframeEffectReadOnly::SetAnimation(Animation* aAnimation)
+{
+  mAnimation = aAnimation;
+}
+
 const AnimationProperty*
 KeyframeEffectReadOnly::GetAnimationOfProperty(nsCSSProperty aProperty) const
 {
@@ -495,6 +503,12 @@ KeyframeEffectReadOnly::SetIsRunningOnCompositor(nsCSSProperty aProperty,
   }
 }
 
+// We need to define this here since Animation is an incomplete type
+// (forward-declared) in the header.
+KeyframeEffectReadOnly::~KeyframeEffectReadOnly()
+{
+}
+
 void
 KeyframeEffectReadOnly::ResetIsRunningOnCompositor()
 {

dom/animation/KeyframeEffect.h

@@ -202,6 +202,8 @@ struct ElementPropertyTransition;
 
 namespace dom {
 
+class Animation;
+
 class KeyframeEffectReadOnly : public AnimationEffectReadOnly
 {
 public:
@@ -253,10 +255,7 @@ public:
   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
@@ -289,10 +288,12 @@ public:
   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 {
@@ -308,8 +309,8 @@ public:
   }
 
   // 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);
@@ -317,10 +318,11 @@ public:
   void SetIsRunningOnCompositor(nsCSSProperty aProperty, bool aIsRunning);
 
 protected:
-  virtual ~KeyframeEffectReadOnly() { }
+  virtual ~KeyframeEffectReadOnly();
   void ResetIsRunningOnCompositor();
 
   nsCOMPtr<Element> mTarget;
+  nsRefPtr<Animation> mAnimation;
   Nullable<TimeDuration> mParentTime;
 
   AnimationTiming mTiming;

layout/style/AnimationCommon.cpp

@@ -915,7 +915,7 @@ AnimationCollection::HasCurrentAnimationsForProperties(
     const Animation& anim = *mAnimations[animIdx];
     const KeyframeEffectReadOnly* effect = anim.GetEffect();
     if (effect &&
-        effect->IsCurrent(anim) &&
+        effect->IsCurrent() &&
         effect->HasAnimationOfProperties(aProperties, aPropertyCount)) {
       return true;
     }

layout/style/nsAnimationManager.cpp

@@ -505,7 +505,7 @@ nsAnimationManager::CheckAnimationRule(nsStyleContext* aStyleContext,
           animationChanged =
             oldEffect->Timing() != newEffect->Timing() ||
             oldEffect->Properties() != newEffect->Properties();
-          oldEffect->SetTiming(newEffect->Timing(), *oldAnim);
+          oldEffect->SetTiming(newEffect->Timing());
           oldEffect->Properties() = newEffect->Properties();
         }