fix(google-maps): maintain subscriptions across event targets (#20897)

This came up during the review of #20873. Currently the class that manages events
for the Google Maps objects assumes that once a target is assigned, it'll either be
maintained or eventually removed which means that the fix from #20873 will
cause any existing event listeners to be dropped.

These changes add some code which will preserve the listeners.

Furthermore, I've added a dedicated testing setup for the `MapEventManager` since
there's a fair bit of logic encapsulated in it and we've only been testing it by proxy
through the various Google Maps components.

Author: crisbeto

src/google-maps/map-event-manager.spec.ts

@@ -0,0 +1,170 @@
+import {NgZone} from '@angular/core';
+import {MapEventManager} from './map-event-manager';
+
+describe('MapEventManager', () => {
+  let dummyZone: NgZone;
+  let manager: MapEventManager;
+  let target: TestEventTarget;
+
+  beforeEach(() => {
+    dummyZone = {
+      run: jasmine.createSpy('NgZone.run').and.callFake((callback: () => void) => callback())
+    } as unknown as NgZone;
+    target = new TestEventTarget();
+    manager = new MapEventManager(dummyZone);
+  });
+
+  afterEach(() => {
+    manager.destroy();
+  });
+
+  it('should register a listener when subscribing to an event', () => {
+    expect(target.addListener).not.toHaveBeenCalled();
+
+    manager.setTarget(target);
+    const stream = manager.getLazyEmitter('click');
+
+    expect(target.addListener).not.toHaveBeenCalled();
+    expect(target.events.get('click')).toBeFalsy();
+
+    const subscription = stream.subscribe();
+    expect(target.addListener).toHaveBeenCalledTimes(1);
+    expect(target.events.get('click')?.size).toBe(1);
+    subscription.unsubscribe();
+  });
+
+  it('should register a listener if the subscription happened before there was a target', () => {
+    const stream = manager.getLazyEmitter('click');
+    const subscription = stream.subscribe();
+
+    expect(target.addListener).not.toHaveBeenCalled();
+    expect(target.events.get('click')).toBeFalsy();
+
+    manager.setTarget(target);
+
+    expect(target.addListener).toHaveBeenCalledTimes(1);
+    expect(target.events.get('click')?.size).toBe(1);
+    subscription.unsubscribe();
+  });
+
+  it('should remove the listener when unsubscribing', () => {
+    const stream = manager.getLazyEmitter('click');
+    const subscription = stream.subscribe();
+    manager.setTarget(target);
+    expect(target.events.get('click')?.size).toBe(1);
+
+    subscription.unsubscribe();
+    expect(target.events.get('click')?.size).toBe(0);
+  });
+
+  it('should remove the listener when the manager is destroyed', () => {
+    const stream = manager.getLazyEmitter('click');
+    stream.subscribe();
+    manager.setTarget(target);
+    expect(target.events.get('click')?.size).toBe(1);
+
+    manager.destroy();
+    expect(target.events.get('click')?.size).toBe(0);
+  });
+
+  it('should remove the listener when the target is changed', () => {
+    const stream = manager.getLazyEmitter('click');
+    stream.subscribe();
+    manager.setTarget(target);
+    expect(target.events.get('click')?.size).toBe(1);
+
+    manager.setTarget(undefined);
+    expect(target.events.get('click')?.size).toBe(0);
+  });
+
+  it('should trigger the subscription to an event', () => {
+    const spy = jasmine.createSpy('subscription');
+    const stream = manager.getLazyEmitter('click');
+    stream.subscribe(spy);
+    manager.setTarget(target);
+    expect(spy).not.toHaveBeenCalled();
+
+    target.triggerListeners('click');
+    expect(spy).toHaveBeenCalledTimes(1);
+  });
+
+  it('should be able to register multiple listeners to the same event', () => {
+    const firstSpy = jasmine.createSpy('subscription one');
+    const secondSpy = jasmine.createSpy('subscription two');
+    const stream = manager.getLazyEmitter('click');
+    manager.setTarget(target);
+    stream.subscribe(firstSpy);
+    stream.subscribe(secondSpy);
+    expect(firstSpy).not.toHaveBeenCalled();
+    expect(secondSpy).not.toHaveBeenCalled();
+    expect(target.events.get('click')?.size).toBe(2);
+
+    target.triggerListeners('click');
+    expect(firstSpy).toHaveBeenCalledTimes(1);
+    expect(secondSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it('should run listeners inside the NgZone', () => {
+    const spy = jasmine.createSpy('subscription');
+    const stream = manager.getLazyEmitter('click');
+    stream.subscribe(spy);
+    manager.setTarget(target);
+    expect(dummyZone.run).not.toHaveBeenCalled();
+
+    target.triggerListeners('click');
+    expect(dummyZone.run).toHaveBeenCalledTimes(1);
+  });
+
+  it('should maintain subscriptions when swapping out targets', () => {
+    const spy = jasmine.createSpy('subscription');
+    const stream = manager.getLazyEmitter('click');
+    stream.subscribe(spy);
+    manager.setTarget(target);
+    expect(spy).not.toHaveBeenCalled();
+
+    target.triggerListeners('click');
+    expect(spy).toHaveBeenCalledTimes(1);
+
+    const alternateTarget = new TestEventTarget();
+    manager.setTarget(alternateTarget);
+
+    expect(spy).toHaveBeenCalledTimes(1);
+    expect(target.events.get('click')?.size).toBe(0);
+    expect(alternateTarget.events.get('click')?.size).toBe(1);
+
+    alternateTarget.triggerListeners('click');
+    expect(spy).toHaveBeenCalledTimes(2);
+
+    manager.setTarget(undefined);
+    expect(alternateTarget.events.get('click')?.size).toBe(0);
+
+    alternateTarget.triggerListeners('click');
+    expect(spy).toHaveBeenCalledTimes(2);
+  });
+
+});
+
+/** Imitates a Google Maps event target and keeps track of the registered events. */
+class TestEventTarget {
+  events = new Map<string, Set<() => void>>();
+
+  addListener = jasmine.createSpy('addListener').and.callFake(
+    (name: string, listener: () => void) => {
+      if (!this.events.has(name)) {
+        this.events.set(name, new Set());
+      }
+      this.events.get(name)!.add(listener);
+
+      return {remove: () => this.events.get(name)!.delete(listener)};
+    });
+
+  triggerListeners(name: string) {
+    const listeners = this.events.get(name);
+
+    if (!listeners) {
+      throw Error(`No listeners registered for "${name}" event.`);
+    }
+
+    listeners.forEach(listener => listener());
+  }
+}

src/google-maps/map-event-manager.ts

@@ -7,7 +7,8 @@
  */
 
 import {NgZone} from '@angular/core';
-import {Observable, Subscriber} from 'rxjs';
+import {BehaviorSubject, Observable, Subscriber} from 'rxjs';
+import {switchMap} from 'rxjs/operators';
 
 type MapEventManagerTarget = {
   addListener: (name: string, callback: (...args: any[]) => void) => google.maps.MapsEventListener;
@@ -18,11 +19,11 @@ export class MapEventManager {
   /** Pending listeners that were added before the target was set. */
   private _pending: {observable: Observable<any>, observer: Subscriber<any>}[] = [];
   private _listeners: google.maps.MapsEventListener[] = [];
-  private _target: MapEventManagerTarget;
+  private _targetStream = new BehaviorSubject<MapEventManagerTarget>(undefined);
 
   /** Clears all currently-registered event listeners. */
   private _clearListeners() {
-    for (let listener of this._listeners) {
+    for (const listener of this._listeners) {
       listener.remove();
     }
 
@@ -33,36 +34,40 @@ export class MapEventManager {
 
   /** Gets an observable that adds an event listener to the map when a consumer subscribes to it. */
   getLazyEmitter<T>(name: string): Observable<T> {
-    const observable = new Observable<T>(observer => {
-      // If the target hasn't been initialized yet, cache the observer so it can be added later.
-      if (!this._target) {
-        this._pending.push({observable, observer});
-        return undefined;
-      }
+    return this._targetStream.pipe(switchMap(target => {
+      const observable = new Observable<T>(observer => {
+        // If the target hasn't been initialized yet, cache the observer so it can be added later.
+        if (!target) {
+          this._pending.push({observable, observer});
+          return undefined;
+        }
 
-      const listener = this._target.addListener(name, (event: T) => {
-        this._ngZone.run(() => observer.next(event));
+        const listener = target.addListener(name, (event: T) => {
+          this._ngZone.run(() => observer.next(event));
+        });
+        this._listeners.push(listener);
+        return () => listener.remove();
       });
-      this._listeners.push(listener);
-      return () => listener.remove();
-    });
 
-    return observable;
+      return observable;
+    }));
   }
 
   /** Sets the current target that the manager should bind events to. */
   setTarget(target: MapEventManagerTarget) {
-    if (target === this._target) {
+    const currentTarget = this._targetStream.value;
+
+    if (target === currentTarget) {
       return;
     }
 
     // Clear the listeners from the pre-existing target.
-    if (this._target) {
+    if (currentTarget) {
       this._clearListeners();
       this._pending = [];
     }
 
-    this._target = target;
+    this._targetStream.next(target);
 
     // Add the listeners that were bound before the map was initialized.
     this._pending.forEach(subscriber => subscriber.observable.subscribe(subscriber.observer));
@@ -73,6 +78,6 @@ export class MapEventManager {
   destroy() {
     this._clearListeners();
     this._pending = [];
-    this._target = undefined;
+    this._targetStream.complete();
   }
 }