Bug 1242872 - Part 3: Factor finding old animations process out. r?dbaron draft
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Sat, 13 Feb 2016 11:42:07 +0900
changeset 330823 754f7d035a29bb4d58fc56fbc819e6d6401ea5ce
parent 330822 856be30f219bcd49c3498ed1061d0b99034ccf22
child 330824 33f46ef817562be8149e2f0e3626a1825afb83cc
push id10851
push userbmo:hiikezoe@mozilla-japan.org
push dateSat, 13 Feb 2016 11:34:41 +0000
reviewersdbaron
bugs1242872
milestone47.0a1
Bug 1242872 - Part 3: Factor finding old animations process out. r?dbaron MozReview-Commit-ID: 45XrTtLJpnz
layout/style/nsAnimationManager.cpp
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -296,16 +296,41 @@ CSSAnimation::ElapsedTimeToTimeStamp(con
 
 ////////////////////////// nsAnimationManager ////////////////////////////
 
 NS_IMPL_CYCLE_COLLECTION(nsAnimationManager, mEventDispatcher)
 
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(nsAnimationManager, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(nsAnimationManager, Release)
 
+// Find the matching animation by |aName| in the old list
+// of animations and remove the matched animation from the list.
+static already_AddRefed<CSSAnimation>
+PullOutOldAnimationInCollection(const nsAString& aName,
+                                AnimationCollection* aCollection)
+{
+  // Find the matching animation with this name in the old list
+  // of animations.  We iterate through both lists in a backwards
+  // direction which means that if there are more animations in
+  // the new list of animations with a given name than in the old
+  // list, it will be the animations towards the of the beginning of
+  // the list that do not match and are treated as new animations.
+  size_t oldIdx = aCollection->mAnimations.Length();
+  while (oldIdx-- != 0) {
+    RefPtr<CSSAnimation> a = aCollection->mAnimations[oldIdx]->AsCSSAnimation();
+    MOZ_ASSERT(a, "All animations in the CSS Animation collection should"
+        " be CSSAnimation objects");
+    if (a->AnimationName() == aName) {
+      aCollection->mAnimations.RemoveElementAt(oldIdx);
+      return a.forget();
+    }
+  }
+  return nullptr;
+}
+
 nsIStyleRule*
 nsAnimationManager::CheckAnimationRule(nsStyleContext* aStyleContext,
                                        mozilla::dom::Element* aElement)
 {
   // Ignore animations for print or print preview, and for elements
   // that are not attached to the document tree.
   if (!mPresContext->IsDynamic() || !aElement->IsInComposedDoc()) {
     return nullptr;
@@ -360,33 +385,25 @@ nsAnimationManager::CheckAnimationRule(n
     // (or potentially optimize BuildAnimations to avoid rebuilding it
     // in the first place).
     if (!collection->mAnimations.IsEmpty()) {
 
       for (size_t newIdx = newAnimations.Length(); newIdx-- != 0;) {
         Animation* newAnim = newAnimations[newIdx];
 
         // Find the matching animation with this name in the old list
-        // of animations.  We iterate through both lists in a backwards
+        // of animations and remove the matched animation from the list.
+        // We iterate through both lists in a backwards
         // direction which means that if there are more animations in
         // the new list of animations with a given name than in the old
         // list, it will be the animations towards the of the beginning of
         // the list that do not match and are treated as new animations.
-        RefPtr<CSSAnimation> oldAnim;
-        size_t oldIdx = collection->mAnimations.Length();
-        while (oldIdx-- != 0) {
-          CSSAnimation* a = collection->mAnimations[oldIdx]->AsCSSAnimation();
-          MOZ_ASSERT(a, "All animations in the CSS Animation collection should"
-                        " be CSSAnimation objects");
-          if (a->AnimationName() ==
-              newAnim->AsCSSAnimation()->AnimationName()) {
-            oldAnim = a;
-            break;
-          }
-        }
+        RefPtr<CSSAnimation> oldAnim =
+          PullOutOldAnimationInCollection(
+            newAnim->AsCSSAnimation()->AnimationName(), collection);
         if (!oldAnim) {
           // FIXME: Bug 1134163 - We shouldn't queue animationstart events
           // until the animation is actually ready to run. However, we
           // currently have some tests that assume that these events are
           // dispatched within the same tick as the animation is added
           // so we need to queue up any animationstart events from newly-created
           // animations.
           newAnim->AsCSSAnimation()->QueueEvents();
@@ -431,26 +448,23 @@ nsAnimationManager::CheckAnimationRule(n
 
         // 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 && oldAnim->IsRelevant()) {
           nsNodeUtils::AnimationChanged(oldAnim);
         }
 
-        // Replace new animation with the (updated) old one and remove the
-        // old one from the array so we don't try to match it any more.
+        // Replace new animation with the (updated) old one.
         //
         // Although we're doing this while iterating this is safe because
-        // we're not changing the length of newAnimations and we've finished
-        // iterating over the list of old iterations.
+        // we're not changing the length of newAnimations.
         newAnim->CancelFromStyle();
         newAnim = nullptr;
         newAnimations.ReplaceElementAt(newIdx, oldAnim);
-        collection->mAnimations.RemoveElementAt(oldIdx);
       }
     }
   } else {
     collection = GetAnimationCollection(aElement,
                                         aStyleContext->GetPseudoType(),
                                         true /* aCreateIfNeeded */);
     for (Animation* animation : newAnimations) {
       // FIXME: Bug 1134163 - As above, we have shouldn't actually need to