cc : Fix the bug in tracking currently running animations for an element

cc : Fix the bug in tracking currently running animations for an element

We are updating this value based on the status of the lst animation of
the element which is wrong.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/f5f0a49c24df3a81193e3d4eaf8e9a519b98ae4f
Cr-Commit-Position: refs/heads/master@{#418701}

[jaydasika, 2016-09-14 20:31:07]

Description was changed from

==========
cc : Fix the bug in tracking currently running animations for an element

We are updating this value based on the status of the lst animation of
the element which is wrong.
==========

to

==========
cc : Fix the bug in tracking currently running animations for an element

We are updating this value based on the status of the lst animation of
the element which is wrong.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[jaydasika, 2016-09-14 20:33:10]

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

[jaydasika, 2016-09-14 20:33:10]

PTAL

[jaydasika, 2016-09-14 20:33:40]

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

[commit-bot: I haz the power, 2016-09-14 20:34:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ajuma, 2016-09-14 21:41:13]

https://codereview.chromium.org/2346583002/diff/1/cc/animation/element_animat...
File cc/animation/element_animations_unittest.cc (right):

https://codereview.chromium.org/2346583002/diff/1/cc/animation/element_animat...
cc/animation/element_animations_unittest.cc:2917: // that the transform is
currently animating.
Would one of the tests above also have failed without the fix for the pending
elements case?

[jaydasika, 2016-09-14 21:50:55]

https://codereview.chromium.org/2346583002/diff/1/cc/animation/element_animat...
File cc/animation/element_animations_unittest.cc (right):

https://codereview.chromium.org/2346583002/diff/1/cc/animation/element_animat...
cc/animation/element_animations_unittest.cc:2917: // that the transform is
currently animating.
On 2016/09/14 21:41:13, ajuma wrote:
> Would one of the tests above also have failed without the fix for the pending
> elements case?

Yes, the test in line 2906.

[ajuma, 2016-09-14 21:55:10]

Thanks! lgtm

[commit-bot: I haz the power, 2016-09-14 22:28:17]

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

[commit-bot: I haz the power, 2016-09-14 22:28:18]

Dry run: This issue passed the CQ dry run.

[jaydasika, 2016-09-14 22:28:31]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-09-14 22:29:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 22:36:33]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-09-14 22:38:27]

Description was changed from

==========
cc : Fix the bug in tracking currently running animations for an element

We are updating this value based on the status of the lst animation of
the element which is wrong.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc : Fix the bug in tracking currently running animations for an element

We are updating this value based on the status of the lst animation of
the element which is wrong.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/f5f0a49c24df3a81193e3d4eaf8e9a519b98ae4f
Cr-Commit-Position: refs/heads/master@{#418701}
==========

[commit-bot: I haz the power, 2016-09-14 22:38:28]

Patchset 1 (id:??) landed as
https://crrev.com/f5f0a49c24df3a81193e3d4eaf8e9a519b98ae4f
Cr-Commit-Position: refs/heads/master@{#418701}

[loyso (OOO), 2016-09-14 23:49:08]

https://codereview.chromium.org/2346583002/diff/1/cc/animation/element_animat...
File cc/animation/element_animations.cc (left):

https://codereview.chromium.org/2346583002/diff/1/cc/animation/element_animat...
cc/animation/element_animations.cc:643: animation->InEffect(last_tick_time_);
NIT: Would be nice to add a DCHECK as a loop invariant.
Given that p = potentially_animating_for_pending_elements, c =
currently_running_for_active_elements, c must be a subset of p (c <= p).
i.e.:
// DNF: (!p && !c) || (p && !c) || (p && c)
DCHECK_LE(c, p);

In C++ you can compare the order of bool values, they are promoted to int as
false -> 0, true -> 1.

[loyso (OOO), 2016-09-14 23:53:21]

On 2016/09/14 23:49:08, loyso wrote:
> Given that p = potentially_animating_for_pending_elements, c =
> currently_running_for_active_elements, c must be a subset of p (c <= p).

"Given that p = potentially_animating_for_pending_elements, c =
currently_running_for_pending_elements", of course. Same for active.

[jaydasika, 2016-09-15 21:28:24]

On 2016/09/14 23:53:21, loyso wrote:
> On 2016/09/14 23:49:08, loyso wrote:
> > Given that p = potentially_animating_for_pending_elements, c =
> > currently_running_for_active_elements, c must be a subset of p (c <= p).
> 
> "Given that p = potentially_animating_for_pending_elements, c =
> currently_running_for_pending_elements", of course. Same for active.

Since this has already landed, will add it in some other CL.