From 453b1e8bef2204c6de9c9b73e482aab57cf07a8a Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 26 Jun 2019 17:47:57 +0200 Subject: [PATCH] fix(overlay): events not being unbound from destroyed backdrop (#16268) Fixes the `click` and `transitionend` event handlers not being removed from the backdrop that is being destroyed. Since they're referring to things inside the `OverlayRef`, they could cause some memory to be retained. --- src/cdk/overlay/overlay-ref.ts | 13 +++++++++---- src/cdk/overlay/overlay.spec.ts | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/cdk/overlay/overlay-ref.ts b/src/cdk/overlay/overlay-ref.ts index 4abf91a8c70..1c9f369368f 100644 --- a/src/cdk/overlay/overlay-ref.ts +++ b/src/cdk/overlay/overlay-ref.ts @@ -37,6 +37,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference { private _positionStrategy: PositionStrategy | undefined; private _scrollStrategy: ScrollStrategy | undefined; private _locationChanges: SubscriptionLike = Subscription.EMPTY; + private _backdropClickHandler = (event: MouseEvent) => this._backdropClick.next(event); /** * Reference to the parent of the `_host` at the time it was detached. Used to restore @@ -390,8 +391,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference { // Forward backdrop clicks such that the consumer of the overlay can perform whatever // action desired when such a click occurs (usually closing the overlay). - this._backdropElement.addEventListener('click', - (event: MouseEvent) => this._backdropClick.next(event)); + this._backdropElement.addEventListener('click', this._backdropClickHandler); // Add class to fade-in the backdrop after one frame. if (typeof requestAnimationFrame !== 'undefined') { @@ -431,8 +431,13 @@ export class OverlayRef implements PortalOutlet, OverlayReference { let timeoutId: number; let finishDetach = () => { // It may not be attached to anything in certain cases (e.g. unit tests). - if (backdropToDetach && backdropToDetach.parentNode) { - backdropToDetach.parentNode.removeChild(backdropToDetach); + if (backdropToDetach) { + backdropToDetach.removeEventListener('click', this._backdropClickHandler); + backdropToDetach.removeEventListener('transitionend', finishDetach); + + if (backdropToDetach.parentNode) { + backdropToDetach.parentNode.removeChild(backdropToDetach); + } } // It is possible that a new portal has been attached to this overlay since we started diff --git a/src/cdk/overlay/overlay.spec.ts b/src/cdk/overlay/overlay.spec.ts index 0a3a54ce88a..c40aecc5269 100644 --- a/src/cdk/overlay/overlay.spec.ts +++ b/src/cdk/overlay/overlay.spec.ts @@ -745,6 +745,30 @@ describe('Overlay', () => { .toBeLessThan(children.indexOf(host), 'Expected backdrop to be before the host in the DOM'); }); + it('should remove the event listener from the backdrop', () => { + let overlayRef = overlay.create(config); + overlayRef.attach(componentPortal); + + viewContainerFixture.detectChanges(); + let backdrop = overlayContainerElement.querySelector('.cdk-overlay-backdrop') as HTMLElement; + expect(backdrop).toBeTruthy(); + expect(backdrop.classList).not.toContain('cdk-overlay-backdrop-showing'); + + let backdropClickHandler = jasmine.createSpy('backdropClickHander'); + overlayRef.backdropClick().subscribe(backdropClickHandler); + + backdrop.click(); + expect(backdropClickHandler).toHaveBeenCalledTimes(1); + + overlayRef.detach(); + dispatchFakeEvent(backdrop, 'transitionend'); + zone.simulateZoneExit(); + viewContainerFixture.detectChanges(); + + backdrop.click(); + expect(backdropClickHandler).toHaveBeenCalledTimes(1); + }); + }); describe('panelClass', () => {