Skip to content

Commit e332f15

Browse files
crisbetotinayuangao
authored andcommitted
fix(tab-link): avoid potential memory leak (#1877)
* Similarly to #1838, the `tab-link` destroy handler may not be called in certain situations, because it is being inherited from the MdRipple class. This PR explicitly calls the destroy handler. * Adds a unit test to make sure that the ripples are being cleaned up properly.
1 parent cf1b4b9 commit e332f15

File tree

3 files changed

+74
-4
lines changed

3 files changed

+74
-4
lines changed

src/lib/core/ripple/ripple.spec.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,11 @@ describe('MdRipple', () => {
6464
beforeEach(() => {
6565
TestBed.configureTestingModule({
6666
imports: [MdRippleModule.forRoot()],
67-
declarations: [BasicRippleContainer, RippleContainerWithInputBindings],
67+
declarations: [
68+
BasicRippleContainer,
69+
RippleContainerWithInputBindings,
70+
RippleContainerWithNgIf,
71+
],
6872
});
6973
});
7074

@@ -180,6 +184,20 @@ describe('MdRipple', () => {
180184
expect(pxStringToFloat(ripple.style.width)).toBeCloseTo(2 * expectedRadius, 1);
181185
expect(pxStringToFloat(ripple.style.height)).toBeCloseTo(2 * expectedRadius, 1);
182186
});
187+
188+
it('cleans up the event handlers when the container gets destroyed', () => {
189+
fixture = TestBed.createComponent(RippleContainerWithNgIf);
190+
fixture.detectChanges();
191+
192+
rippleElement = fixture.debugElement.nativeElement.querySelector('[md-ripple]');
193+
rippleBackground = rippleElement.querySelector('.md-ripple-background');
194+
195+
fixture.componentInstance.isDestroyed = true;
196+
fixture.detectChanges();
197+
198+
rippleElement.dispatchEvent(createMouseEvent('mousedown'));
199+
expect(rippleBackground.classList).not.toContain('md-ripple-active');
200+
});
183201
});
184202

185203
describe('configuring behavior', () => {
@@ -367,3 +385,9 @@ class RippleContainerWithInputBindings {
367385
backgroundColor = '';
368386
@ViewChild(MdRipple) ripple: MdRipple;
369387
}
388+
389+
@Component({ template: `<div id="container" md-ripple *ngIf="!isDestroyed"></div>` })
390+
class RippleContainerWithNgIf {
391+
@ViewChild(MdRipple) ripple: MdRipple;
392+
isDestroyed = false;
393+
}

src/lib/tabs/tab-nav-bar/tab-nav-bar.spec.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ describe('MdTabNavBar', () => {
1010
TestBed.configureTestingModule({
1111
imports: [MdTabsModule.forRoot()],
1212
declarations: [
13-
SimpleTabNavBarTestApp
13+
SimpleTabNavBarTestApp,
14+
TabLinkWithNgIf,
1415
],
1516
});
1617

@@ -38,6 +39,25 @@ describe('MdTabNavBar', () => {
3839
expect(component.activeIndex).toBe(2);
3940
});
4041
});
42+
43+
it('should clean up the ripple event handlers on destroy', () => {
44+
let fixture: ComponentFixture<TabLinkWithNgIf> = TestBed.createComponent(TabLinkWithNgIf);
45+
fixture.detectChanges();
46+
47+
let link = fixture.debugElement.nativeElement.querySelector('[md-tab-link]');
48+
let rippleBackground = link.querySelector('.md-ripple-background');
49+
let mouseEvent = document.createEvent('MouseEvents');
50+
51+
fixture.componentInstance.isDestroyed = true;
52+
fixture.detectChanges();
53+
54+
mouseEvent.initMouseEvent('mousedown', false, false, window, 0, 0, 0, 0, 0, false, false,
55+
false, false, 0, null);
56+
57+
link.dispatchEvent(mouseEvent);
58+
59+
expect(rippleBackground.classList).not.toContain('md-ripple-active');
60+
});
4161
});
4262

4363
@Component({
@@ -53,3 +73,14 @@ describe('MdTabNavBar', () => {
5373
class SimpleTabNavBarTestApp {
5474
activeIndex = 0;
5575
}
76+
77+
@Component({
78+
template: `
79+
<nav md-tab-nav-bar>
80+
<a md-tab-link *ngIf="!isDestroyed">Link</a>
81+
</nav>
82+
`
83+
})
84+
class TabLinkWithNgIf {
85+
isDestroyed = false;
86+
}

src/lib/tabs/tab-nav-bar/tab-nav-bar.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1-
import {Component, Input, ViewChild, ElementRef, ViewEncapsulation, Directive} from '@angular/core';
1+
import {
2+
Component,
3+
Input,
4+
ViewChild,
5+
ElementRef,
6+
ViewEncapsulation,
7+
Directive,
8+
OnDestroy,
9+
} from '@angular/core';
10+
211
import {MdInkBar} from '../ink-bar';
312
import {MdRipple} from '../../core/ripple/ripple';
413

@@ -50,8 +59,14 @@ export class MdTabLink {
5059
@Directive({
5160
selector: '[md-tab-link]',
5261
})
53-
export class MdTabLinkRipple extends MdRipple {
62+
export class MdTabLinkRipple extends MdRipple implements OnDestroy {
5463
constructor(private _element: ElementRef) {
5564
super(_element);
5665
}
66+
67+
// In certain cases the parent destroy handler
68+
// may not get called. See Angular issue #11606.
69+
ngOnDestroy() {
70+
super.ngOnDestroy();
71+
}
5772
}

0 commit comments

Comments
 (0)