Web Animations: Don't mark players as outdated upon currentTime access

Accessing currentTime on a Player logically doesn't mark it as outdated,
although currently it does as it updates time to match the timeline.
Removing this behaviour makes some of the logic in serviceAnimations and
Player::update slightly nicer. We also tweak the logic somewhat so as to
remove a source of rounding errors in timing calculations.

BUG=334926

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

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

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

@@ -93,6 +93,7 @@ void DocumentTimeline::serviceAnimations()
     TRACE_EVENT0("webkit", "DocumentTimeline::serviceAnimations");
 
     m_timing->cancelWake();
+    m_hasOutdatedPlayer = false;
 
     double timeToNextEffect = std::numeric_limits<double>::infinity();
     Vector<Player*> players;
@@ -115,7 +116,7 @@ void DocumentTimeline::serviceAnimations()
     else if (timeToNextEffect != std::numeric_limits<double>::infinity())
         m_timing->wakeAfter(timeToNextEffect - s_minimumDelay);
 
-    m_hasOutdatedPlayer = false;
+    ASSERT(!m_hasOutdatedPlayer);
 }
 
 void DocumentTimeline::setZeroTime(double zeroTime)

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

@@ -109,26 +109,31 @@ double Player::currentTimeWithLag() const
 void Player::updateTimingState(double newCurrentTime)
 {
     ASSERT(!isNull(newCurrentTime));
+    bool oldHeld = m_held;
     m_held = m_paused || !m_playbackRate || limited(newCurrentTime);
     if (m_held) {
+        if (!oldHeld || m_holdTime != newCurrentTime)
+            setOutdated();
         m_holdTime = newCurrentTime;
         m_storedTimeLag = nullValue();
     } else {
         m_holdTime = nullValue();
         m_storedTimeLag = currentTimeWithoutLag() - newCurrentTime;
+        setOutdated();
     }
-    setOutdated();
 }
 
 void Player::updateCurrentTimingState()
 {
     if (m_held) {
         updateTimingState(m_holdTime);
-    } else {
-        updateTimingState(currentTimeWithLag());
-        if (m_held && limited(m_holdTime))
-            m_holdTime = m_playbackRate < 0 ? 0 : sourceEnd();
+        return;
     }
+    if (!limited(currentTimeWithLag()))
+        return;
+    m_held = true;
+    m_holdTime = m_playbackRate < 0 ? 0 : sourceEnd();
+    m_storedTimeLag = nullValue();
 }
 
 double Player::currentTime()
@@ -152,8 +157,10 @@ void Player::setStartTime(double newStartTime)
         return;
     updateCurrentTimingState(); // Update the value of held
     m_startTime = newStartTime;
-    if (!m_held)
-        updateCurrentTimingState();
+    if (m_held)
+        return;
+    updateCurrentTimingState();
+    setOutdated();
 }
 
 void Player::setSource(TimedItem* newSource)
@@ -275,15 +282,12 @@ void Player::cancelAnimationOnCompositor()
 
 bool Player::update()
 {
-    if (!m_timeline)
-        return false;
-
-    double inheritedTime = isNull(m_timeline->currentTime()) ? nullValue() : currentTime();
     m_outdated = false;
 
-    if (!m_content)
+    if (!m_timeline || !m_content)
         return false;
 
+    double inheritedTime = isNull(m_timeline->currentTime()) ? nullValue() : currentTime();
     m_content->updateInheritedTime(inheritedTime);
 
     ASSERT(!m_outdated);

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

@@ -121,7 +121,8 @@ private:
     bool m_held;
     bool m_isPausedForTesting;
 
-    // This indicates timing information relevant to the player has changed
+    // This indicates timing information relevant to the player has changed by
+    // means other than the ordinary progression of time
     bool m_outdated;
 
     unsigned m_sequenceNumber;

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

@@ -113,6 +113,19 @@ TEST_F(AnimationPlayerTest, InitialState)
 }
 
 
+TEST_F(AnimationPlayerTest, CurrentTimeDoesNotSetOutdated)
+{
+    EXPECT_FALSE(player->outdated());
+    EXPECT_EQ(0, player->currentTime());
+    EXPECT_FALSE(player->outdated());
+    // FIXME: We should split updateTimeline into a version that doesn't update
+    // the player and one that does, as most of the tests don't require update()
+    // to be called.
+    document->animationClock().updateTime(10);
+    EXPECT_EQ(10, player->currentTime());
+    EXPECT_FALSE(player->outdated());
+}
+
 TEST_F(AnimationPlayerTest, SetCurrentTime)
 {
     player->setCurrentTime(10);