Thanks for the great bug-report!

Confirmed that over a ~30s viewing of this demo, the map on the active tree does seem to be growing out of control. We see the pending one being cleared periodically but not the active one.

[1:8:1219/094135.086538:INFO:layer_tree_impl.cc(681)] 1: element_id_to_transform_animations_.size(): 98
[1:8:1219/094135.086620:INFO:layer_tree_impl.cc(681)] 0: element_id_to_transform_animations_.size(): 12
[1:8:1219/094135.094711:INFO:layer_tree_impl.cc(681)] 1: element_id_to_transform_animations_.size(): 98
[1:8:1219/094135.094824:INFO:layer_tree_impl.cc(681)] 0: element_id_to_transform_animations_.size(): 1

I will investigate, but not sure if I will be able to get to this before the winter holidays.

Coming back to looking at this. The immediate observation is that it's not clear why we bother filling element_id_to_transform_animations_ on the active tree (assuming we have a pending tree), since it is only used in LayerTreeImpl::UpdatePropertyTreeAnimationFromMainThread which is only called on the sync_tree == pending_tree.

I wonder if the correct solution might be to not populate the map in the active tree if !CommitToActiveTree. Alternatively we could sync the maps during LayerTreeHostImpl::ActivateSyncTree.

Note that this leakage likely occurs to:

  * element_id_to_opacity_animations_
  * element_id_to_transform_animations_
  * element_id_to_filter_animations_

It may or may not occur to element_id_to_scrollable_layer_. I just found LayerTreeImpl::RemoveFromElementLayerList which seems to clear that map. I am now looking at whether RemoveFromElementLayerList should be used for the animations maps too.

I tested a patch which uses RemoveFromElementLayerList and it does stop the memory leak, but given that the original patch was aiming to reduce references to LayerImpl::element_id_ and that would just add three back on, I don't think it is the correct approach.

Next I'm going to try syncing the maps during activation (because within a tree you understandably don't know if it the pending or active tree, so we can't really track whether we need element_id_to_transform_animations_).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d362daef89cdaaf03ff8e474119122bf121d2689

commit d362daef89cdaaf03ff8e474119122bf121d2689
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Mon Jan 14 20:30:33 2019

Don't track element_id_to_*_animations_ maps on non-sync trees

For the active tree, the element_id_to_*_animations_ maps were not being
cleared, ever. The only code that cleared them was
LayerTreeImpl::UpdatePropertyTreeAnimationFromMainThread which was only
called for the sync tree (which is usually the pending tree). However,
they also aren't used on the active tree.

To fix this, only track them on the sync tree.

Bug:  916512 
Change-Id: Ib9cae536ff38fb84700934c9aed9216311932c9c
Reviewed-on: https://chromium-review.googlesource.com/c/1403334
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622582}
[modify] https://crrev.com/d362daef89cdaaf03ff8e474119122bf121d2689/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/d362daef89cdaaf03ff8e474119122bf121d2689/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/d362daef89cdaaf03ff8e474119122bf121d2689/cc/trees/layer_tree_impl_unittest.cc

This should be fixed by the above CL. Since the original regression landed in M62, not planning to merge this back - it should make it into the next Chrome release (M73). The fix should be available in the next M73 canary release.