core: make session an event emitter (#14147)

lighthouse-core/fraggle-rock/gather/session.js

@@ -5,28 +5,47 @@
  */
 'use strict';
 
+const EventEmitter = require('events').EventEmitter;
 const LHError = require('../../lib/lh-error.js');
 
 // Controls how long to wait for a response after sending a DevTools protocol command.
 const DEFAULT_PROTOCOL_TIMEOUT = 30000;
 
+/** @typedef {new () => LH.Protocol.StrictEventEmitter<LH.CrdpEvents>} CrdpEventMessageEmitter */
+const CrdpEventEmitter = /** @type {CrdpEventMessageEmitter} */ (EventEmitter);
+
 /** @implements {LH.Gatherer.FRProtocolSession} */
-class ProtocolSession {
+class ProtocolSession extends CrdpEventEmitter {
   /**
    * @param {LH.Puppeteer.CDPSession} cdpSession
    */
   constructor(cdpSession) {
+    super();
+
     this._cdpSession = cdpSession;
     /** @type {LH.Crdp.Target.TargetInfo|undefined} */
     this._targetInfo = undefined;
     /** @type {number|undefined} */
     this._nextProtocolTimeout = undefined;
+
+    this._handleProtocolEvent = this._handleProtocolEvent.bind(this);
+    this._cdpSession.on('*', this._handleProtocolEvent);
   }
 
   id() {
     return this._cdpSession.id();
   }
 
+  /**
+   * Re-emit protocol events from the underlying CDPSession.
+   * @template {keyof LH.CrdpEvents} E
+   * @param {E} method
+   * @param {LH.CrdpEvents[E]} params
+   */
+  _handleProtocolEvent(method, ...params) {
+    this.emit(method, ...params);
+  }
+
   /** @param {LH.Crdp.Target.TargetInfo} targetInfo */
   setTargetInfo(targetInfo) {
     this._targetInfo = targetInfo;
@@ -53,36 +72,6 @@ class ProtocolSession {
     this._nextProtocolTimeout = ms;
   }
 
-  /**
-   * Bind listeners for protocol events.
-   * @template {keyof LH.CrdpEvents} E
-   * @param {E} eventName
-   * @param {(...args: LH.CrdpEvents[E]) => void} callback
-   */
-  on(eventName, callback) {
-    this._cdpSession.on(eventName, /** @type {*} */ (callback));
-  }
-
-  /**
-   * Bind listeners for protocol events.
-   * @template {keyof LH.CrdpEvents} E
-   * @param {E} eventName
-   * @param {(...args: LH.CrdpEvents[E]) => void} callback
-   */
-  once(eventName, callback) {
-    this._cdpSession.once(eventName, /** @type {*} */ (callback));
-  }
-
-  /**
-   * Bind listeners for protocol events.
-   * @template {keyof LH.CrdpEvents} E
-   * @param {E} eventName
-   * @param {(...args: LH.CrdpEvents[E]) => void} callback
-   */
-  off(eventName, callback) {
-    this._cdpSession.off(eventName, /** @type {*} */ (callback));
-  }
-
   /**
    * @template {keyof LH.CrdpCommands} C
    * @param {C} method
@@ -116,7 +105,7 @@ class ProtocolSession {
    * @return {Promise<void>}
    */
   async dispose() {
-    this._cdpSession.removeAllListeners();
+    this._cdpSession.off('*', this._handleProtocolEvent);
     await this._cdpSession.detach();
   }
 }

lighthouse-core/gather/driver/network-monitor.js

@@ -63,6 +63,8 @@ class NetworkMonitor {
     this.removeListener = emitter.removeListener.bind(emitter);
     /** @type {typeof emitter['removeAllListeners']} */
     this.removeAllListeners = emitter.removeAllListeners.bind(emitter);
+    /** @type {typeof emitter['listenerCount']} */
+    this.listenerCount = emitter.listenerCount.bind(emitter);
   }
 
   /**

lighthouse-core/test/fraggle-rock/gather/session-test.js

@@ -4,6 +4,8 @@
  * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
  */
 
+import {EventEmitter} from 'events';
+
 import {jest} from '@jest/globals';
 import {CDPSession} from 'puppeteer/lib/cjs/puppeteer/common/Connection.js';
 
@@ -27,35 +29,70 @@ describe('ProtocolSession', () => {
 
   beforeEach(() => {
     // @ts-expect-error - Individual mock functions are applied as necessary.
-    puppeteerSession = new CDPSession({_rawSend: fnAny()});
+    puppeteerSession = new CDPSession({_rawSend: fnAny(), send: fnAny()}, '', 'root');
     session = new ProtocolSession(puppeteerSession);
   });
 
-  /** @type {Array<'on'|'off'|'once'>} */
-  const delegateMethods = ['on', 'once', 'off'];
-  for (const method of delegateMethods) {
-    describe(`.${method}`, () => {
-      it('delegates to puppeteer', async () => {
-        const puppeteerFn = puppeteerSession[method] = fnAny();
-        const callback = () => undefined;
+  describe('responds to events from the underlying CDPSession', () => {
+    it('once', async () => {
+      const callback = fnAny();
 
-        session[method]('Page.frameNavigated', callback);
-        expect(puppeteerFn).toHaveBeenCalledWith('Page.frameNavigated', callback);
-      });
+      session.once('Page.frameNavigated', callback);
+      puppeteerSession.emit('Page.frameNavigated', {id: 1});
+      expect(callback).toHaveBeenCalledTimes(1);
+      expect(callback).toHaveBeenCalledWith({id: 1});
+
+      puppeteerSession.emit('Page.frameNavigated', {id: 2});
+      expect(callback).toHaveBeenCalledTimes(1);
+    });
+
+    it('on', async () => {
+      const callback = fnAny();
+
+      session.on('Page.frameNavigated', callback);
+      puppeteerSession.emit('Page.frameNavigated', {id: 1});
+      expect(callback).toHaveBeenCalledTimes(1);
+      expect(callback).toHaveBeenCalledWith({id: 1});
+
+      puppeteerSession.emit('Page.frameNavigated', {id: 2});
+      expect(callback).toHaveBeenCalledTimes(2);
+      expect(callback).toHaveBeenCalledWith({id: 2});
     });
-  }
+
+    it('off', async () => {
+      const callback = fnAny();
+
+      session.on('Page.frameNavigated', callback);
+      puppeteerSession.emit('Page.frameNavigated', {id: 1});
+      expect(callback).toHaveBeenCalledTimes(1);
+      expect(callback).toHaveBeenCalledWith({id: 1});
+
+      session.off('Page.frameNavigated', callback);
+      puppeteerSession.emit('Page.frameNavigated', {id: 2});
+      expect(callback).toHaveBeenCalledTimes(1);
+    });
+  });
 
   describe('.dispose', () => {
     it('should detach from the session', async () => {
       const detach = fnAny();
-      const removeAllListeners = fnAny();
+      class MockCdpSession extends EventEmitter {
+        constructor() {
+          super();
+
+          this.detach = detach;
+        }
+      }
+
       // @ts-expect-error - we want to use a more limited test.
-      puppeteerSession = {detach, emit: fnAny(), removeAllListeners};
+      puppeteerSession = new MockCdpSession();
       session = new ProtocolSession(puppeteerSession);
 
+      expect(puppeteerSession.listenerCount('*')).toBe(1);
+
       await session.dispose();
       expect(detach).toHaveBeenCalled();
-      expect(removeAllListeners).toHaveBeenCalled();
+      expect(puppeteerSession.listenerCount('*')).toBe(0);
     });
   });
 

lighthouse-core/test/gather/driver/network-monitor-test.js

@@ -151,14 +151,13 @@ describe('NetworkMonitor', () => {
     });
 
     it('should have idempotent enable', async () => {
-      // Two initial calls for TargetManager.enable().
-      expect(rootCdpSessionMock.on).toHaveBeenCalledTimes(2);
+      const initialListenerCount = targetManager.listenerCount('protocolevent');
       await monitor.enable();
       await monitor.enable();
       await monitor.enable();
       await monitor.enable();
-      // Only one more call.
-      expect(rootCdpSessionMock.on).toHaveBeenCalledTimes(3);
+      // Only one more listener added for `monitor.enable()`.
+      expect(targetManager.listenerCount('protocolevent')).toBe(initialListenerCount + 1);
     });
   });
 

lighthouse-core/test/gather/driver/target-manager-test.js

@@ -118,22 +118,35 @@ describe('TargetManager', () => {
     });
 
     it('should listen to target before resuming', async () => {
+      let targetListeningAsserted = false;
+
+      // Intercept listener for all protocol events and ensure target is still paused.
       sessionMock.on = /** @type {typeof sessionMock.on} */ (fnAny()
         .mockImplementation(/** @param {string} eventName */ (eventName) => {
-          // Intercept listener for all protocol events and ensure target is still paused.
-          if (eventName === '*') {
-            expect(sendMock.findAllInvocations('Target.getTargetInfo')).toHaveLength(1);
-            expect(sendMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(0);
-            expect(sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger')).toHaveLength(0);
+          const getTargetInfoCount = sendMock.findAllInvocations('Target.getTargetInfo').length;
+          const setAutoAttachCount = sendMock.findAllInvocations('Target.setAutoAttach').length;
+          const resumeCount = sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger').length;
+
+          // There may be many listeners for all protocol events, so just ensure this one occurred.
+          if (eventName === '*' &&
+              getTargetInfoCount === 1 && setAutoAttachCount === 0 && resumeCount === 0) {
+            targetListeningAsserted = true;
           }
         }));
 
       sendMock
         .mockResponse('Target.getTargetInfo', {targetInfo})
         .mockResponse('Network.enable')
         .mockResponse('Target.setAutoAttach');
+
+      expect(sendMock.findAllInvocations('Target.getTargetInfo')).toHaveLength(0);
+      expect(sendMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(0);
+      expect(sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger')).toHaveLength(0);
+
       await targetManager.enable();
 
+      expect(targetListeningAsserted).toBe(true);
+
       expect(sendMock.findAllInvocations('Target.getTargetInfo')).toHaveLength(1);
       expect(sendMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(1);
       expect(sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger')).toHaveLength(1);

types/protocol.d.ts

@@ -67,6 +67,8 @@ declare module Protocol {
     once<E extends keyof TEventRecord>(event: E, listener: (...args: TEventRecord[E]) => void): void;
 
     emit<E extends keyof TEventRecord>(event: E, ...request: TEventRecord[E]): void;
+
+    listenerCount<E extends keyof TEventRecord>(event: E): number;
   }
 }