Web Animations CSS: Record if style recalc is for animation

Web Animations CSS: Record if style recalc is for animation

Based on rolled back patch from issue 86763003

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

[dstockwell, 2013-12-16 09:54:07]

https://codereview.chromium.org/110123005/diff/20001/LayoutTests/transitions/...
File LayoutTests/transitions/interrupted-immediately.html (right):

https://codereview.chromium.org/110123005/diff/20001/LayoutTests/transitions/...
LayoutTests/transitions/interrupted-immediately.html:32:
window.setTimeout(function() {
Do we need the setTimeout?

Can this be something like:

box.style.left = '300px';
box.offsetTop; // force transition start

box.style.left = '0px'
box.offsetTop; // force transition interruption

box.addEventListener('transitionend', function() {
  // Evaluate ending condition
  testRunner.notifyDone();
});

// Force at least one timing update and recalc after the interruption.
requestAnimationFrame(function() {});

https://codereview.chromium.org/110123005/diff/20001/Source/core/animation/cs...
File Source/core/animation/css/CSSAnimations.cpp (right):

https://codereview.chromium.org/110123005/diff/20001/Source/core/animation/cs...
Source/core/animation/css/CSSAnimations.cpp:579: // In release builds we avoid
the cost of populating and testing listedProperties if the style recalc is due
to animation.
.. cost of populating and testing listedProperties if .. -> 
.. cost of checking for new and interrupted transitions if ..

[Timothy Loh, 2013-12-16 22:53:23]

Does this test fix this issue: http://jsfiddle.net/C7WDL/ ? If so, worthwhile to
add a test for it (I wouldn't mind having that part of the change in a separate
patch too, but up to you).

[Eric Willigers, 2013-12-16 23:26:32]

On 2013/12/16 22:53:23, Timothy Loh wrote:
> Does this test fix this issue: http://jsfiddle.net/C7WDL/ ? If so, worthwhile
to
> add a test for it (I wouldn't mind having that part of the change in a
separate
> patch too, but up to you).

No, we still see "we shouldn't reach here!"

[Eric Willigers, 2013-12-16 23:26:49]

https://codereview.chromium.org/110123005/diff/20001/LayoutTests/transitions/...
File LayoutTests/transitions/interrupted-immediately.html (right):

https://codereview.chromium.org/110123005/diff/20001/LayoutTests/transitions/...
LayoutTests/transitions/interrupted-immediately.html:32:
window.setTimeout(function() {
On 2013/12/16 09:54:08, dstockwell wrote:
> Do we need the setTimeout?

We didn't need the first setTimeout for interrupting the transition.

> Can this be something like:
> box.addEventListener('transitionend', function() {

With that code, the test fails (times out).

[dstockwell, 2013-12-17 00:15:31]

https://codereview.chromium.org/110123005/diff/20001/LayoutTests/transitions/...
File LayoutTests/transitions/interrupted-immediately.html (right):

https://codereview.chromium.org/110123005/diff/20001/LayoutTests/transitions/...
LayoutTests/transitions/interrupted-immediately.html:32:
window.setTimeout(function() {
On 2013/12/16 23:26:50, Eric Willigers wrote:
> On 2013/12/16 09:54:08, dstockwell wrote:
> > Do we need the setTimeout?
> 
> We didn't need the first setTimeout for interrupting the transition.
> 
> > Can this be something like:
> > box.addEventListener('transitionend', function() {
> 
> With that code, the test fails (times out). 

Yeah, what I had is clearly wrong. Since we don't expect to start a new
transition in that case. We should be able to leverage
internals.numberOfActiveAnimations() here

[dstockwell, 2013-12-17 03:16:47]

lgtm

OK to land this now and remove the setTimeouts from the test in a subsequent
change.

[commit-bot: I haz the power, 2013-12-17 03:32:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ericwilligers@chromium.org/110123005/6...

[commit-bot: I haz the power, 2013-12-17 06:04:25]

Retried try job too often on linux_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2013-12-17 11:48:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ericwilligers@chromium.org/110123005/8...

[commit-bot: I haz the power, 2013-12-17 13:02:24]

Change committed as 164013