Skip to content

Commit

Permalink
fix(overlay): remove potentially leaky observable (#19146)
Browse files Browse the repository at this point in the history
Removes an unnecessary observable that has the potential for memory leaks.

Fixes #10573.
  • Loading branch information
crisbeto authored and jelbourn committed May 8, 2020
1 parent ddf49a0 commit 6f79527
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export class OverlayKeyboardDispatcher implements OnDestroy {
// (e.g. for select and autocomplete). We skip overlays without keydown event subscriptions,
// because we don't want overlays that don't handle keyboard events to block the ones below
// them that do.
if (overlays[i]._keydownEventSubscriptions > 0) {
if (overlays[i]._keydownEvents.observers.length > 0) {
overlays[i]._keydownEvents.next(event);
break;
}
Expand Down
18 changes: 2 additions & 16 deletions src/cdk/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {Direction, Directionality} from '@angular/cdk/bidi';
import {ComponentPortal, Portal, PortalOutlet, TemplatePortal} from '@angular/cdk/portal';
import {ComponentRef, EmbeddedViewRef, NgZone} from '@angular/core';
import {Location} from '@angular/common';
import {Observable, Subject, merge, SubscriptionLike, Subscription, Observer} from 'rxjs';
import {Observable, Subject, merge, SubscriptionLike, Subscription} from 'rxjs';
import {take, takeUntil} from 'rxjs/operators';
import {OverlayKeyboardDispatcher} from './keyboard/overlay-keyboard-dispatcher';
import {OverlayConfig} from './overlay-config';
Expand Down Expand Up @@ -45,23 +45,9 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
*/
private _previousHostParent: HTMLElement;

private _keydownEventsObservable: Observable<KeyboardEvent> =
new Observable((observer: Observer<KeyboardEvent>) => {
const subscription = this._keydownEvents.subscribe(observer);
this._keydownEventSubscriptions++;

return () => {
subscription.unsubscribe();
this._keydownEventSubscriptions--;
};
});

/** Stream of keydown events dispatched to this overlay. */
_keydownEvents = new Subject<KeyboardEvent>();

/** Amount of subscriptions to the keydown events. */
_keydownEventSubscriptions = 0;

constructor(
private _portalOutlet: PortalOutlet,
private _host: HTMLElement,
Expand Down Expand Up @@ -265,7 +251,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference {

/** Gets an observable of keydown events targeted to this overlay. */
keydownEvents(): Observable<KeyboardEvent> {
return this._keydownEventsObservable;
return this._keydownEvents.asObservable();
}

/** Gets the current overlay configuration, which is immutable. */
Expand Down
1 change: 0 additions & 1 deletion tools/public_api_guard/cdk/overlay.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ export declare class OverlayPositionBuilder {
}

export declare class OverlayRef implements PortalOutlet, OverlayReference {
_keydownEventSubscriptions: number;
_keydownEvents: Subject<KeyboardEvent>;
get backdropElement(): HTMLElement | null;
get hostElement(): HTMLElement;
Expand Down

0 comments on commit 6f79527

Please sign in to comment.