Fix animations not starting on detached elements

Fix animations not starting on detached elements

An earlier fix for composited animations disabled animations
from starting on elements that aren't attached to a live DOM.
This patch fixes the behavioural regression.
This also avoids starting animations on the compositor for
detached elements that depend on style
e.g. {transform: 'translateX(10em)'}. Instead these animations
will run on the main thread.

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

[alancutter (OOO until 2018), 2015-06-22 07:03:56]

alancutter@chromium.org changed reviewers:
+ dstockwell@chromium.org, shans@chromium.org

[shans, 2015-06-22 11:51:39]

https://codereview.chromium.org/1196163003/diff/20001/LayoutTests/animations/...
File LayoutTests/animations/insert-composited-animation.html (right):

https://codereview.chromium.org/1196163003/diff/20001/LayoutTests/animations/...
LayoutTests/animations/insert-composited-animation.html:11: transform:
translate(50px, 50px);
Need a test to ensure connected divs with 'em' translate animations go to the
compositor too.

https://codereview.chromium.org/1196163003/diff/20001/Source/core/animation/E...
File Source/core/animation/EffectInput.cpp (left):

https://codereview.chromium.org/1196163003/diff/20001/Source/core/animation/E...
Source/core/animation/EffectInput.cpp:203: return nullptr;
doesn't this prevent elements that aren't in the document from being animated
using the API at all? That seems wrong.

[esprehn, 2015-06-23 01:07:28]

Can we just change the spec? Running a style animation on a detached element
doesn't make any sense, you can't compute style on a detached fragment (in any
browser...)

[shans, 2015-06-23 01:14:20]

On 2015/06/23 01:07:28, esprehn wrote:
> Can we just change the spec? Running a style animation on a detached element
> doesn't make any sense, you can't compute style on a detached fragment (in any
> browser...)

Running animations doesn't require style computation, only applying.

You can't apply animations on detached elements.

[alancutter (OOO until 2018), 2015-06-23 03:47:04]

https://codereview.chromium.org/1196163003/diff/20001/LayoutTests/animations/...
File LayoutTests/animations/insert-composited-animation.html (right):

https://codereview.chromium.org/1196163003/diff/20001/LayoutTests/animations/...
LayoutTests/animations/insert-composited-animation.html:11: transform:
translate(50px, 50px);
On 2015/06/22 at 11:51:39, shans wrote:
> Need a test to ensure connected divs with 'em' translate animations go to the
compositor too.

Done.

https://codereview.chromium.org/1196163003/diff/20001/Source/core/animation/E...
File Source/core/animation/EffectInput.cpp (left):

https://codereview.chromium.org/1196163003/diff/20001/Source/core/animation/E...
Source/core/animation/EffectInput.cpp:203: return nullptr;
On 2015/06/22 at 11:51:39, shans wrote:
> doesn't this prevent elements that aren't in the document from being animated
using the API at all? That seems wrong.

Correct, this patch fixes that.

[shans, 2015-06-23 03:56:44]

lgtm

[shans, 2015-06-23 03:56:51]

The CQ bit was checked by shans@chromium.org

[commit-bot: I haz the power, 2015-06-23 03:57:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1196163003/40001

[commit-bot: I haz the power, 2015-06-23 04:03:15]

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

[commit-bot: I haz the power, 2015-06-23 04:03:16]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/3...)

[alancutter (OOO until 2018), 2015-06-23 04:08:49]

On 2015/06/23 at 04:03:16, commit-bot wrote:
> Try jobs failed on following builders:
>   blink_presubmit on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/3...)

Still need core/OWNERS lgtms for the Internals changes.

[Timothy Loh, 2015-06-23 04:14:18]

"Still need core/OWNERS lgtms for the Internals changes." -> stamp

[Timothy Loh, 2015-06-23 04:14:31]

On 2015/06/23 04:14:18, Timothy Loh wrote:
> "Still need core/OWNERS lgtms for the Internals changes." -> stamp

eh that didn't work, lgtm :p

[alancutter (OOO until 2018), 2015-06-23 04:24:23]

The CQ bit was checked by alancutter@chromium.org

[commit-bot: I haz the power, 2015-06-23 04:24:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1196163003/40001

[commit-bot: I haz the power, 2015-06-23 04:35:41]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197624