Skip to content
Permalink
Browse files

fix(animations): leaking detached nodes when parent has a leave trans…

…ition (#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
  • Loading branch information
crisbeto authored and kara committed Dec 14, 2019
1 parent ee1eebd commit 6607fb496ad5fbe14941b27df9276b556123b8b4
@@ -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
@@ -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', () => {

0 comments on commit 6607fb4

Please sign in to comment.
You can’t perform that action at this time.