Bug 1234095 - Rework sorting to handle to script-generated animations; r=heycam
authorBrian Birtles <birtles@gmail.com>
Thu, 14 Jan 2016 10:24:24 +0900
changeset 279840 ce9161cb9392605a34deef3b338f6a8515d8ebcc
parent 279839 cae1ea0a5e649a344eca1afb0b5c8ee7b6538bda
child 279890 fec6617d9b4666a844a35cd6047341a1307a3cf0
push id70244
push userbbirtles@mozilla.com
push dateThu, 14 Jan 2016 01:25:30 +0000
treeherdermozilla-inbound@ce9161cb9392 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1234095
milestone46.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 1234095 - Rework sorting to handle to script-generated animations; 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
@@ -6,21 +6,23 @@
 
 #include "Animation.h"
 #include "AnimationUtils.h"
 #include "mozilla/dom/AnimationBinding.h"
 #include "mozilla/dom/AnimationPlaybackEvent.h"
 #include "mozilla/AutoRestore.h"
 #include "mozilla/AsyncEventDispatcher.h" // For AsyncEventDispatcher
 #include "mozilla/Maybe.h" // For Maybe
+#include "nsAnimationManager.h" // For CSSAnimation
 #include "nsDOMMutationObserver.h" // For nsAutoAnimationMutationBatch
 #include "nsIDocument.h" // For nsIDocument
 #include "nsIPresShell.h" // For nsIPresShell
 #include "nsLayoutUtils.h" // For PostRestyleEvent (remove after bug 1073336)
 #include "nsThreadUtils.h" // For nsRunnableMethod and nsRevocableEventPtr
+#include "nsTransitionManager.h" // For CSSTransition
 #include "PendingAnimationTracker.h" // For PendingAnimationTracker
 
 namespace mozilla {
 namespace dom {
 
 // Static members
 uint64_t Animation::sNextAnimationIndex = 0;
 
@@ -662,24 +664,68 @@ Animation::UpdateRelevance()
   } else if (!wasRelevant && mIsRelevant) {
     nsNodeUtils::AnimationAdded(this);
   }
 }
 
 bool
 Animation::HasLowerCompositeOrderThan(const Animation& aOther) const
 {
-  // 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,
+  // 0. Object-equality case
+  if (&aOther == this) {
+    return false;
+  }
+
+  // 1. CSS Transitions sort lowest
+  {
+    auto asCSSTransitionForSorting =
+      [] (const Animation& anim) -> const CSSTransition*
+      {
+        const CSSTransition* transition = anim.AsCSSTransition();
+        return transition && transition->IsTiedToMarkup() ?
+               transition :
+               nullptr;
+      };
+    auto thisTransition  = asCSSTransitionForSorting(*this);
+    auto otherTransition = asCSSTransitionForSorting(aOther);
+    if (thisTransition && otherTransition) {
+      return thisTransition->HasLowerCompositeOrderThan(*otherTransition);
+    }
+    if (thisTransition || otherTransition) {
+      return thisTransition;
+    }
+  }
+
+  // 2. CSS Animations sort next
+  {
+    auto asCSSAnimationForSorting =
+      [] (const Animation& anim) -> const CSSAnimation*
+      {
+        const CSSAnimation* animation = anim.AsCSSAnimation();
+        return animation && animation->IsTiedToMarkup() ? animation : nullptr;
+      };
+    auto thisAnimation  = asCSSAnimationForSorting(*this);
+    auto otherAnimation = asCSSAnimationForSorting(aOther);
+    if (thisAnimation && otherAnimation) {
+      return thisAnimation->HasLowerCompositeOrderThan(*otherAnimation);
+    }
+    if (thisAnimation || otherAnimation) {
+      return thisAnimation;
+    }
+  }
+
+  // Subclasses of Animation repurpose mAnimationIndex to implement their
+  // own brand of composite ordering. However, by this point we should have
+  // handled any such custom composite ordering so we should now have unique
+  // animation indices.
+  MOZ_ASSERT(mAnimationIndex != aOther.mAnimationIndex,
              "Animation indices should be unique");
 
+  // 3. Finally, generic animations sort by their position in the global
+  // animation array.
   return mAnimationIndex < aOther.mAnimationIndex;
 }
 
 void
 Animation::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
                         nsCSSPropertySet& aSetProperties)
 {
   if (!mEffect) {
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -278,17 +278,17 @@ public:
             mPendingState == PendingState::PlayPending);
   }
   bool IsRelevant() const { return mIsRelevant; }
   void UpdateRelevance();
 
   /**
    * Returns true if this Animation has a lower composite order than aOther.
    */
-  virtual bool HasLowerCompositeOrderThan(const Animation& aOther) const;
+  bool HasLowerCompositeOrderThan(const Animation& aOther) const;
 
    /**
    * Returns the level at which the effect(s) associated with this Animation
    * are applied to the CSS cascade.
    */
   virtual EffectCompositor::CascadeLevel CascadeLevel() const
   {
     return EffectCompositor::CascadeLevel::Animations;
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -119,53 +119,34 @@ CSSAnimation::PauseFromStyle()
 void
 CSSAnimation::Tick()
 {
   Animation::Tick();
   QueueEvents();
 }
 
 bool
-CSSAnimation::HasLowerCompositeOrderThan(const Animation& aOther) const
+CSSAnimation::HasLowerCompositeOrderThan(const CSSAnimation& aOther) const
 {
+  MOZ_ASSERT(IsTiedToMarkup() && aOther.IsTiedToMarkup(),
+             "Should only be called for CSS animations that are sorted "
+             "as CSS animations (i.e. tied to CSS markup)");
+
   // 0. Object-equality case
   if (&aOther == this) {
     return false;
   }
 
-  // 1. Transitions sort lower
-  //
-  // FIXME: We need to differentiate between transitions and generic Animations.
-  // Generic animations don't exist yet (that's bug 1096773) so for now we're
-  // ok.
-  const CSSAnimation* otherAnimation = aOther.AsCSSAnimation();
-  if (!otherAnimation) {
-    MOZ_ASSERT(aOther.AsCSSTransition(),
-               "Animation being compared is a CSS transition");
-    return false;
+  // 1. Sort by document order
+  if (!mOwningElement.Equals(aOther.mOwningElement)) {
+    return mOwningElement.LessThan(aOther.mOwningElement);
   }
 
-  // 2. CSS animations that correspond to an animation-name property sort lower
-  //    than other CSS animations (e.g. those created or kept-alive by script).
-  if (!IsTiedToMarkup()) {
-    return !otherAnimation->IsTiedToMarkup() ?
-           Animation::HasLowerCompositeOrderThan(aOther) :
-           false;
-  }
-  if (!otherAnimation->IsTiedToMarkup()) {
-    return true;
-  }
-
-  // 3. Sort by document order
-  if (!mOwningElement.Equals(otherAnimation->mOwningElement)) {
-    return mOwningElement.LessThan(otherAnimation->mOwningElement);
-  }
-
-  // 4. (Same element and pseudo): Sort by position in animation-name
-  return mAnimationIndex < otherAnimation->mAnimationIndex;
+  // 2. (Same element and pseudo): Sort by position in animation-name
+  return mAnimationIndex < aOther.mAnimationIndex;
 }
 
 void
 CSSAnimation::QueueEvents()
 {
   if (!mEffect) {
     return;
   }
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -125,17 +125,17 @@ public:
     Animation::CancelFromStyle();
   }
 
   void Tick() override;
   void QueueEvents();
 
   bool IsStylePaused() const { return mIsStylePaused; }
 
-  bool HasLowerCompositeOrderThan(const Animation& aOther) const override;
+  bool HasLowerCompositeOrderThan(const CSSAnimation& aOther) const;
 
   void SetAnimationIndex(uint64_t aIndex)
   {
     MOZ_ASSERT(IsTiedToMarkup());
     mAnimationIndex = aIndex;
   }
   void CopyAnimationIndex(const CSSAnimation& aOther)
   {
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -170,53 +170,40 @@ CSSTransition::TransitionProperty() cons
   // returning the same value in that case.
   dom::KeyframeEffectReadOnly* effect = GetEffect();
   MOZ_ASSERT(effect && effect->AsTransition(),
              "Transition should have a transition effect");
   return effect->AsTransition()->TransitionProperty();
 }
 
 bool
-CSSTransition::HasLowerCompositeOrderThan(const Animation& aOther) const
+CSSTransition::HasLowerCompositeOrderThan(const CSSTransition& aOther) const
 {
+  MOZ_ASSERT(IsTiedToMarkup() && aOther.IsTiedToMarkup(),
+             "Should only be called for CSS transitions that are sorted "
+             "as CSS transitions (i.e. tied to CSS markup)");
+
   // 0. Object-equality case
   if (&aOther == this) {
     return false;
   }
 
-  // 1. Transitions sort lowest
-  const CSSTransition* otherTransition = aOther.AsCSSTransition();
-  if (!otherTransition) {
-    return true;
+  // 1. Sort by document order
+  if (!mOwningElement.Equals(aOther.mOwningElement)) {
+    return mOwningElement.LessThan(aOther.mOwningElement);
   }
 
-  // 2. CSS transitions that correspond to a transition-property property sort
-  // lower than CSS transitions owned by script.
-  if (!IsTiedToMarkup()) {
-    return !otherTransition->IsTiedToMarkup() ?
-           Animation::HasLowerCompositeOrderThan(aOther) :
-           false;
-  }
-  if (!otherTransition->IsTiedToMarkup()) {
-    return true;
+  // 2. (Same element and pseudo): Sort by transition generation
+  if (mAnimationIndex != aOther.mAnimationIndex) {
+    return mAnimationIndex < aOther.mAnimationIndex;
   }
 
-  // 3. Sort by document order
-  if (!mOwningElement.Equals(otherTransition->mOwningElement)) {
-    return mOwningElement.LessThan(otherTransition->mOwningElement);
-  }
-
-  // 4. (Same element and pseudo): Sort by transition generation
-  if (mAnimationIndex != otherTransition->mAnimationIndex) {
-    return mAnimationIndex < otherTransition->mAnimationIndex;
-  }
-
-  // 5. (Same transition generation): Sort by transition property
+  // 3. (Same transition generation): Sort by transition property
   return nsCSSProps::GetStringValue(TransitionProperty()) <
-         nsCSSProps::GetStringValue(otherTransition->TransitionProperty());
+         nsCSSProps::GetStringValue(aOther.TransitionProperty());
 }
 
 ////////////////////////// nsTransitionManager ////////////////////////////
 
 NS_IMPL_CYCLE_COLLECTION(nsTransitionManager, mEventDispatcher)
 
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(nsTransitionManager, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(nsTransitionManager, Release)
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -133,17 +133,17 @@ public:
     // returning CascadeLevel::Animations.
     mOwningElement = OwningElementRef();
   }
 
   void Tick() override;
 
   nsCSSProperty TransitionProperty() const;
 
-  bool HasLowerCompositeOrderThan(const Animation& aOther) const override;
+  bool HasLowerCompositeOrderThan(const CSSTransition& aOther) const;
   EffectCompositor::CascadeLevel CascadeLevel() const override
   {
     return IsTiedToMarkup() ?
            EffectCompositor::CascadeLevel::Transitions :
            EffectCompositor::CascadeLevel::Animations;
   }
 
   void SetCreationSequence(uint64_t aIndex)