From 8ab5de6ac9d7acafa39a9c36ca6d9043b2fed27e Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Thu, 7 Dec 2017 22:42:14 +0100
Subject: [PATCH] fix(drawer): infinite loop when two-way opened binding is
 toggled mid-animation

As of https://github.com/angular/material2/commit/004e0fef2fe5172b6e6ed2c4d368d21dab051355 the drawer supports a two-way binding on the `opened` property, however because the `openedChange` emits at the end of the animation and two-way bindings invoke their setter which then trigger an animation which in turn invokes `openedChange`, there is the potential for an infinite loop if the user changes the `opened` state mid-animation. These changes avoid the issue by turning the `openedChanged` into an async stream and emitting the current value at the end of the animation, rather than determining the value based on the animation state.

Fixes #8869.
---
 src/lib/sidenav/drawer.spec.ts | 35 +++++++++++++++++++++++++++++++++-
 src/lib/sidenav/drawer.ts      | 11 ++++++-----
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/src/lib/sidenav/drawer.spec.ts b/src/lib/sidenav/drawer.spec.ts
index 493490d8b6bf..25059f485f11 100644
--- a/src/lib/sidenav/drawer.spec.ts
+++ b/src/lib/sidenav/drawer.spec.ts
@@ -8,7 +8,7 @@ import {
 } from '@angular/core/testing';
 import {Component, ElementRef, ViewChild} from '@angular/core';
 import {By} from '@angular/platform-browser';
-import {NoopAnimationsModule} from '@angular/platform-browser/animations';
+import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations';
 import {MatDrawer, MatSidenavModule, MatDrawerContainer} from './index';
 import {A11yModule} from '@angular/cdk/a11y';
 import {PlatformModule} from '@angular/cdk/platform';
@@ -138,6 +138,7 @@ describe('MatDrawer', () => {
       expect(testComponent.openCount).toBe(0);
       expect(testComponent.closeCount).toBe(0);
 
+      tick();
       fixture.debugElement.query(By.css('.close')).nativeElement.click();
       fixture.detectChanges();
 
@@ -367,6 +368,37 @@ describe('MatDrawer', () => {
 
       expect(fixture.componentInstance.isOpen).toBe(true);
     }));
+
+    it('should not throw when a two-way binding is toggled quickly while animating',
+      fakeAsync(() => {
+        TestBed
+          .resetTestingModule()
+          .configureTestingModule({
+            imports: [MatSidenavModule, BrowserAnimationsModule],
+            declarations: [DrawerOpenBinding],
+          })
+          .compileComponents();
+
+        const fixture = TestBed.createComponent(DrawerOpenBinding);
+        fixture.detectChanges();
+
+        // Note that we need actual timeouts and the `BrowserAnimationsModule`
+        // in order to test it correctly.
+        setTimeout(() => {
+          fixture.componentInstance.isOpen = !fixture.componentInstance.isOpen;
+          expect(() => fixture.detectChanges()).not.toThrow();
+
+          setTimeout(() => {
+            fixture.componentInstance.isOpen = !fixture.componentInstance.isOpen;
+            expect(() => fixture.detectChanges()).not.toThrow();
+          }, 1);
+
+          tick(1);
+        }, 1);
+
+        tick(1);
+      }));
+
   });
 
   describe('focus trapping behavior', () => {
@@ -496,6 +528,7 @@ describe('MatDrawerContainer', () => {
 
     fixture.componentInstance.renderDrawer = false;
     fixture.detectChanges();
+    tick();
 
     expect(parseInt(contentElement.style.marginLeft)).toBeLessThan(initialMargin);
   }));
diff --git a/src/lib/sidenav/drawer.ts b/src/lib/sidenav/drawer.ts
index 9a5fbbc86c1a..407e50b2eae5 100644
--- a/src/lib/sidenav/drawer.ts
+++ b/src/lib/sidenav/drawer.ts
@@ -195,7 +195,9 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
   _animationState: 'open-instant' | 'open' | 'void' = 'void';
 
   /** Event emitted when the drawer open state is changed. */
-  @Output() openedChange: EventEmitter<boolean> = new EventEmitter<boolean>();
+  @Output() openedChange: EventEmitter<boolean> =
+    // Note this has to be async in order to avoid some issues with two-bindings (see #8872).
+    new EventEmitter<boolean>(/* isAsync */true);
 
   /** Event emitted when the drawer has been opened. */
   @Output('opened')
@@ -390,10 +392,9 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
   _onAnimationEnd(event: AnimationEvent) {
     const {fromState, toState} = event;
 
-    if (toState.indexOf('open') === 0 && fromState === 'void') {
-      this.openedChange.emit(true);
-    } else if (toState === 'void' && fromState.indexOf('open') === 0) {
-      this.openedChange.emit(false);
+    if ((toState.indexOf('open') === 0 && fromState === 'void') ||
+        (toState === 'void' && fromState.indexOf('open') === 0)) {
+      this.openedChange.emit(this._opened);
     }
   }