Web Animations: Don't compare start time for AnimationPlayer sorting

As per recent spec changes, the start time should no longer factor in to
AnimationPlayer sort order. I've left in the SortInfo class instead of
simply holding on to an unsigned sequence number since we may in the
future add a mechanism for changing sort order, although this is easy
to remove later if won't be adding such functionality.

Review URL: https://codereview.chromium.org/399033002

git-svn-id: svn://svn.chromium.org/blink/trunk@178531 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/animation/AnimationPlayer.cpp

@@ -62,7 +62,7 @@ AnimationPlayer::AnimationPlayer(ExecutionContext* executionContext, AnimationTi
     , m_startTime(nullValue())
     , m_holdTime(nullValue())
     , m_storedTimeLag(0)
-    , m_sortInfo(nextSequenceNumber(), timeline.effectiveTime())
+    , m_sortInfo(nextSequenceNumber())
     , m_content(content)
     , m_timeline(&timeline)
     , m_paused(false)
@@ -183,7 +183,6 @@ void AnimationPlayer::setStartTimeInternal(double newStartTime, bool isUpdateFro
     bool hadStartTime = hasStartTime();
     double previousCurrentTime = currentTimeInternal();
     m_startTime = newStartTime;
-    m_sortInfo.m_startTime = newStartTime;
     updateCurrentTimingState();
     if (previousCurrentTime != currentTimeInternal()) {
         setOutdated();
@@ -415,16 +414,6 @@ void AnimationPlayer::cancel()
     setSource(0);
 }
 
-bool AnimationPlayer::SortInfo::operator<(const SortInfo& other) const
-{
-    ASSERT(!std::isnan(m_startTime) && !std::isnan(other.m_startTime));
-    if (m_startTime < other.m_startTime)
-        return true;
-    if (m_startTime > other.m_startTime)
-        return false;
-    return m_sequenceNumber < other.m_sequenceNumber;
-}
-
 #if !ENABLE(OILPAN)
 bool AnimationPlayer::canFree() const
 {

third_party/WebKit/Source/core/animation/AnimationPlayer.h

@@ -126,20 +126,16 @@ public:
     class SortInfo {
     public:
         friend class AnimationPlayer;
-        bool operator<(const SortInfo& other) const;
-        double startTime() const { return m_startTime; }
-        bool hasLowerSequenceNumber(const SortInfo& other) const
+        bool operator<(const SortInfo& other) const
         {
             return m_sequenceNumber < other.m_sequenceNumber;
         }
     private:
-        SortInfo(unsigned sequenceNumber, double startTime)
+        SortInfo(unsigned sequenceNumber)
             : m_sequenceNumber(sequenceNumber)
-            , m_startTime(startTime)
         {
         }
         unsigned m_sequenceNumber;
-        double m_startTime;
     };
 
     const SortInfo& sortInfo() const { return m_sortInfo; }

third_party/WebKit/Source/core/animation/AnimationPlayerTest.cpp

@@ -709,29 +709,9 @@ TEST_F(AnimationAnimationPlayerTest, AttachedAnimationPlayers)
 
 TEST_F(AnimationAnimationPlayerTest, HasLowerPriority)
 {
-    // Sort time defaults to timeline current time
-    updateTimeline(15);
     RefPtrWillBeRawPtr<AnimationPlayer> player1 = timeline->createAnimationPlayer(0);
     RefPtrWillBeRawPtr<AnimationPlayer> player2 = timeline->createAnimationPlayer(0);
-    player2->setStartTimeInternal(10);
-    RefPtrWillBeRawPtr<AnimationPlayer> player3 = timeline->createAnimationPlayer(0);
-    RefPtrWillBeRawPtr<AnimationPlayer> player4 = timeline->createAnimationPlayer(0);
-    player4->setStartTimeInternal(20);
-    RefPtrWillBeRawPtr<AnimationPlayer> player5 = timeline->createAnimationPlayer(0);
-    player5->setStartTimeInternal(10);
-    RefPtrWillBeRawPtr<AnimationPlayer> player6 = timeline->createAnimationPlayer(0);
-    player6->setStartTimeInternal(-10);
-    Vector<RefPtrWillBeMember<AnimationPlayer> > players;
-    players.append(player6);
-    players.append(player2);
-    players.append(player5);
-    players.append(player1);
-    players.append(player3);
-    players.append(player4);
-    for (size_t i = 0; i < players.size(); i++) {
-        for (size_t j = 0; j < players.size(); j++)
-            EXPECT_EQ(i < j, AnimationPlayer::hasLowerPriority(players[i].get(), players[j].get()));
-    }
+    EXPECT_TRUE(AnimationPlayer::hasLowerPriority(player1.get(), player2.get()));
 }
 
 }

third_party/WebKit/Source/core/animation/AnimationStack.cpp

@@ -105,10 +105,6 @@ WillBeHeapHashMap<CSSPropertyID, RefPtrWillBeMember<Interpolation> > AnimationSt
             const SampledEffect& effect = *effects[i];
             if (effect.priority() != priority || (cancelledAnimationPlayers && effect.animation() && cancelledAnimationPlayers->contains(effect.animation()->player())))
                 continue;
-            if (newAnimations && effect.sortInfo().startTime() > timelineCurrentTime) {
-                copyNewAnimationsToActiveInterpolationMap(*newAnimations, result);
-                newAnimations = 0;
-            }
             copyToActiveInterpolationMap(effect.interpolations(), result);
         }
     }

third_party/WebKit/Source/core/animation/AnimationStackTest.cpp

@@ -88,7 +88,7 @@ TEST_F(AnimationAnimationStackTest, ActiveAnimationsSorted)
     play(makeAnimation(makeAnimationEffect(CSSPropertyFontSize, AnimatableDouble::create(3))).get(), 5);
     WillBeHeapHashMap<CSSPropertyID, RefPtrWillBeMember<Interpolation> > result = AnimationStack::activeInterpolations(&element->activeAnimations()->defaultStack(), 0, 0, Animation::DefaultPriority, 0);
     EXPECT_EQ(1u, result.size());
-    EXPECT_TRUE(interpolationValue(result.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(2).get()));
+    EXPECT_TRUE(interpolationValue(result.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(3).get()));
 }
 
 TEST_F(AnimationAnimationStackTest, NewAnimations)
@@ -102,7 +102,7 @@ TEST_F(AnimationAnimationStackTest, NewAnimations)
     newAnimations.append(inert2.get());
     WillBeHeapHashMap<CSSPropertyID, RefPtrWillBeMember<Interpolation> > result = AnimationStack::activeInterpolations(&element->activeAnimations()->defaultStack(), &newAnimations, 0, Animation::DefaultPriority, 10);
     EXPECT_EQ(2u, result.size());
-    EXPECT_TRUE(interpolationValue(result.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(1).get()));
+    EXPECT_TRUE(interpolationValue(result.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(3).get()));
     EXPECT_TRUE(interpolationValue(result.get(CSSPropertyZIndex))->equals(AnimatableDouble::create(4).get()));
 }
 
@@ -128,36 +128,27 @@ TEST_F(AnimationAnimationStackTest, ForwardsFillDiscarding)
     updateTimeline(11);
     Heap::collectAllGarbage();
     interpolations = AnimationStack::activeInterpolations(&element->activeAnimations()->defaultStack(), 0, 0, Animation::DefaultPriority, 0);
-    EXPECT_TRUE(interpolationValue(interpolations.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(2).get()));
+    EXPECT_TRUE(interpolationValue(interpolations.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(3).get()));
     EXPECT_EQ(3u, effects().size());
     EXPECT_EQ(1u, interpolations.size());
-    EXPECT_EQ(2, effects()[0]->sortInfo().startTime());
-    EXPECT_EQ(4, effects()[1]->sortInfo().startTime());
-    EXPECT_EQ(6, effects()[2]->sortInfo().startTime());
 
     updateTimeline(13);
     Heap::collectAllGarbage();
     interpolations = AnimationStack::activeInterpolations(&element->activeAnimations()->defaultStack(), 0, 0, Animation::DefaultPriority, 0);
-    EXPECT_TRUE(interpolationValue(interpolations.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(2).get()));
+    EXPECT_TRUE(interpolationValue(interpolations.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(3).get()));
     EXPECT_EQ(3u, effects().size());
-    EXPECT_EQ(2, effects()[0]->sortInfo().startTime());
-    EXPECT_EQ(4, effects()[1]->sortInfo().startTime());
-    EXPECT_EQ(6, effects()[2]->sortInfo().startTime());
 
     updateTimeline(15);
     Heap::collectAllGarbage();
     interpolations = AnimationStack::activeInterpolations(&element->activeAnimations()->defaultStack(), 0, 0, Animation::DefaultPriority, 0);
-    EXPECT_TRUE(interpolationValue(interpolations.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(2).get()));
+    EXPECT_TRUE(interpolationValue(interpolations.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(3).get()));
     EXPECT_EQ(2u, effects().size());
-    EXPECT_EQ(4, effects()[0]->sortInfo().startTime());
-    EXPECT_EQ(6, effects()[1]->sortInfo().startTime());
 
     updateTimeline(17);
     Heap::collectAllGarbage();
     interpolations = AnimationStack::activeInterpolations(&element->activeAnimations()->defaultStack(), 0, 0, Animation::DefaultPriority, 0);
-    EXPECT_TRUE(interpolationValue(interpolations.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(2).get()));
+    EXPECT_TRUE(interpolationValue(interpolations.get(CSSPropertyFontSize))->equals(AnimatableDouble::create(3).get()));
     EXPECT_EQ(1u, effects().size());
-    EXPECT_EQ(6, effects()[0]->sortInfo().startTime());
 }
 
 }

third_party/WebKit/Source/core/animation/AnimationTimeline.cpp

@@ -44,7 +44,7 @@ namespace {
 
 bool compareAnimationPlayers(const RefPtrWillBeMember<blink::AnimationPlayer>& left, const RefPtrWillBeMember<blink::AnimationPlayer>& right)
 {
-    return left->sortInfo().hasLowerSequenceNumber(right->sortInfo());
+    return AnimationPlayer::hasLowerPriority(left.get(), right.get());
 }
 
 }