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

Source/core/animation/Player.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 74 matching lines @@
     return (timelineTime - m_startTime) * m_playbackRate;
 }
 
 double Player::currentTimeWithLag() const
 {
     ASSERT(!m_held);
     double time = currentTimeWithoutLag();
     return std::isinf(time) ? time : time - m_storedTimeLag;
 }
 
-void Player::updateTimingState(double newCurrentTime)
+void Player::updateTimingState(double newCurrentTime, bool shouldSetOutdated)
 {
     ASSERT(!isNull(newCurrentTime));
     m_held = m_paused || !m_playbackRate || limited(newCurrentTime);
     if (m_held) {
         m_holdTime = newCurrentTime;
         m_storedTimeLag = nullValue();
     } else {
         m_holdTime = nullValue();
         m_storedTimeLag = currentTimeWithoutLag() - newCurrentTime;
     }
-    setOutdated();
+    if (shouldSetOutdated)

[dstockwell, 2014/02/24 07:44:07]

Shouldn't we be able to derive this from above? Changes in the values of
m_holdTime/m_storedTimeLag indicate shouldSetOutdated?

[Timothy Loh, 2014/03/04 03:13:07]

Ok this ends up being a little more complicated because the calculation for
storedTimeLag currently is something like (currentTimeWithoutLag -
(currentTimeWithoutLag - storedTimeLag)) when we aren't held. This probably
introduces both floating point issues and timing issues (if we aren't in an
animation frame the timeline's time could advance between calls).

I've tweaked things around to hopefully avoid these issues in patch set 2. I
don't bother checking for changes in m_storedTimeLag since if we're not held
then currentTime() calls shouldn't come through here and for the reasons above.

+        setOutdated();
 }
 
-void Player::updateCurrentTimingState()
+void Player::updateCurrentTimingState(bool shouldSetOutdated)
 {
     if (m_held) {
-        updateTimingState(m_holdTime);
+        updateTimingState(m_holdTime, shouldSetOutdated);
     } else {
-        updateTimingState(currentTimeWithLag());
+        updateTimingState(currentTimeWithLag(), shouldSetOutdated);
         if (m_held && limited(m_holdTime))
             m_holdTime = m_playbackRate < 0 ? 0 : sourceEnd();
     }
 }
 
 double Player::currentTime()
 {
-    updateCurrentTimingState();
+    updateCurrentTimingState(false);
     if (m_held)
         return m_holdTime;
     return currentTimeWithLag();
 }
 
 void Player::setCurrentTime(double newCurrentTime)
 {
     if (!std::isfinite(newCurrentTime))
         return;
     updateTimingState(newCurrentTime);
@@ skipping 121 matching lines @@
 }
 
 void Player::cancelAnimationOnCompositor()
 {
     if (hasActiveAnimationsOnCompositor())
         toAnimation(m_content.get())->cancelAnimationOnCompositor();
 }
 
 bool Player::update()
 {
-    if (!m_timeline)
+    m_outdated = false;
+
+    if (!m_timeline || !m_content)
         return false;
 
     double inheritedTime = isNull(m_timeline->currentTime()) ? nullValue() : currentTime();
-    m_outdated = false;
-
-    if (!m_content)
-        return false;
-
     m_content->updateInheritedTime(inheritedTime);
 
     ASSERT(!m_outdated);
     return m_content->isCurrent() || m_content->isInEffect();
 }
 
 double Player::timeToEffectChange()
 {
     ASSERT(!m_outdated);
     if (!m_content || !m_playbackRate)
@@ skipping 17 matching lines @@
 {
     RELEASE_ASSERT(!paused());
     updateTimingState(pauseTime);
     if (!m_isPausedForTesting && hasActiveAnimationsOnCompositor())
         toAnimation(m_content.get())->pauseAnimationForTestingOnCompositor(currentTime());
     m_isPausedForTesting = true;
     pause();
 }
 
 } // namespace