Bug 1332958 - Do not call Animation::PostUpdate() if the CSS animation is newly created. r?heycam draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Mon, 23 Jan 2017 10:40:39 +0900
changeset 464834 d93b3aa2bddc8fa010037d7ea4cb6b857aa51d1f
parent 464800 bd0cd9af94d9334b862d9891013fed56fb9b3b7c
child 543005 ec4525a4779446f36b51bc91fa9e2537196766d4
push id42445
push userhikezoe@mozilla.com
push dateMon, 23 Jan 2017 01:46:26 +0000
reviewersheycam
bugs1332958
milestone53.0a1
Bug 1332958 - Do not call Animation::PostUpdate() if the CSS animation is newly created. r?heycam MozReview-Commit-ID: 2UX8cax0o4N
dom/animation/Animation.h
layout/style/nsAnimationManager.cpp
layout/style/nsAnimationManager.h
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -328,16 +328,18 @@ public:
    * where the Animation may have been cancelled.
    *
    * We need to do this synchronously because after a CSS animation/transition
    * is canceled, it will be released by its owning element and may not still
    * exist when we would normally go to queue events on the next tick.
    */
   virtual void MaybeQueueCancelEvent(StickyTimeDuration aActiveTime) {};
 
+  void PostUpdate();
+
 protected:
   void SilentlySetCurrentTime(const TimeDuration& aNewCurrentTime);
   void SilentlySetPlaybackRate(double aPlaybackRate);
   void CancelNoUpdate();
   void PlayNoUpdate(ErrorResult& aRv, LimitBehavior aLimitBehavior);
   void PauseNoUpdate(ErrorResult& aRv);
   void ResumeAt(const TimeDuration& aReadyTime);
   void PauseAt(const TimeDuration& aReadyTime);
@@ -367,17 +369,16 @@ protected:
   };
 
   virtual void UpdateTiming(SeekFlag aSeekFlag,
                             SyncNotifyFlag aSyncNotifyFlag);
   void UpdateFinishedState(SeekFlag aSeekFlag,
                            SyncNotifyFlag aSyncNotifyFlag);
   void UpdateEffect();
   void FlushStyle() const;
-  void PostUpdate();
   void ResetFinishedPromise();
   void MaybeResolveFinishedPromise();
   void DoFinishNotification(SyncNotifyFlag aSyncNotifyFlag);
   void DoFinishNotificationImmediately();
   void DispatchPlaybackEvent(const nsAString& aName);
 
   /**
    * Remove this animation from the pending animation tracker and reset
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -336,17 +336,18 @@ PopExistingAnimation(const nsAString& aN
 }
 
 static void
 UpdateOldAnimationPropertiesWithNew(
     CSSAnimation& aOld,
     TimingParams& aNewTiming,
     nsTArray<Keyframe>& aNewKeyframes,
     bool aNewIsStylePaused,
-    nsStyleContext* aStyleContext)
+    nsStyleContext* aStyleContext,
+    uint64_t aAnimationIndex)
 {
   bool animationChanged = false;
 
   // Update the old from the new so we can keep the original object
   // identity (and any expando properties attached to it).
   if (aOld.GetEffect()) {
     dom::AnimationEffectReadOnly* oldEffect = aOld.GetEffect();
     animationChanged = oldEffect->SpecifiedTiming() != aNewTiming;
@@ -375,18 +376,24 @@ UpdateOldAnimationPropertiesWithNew(
       aOld.PlayFromStyle();
       animationChanged = true;
     }
   }
 
   // Updating the effect timing above might already have caused the
   // animation to become irrelevant so only add a changed record if
   // the animation is still relevant.
-  if (animationChanged && aOld.IsRelevant()) {
-    nsNodeUtils::AnimationChanged(&aOld);
+  if (aOld.IsRelevant()) {
+    if (animationChanged) {
+      nsNodeUtils::AnimationChanged(&aOld);
+    }
+
+    if (aAnimationIndex != aOld.AnimationIndex()) {
+      aOld.PostUpdate();
+    }
   }
 }
 
 void
 nsAnimationManager::UpdateAnimations(nsStyleContext* aStyleContext,
                                      mozilla::dom::Element* aElement)
 {
   MOZ_ASSERT(mPresContext->IsDynamic(),
@@ -534,17 +541,18 @@ public:
 
   // Returns a new animation set up with given StyleAnimation and
   // keyframe rules.
   // Or returns an existing animation matching StyleAnimation's name updated
   // with the new StyleAnimation and keyframe rules.
   already_AddRefed<CSSAnimation>
   Build(nsPresContext* aPresContext,
         const StyleAnimation& aSrc,
-        const nsCSSKeyframesRule* aRule);
+        const nsCSSKeyframesRule* aRule,
+        uint64_t aAnimationIndex);
 
 private:
   nsTArray<Keyframe> BuildAnimationFrames(nsPresContext* aPresContext,
                                           const StyleAnimation& aSrc,
                                           const nsCSSKeyframesRule* aRule);
   Maybe<ComputedTimingFunction> GetKeyframeTimingFunction(
     nsPresContext* aPresContext,
     nsCSSKeyframeRule* aKeyframeRule,
@@ -595,17 +603,18 @@ private:
 };
 
 static Maybe<ComputedTimingFunction>
 ConvertTimingFunction(const nsTimingFunction& aTimingFunction);
 
 already_AddRefed<CSSAnimation>
 CSSAnimationBuilder::Build(nsPresContext* aPresContext,
                            const StyleAnimation& aSrc,
-                           const nsCSSKeyframesRule* aRule)
+                           const nsCSSKeyframesRule* aRule,
+                           uint64_t aAnimationIndex)
 {
   MOZ_ASSERT(aPresContext);
   MOZ_ASSERT(aRule);
 
   TimingParams timing = TimingParamsFrom(aSrc);
 
   nsTArray<Keyframe> keyframes =
     BuildAnimationFrames(aPresContext, aSrc, aRule);
@@ -626,17 +635,18 @@ CSSAnimationBuilder::Build(nsPresContext
     // spec says to do, but WebKit seems to honor at least some of
     // them.  See
     // http://lists.w3.org/Archives/Public/www-style/2011Apr/0079.html
     // In order to honor what the spec said, we'd copy more data over.
     UpdateOldAnimationPropertiesWithNew(*oldAnim,
                                         timing,
                                         keyframes,
                                         isStylePaused,
-                                        mStyleContext);
+                                        mStyleContext,
+                                        aAnimationIndex);
     return oldAnim.forget();
   }
 
   // mTarget is non-null here, so we emplace it directly.
   Maybe<OwningAnimationTarget> target;
   target.emplace(mTarget, mStyleContext->GetPseudoType());
   KeyframeEffectParams effectOptions;
   RefPtr<KeyframeEffectReadOnly> effect =
@@ -1089,14 +1099,15 @@ nsAnimationManager::BuildAnimations(nsSt
     nsCSSKeyframesRule* rule =
       src.GetName().IsEmpty()
       ? nullptr
       : mPresContext->StyleSet()->AsGecko()->KeyframesRuleForName(src.GetName());
     if (!rule) {
       continue;
     }
 
-    RefPtr<CSSAnimation> dest = builder.Build(mPresContext, src, rule);
+    RefPtr<CSSAnimation> dest =
+      builder.Build(mPresContext, src, rule, static_cast<uint64_t>(animIdx));
     dest->SetAnimationIndex(static_cast<uint64_t>(animIdx));
     aAnimations.AppendElement(dest);
   }
 }
 
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -136,20 +136,23 @@ public:
   bool HasLowerCompositeOrderThan(const CSSAnimation& aOther) const;
 
   void SetAnimationIndex(uint64_t aIndex)
   {
     MOZ_ASSERT(IsTiedToMarkup());
     if (IsRelevant() &&
         mAnimationIndex != aIndex) {
       nsNodeUtils::AnimationChanged(this);
-      PostUpdate();
     }
     mAnimationIndex = aIndex;
   }
+  uint64_t AnimationIndex() const
+  {
+    return mAnimationIndex;
+  }
 
   // Sets the owning element which is used for determining the composite
   // order of CSSAnimation objects generated from CSS markup.
   //
   // @see mOwningElement
   void SetOwningElement(const OwningElementRef& aElement)
   {
     mOwningElement = aElement;