Web Animations: Remove timeLag from Player API and model

Web Animations: Remove timeLag from Player API and model

API changes are guarded by experimental feature flag. No visible changes to CSS
animations and element.animate.

This is a prototype for behavior we plan to incorporate into the Web Animations
specification:
* startTime is effective start time or NaN when paused or starting
* timeLag is removed

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178619

[shans, 2014-07-04 06:19:27]

LGTM, please get an additional review

[dstockwell, 2014-07-04 06:21:05]

API changes are guarded by experimental feature flag. No visible changes to CSS
animations and element.animate.

This is a prototype for behavior we plan to incorporate into the Web Animations
specification.

[samli, 2014-07-14 07:03:58]

On 2014/07/04 06:21:05, dstockwell wrote:
> API changes are guarded by experimental feature flag. No visible changes to
CSS
> animations and element.animate.
> 
> This is a prototype for behavior we plan to incorporate into the Web
Animations
> specification.

This change fixes http://crbug.com/376597

Just need to remove player.timelag in LayoutTests/web-animations-api/player.html
and remove the test from TestExpectations.

[Timothy Loh, 2014-07-18 02:48:04]

lgtm

seems fine to me.

[dstockwell, 2014-07-21 23:58:18]

The CQ bit was checked by dstockwell@chromium.org

[commit-bot: I haz the power, 2014-07-21 23:58:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dstockwell@chromium.org/365163004/40001

[commit-bot: I haz the power, 2014-07-21 23:58:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-21 23:58:50]

Failed to apply patch for Source/core/animation/AnimationPlayer.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/animation/AnimationPlayer.cpp
  Hunk #1 FAILED at 61.
  Hunk #3 FAILED at 151.
  Hunk #4 succeeded at 209 (offset -1 lines).
  Hunk #5 succeeded at 218 (offset -1 lines).
  Hunk #6 succeeded at 227 (offset -1 lines).
  Hunk #7 succeeded at 255 (offset -1 lines).
  Hunk #8 succeeded at 272 (offset -1 lines).
  Hunk #9 succeeded at 310 (offset -1 lines).
  Hunk #10 succeeded at 334 (offset -1 lines).
  Hunk #11 succeeded at 361 (offset -1 lines).
  Hunk #12 succeeded at 423 (offset -11 lines).
  2 out of 12 hunks FAILED -- saving rejects to file
Source/core/animation/AnimationPlayer.cpp.rej

Patch:       Source/core/animation/AnimationPlayer.cpp
Index: Source/core/animation/AnimationPlayer.cpp
diff --git a/Source/core/animation/AnimationPlayer.cpp
b/Source/core/animation/AnimationPlayer.cpp
index
1e313071acc87019d621f4f2921c1910a856e39f..79586561504d42f5766ec32d22f2853591f0ee8c
100644
--- a/Source/core/animation/AnimationPlayer.cpp
+++ b/Source/core/animation/AnimationPlayer.cpp
@@ -61,7 +61,6 @@ AnimationPlayer::AnimationPlayer(ExecutionContext*
executionContext, AnimationTi
     , m_playbackRate(1)
     , m_startTime(nullValue())
     , m_holdTime(nullValue())
-    , m_storedTimeLag(0)
     , m_sortInfo(nextSequenceNumber(), timeline.effectiveTime())
     , m_content(content)
     , m_timeline(&timeline)
@@ -99,49 +98,46 @@ bool AnimationPlayer::limited(double currentTime) const
     return (m_playbackRate < 0 && currentTime <= 0) || (m_playbackRate > 0 &&
currentTime >= sourceEnd());
 }
 
-double AnimationPlayer::currentTimeWithoutLag() const
-{
-    if (isNull(m_startTime) || !m_timeline)
-        return 0;
-    return (m_timeline->effectiveTime() - m_startTime) * m_playbackRate;
-}
-
-double AnimationPlayer::currentTimeWithLag() const
+void AnimationPlayer::setCurrentTimeInternal(double newCurrentTime)
 {
-    ASSERT(!m_held);
-    double time = currentTimeWithoutLag();
-    return std::isinf(time) ? time : time - m_storedTimeLag;
-}
+    ASSERT(std::isfinite(newCurrentTime));
 
-void AnimationPlayer::updateTimingState(double newCurrentTime)
-{
-    ASSERT(!isNull(newCurrentTime));
     bool oldHeld = m_held;
+    bool outdated = false;
     m_held = m_paused || !m_playbackRate || limited(newCurrentTime);
+
     if (m_held) {
         if (!oldHeld || m_holdTime != newCurrentTime)
-            setOutdated();
+            outdated = true;
         m_holdTime = newCurrentTime;
-        m_storedTimeLag = nullValue();
+        if (m_paused || !m_playbackRate)
+            m_startTime = nullValue();
     } else {
         m_holdTime = nullValue();
-        m_storedTimeLag = currentTimeWithoutLag() - newCurrentTime;
+        m_startTime = m_timeline->effectiveTime() - newCurrentTime /
m_playbackRate;
         m_finished = false;
+        outdated = true;
+    }
+
+    if (outdated) {
         setOutdated();
+        cancelAnimationOnCompositor();
+        if (!m_held)
+            schedulePendingAnimationOnCompositor();
     }
 }
 
+// Update timing to reflect updated animation clock due to tick
 void AnimationPlayer::updateCurrentTimingState()
 {
     if (m_held) {
-        updateTimingState(m_holdTime);
+        setCurrentTimeInternal(m_holdTime);
         return;
     }
-    if (!limited(currentTimeWithLag()))
+    if (!limited(calculateCurrentTime()))
         return;
     m_held = true;
     m_holdTime = m_playbackRate < 0 ? 0 : sourceEnd();
-    m_storedTimeLag = nullValue();
 }
 
 double AnimationPlayer::currentTime()
@@ -154,38 +150,42 @@ double AnimationPlayer::currentTimeInternal()
     updateCurrentTimingState();
     if (m_held)
         return m_holdTime;
-    return currentTimeWithLag();
+    return calculateCurrentTime();
 }
 
-void AnimationPlayer::setCurrentTime(double newCurrentTime)
+double AnimationPlayer::calculateCurrentTime() const
 {
-    setCurrentTimeInternal(newCurrentTime / 1000);
+    ASSERT(!m_held);
+    if (isNull(m_startTime) || !m_timeline)
+        return 0;
+    return (m_timeline->effectiveTime() - m_startTime) * m_playbackRate;
 }
 
-void AnimationPlayer::setCurrentTimeInternal(double newCurrentTime)
+void AnimationPlayer::setCurrentTime(double newCurrentTime)
 {
     if (!std::isfinite(newCurrentTime))
         return;
-    updateTimingState(newCurrentTime);
-    cancelAnimationOnCompositor();
-    schedulePendingAnimationOnCompositor();
+    setCurrentTimeInternal(newCurrentTime / 1000);
 }
 
 void AnimationPlayer::setStartTimeInternal(double newStartTime, bool
isUpdateFromCompositor)
 {
     ASSERT(!isUpdateFromCompositor || !hasStartTime());
 
+    if (m_paused)
+        return;
     if (!std::isfinite(newStartTime))
         return;
     if (newStartTime == m_startTime)
         return;
-    updateCurrentTimingState(); // Update the value of held
+
     bool hadStartTime = hasStartTime();
     double previousCurrentTime = currentTimeInternal();
     m_startTime = newStartTime;
     m_sortInfo.m_startTime = newStartTime;
-    updateCurrentTimingState();
-    if (previousCurrentTime != currentTimeInternal()) {
+    double newCurrentTime = currentTimeInternal();
+
+    if (previousCurrentTime != newCurrentTime) {
         setOutdated();
     } else if (!hadStartTime && m_timeline) {
         // Even though this player is not outdated, time to effect change is
@@ -213,7 +213,7 @@ void AnimationPlayer::setSource(AnimationNode* newSource)
             newSource->player()->cancel();
         newSource->attach(this);
     }
-    updateTimingState(storedCurrentTime);
+    setCurrentTimeInternal(storedCurrentTime);
     schedulePendingAnimationOnCompositor();
 }
 
@@ -222,7 +222,7 @@ void AnimationPlayer::pause()
     if (m_paused)
         return;
     m_paused = true;
-    updateTimingState(currentTimeInternal());
+    setCurrentTimeInternal(currentTimeInternal());
     cancelAnimationOnCompositor();
 }
 
@@ -231,14 +231,12 @@ void AnimationPlayer::unpause()
     if (!m_paused)
         return;
     m_paused = false;
-    updateTimingState(currentTimeInternal());
-    schedulePendingAnimationOnCompositor();
+    setCurrentTimeInternal(currentTimeInternal());
 }
 
 void AnimationPlayer::play()
 {
     cancelAnimationOnCompositor();
-    // Note, unpause schedules pending animation on compositor if necessary.
     unpause();
     if (!m_content)
         return;
@@ -261,8 +259,6 @@ void AnimationPlayer::reverse()
             setCurrentTimeInternal(0);
     }
     setPlaybackRate(-m_playbackRate);
-    cancelAnimationOnCompositor();
-    // Note, unpause schedules pending animation on compositor if necessary.
     unpause();
 }
 
@@ -280,7 +276,6 @@ void AnimationPlayer::finish(ExceptionState& exceptionState)
         setCurrentTimeInternal(sourceEnd());
     }
     ASSERT(finished());
-    cancelAnimationOnCompositor();
 }
 
 const AtomicString& AnimationPlayer::interfaceName() const
@@ -319,9 +314,7 @@ void AnimationPlayer::setPlaybackRate(double playbackRate)
     if ((m_playbackRate < 0 && playbackRate >= 0) || (m_playbackRate > 0 &&
playbackRate <= 0))
         m_finished = false;
     m_playbackRate = playbackRate;
-    updateTimingState(storedCurrentTime);
-    cancelAnimationOnCompositor();
-    schedulePendingAnimationOnCompositor();
+    setCurrentTimeInternal(storedCurrentTime);
 }
 
 void AnimationPlayer::setOutdated()
@@ -345,7 +338,7 @@ bool AnimationPlayer::maybeStartAnimationOnCompositor()
     if (!canStartAnimationOnCompositor())
         return false;
 
-    return
toAnimation(m_content.get())->maybeStartAnimationOnCompositor(timeline()->zeroTime()
+ startTimeInternal() + timeLagInternal());
+    return
toAnimation(m_content.get())->maybeStartAnimationOnCompositor(timeline()->zeroTime()
+ startTimeInternal());
 }
 
 void AnimationPlayer::schedulePendingAnimationOnCompositor()
@@ -372,11 +365,12 @@ void AnimationPlayer::cancelAnimationOnCompositor()
 
 bool AnimationPlayer::update(TimingUpdateReason reason)
 {
-    m_outdated = false;
-
     if (!m_timeline)
         return false;
 
+    updateCurrentTimingState();
+    m_outdated = false;
+
     if (m_content) {
         double inheritedTime = isNull(m_timeline->currentTimeInternal()) ?
nullValue() : currentTimeInternal();
         m_content->updateInheritedTime(inheritedTime, reason);
@@ -443,7 +437,7 @@ bool AnimationPlayer::addEventListener(const AtomicString&
eventType, PassRefPtr
 void AnimationPlayer::pauseForTesting(double pauseTime)
 {
     RELEASE_ASSERT(!paused());
-    updateTimingState(pauseTime);
+    setCurrentTimeInternal(pauseTime);
     if (!m_isPausedForTesting && hasActiveAnimationsOnCompositor())
        
toAnimation(m_content.get())->pauseAnimationForTestingOnCompositor(currentTimeInternal());
     m_isPausedForTesting = true;

[dstockwell, 2014-07-22 03:14:52]

The CQ bit was checked by dstockwell@chromium.org

[commit-bot: I haz the power, 2014-07-22 03:15:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dstockwell@chromium.org/365163004/60001

[commit-bot: I haz the power, 2014-07-22 04:21:56]

Change committed as 178619