Skip to content

Commit

Permalink
fix(memory): Unsubscribe event listeners when using Observable.fromEv…
Browse files Browse the repository at this point in the history
…ent (#5325)

* Fixed memory leak in CdkMonitorFocus

* Unsubscribe events that are registered under window/document using Observable.fromEvent.

* Fixed tslint problems.
  • Loading branch information
Eddman authored and tinayuangao committed Jun 27, 2017
1 parent 288f00e commit 1b351cd
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
10 changes: 7 additions & 3 deletions src/lib/autocomplete/autocomplete-trigger.ts
Expand Up @@ -101,6 +101,9 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
/** Whether or not the placeholder state is being overridden. */
private _manuallyFloatingPlaceholder = false;

/** The subscription for closing actions (some are bound to document). */
private _closingActionsSubscription: Subscription;

/** View -> model callback called when value changes */
_onChange: (value: any) => void = () => {};

Expand Down Expand Up @@ -157,7 +160,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {

if (this._overlayRef && !this._overlayRef.hasAttached()) {
this._overlayRef.attach(this._portal);
this._subscribeToClosingActions();
this._closingActionsSubscription = this._subscribeToClosingActions();
}

this.autocomplete._setVisibility();
Expand All @@ -169,6 +172,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
closePanel(): void {
if (this._overlayRef && this._overlayRef.hasAttached()) {
this._overlayRef.detach();
this._closingActionsSubscription.unsubscribe();
}

this._panelOpen = false;
Expand Down Expand Up @@ -332,9 +336,9 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
* This method listens to a stream of panel closing actions and resets the
* stream every time the option list changes.
*/
private _subscribeToClosingActions(): void {
private _subscribeToClosingActions(): Subscription {
// When the zone is stable initially, and when the option list changes...
RxChain.from(merge(first.call(this._zone.onStable), this.autocomplete.options.changes))
return RxChain.from(merge(first.call(this._zone.onStable), this.autocomplete.options.changes))
// create a new stream of panelClosingActions, replacing any previous streams
// that were created, and flatten it so our stream only emits closing events...
.call(switchMap, () => {
Expand Down
5 changes: 4 additions & 1 deletion src/lib/core/style/focus-origin-monitor.ts
Expand Up @@ -20,6 +20,7 @@ import {
} from '@angular/core';
import {Observable} from 'rxjs/Observable';
import {Subject} from 'rxjs/Subject';
import {Subscription} from 'rxjs/Subscription';
import {Platform} from '../platform/platform';
import {of as observableOf} from 'rxjs/observable/of';

Expand Down Expand Up @@ -316,18 +317,20 @@ export class FocusOriginMonitor {
selector: '[cdkMonitorElementFocus], [cdkMonitorSubtreeFocus]',
})
export class CdkMonitorFocus implements OnDestroy {
private _monitorSubscription: Subscription;
@Output() cdkFocusChange = new EventEmitter<FocusOrigin>();

constructor(private _elementRef: ElementRef, private _focusOriginMonitor: FocusOriginMonitor,
renderer: Renderer2) {
this._focusOriginMonitor.monitor(
this._monitorSubscription = this._focusOriginMonitor.monitor(
this._elementRef.nativeElement, renderer,
this._elementRef.nativeElement.hasAttribute('cdkMonitorSubtreeFocus'))
.subscribe(origin => this.cdkFocusChange.emit(origin));
}

ngOnDestroy() {
this._focusOriginMonitor.stopMonitoring(this._elementRef.nativeElement);
this._monitorSubscription.unsubscribe();
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/lib/tabs/tab-nav-bar/tab-nav-bar.ts
Expand Up @@ -25,8 +25,8 @@ import {CanDisable, mixinDisabled} from '../../core/common-behaviors/disabled';
import {MdRipple} from '../../core';
import {ViewportRuler} from '../../core/overlay/position/viewport-ruler';
import {Directionality, MD_RIPPLE_GLOBAL_OPTIONS, Platform, RippleGlobalOptions} from '../../core';
import {Observable} from 'rxjs/Observable';
import {Subject} from 'rxjs/Subject';
import {Subscription} from 'rxjs/Subscription';
import {takeUntil, auditTime} from '../../core/rxjs/index';
import {of as observableOf} from 'rxjs/observable/of';
import {merge} from 'rxjs/observable/merge';
Expand All @@ -53,6 +53,9 @@ export class MdTabNav implements AfterContentInit, OnDestroy {

@ViewChild(MdInkBar) _inkBar: MdInkBar;

/** Subscription for window.resize event **/
private _resizeSubscription: Subscription;

constructor(@Optional() private _dir: Directionality, private _ngZone: NgZone) { }

/** Notifies the component that the active link has been changed. */
Expand All @@ -62,7 +65,7 @@ export class MdTabNav implements AfterContentInit, OnDestroy {
}

ngAfterContentInit(): void {
this._ngZone.runOutsideAngular(() => {
this._resizeSubscription = this._ngZone.runOutsideAngular(() => {
let dirChange = this._dir ? this._dir.change : observableOf(null);
let resize = typeof window !== 'undefined' ?
auditTime.call(fromEvent(window, 'resize'), 10) :
Expand All @@ -83,6 +86,7 @@ export class MdTabNav implements AfterContentInit, OnDestroy {

ngOnDestroy() {
this._onDestroy.next();
this._resizeSubscription.unsubscribe();
}

/** Aligns the ink bar to the active link. */
Expand Down

0 comments on commit 1b351cd

Please sign in to comment.