UI Compositor: Fix threaded animation survival if layer removed and added.

UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines inside same Compositor(AnimationHost) without any animation losses.

See http://crbug.com/592873 (To be designed separately)

BUG=591474
Committed: https://crrev.com/3338dfbd7f61e4ebb5a1e1519e0ef930cd7499d8
Cr-Commit-Position: refs/heads/master@{#380047}

[loyso (OOO), 2016-03-08 02:32:31]

Description was changed from

==========
UI Compositor: Fix threaded animation survival if layer removed and added.

BUG=591474
==========

to

==========
UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines within same
Compositor(AnimationHost) without any animation losses. (To be designed
separately)

BUG=591474
==========

[loyso (OOO), 2016-03-08 02:32:31]

loyso@chromium.org changed reviewers:
+ ajuma@chromium.org, vollick@chromium.org

[loyso (OOO), 2016-03-08 03:18:49]

Description was changed from

==========
UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines within same
Compositor(AnimationHost) without any animation losses. (To be designed
separately)

BUG=591474
==========

to

==========
UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines within same
Compositor(AnimationHost) without any animation losses.

See http://crbug.com/592873 (To be designed separately)

BUG=591474
==========

[loyso (OOO), 2016-03-08 03:49:16]

Description was changed from

==========
UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines within same
Compositor(AnimationHost) without any animation losses.

See http://crbug.com/592873 (To be designed separately)

BUG=591474
==========

to

==========
UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines inside same
Compositor(AnimationHost) without any animation losses.

See http://crbug.com/592873 (To be designed separately)

BUG=591474
==========

[ajuma, 2016-03-08 14:09:07]

Note that layers can also move from one Compositor to another (e.g. when
dragging a tab from one monitor to another). I think your fix still works for
this case (since you register the animation controller with the destination
Compositor's animation registrar), but please add a test that verifies this.

With that test added, lgtm as a minimal mergeable fix.

[loyso (OOO), 2016-03-08 23:43:53]

On 2016/03/08 14:09:07, ajuma wrote:
> Note that layers can also move from one Compositor to another (e.g. when
> dragging a tab from one monitor to another). I think your fix still works for
> this case (since you register the animation controller with the destination
> Compositor's animation registrar), but please add a test that verifies this.
Done.
Also, DCHECK was failing on some tests. Changed DCHECK to |if| condition.

[ajuma, 2016-03-08 23:51:01]

https://codereview.chromium.org/1776513002/diff/40001/ui/compositor/layer_ani...
File ui/compositor/layer_animator.cc (right):

https://codereview.chromium.org/1776513002/diff/40001/ui/compositor/layer_ani...
ui/compositor/layer_animator.cc:166: animation_controller_state_->id() ==
delegate_->GetCcLayer()->id()) {
To turn this back into a DCHECK, clear the animation_controller_state when
::SwitchToLayer gets called (since that's how we'd wind up with a new cc layer
id).

[loyso (OOO), 2016-03-09 00:01:03]

On 2016/03/08 23:51:01, ajuma wrote:
> To turn this back into a DCHECK, clear the animation_controller_state when
> ::SwitchToLayer gets called (since that's how we'd wind up with a new cc layer
> id).
Nice! Thanks! I've forgotten that Layer:SwitchToLayer is plumbed to
LayerAnimator::SwitchToLayer directly.

[loyso (OOO), 2016-03-09 00:09:36]

On 2016/03/08 23:51:01, ajuma wrote:
> To turn this back into a DCHECK, clear the animation_controller_state when
> ::SwitchToLayer gets called (since that's how we'd wind up with a new cc layer
> id).
Done!

[ajuma, 2016-03-09 00:15:09]

Thanks, lgtm.

[loyso (OOO), 2016-03-09 02:23:53]

The CQ bit was checked by loyso@chromium.org

[commit-bot: I haz the power, 2016-03-09 02:24:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1776513002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1776513002/60001

[commit-bot: I haz the power, 2016-03-09 03:07:40]

Description was changed from

==========
UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines inside same
Compositor(AnimationHost) without any animation losses.

See http://crbug.com/592873 (To be designed separately)

BUG=591474
==========

to

==========
UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines inside same
Compositor(AnimationHost) without any animation losses.

See http://crbug.com/592873 (To be designed separately)

BUG=591474
==========

[commit-bot: I haz the power, 2016-03-09 03:07:41]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-09 03:09:37]

Description was changed from

==========
UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines inside same
Compositor(AnimationHost) without any animation losses.

See http://crbug.com/592873 (To be designed separately)

BUG=591474
==========

to

==========
UI Compositor: Fix threaded animation survival if layer removed and added.

This is minimal fix, scoped in ui/compositor. To be merged.

In general, we must allow to move AnimationPlayers between timelines inside same
Compositor(AnimationHost) without any animation losses.

See http://crbug.com/592873 (To be designed separately)

BUG=591474

Committed: https://crrev.com/3338dfbd7f61e4ebb5a1e1519e0ef930cd7499d8
Cr-Commit-Position: refs/heads/master@{#380047}
==========

[commit-bot: I haz the power, 2016-03-09 03:09:38]

Patchset 4 (id:??) landed as
https://crrev.com/3338dfbd7f61e4ebb5a1e1519e0ef930cd7499d8
Cr-Commit-Position: refs/heads/master@{#380047}