Fix assumptions made in LAC::MarkAnimationsForDeletion

Fix assumptions made in LAC::MarkAnimationsForDeletion

lof84 correctly noticed that this function was using broken logic to
determine if we were on the impl thread (i.e., a non-null events
vector). This got broken in my cl
https://codereview.chromium.org/1088773003, which collects events on
the main thread as well.

The test in this CL was taken directly from lof84's original patch:
http://crrev.com/1154323011

BUG=None
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/7324fb5f10b801ed49b4e1c8d743becded6d2ed3
Cr-Commit-Position: refs/heads/master@{#335262}

[Ian Vollick, 2015-06-05 18:51:31]

vollick@chromium.org changed reviewers:
+ enne@chromium.org, lof84@yandex-team.ru

[Lof, 2015-06-05 20:07:43]

https://codereview.chromium.org/1151763011/diff/1/cc/trees/layer_tree_host_un...
File cc/trees/layer_tree_host_unittest_animation.cc (right):

https://codereview.chromium.org/1151763011/diff/1/cc/trees/layer_tree_host_un...
cc/trees/layer_tree_host_unittest_animation.cc:1089:
TEST_F(LayerTreeHostAnimationTestNotifyAnimationFinished,
Could you please use SINGLE_AND_MULTI_THREAD_TEST_F macro?

SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostAnimationTestNotifyAnimationFinished);

[Lof, 2015-06-11 14:47:37]

Hello. Are there any difficulties? I could make CL.

[Ian Vollick, 2015-06-11 14:55:01]

The CQ bit was checked by vollick@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-06-11 14:55:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1151763011/20001

[commit-bot: I haz the power, 2015-06-11 15:05:01]

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

[commit-bot: I haz the power, 2015-06-11 15:05:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ian Vollick, 2015-06-11 21:00:38]

On 2015/06/11 at 15:05:02, commit-bot wrote:
> Dry run: Try jobs failed on following builders:
>   linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

I'm sorry -- it looks like I needed to rebase this patch and I lost track of it.
I'll get it sorted out shortly.

[Ian Vollick, 2015-06-11 21:47:40]

On 2015/06/11 at 21:00:38, vollick wrote:
> On 2015/06/11 at 15:05:02, commit-bot wrote:
> > Dry run: Try jobs failed on following builders:
> >   linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> 
> I'm sorry -- it looks like I needed to rebase this patch and I lost track of
it. I'll get it sorted out shortly.

Ah, FakeContentLayer died :) I've updated the test to use FakePictureLayer
instead. Long-live impl-paint!

enne, PTAL.

[Ian Vollick, 2015-06-12 14:33:38]

vollick@chromium.org changed reviewers:
+ ajuma@chromium.org
- enne@chromium.org

[Ian Vollick, 2015-06-12 14:33:51]

On 2015/06/11 at 21:47:40, vollick wrote:
> On 2015/06/11 at 21:00:38, vollick wrote:
> > On 2015/06/11 at 15:05:02, commit-bot wrote:
> > > Dry run: Try jobs failed on following builders:
> > >   linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> > 
> > I'm sorry -- it looks like I needed to rebase this patch and I lost track of
it. I'll get it sorted out shortly.
> 
> Ah, FakeContentLayer died :) I've updated the test to use FakePictureLayer
instead. Long-live impl-paint!
> 
> enne, PTAL.

+ajuma@

[ajuma, 2015-06-16 13:51:53]

https://codereview.chromium.org/1151763011/diff/40001/cc/animation/layer_anim...
File cc/animation/layer_animation_controller.cc (right):

https://codereview.chromium.org/1151763011/diff/40001/cc/animation/layer_anim...
cc/animation/layer_animation_controller.cc:857: if (events && impl_animation) {
This will prevent Finished events from being produced on the main thread, but
Start events will still get produced (and need to be, since the property tree
rebuild logic from the other CL depends on this). This seems asymmetric. Would
it make sense to continue to produce Finished events on the main thread?

[Ian Vollick, 2015-06-19 14:13:24]

On 2015/06/16 at 13:51:53, ajuma wrote:
>
https://codereview.chromium.org/1151763011/diff/40001/cc/animation/layer_anim...
> File cc/animation/layer_animation_controller.cc (right):
> 
>
https://codereview.chromium.org/1151763011/diff/40001/cc/animation/layer_anim...
> cc/animation/layer_animation_controller.cc:857: if (events && impl_animation)
{
> This will prevent Finished events from being produced on the main thread, but
Start events will still get produced (and need to be, since the property tree
rebuild logic from the other CL depends on this). This seems asymmetric. Would
it make sense to continue to produce Finished events on the main thread?

Thanks for explaining this to me offline. I've updated the patch so that the
finished events are still generated on the main and impl threads, and the test
is still happy.

[ajuma, 2015-06-19 14:16:39]

Thanks! lgtm.

[Ian Vollick, 2015-06-19 14:17:30]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2015-06-19 14:17:57]

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

[commit-bot: I haz the power, 2015-06-19 15:42:53]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-06-19 15:43:55]

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