Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(281)

Issue 275753002: Web Animations: Timeline should not advance during task execution (Closed)

Created:
6 years, 7 months ago by dstockwell
Modified:
6 years, 7 months ago
Reviewers:
Noel Gordon
CC:
blink-reviews, rwlbuis, shans, rjwright, Mike Lawther (Google), blink-reviews-animation_chromium.org, sof, eae+blinkwatch, dglazkov+blink, dstockwell, Timothy Loh, blink-reviews-dom_chromium.org, darktears, Steve Block, Eric Willigers, abarth-chromium
Visibility:
Public.

Description

Web Animations: Timeline should not advance during task execution Previously the animation clock was only frozen during animation frame callbacks. This meant that the timeline was able to advance during other tasks. This patch allows the clock to advance once per task, either to the compositor supplied frame start time or to an approximation of the next expected frame time. http://dev.w3.org/fxtf/web-animations/#script-execution-and-live-updates-to-the-model This is a reland of r173583[1], the previous patch hit added assertions due to double rounding issues, the new approach avoids these. Also ensures that other unit tests see a non-zero currentTime (seen in some webkit_unit_test runs on Android). [1] https://src.chromium.org/viewvc/blink?view=rev&revision=173583 BUG=367903 TBR=abarth Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174778

Patch Set 1 : Original patch #

Patch Set 2 : Updated logic #

Patch Set 3 : Rebase. #

Patch Set 4 : #

Patch Set 5 : Add back missing test changes. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+174 lines, -77 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M LayoutTests/web-animations-api/element-animate-position-crash.html View 3 4 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/web-animations-api/element-animate-position-crash-expected.txt View 3 4 1 chunk +1 line, -2 lines 0 comments Download
M LayoutTests/web-animations-api/timeline-time.html View 3 4 1 chunk +38 lines, -18 lines 0 comments Download
M Source/core/animation/AnimationClock.h View 1 2 1 chunk +12 lines, -18 lines 0 comments Download
M Source/core/animation/AnimationClock.cpp View 1 2 3 1 chunk +34 lines, -5 lines 0 comments Download
M Source/core/animation/AnimationClockTest.cpp View 1 2 2 chunks +79 lines, -23 lines 0 comments Download
M Source/core/animation/DocumentAnimations.cpp View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/dom/Document.h View 1 2 4 chunks +3 lines, -3 lines 0 comments Download
M Source/core/dom/Document.cpp View 1 2 2 chunks +0 lines, -2 lines 0 comments Download
M Source/web/WebKit.cpp View 1 2 2 chunks +6 lines, -2 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
dstockwell
6 years, 7 months ago (2014-05-09 05:14:02 UTC) #1
Noel Gordon
LGTM let's see if it fixed the failing SVG unit test.
6 years, 7 months ago (2014-05-09 05:19:57 UTC) #2
dstockwell
The CQ bit was checked by dstockwell@chromium.org
6 years, 7 months ago (2014-05-09 05:21:30 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dstockwell@chromium.org/275753002/40001
6 years, 7 months ago (2014-05-09 05:23:25 UTC) #4
commit-bot: I haz the power
Change committed as 173729
6 years, 7 months ago (2014-05-09 06:35:56 UTC) #5
dstockwell
A revert of this CL has been created in https://codereview.chromium.org/275863002/ by dstockwell@chromium.org. The reason for ...
6 years, 7 months ago (2014-05-09 11:41:22 UTC) #6
dstockwell
The CQ bit was checked by dstockwell@chromium.org
6 years, 7 months ago (2014-05-20 05:50:47 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dstockwell@chromium.org/275753002/80001
6 years, 7 months ago (2014-05-20 05:50:57 UTC) #8
commit-bot: I haz the power
Change committed as 174346
6 years, 7 months ago (2014-05-20 06:03:13 UTC) #9
dstockwell
A revert of this CL has been created in https://codereview.chromium.org/299723002/ by dstockwell@chromium.org. The reason for ...
6 years, 7 months ago (2014-05-20 18:05:32 UTC) #10
dstockwell
The CQ bit was checked by dstockwell@chromium.org
6 years, 7 months ago (2014-05-26 00:07:55 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dstockwell@chromium.org/275753002/190001
6 years, 7 months ago (2014-05-26 00:08:06 UTC) #12
commit-bot: I haz the power
6 years, 7 months ago (2014-05-26 00:32:48 UTC) #13
Message was sent while issue was closed.
Change committed as 174778

Powered by Google App Engine
This is Rietveld 408576698