From 998a583c0a1a3c2882752d9af596c3ef8e8e4fba Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@users.noreply.github.com>
Date: Thu, 9 Feb 2017 20:46:47 +0100
Subject: [PATCH] fix(select): avoid going into infinite loop under certain
 conditions (#2955)

* fix(select): avoid going into infinite loop under certain conditions

Currently `md-select` calls `writeValue` recursively until the `QueryList` with all of the options is initialized. This usually means 1-2 iterations max, however in certain conditions (e.g. a sibling component throws an error on init) this may not happen and the browser would get thrown into an infinite loop.

This change switches to saving the attempted value assignments in a property and applying it after initialization.

Fixes #2950.

* fix: more elegant approach to initial value and add unit test

* fix: button not being removed after test

* chore: remove unnecessary expression
---
 src/lib/dialog/dialog.spec.ts |  2 ++
 src/lib/select/select.spec.ts | 34 +++++++++++++++++++++++++++++++++-
 src/lib/select/select.ts      | 29 +++++++++++++++--------------
 3 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/src/lib/dialog/dialog.spec.ts b/src/lib/dialog/dialog.spec.ts
index bbc35dece8d..103d0474a7e 100644
--- a/src/lib/dialog/dialog.spec.ts
+++ b/src/lib/dialog/dialog.spec.ts
@@ -381,6 +381,8 @@ describe('MdDialog', () => {
 
       expect(document.activeElement.id)
           .toBe('dialog-trigger', 'Expected that the trigger was refocused after dialog close');
+
+      document.body.removeChild(button);
     }));
   });
 
diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts
index f618cf5b88b..59c2e5de58d 100644
--- a/src/lib/select/select.spec.ts
+++ b/src/lib/select/select.spec.ts
@@ -7,6 +7,7 @@ import {
   ViewChild,
   ViewChildren,
   ChangeDetectionStrategy,
+  OnInit,
 } from '@angular/core';
 import {MdSelectModule} from './index';
 import {OverlayContainer} from '../core/overlay/overlay-container';
@@ -34,6 +35,8 @@ describe('MdSelect', () => {
         SelectWithChangeEvent,
         CustomSelectAccessor,
         CompWithCustomSelect,
+        SelectWithErrorSibling,
+        ThrowsErrorOnInit,
         BasicSelectOnPush
       ],
       providers: [
@@ -1239,6 +1242,14 @@ describe('MdSelect', () => {
       });
     }));
 
+    it('should not crash the browser when a sibling throws an error on init', async(() => {
+      // Note that this test can be considered successful if the error being thrown didn't
+      // end up crashing the testing setup altogether.
+      expect(() => {
+        TestBed.createComponent(SelectWithErrorSibling).detectChanges();
+      }).toThrowError(new RegExp('Oh no!', 'g'));
+    }));
+
   });
 
   describe('change event', () => {
@@ -1281,7 +1292,7 @@ describe('MdSelect', () => {
     beforeEach(() => {
       fixture = TestBed.createComponent(BasicSelectOnPush);
       fixture.detectChanges();
-      trigger = fixture.debugElement.query(By.css('.md-select-trigger')).nativeElement;
+      trigger = fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement;
     });
 
     it('should update the trigger based on the value', () => {
@@ -1474,6 +1485,27 @@ class CompWithCustomSelect {
   @ViewChild(CustomSelectAccessor) customAccessor: CustomSelectAccessor;
 }
 
+@Component({
+  selector: 'select-infinite-loop',
+  template: `
+    <md-select [(ngModel)]="value"></md-select>
+    <throws-error-on-init></throws-error-on-init>
+  `
+})
+class SelectWithErrorSibling {
+  value: string;
+}
+
+@Component({
+  selector: 'throws-error-on-init',
+  template: ''
+})
+export class ThrowsErrorOnInit implements OnInit {
+  ngOnInit() {
+    throw new Error('Oh no!');
+  }
+}
+
 @Component({
   selector: 'basic-select-on-push',
   changeDetection: ChangeDetectionStrategy.OnPush,
diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts
index 9d99134b0a3..c1c0a3e3797 100644
--- a/src/lib/select/select.ts
+++ b/src/lib/select/select.ts
@@ -25,6 +25,8 @@ import {ControlValueAccessor, NgControl} from '@angular/forms';
 import {coerceBooleanProperty} from '../core/coercion/boolean-property';
 import {ConnectedOverlayDirective} from '../core/overlay/overlay-directives';
 import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
+import 'rxjs/add/operator/startWith';
+
 
 /**
  * The following style constants are necessary to save here in order
@@ -243,8 +245,8 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
 
   ngAfterContentInit() {
     this._initKeyManager();
-    this._resetOptions();
-    this._changeSubscription = this.options.changes.subscribe(() => {
+
+    this._changeSubscription = this.options.changes.startWith(null).subscribe(() => {
       this._resetOptions();
 
       if (this._control) {
@@ -257,8 +259,14 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
 
   ngOnDestroy() {
     this._dropSubscriptions();
-    this._changeSubscription.unsubscribe();
-    this._tabSubscription.unsubscribe();
+
+    if (this._changeSubscription) {
+      this._changeSubscription.unsubscribe();
+    }
+
+    if (this._tabSubscription) {
+      this._tabSubscription.unsubscribe();
+    }
   }
 
   /** Toggles the overlay panel open or closed. */
@@ -292,17 +300,10 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
    * @param value New value to be written to the model.
    */
   writeValue(value: any): void {
-    if (!this.options) {
-      // In reactive forms, writeValue() will be called synchronously before
-      // the select's child options have been created. It's necessary to call
-      // writeValue() again after the options have been created to ensure any
-      // initial view value is set.
-      Promise.resolve(null).then(() => this.writeValue(value));
-      return;
+    if (this.options) {
+      this._setSelectionByValue(value);
+      this._changeDetectorRef.markForCheck();
     }
-
-    this._setSelectionByValue(value);
-    this._changeDetectorRef.markForCheck();
   }
 
   /**