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

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
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168628

[Timothy Loh, 2014-02-19 05:51:39]

I think this depends on 171983002 and 168983009 landing first.

[Eric Willigers, 2014-02-23 23:44:55]

lgtm

[dstockwell, 2014-02-24 07:44:06]

https://codereview.chromium.org/172003002/diff/1/Source/core/animation/Player...
File Source/core/animation/Player.cpp (right):

https://codereview.chromium.org/172003002/diff/1/Source/core/animation/Player...
Source/core/animation/Player.cpp:106: if (shouldSetOutdated)
Shouldn't we be able to derive this from above? Changes in the values of
m_holdTime/m_storedTimeLag indicate shouldSetOutdated?

[dstockwell, 2014-02-27 09:14:50]

On 2014/02/24 07:44:06, dstockwell wrote:
>
https://codereview.chromium.org/172003002/diff/1/Source/core/animation/Player...
> File Source/core/animation/Player.cpp (right):
> 
>
https://codereview.chromium.org/172003002/diff/1/Source/core/animation/Player...
> Source/core/animation/Player.cpp:106: if (shouldSetOutdated)
> Shouldn't we be able to derive this from above? Changes in the values of
> m_holdTime/m_storedTimeLag indicate shouldSetOutdated?

Ping?

[Timothy Loh, 2014-02-28 02:39:16]

On 2014/02/27 09:14:50, dstockwell wrote:
> On 2014/02/24 07:44:06, dstockwell wrote:
> >
>
https://codereview.chromium.org/172003002/diff/1/Source/core/animation/Player...
> > File Source/core/animation/Player.cpp (right):
> > 
> >
>
https://codereview.chromium.org/172003002/diff/1/Source/core/animation/Player...
> > Source/core/animation/Player.cpp:106: if (shouldSetOutdated)
> > Shouldn't we be able to derive this from above? Changes in the values of
> > m_holdTime/m_storedTimeLag indicate shouldSetOutdated?
> 
> Ping?

Your comment seems pretty reasonable to me, I'll fix this up next week.

[Timothy Loh, 2014-03-04 03:13:06]

https://codereview.chromium.org/172003002/diff/1/Source/core/animation/Player...
File Source/core/animation/Player.cpp (right):

https://codereview.chromium.org/172003002/diff/1/Source/core/animation/Player...
Source/core/animation/Player.cpp:106: if (shouldSetOutdated)
On 2014/02/24 07:44:07, dstockwell wrote:
> Shouldn't we be able to derive this from above? Changes in the values of
> m_holdTime/m_storedTimeLag indicate shouldSetOutdated?

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.

[dstockwell, 2014-03-06 02:15:29]

lgtm, test?

[Timothy Loh, 2014-03-06 04:58:42]

The CQ bit was checked by timloh@chromium.org

[commit-bot: I haz the power, 2014-03-06 04:58:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/timloh@chromium.org/172003002/140001

[Timothy Loh, 2014-03-06 04:58:55]

On 2014/03/06 02:15:29, dstockwell wrote:
> lgtm, test?

Added; also updated description.

[commit-bot: I haz the power, 2014-03-06 07:26:08]

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

[commit-bot: I haz the power, 2014-03-06 07:26:08]

Try jobs failed on following builders: win_blink_rel

[Timothy Loh, 2014-03-06 11:18:46]

The CQ bit was checked by timloh@chromium.org

[commit-bot: I haz the power, 2014-03-06 11:19:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/timloh@chromium.org/172003002/140001

[commit-bot: I haz the power, 2014-03-06 11:57:35]

Change committed as 168628