Bug 1040543 part 10 - Move mIsLastNotification from AnimationPlayer to Animation; r=bz
authorBrian Birtles <birtles@gmail.com>
Sun, 10 Aug 2014 17:06:51 +1000
changeset 198792 851652501f189f119c679de6a11ad00b7b105f4c
parent 198791 3a12123bef3f2628252113aa71582bc9f3894350
child 198793 4234319389c31359a13512e6348457a7aed8dbb7
push id27286
push usernigelbabu@gmail.com
push dateMon, 11 Aug 2014 06:26:45 +0000
treeherdermozilla-central@8c4a1b3a2a8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1040543
milestone34.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 1040543 part 10 - Move mIsLastNotification from AnimationPlayer to Animation; r=bz In this fourth step of dividing functionality between AnimationPlayer and Animation, we move the mIsLastNotification and related methods/enums from AnimationPlayer to Animation. It is somewhat unclear where this belongs. This member is used to determine which event to send for CSS Animations. The thinking behind moving this to Animation is that if an animation that has already dispatched its animationstart event was transferred to a new animation player with a similar current time then I think it is expected that such an animation would *not* dispatch another animationstart event. That suggests that event-state is a property of the Animation not the AnimationPlayer. Obviously, this needs to be defined somewhere (namely, the CSS Animations <-> Web Animations integration spec likely to become "CSS Animations Level 4"). Once that behavior is agreed upon, if AnimationPlayer proves to be the more suitable home for this member then it should be relatively straightforward to move the member back at that time.
dom/animation/Animation.h
dom/animation/AnimationPlayer.h
layout/style/AnimationCommon.cpp
layout/style/nsAnimationManager.cpp
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -120,16 +120,17 @@ namespace dom {
 
 class Animation MOZ_FINAL : public nsWrapperCache
 {
 public:
   Animation(nsIDocument* aDocument, const AnimationTiming &aTiming)
     : mDocument(aDocument)
     , mTiming(aTiming)
     , mIsFinishedTransition(false)
+    , mLastNotification(LAST_NOTIFICATION_NONE)
   {
     SetIsDOMBinding();
   }
 
   NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(Animation)
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(Animation)
 
   nsIDocument* GetParentObject() const { return mDocument; }
@@ -203,16 +204,25 @@ public:
       return false;
     }
 
     ComputedTiming computedTiming = GetComputedTiming();
     return computedTiming.mPhase == ComputedTiming::AnimationPhase_Before ||
            computedTiming.mPhase == ComputedTiming::AnimationPhase_Active;
   }
 
+  enum {
+    LAST_NOTIFICATION_NONE = uint64_t(-1),
+    LAST_NOTIFICATION_END = uint64_t(-2)
+  };
+  uint64_t LastNotification() const { return mLastNotification; }
+  void SetLastNotification(uint64_t aLastNotification) {
+    mLastNotification = aLastNotification;
+  }
+
   bool HasAnimationOfProperty(nsCSSProperty aProperty) const;
   const InfallibleTArray<AnimationProperty>& Properties() const {
     return mProperties;
   }
   InfallibleTArray<AnimationProperty>& Properties() {
     return mProperties;
   }
 
@@ -223,16 +233,19 @@ protected:
   // the target element, can be empty.
   nsRefPtr<nsIDocument> mDocument;
   Nullable<TimeDuration> mParentTime;
 
   AnimationTiming mTiming;
   // A flag to mark transitions that have finished and are due to
   // be removed on the next throttle-able cycle.
   bool mIsFinishedTransition;
+  // One of the LAST_NOTIFICATION_* constants, or an integer for the iteration
+  // whose start we last notified on.
+  uint64_t mLastNotification;
 
   InfallibleTArray<AnimationProperty> mProperties;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_Animation_h
--- a/dom/animation/AnimationPlayer.h
+++ b/dom/animation/AnimationPlayer.h
@@ -31,17 +31,16 @@ namespace dom {
 class AnimationPlayer : public nsWrapperCache
 {
 protected:
   virtual ~AnimationPlayer() { }
 
 public:
   explicit AnimationPlayer(AnimationTimeline* aTimeline)
     : mIsRunningOnCompositor(false)
-    , mLastNotification(LAST_NOTIFICATION_NONE)
     , mTimeline(aTimeline)
   {
     SetIsDOMBinding();
   }
 
   NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(AnimationPlayer)
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(AnimationPlayer)
 
@@ -95,24 +94,16 @@ public:
 
   nsString mName;
   // The beginning of the delay period.
   TimeStamp mStartTime;
   TimeStamp mPauseStart;
   uint8_t mPlayState;
   bool mIsRunningOnCompositor;
 
-  enum {
-    LAST_NOTIFICATION_NONE = uint64_t(-1),
-    LAST_NOTIFICATION_END = uint64_t(-2)
-  };
-  // One of the above constants, or an integer for the iteration
-  // whose start we last notified on.
-  uint64_t mLastNotification;
-
   nsRefPtr<AnimationTimeline> mTimeline;
   nsRefPtr<Animation> mSource;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_AnimationPlayer_h
--- a/layout/style/AnimationCommon.cpp
+++ b/layout/style/AnimationCommon.cpp
@@ -481,23 +481,24 @@ AnimationPlayerCollection::EnsureStyleRu
           player->GetSource()->Properties().IsEmpty()) {
         continue;
       }
 
       // The GetComputedTiming() call here handles pausing.  But:
       // FIXME: avoid recalculating every time when paused.
       ComputedTiming computedTiming = player->GetSource()->GetComputedTiming();
 
-      // XXX We shouldn't really be using mLastNotification as a general
+      // XXX We shouldn't really be using LastNotification() as a general
       // indicator that the animation has finished, it should be reserved for
       // events. If we use it differently in the future this use might need
       // changing.
       if (!player->mIsRunningOnCompositor ||
           (computedTiming.mPhase == ComputedTiming::AnimationPhase_After &&
-           player->mLastNotification != AnimationPlayer::LAST_NOTIFICATION_END))
+           player->GetSource()->LastNotification()
+             != Animation::LAST_NOTIFICATION_END))
       {
         aFlags = EnsureStyleRule_IsNotThrottled;
         break;
       }
     }
   }
 
   if (aFlags == EnsureStyleRule_IsThrottled) {
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -55,61 +55,57 @@ nsAnimationManager::GetEventsForCurrentT
     switch (computedTiming.mPhase) {
       case ComputedTiming::AnimationPhase_Null:
       case ComputedTiming::AnimationPhase_Before:
         // Do nothing
         break;
 
       case ComputedTiming::AnimationPhase_Active:
         // Dispatch 'animationstart' or 'animationiteration' when needed.
-        if (computedTiming.mCurrentIteration != player->mLastNotification) {
+        if (computedTiming.mCurrentIteration != anim->LastNotification()) {
           // Notify 'animationstart' even if a negative delay puts us
           // past the first iteration.
           // Note that when somebody changes the animation-duration
           // dynamically, this will fire an extra iteration event
           // immediately in many cases.  It's not clear to me if that's the
           // right thing to do.
           uint32_t message =
-            player->mLastNotification ==
-              AnimationPlayer::LAST_NOTIFICATION_NONE
-              ? NS_ANIMATION_START
-              : NS_ANIMATION_ITERATION;
-
-          player->mLastNotification = computedTiming.mCurrentIteration;
+            anim->LastNotification() == Animation::LAST_NOTIFICATION_NONE
+                                        ? NS_ANIMATION_START
+                                        : NS_ANIMATION_ITERATION;
+          anim->SetLastNotification(computedTiming.mCurrentIteration);
           TimeDuration iterationStart =
             anim->Timing().mIterationDuration *
             computedTiming.mCurrentIteration;
           TimeDuration elapsedTime =
             std::max(iterationStart, anim->InitialAdvance());
           AnimationEventInfo ei(aCollection->mElement, player->mName, message,
                                 elapsedTime, aCollection->PseudoElement());
           aEventsToDispatch.AppendElement(ei);
         }
         break;
 
       case ComputedTiming::AnimationPhase_After:
         // If we skipped the animation interval entirely, dispatch
         // 'animationstart' first
-        if (player->mLastNotification ==
-            AnimationPlayer::LAST_NOTIFICATION_NONE) {
+        if (anim->LastNotification() == Animation::LAST_NOTIFICATION_NONE) {
           // Notifying for start of 0th iteration.
           // (This is overwritten below but we set it here to maintain
           // internal consistency.)
-          player->mLastNotification = 0;
+          anim->SetLastNotification(0);
           TimeDuration elapsedTime =
             std::min(anim->InitialAdvance(), computedTiming.mActiveDuration);
           AnimationEventInfo ei(aCollection->mElement,
                                 player->mName, NS_ANIMATION_START,
                                 elapsedTime, aCollection->PseudoElement());
           aEventsToDispatch.AppendElement(ei);
         }
         // Dispatch 'animationend' when needed.
-        if (player->mLastNotification !=
-            AnimationPlayer::LAST_NOTIFICATION_END) {
-          player->mLastNotification = AnimationPlayer::LAST_NOTIFICATION_END;
+        if (anim->LastNotification() != Animation::LAST_NOTIFICATION_END) {
+          anim->SetLastNotification(Animation::LAST_NOTIFICATION_END);
           AnimationEventInfo ei(aCollection->mElement,
                                 player->mName, NS_ANIMATION_END,
                                 computedTiming.mActiveDuration,
                                 aCollection->PseudoElement());
           aEventsToDispatch.AppendElement(ei);
         }
         break;
     }