fix(animations): leaking detached nodes when parent has a leave transition (#34409)

In the TransitionAnimationEngine we keep track of the existing elements with animations and we clear the cached data when they're removed. We also have some logic where we transition away the child elements when a parent is removed, however in that case we never cleared the cached element data which resulted in a memory leak. The leak is particularly visible in Material where whenever there's an animated overlay with a component inside of it that has an animation, the child component would always be retained in memory.

Fixes #25744.

PR Close #34409
This commit is contained in:
crisbeto 2019-12-14 17:29:35 +01:00 committed by Kara Erickson
parent 1144ce97f9
commit 835ed0f35f
2 changed files with 44 additions and 3 deletions

View File

@ -308,11 +308,13 @@ export class AnimationTransitionNamespace {
}
}
private _signalRemovalForInnerTriggers(rootElement: any, context: any, animate: boolean = false) {
private _signalRemovalForInnerTriggers(rootElement: any, context: any) {
const elements = this._engine.driver.query(rootElement, NG_TRIGGER_SELECTOR, true);
// emulate a leave animation for all inner nodes within this node.
// If there are no animations found for any of the nodes then clear the cache
// for the element.
this._engine.driver.query(rootElement, NG_TRIGGER_SELECTOR, true).forEach(elm => {
elements.forEach(elm => {
// this means that an inner remove() operation has already kicked off
// the animation on this element...
if (elm[REMOVAL_FLAG]) return;
@ -324,6 +326,11 @@ export class AnimationTransitionNamespace {
this.clearElementCache(elm);
}
});
// If the child elements were removed along with the parent, their animations might not
// have completed. Clear all the elements from the cache so we don't end up with a memory leak.
this._engine.afterFlushAnimationsDone(
() => elements.forEach(elm => this.clearElementCache(elm)));
}
triggerLeaveAnimation(
@ -388,7 +395,7 @@ export class AnimationTransitionNamespace {
const engine = this._engine;
if (element.childElementCount) {
this._signalRemovalForInnerTriggers(element, context, true);
this._signalRemovalForInnerTriggers(element, context);
}
// this means that a * => VOID animation was detected and kicked off

View File

@ -142,6 +142,40 @@ const DEFAULT_NAMESPACE_ID = 'id';
.duration)
.toEqual(1234);
});
it('should clear child node data when a parent node with leave transition is removed', () => {
const engine = makeEngine();
const child = document.createElement('div');
const parentTrigger = trigger('parent', [
transition(':leave', [style({height: '0px'}), animate(1000, style({height: '100px'}))])
]);
const childTrigger = trigger(
'child',
[transition(':enter', [style({opacity: '0'}), animate(1000, style({opacity: '1'}))])]);
registerTrigger(element, engine, parentTrigger);
registerTrigger(child, engine, childTrigger);
element.appendChild(child);
engine.insertNode(DEFAULT_NAMESPACE_ID, child, element, true);
setProperty(element, engine, 'parent', 'value');
setProperty(child, engine, 'child', 'visible');
engine.flush();
expect(engine.statesByElement.has(element))
.toBe(true, 'Expected parent data to be defined.');
expect(engine.statesByElement.has(child)).toBe(true, 'Expected child data to be defined.');
engine.removeNode(DEFAULT_NAMESPACE_ID, element, true, true);
engine.flush();
engine.players[0].finish();
expect(engine.statesByElement.has(element))
.toBe(false, 'Expected parent data to be cleared.');
expect(engine.statesByElement.has(child)).toBe(false, 'Expected child data to be cleared.');
});
});
describe('event listeners', () => {