Bug 1203009 part 4 - Implement new composite ordering; r=heycam
authorBrian Birtles <birtles@gmail.com>
Tue, 15 Sep 2015 11:20:56 +0900
changeset 295303 37b6deedaab676639f504ad9c263486c0062da18
parent 295302 b7af8129961f8f9401923debb1de40d73c90ae72
child 295304 44462cfe313478176cb383aa4b1068b6d3feaea8
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1203009
milestone43.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 1203009 part 4 - Implement new composite ordering; r=heycam
dom/animation/Animation.cpp
dom/animation/Animation.h
layout/style/nsAnimationManager.cpp
layout/style/nsAnimationManager.h
layout/style/nsTransitionManager.cpp
layout/style/nsTransitionManager.h
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -512,21 +512,21 @@ Animation::UpdateRelevance()
   } else if (!wasRelevant && mIsRelevant) {
     nsNodeUtils::AnimationAdded(this);
   }
 }
 
 bool
 Animation::HasLowerCompositeOrderThan(const Animation& aOther) const
 {
-  // We only ever sort non-idle animations so we don't ever expect
-  // mAnimationIndex to be set to kNoIndex
-  MOZ_ASSERT(mAnimationIndex != kNoIndex &&
-             aOther.mAnimationIndex != kNoIndex,
-             "Animations to compare should not be idle");
+  // Due to the way subclasses of this repurpose the mAnimationIndex to
+  // implement their own brand of composite ordering it is possible for
+  // two animations to have an identical mAnimationIndex member.
+  // However, these subclasses override this method so we shouldn't see
+  // identical animation indices here.
   MOZ_ASSERT(mAnimationIndex != aOther.mAnimationIndex || &aOther == this,
              "Animation indices should be unique");
 
   return mAnimationIndex < aOther.mAnimationIndex;
 }
 
 bool
 Animation::CanThrottle() const
@@ -837,26 +837,16 @@ Animation::PauseAt(const TimeDuration& a
   if (mReady) {
     mReady->MaybeResolve(this);
   }
 }
 
 void
 Animation::UpdateTiming(SeekFlag aSeekFlag, SyncNotifyFlag aSyncNotifyFlag)
 {
-  // Update the animation index each time we transition in or out of the
-  // idle state
-  if (!IsUsingCustomCompositeOrder()) {
-    if (PlayState() == AnimationPlayState::Idle) {
-      mAnimationIndex = kNoIndex;
-    } else if (mAnimationIndex == kNoIndex) {
-      mAnimationIndex = sNextAnimationIndex++;
-    }
-  }
-
   // We call UpdateFinishedState before UpdateEffect because the former
   // can change the current time, which is used by the latter.
   UpdateFinishedState(aSeekFlag, aSyncNotifyFlag);
   UpdateEffect();
 
   // Unconditionally Add/Remove from the timeline. This is ok because if the
   // animation has already been added/removed (which will be true more often
   // than not) the work done by AnimationTimeline/DocumentTimeline is still
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -52,17 +52,17 @@ class Animation
 protected:
   virtual ~Animation() {}
 
 public:
   explicit Animation(nsIGlobalObject* aGlobal)
     : DOMEventTargetHelper(aGlobal)
     , mPlaybackRate(1.0)
     , mPendingState(PendingState::NotPending)
-    , mAnimationIndex(kNoIndex)
+    , mAnimationIndex(sNextAnimationIndex++)
     , mIsRunningOnCompositor(false)
     , mFinishedAtLastComposeStyle(false)
     , mIsRelevant(false)
     , mFinishedIsResolved(false)
   {
   }
 
   NS_DECL_ISUPPORTS_INHERITED
@@ -341,18 +341,18 @@ protected:
     DidSeek
   };
 
   enum class SyncNotifyFlag {
     Sync,
     Async
   };
 
-  void UpdateTiming(SeekFlag aSeekFlag,
-                    SyncNotifyFlag aSyncNotifyFlag);
+  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);
@@ -401,17 +401,16 @@ protected:
   // checking if this animation is tracked by a PendingAnimationTracker because
   // the animation will continue to be pending even after it has been removed
   // from the PendingAnimationTracker while it is waiting for the next tick
   // (see TriggerOnNextTick for details).
   enum class PendingState { NotPending, PlayPending, PausePending };
   PendingState mPendingState;
 
   static uint64_t sNextAnimationIndex;
-  static const uint64_t kNoIndex = UINT64_MAX;
 
   // The relative position of this animation within the global animation list.
   // This is kNoIndex while the animation is in the idle state and is updated
   // each time the animation transitions out of the idle state.
   //
   // Note that subclasses such as CSSTransition and CSSAnimation may repurpose
   // this member to implement their own brand of sorting. As a result, it is
   // possible for two different objects to have the same index.
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -297,16 +297,28 @@ CSSAnimation::GetAnimationManager() cons
   nsPresContext* context = GetPresContext();
   if (!context) {
     return nullptr;
   }
 
   return context->AnimationManager();
 }
 
+void
+CSSAnimation::UpdateTiming(SeekFlag aSeekFlag, SyncNotifyFlag aSyncNotifyFlag)
+{
+  if (mNeedsNewAnimationIndexWhenRun &&
+      PlayState() != AnimationPlayState::Idle) {
+    mAnimationIndex = sNextAnimationIndex++;
+    mNeedsNewAnimationIndexWhenRun = false;
+  }
+
+  Animation::UpdateTiming(aSeekFlag, aSyncNotifyFlag);
+}
+
 ////////////////////////// nsAnimationManager ////////////////////////////
 
 NS_IMPL_CYCLE_COLLECTION(nsAnimationManager, mEventDispatcher)
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsAnimationManager)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsAnimationManager)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsAnimationManager)
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -93,18 +93,31 @@ public:
   virtual AnimationPlayState PlayStateFromJS() const override;
   virtual void PlayFromJS(ErrorResult& aRv) override;
 
   void PlayFromStyle();
   void PauseFromStyle();
   void CancelFromStyle() override
   {
     mOwningElement = OwningElementRef();
+
+    // When an animation is disassociated with style it enters an odd state
+    // where its composite order is undefined until it first transitions
+    // out of the idle state.
+    //
+    // Even if the composite order isn't defined we don't want it to be random
+    // in case we need to determine the order to dispatch events associated
+    // with an animation in this state. To solve this we treat the animation as
+    // if it had been added to the end of the global animation list so that
+    // its sort order is defined. We'll update this index again once the
+    // animation leaves the idle state.
+    mAnimationIndex = sNextAnimationIndex++;
+    mNeedsNewAnimationIndexWhenRun = true;
+
     Animation::CancelFromStyle();
-    MOZ_ASSERT(mAnimationIndex == kNoIndex);
   }
 
   void Tick() override;
   void QueueEvents();
   bool HasEndEventToQueue() const override;
 
   bool IsStylePaused() const { return mIsStylePaused; }
 
@@ -143,17 +156,21 @@ public:
   bool mInEffectForCascadeResults;
 
 protected:
   virtual ~CSSAnimation()
   {
     MOZ_ASSERT(!mOwningElement.IsSet(), "Owning element should be cleared "
                                         "before a CSS animation is destroyed");
   }
-  virtual CommonAnimationManager* GetAnimationManager() const override;
+
+  // Animation overrides
+  CommonAnimationManager* GetAnimationManager() const override;
+  void UpdateTiming(SeekFlag aSeekFlag,
+                    SyncNotifyFlag aSyncNotifyFlag) override;
 
   nsString mAnimationName;
 
   // The (pseudo-)element whose computed animation-name refers to this
   // animation (if any).
   //
   // This is used for determining the relative composite order of animations
   // generated from CSS markup.
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -108,16 +108,28 @@ CSSTransition::GetAnimationManager() con
   if (!context) {
     return nullptr;
   }
 
   return context->TransitionManager();
 }
 
 void
+CSSTransition::UpdateTiming(SeekFlag aSeekFlag, SyncNotifyFlag aSyncNotifyFlag)
+{
+  if (mNeedsNewAnimationIndexWhenRun &&
+      PlayState() != AnimationPlayState::Idle) {
+    mAnimationIndex = sNextAnimationIndex++;
+    mNeedsNewAnimationIndexWhenRun = false;
+  }
+
+  Animation::UpdateTiming(aSeekFlag, aSyncNotifyFlag);
+}
+
+void
 CSSTransition::QueueEvents()
 {
   AnimationPlayState playState = PlayState();
   bool newlyFinished = !mWasFinishedOnLastTick &&
                        playState == AnimationPlayState::Finished;
   mWasFinishedOnLastTick = playState == AnimationPlayState::Finished;
 
   if (!newlyFinished || !mEffect || !mOwningElement.IsSet()) {
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -110,18 +110,27 @@ public:
     DoPlay(rv, Animation::LimitBehavior::Continue);
     // play() should not throw when LimitBehavior is Continue
     MOZ_ASSERT(!rv.Failed(), "Unexpected exception playing transition");
   }
 
   void CancelFromStyle() override
   {
     mOwningElement = OwningElementRef();
+
+    // The animation index to use for compositing will be established when
+    // this transition next transitions out of the idle state but we still
+    // update it now so that the sort order of this transition remains
+    // defined until that moment.
+    //
+    // See longer explanation in CSSAnimation::CancelFromStyle.
+    mAnimationIndex = sNextAnimationIndex++;
+    mNeedsNewAnimationIndexWhenRun = true;
+
     Animation::CancelFromStyle();
-    MOZ_ASSERT(mAnimationIndex == kNoIndex);
   }
 
   void Tick() override;
 
   nsCSSProperty TransitionProperty() const;
 
   bool HasLowerCompositeOrderThan(const Animation& aOther) const override;
   bool IsUsingCustomCompositeOrder() const override
@@ -146,17 +155,20 @@ public:
 
 protected:
   virtual ~CSSTransition()
   {
     MOZ_ASSERT(!mOwningElement.IsSet(), "Owning element should be cleared "
                                         "before a CSS transition is destroyed");
   }
 
-  virtual CommonAnimationManager* GetAnimationManager() const override;
+  // Animation overrides
+  CommonAnimationManager* GetAnimationManager() const override;
+  void UpdateTiming(SeekFlag aSeekFlag,
+                    SyncNotifyFlag aSyncNotifyFlag) override;
 
   void QueueEvents();
   bool HasEndEventToQueue() const override;
 
   // The (pseudo-)element whose computed transition-property refers to this
   // transition (if any).
   //
   // This is used for determining the relative composite order of transitions