fix(cdk/overlay): backdrop timeouts not being cleared in some cases (#…

…23972)

When we start the detach sequence of a backdrop, we bind a `transitionend` event as well as a 500ms timeout in case the event doesn't fire. If the event fires before 500ms, we clear the timeout so users don't have to flush it in tests, however we had the following problems:
1. Transparent backdrops don't have a transition so they were always being cleaned up after the 500ms timeout.
2. We weren't clearing the timeout if the overlay was disposed while the backdrop transition is running. This meant that the timeout would still be in memory, even though the element was removed from the DOM.
3. We had a memory leak where the `click` and `transitionend` events weren't being removed from the backdrop if the overlay is disposed while detaching.

Basically all Material components had one of these two issues. These changes resolve the problems by:
1. Clearing the timeout when the overlay is disposed.
2. Setting a 1ms transition on the transparent overlay so its `transitionend` can fire (almost) instantly.
3. Removing the `click` and `transitionend` events when the backdrop is disposed.

src/cdk/overlay/_index.scss

@@ -96,12 +96,18 @@ $backdrop-animation-timing-function: cubic-bezier(0.25, 0.8, 0.25, 1) !default;
   }
 
   .cdk-overlay-transparent-backdrop {
+    // Define a transition on the visibility so that the `transitionend` event can fire immediately.
+    transition: visibility 1ms linear, opacity 1ms linear;
+    visibility: hidden;
+    opacity: 1;
+
     // Note: as of Firefox 57, having the backdrop be `background: none` will prevent it from
     // capturing the user's mouse scroll events. Since we also can't use something like
     // `rgba(0, 0, 0, 0)`, we work around the inconsistency by not setting the background at
     // all and using `opacity` to make the element transparent.
-    &, &.cdk-overlay-backdrop-showing {
+    &.cdk-overlay-backdrop-showing {
       opacity: 0;
+      visibility: visible;
     }
   }
 

src/cdk/overlay/overlay-ref.ts

@@ -31,13 +31,17 @@ export type ImmutableObject<T> = {
  */
 export class OverlayRef implements PortalOutlet, OverlayReference {
   private _backdropElement: HTMLElement | null = null;
+  private _backdropTimeout: number | undefined;
   private readonly _backdropClick = new Subject<MouseEvent>();
   private readonly _attachments = new Subject<void>();
   private readonly _detachments = new Subject<void>();
   private _positionStrategy: PositionStrategy | undefined;
   private _scrollStrategy: ScrollStrategy | undefined;
   private _locationChanges: SubscriptionLike = Subscription.EMPTY;
   private _backdropClickHandler = (event: MouseEvent) => this._backdropClick.next(event);
+  private _backdropTransitionendHandler = (event: TransitionEvent) => {
+    this._disposeBackdrop(event.target as HTMLElement | null);
+  };
 
   /**
    * Reference to the parent of the `_host` at the time it was detached. Used to restore
@@ -418,26 +422,10 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
       return;
     }
 
-    let timeoutId: number;
-    const finishDetach = () => {
-      // It may not be attached to anything in certain cases (e.g. unit tests).
-      if (backdropToDetach) {
-        backdropToDetach.removeEventListener('click', this._backdropClickHandler);
-        backdropToDetach.removeEventListener('transitionend', finishDetach);
-        this._disposeBackdrop(backdropToDetach);
-      }
-
-      if (this._config.backdropClass) {
-        this._toggleClasses(backdropToDetach!, this._config.backdropClass, false);
-      }
-
-      clearTimeout(timeoutId);
-    };
-
     backdropToDetach.classList.remove('cdk-overlay-backdrop-showing');
 
     this._ngZone.runOutsideAngular(() => {
-      backdropToDetach!.addEventListener('transitionend', finishDetach);
+      backdropToDetach!.addEventListener('transitionend', this._backdropTransitionendHandler);
     });
 
     // If the backdrop doesn't have a transition, the `transitionend` event won't fire.
@@ -447,7 +435,12 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
     // Run this outside the Angular zone because there's nothing that Angular cares about.
     // If it were to run inside the Angular zone, every test that used Overlay would have to be
     // either async or fakeAsync.
-    timeoutId = this._ngZone.runOutsideAngular(() => setTimeout(finishDetach, 500));
+    this._backdropTimeout = this._ngZone.runOutsideAngular(() =>
+      setTimeout(() => {
+        console.log('fallback');
+        this._disposeBackdrop(backdropToDetach);
+      }, 500),
+    );
   }
 
   /** Toggles a single CSS class or an array of classes on an element. */
@@ -505,6 +498,8 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
   /** Removes a backdrop element from the DOM. */
   private _disposeBackdrop(backdrop: HTMLElement | null) {
     if (backdrop) {
+      backdrop.removeEventListener('click', this._backdropClickHandler);
+      backdrop.removeEventListener('transitionend', this._backdropTransitionendHandler);
       backdrop.remove();
 
       // It is possible that a new portal has been attached to this overlay since we started
@@ -514,6 +509,11 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
         this._backdropElement = null;
       }
     }
+
+    if (this._backdropTimeout) {
+      clearTimeout(this._backdropTimeout);
+      this._backdropTimeout = undefined;
+    }
   }
 }
 

src/cdk/overlay/overlay.spec.ts

@@ -348,6 +348,16 @@ describe('Overlay', () => {
     // `fakeAsync` will throw if we have an unflushed timer.
   }));
 
+  it('should clear the backdrop timeout if the overlay is disposed', fakeAsync(() => {
+    const overlayRef = overlay.create({hasBackdrop: true});
+    overlayRef.attach(componentPortal);
+    overlayRef.detach();
+    overlayRef.dispose();
+
+    // Note: we don't `tick` or `flush` here. The assertion is that
+    // `fakeAsync` will throw if we have an unflushed timer.
+  }));
+
   it('should be able to use the `Overlay` provider during app initialization', () => {
     /** Dummy provider that depends on `Overlay`. */
     @Injectable()