cc: Clear entries from animations maps only when main thread wins

cc: Clear entries from animations maps only when main thread wins

Because we will may need these values in future commits.

BUG=624366
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel

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

[jaydasika, 2016-06-29 23:53:23]

Description was changed from

==========
cc: Clear entries from animations maps only when main thread wins

Because we will may need these values in future commits.

BUG=624366
==========

to

==========
cc: Clear entries from animations maps only when main thread wins

Because we will may need these values in future commits.

BUG=624366
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-06-30 00:06:53]

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

[jaydasika, 2016-06-30 00:06:54]

I don't know if this fixes the bug. I couldn't repro it reliably. But the DCHECK
added in this CL fail on that page. So, I am hoping this will fix it.

[jaydasika, 2016-06-30 00:26:39]

https://codereview.chromium.org/2105173006/diff/20001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2105173006/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:644: transform_animations_map_.erase(id);
Just realized map entries should be erased in RemoveLayer too.

[jaydasika, 2016-06-30 18:52:00]

https://codereview.chromium.org/2105173006/diff/20001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2105173006/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:644: transform_animations_map_.erase(id);
On 2016/06/30 00:26:39, jaydasika wrote:
> Just realized map entries should be erased in RemoveLayer too. 

Done.

[ajuma, 2016-06-30 21:02:22]

lgtm

[jaydasika, 2016-06-30 21:04:36]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-06-30 21:05:38]

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-06-30 21:58:56]

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

[commit-bot: I haz the power, 2016-06-30 21:58:57]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jaydasika, 2016-07-01 18:25:39]

The CQ bit was checked by jaydasika@chromium.org

[jaydasika, 2016-07-01 18:25:39]

The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2105173006/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-07-01 18:26:32]

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-07-01 19:08:58]

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

[commit-bot: I haz the power, 2016-07-01 19:08:59]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jaydasika, 2016-07-06 22:48:08]

There is a perf test that is hitting the DCHECK. I am unable to repro it.
Probably because the DCHECK is hit during an animation (?). Can I remove the
DCHECK and land this CL ?

[ajuma, 2016-07-06 23:17:41]

On 2016/07/06 22:48:08, jaydasika wrote:
> There is a perf test that is hitting the DCHECK. I am unable to repro it.
> Probably because the DCHECK is hit during an animation (?). Can I remove the
> DCHECK and land this CL ?

A mismatch between the transform on the layer and the transform in the transform
node could be a real bug, so it'd be good to try to investigate this some more.

[jaydasika, 2016-07-18 21:27:20]

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

[commit-bot: I haz the power, 2016-07-18 21:28:35]

Dry run: 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-07-18 21:28:37]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.blink
   For more details, see http://crbug.com/617627.

[jaydasika, 2016-07-18 21:33:45]

The DCHECK was failing for the case where there are no animations on compositor
between two commits and the next commit wants the compositor value transform to
win.
But, as transform_animations_map is empty, main thread value stays in the
property trees.

Removed the DCHECK and added a test for this case.

[jaydasika, 2016-07-18 21:34:20]

Description was changed from

==========
cc: Clear entries from animations maps only when main thread wins

Because we will may need these values in future commits.

BUG=624366
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Clear entries from animations maps only when main thread wins

Because we will may need these values in future commits.

BUG=624366
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[ajuma, 2016-07-18 21:45:55]

On 2016/07/18 21:33:45, jaydasika wrote:
> The DCHECK was failing for the case where there are no animations on
compositor
> between two commits and the next commit wants the compositor value transform
to
> win.
> But, as transform_animations_map is empty, main thread value stays in the
> property trees.
> 
> Removed the DCHECK and added a test for this case.

Thanks, lgtm

[commit-bot: I haz the power, 2016-07-18 22:52:58]

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

[commit-bot: I haz the power, 2016-07-18 22:52:59]

Dry run: This issue passed the CQ dry run.

[jaydasika, 2016-07-18 23:12:03]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-07-18 23:13:20]

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-07-18 23:29:10]

Description was changed from

==========
cc: Clear entries from animations maps only when main thread wins

Because we will may need these values in future commits.

BUG=624366
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc: Clear entries from animations maps only when main thread wins

Because we will may need these values in future commits.

BUG=624366
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-07-18 23:29:11]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-07-18 23:29:22]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-18 23:30:19]

Description was changed from

==========
cc: Clear entries from animations maps only when main thread wins

Because we will may need these values in future commits.

BUG=624366
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc: Clear entries from animations maps only when main thread wins

Because we will may need these values in future commits.

BUG=624366
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel

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

[commit-bot: I haz the power, 2016-07-18 23:30:21]

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