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

Author: crisbeto

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);
     }));
   });
 

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,

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();
   }
 
   /**