Web Animations CSS: Unfreeze AnimationClock if sampling timelines does not trigger style recalc

Web Animations CSS: Unfreeze AnimationClock if sampling timelines does not trigger style recalc

When sampling the animations and transitions timelines, we set the
AnimationClock's time and freeze it so that subsequent accesses of the clock's
time during style recalc receive the same value. The clock is unfrozen when
style recalc completes.

However, if a timeline is sampled after its last Player is removed, it will not
trigger style recalc *. This can occur if a transition is cancelled
prematurely. In this case, we will fail to unfreeze the clock, so transitions
started subsequently will see a stale clock time.

This change modifies DocumentTimeline::serviceAnimations() to unfreeze the
AnimationClock if it did not trigger a style recalc. It also adds a test to
verify this behavior.

* In non-CSS uses of the Web Animations model, a timeline can also fail to
trigger style recalc if its animations have no effect or no target.

BUG=303430, 248938
R=timloh@chromium.org

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

[Timothy Loh, 2013-10-30 03:55:06]

lgtm

https://codereview.chromium.org/46043014/diff/70001/LayoutTests/transitions/c...
File LayoutTests/transitions/cancel-and-start-new.html (right):

https://codereview.chromium.org/46043014/diff/70001/LayoutTests/transitions/c...
LayoutTests/transitions/cancel-and-start-new.html:54: var result = "<span
style='color:red'>FAIL(was: " + left + ", expected: 150)</span>";
expected: 200?

https://codereview.chromium.org/46043014/diff/70001/Source/core/animation/Ani...
File Source/core/animation/AnimationClock.h (left):

https://codereview.chromium.org/46043014/diff/70001/Source/core/animation/Ani...
Source/core/animation/AnimationClock.h:48: ASSERT(m_time <= time);
Could you mention why  we're removing this assertion?

[Steve Block, 2013-11-01 03:04:58]

https://codereview.chromium.org/46043014/diff/70001/LayoutTests/transitions/c...
File LayoutTests/transitions/cancel-and-start-new.html (right):

https://codereview.chromium.org/46043014/diff/70001/LayoutTests/transitions/c...
LayoutTests/transitions/cancel-and-start-new.html:54: var result = "<span
style='color:red'>FAIL(was: " + left + ", expected: 150)</span>";
On 2013/10/30 03:55:07, Timothy Loh wrote:
> expected: 200?

Done.

https://codereview.chromium.org/46043014/diff/70001/Source/core/animation/Ani...
File Source/core/animation/AnimationClock.h (left):

https://codereview.chromium.org/46043014/diff/70001/Source/core/animation/Ani...
Source/core/animation/AnimationClock.h:48: ASSERT(m_time <= time);
I removed it because we were hitting it in some unit tests. But it turns out
that we no longer do so in the latest version. Will reinstate.

[commit-bot: I haz the power, 2013-11-01 03:08:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/steveblock@chromium.org/46043014/290001

[commit-bot: I haz the power, 2013-11-01 04:46:00]

Change committed as 161134

[dstockwell, 2013-11-05 05:05:43]

https://codereview.chromium.org/46043014/diff/290001/Source/core/frame/FrameV...
File Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/46043014/diff/290001/Source/core/frame/FrameV...
Source/core/frame/FrameView.cpp:2030: bool didTriggerStyleRecalc =
frame->document()->timeline()->serviceAnimations();
It's unfortunate that we have to thread this all the way out. Wouldn't it be
sufficient to test !(frame->document->needsStyleRecalc() ||
...->childNeedsStyleRecalc()) ?