fix(drawer): infinite loop when two-way opened binding is toggled mid-animation (#8872)

As of 004e0fe 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.

Author: crisbeto

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';
@@ -139,6 +139,7 @@ describe('MatDrawer', () => {
       expect(testComponent.openCount).toBe(0);
       expect(testComponent.closeCount).toBe(0);
 
+      tick();
       fixture.debugElement.query(By.css('.close')).nativeElement.click();
       fixture.detectChanges();
 
@@ -368,6 +369,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', () => {
@@ -512,6 +544,7 @@ describe('MatDrawerContainer', () => {
 
     fixture.componentInstance.renderDrawer = false;
     fixture.detectChanges();
+    tick();
 
     expect(parseInt(contentElement.style.marginLeft)).toBeLessThan(initialMargin);
   }));

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')
@@ -402,10 +404,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);
     }
   }