core(tsc): add type checking to use of CRDP events

lighthouse-core/gather/connections/connection.js

@@ -9,11 +9,17 @@ const EventEmitter = require('events').EventEmitter;
 const log = require('lighthouse-logger');
 const LHError = require('../../lib/errors');
 
+/**
+ * @typedef {LH.StrictEventEmitter<{'protocolevent': LH.Protocol.RawEventMessage}>} CrdpEventMessageEmitter
+ */
+
 class Connection {
   constructor() {
     this._lastCommandId = 0;
     /** @type {Map<number, {resolve: function(Promise<*>), method: string, options: {silent?: boolean}}>}*/
     this._callbacks = new Map();
+
+    /** @type {?CrdpEventMessageEmitter} */
     this._eventEmitter = new EventEmitter();
   }
 
@@ -31,15 +37,13 @@ class Connection {
     return Promise.reject(new Error('Not implemented'));
   }
 
-
   /**
    * @return {Promise<string>}
    */
   wsEndpoint() {
     return Promise.reject(new Error('Not implemented'));
   }
 
-
   /**
    * Call protocol methods
    * @param {string} method
@@ -57,22 +61,6 @@ class Connection {
     });
   }
 
-  /**
-   * Bind listeners for connection events
-   * @param {'notification'} eventName
-   * @param {(body: {method: string, params: object}) => void} cb
-   */
-  on(eventName, cb) {
-    if (eventName !== 'notification') {
-      throw new Error('Only supports "notification" events');
-    }
-
-    if (!this._eventEmitter) {
-      throw new Error('Attempted to add event listener after connection disposed.');
-    }
-    this._eventEmitter.on(eventName, cb);
-  }
-
   /* eslint-disable no-unused-vars */
 
   /**
@@ -91,13 +79,15 @@ class Connection {
    * @protected
    */
   handleRawMessage(message) {
-    const object = JSON.parse(message);
-    // Remote debugging protocol is JSON RPC 2.0 compiant. In terms of that transport,
-    // responses to the commands carry "id" property, while notifications do not.
-    if (!object.id) {
+    const object = /** @type {LH.Protocol.RawMessage} */(JSON.parse(message));
+
+    // Responses to commands carry "id" property, while events do not.
+    if (!('id' in object)) {
+      // tsc doesn't currently narrow type in !in branch, so manually cast.
+      const eventMessage = /** @type {LH.Protocol.RawEventMessage} */(object);
       log.formatProtocol('<= event',
-          {method: object.method, params: object.params}, 'verbose');
-      this.emitNotification(object.method, object.params);
+          {method: eventMessage.method, params: eventMessage.params}, 'verbose');
+      this.emitProtocolEvent(eventMessage);
       return;
     }
 
@@ -126,15 +116,14 @@ class Connection {
   }
 
   /**
-   * @param {string} method
-   * @param {object=} params
-   * @protected
+   * @param {LH.Protocol.RawEventMessage} eventMessage
    */
-  emitNotification(method, params) {
+  emitProtocolEvent(eventMessage) {
     if (!this._eventEmitter) {
       throw new Error('Attempted to emit event after connection disposed.');
     }
-    this._eventEmitter.emit('notification', {method, params});
+
+    this._eventEmitter.emit('protocolevent', eventMessage);
   }
 
   /**
@@ -148,4 +137,20 @@ class Connection {
   }
 }
 
+// Declared outside class body because function expressions can be typed via more expressive @type
+/**
+ * Bind listeners for connection events
+ * @type {CrdpEventMessageEmitter['on']}
+ */
+Connection.prototype.on = function on(eventName, cb) {
+  if (eventName !== 'protocolevent') {
+    throw new Error('Only supports "protocolevent" events');
+  }
+
+  if (!this._eventEmitter) {
+    throw new Error('Attempted to add event listener after connection disposed.');
+  }
+  this._eventEmitter.on(eventName, cb);
+};
+
 module.exports = Connection;

lighthouse-core/gather/connections/extension.js

@@ -30,7 +30,11 @@ class ExtensionConnection extends Connection {
   _onEvent(source, method, params) {
     // log events received
     log.log('<=', method, params);
-    this.emitNotification(method, params);
+
+    // Warning: type cast, assuming that debugger API is giving us a valid protocol event.
+    // Must be cast together since types of `params` and `method` come as a pair.
+    const eventMessage = /** @type {LH.Protocol.RawEventMessage} */({method, params});
+    this.emitProtocolEvent(eventMessage);
   }
 
   /**

lighthouse-core/gather/driver.js

@@ -29,6 +29,10 @@ const DEFAULT_NETWORK_QUIET_THRESHOLD = 5000;
 // Controls how long to wait between longtasks before determining the CPU is idle, off by default
 const DEFAULT_CPU_QUIET_THRESHOLD = 0;
 
+/**
+ * @typedef {LH.StrictEventEmitter<LH.CrdpEvents>} CrdpEventEmitter
+ */
+
 class Driver {
   static get MAX_WAIT_FOR_FULLY_LOADED() {
     return 45 * 1000;
@@ -39,6 +43,10 @@ class Driver {
    */
   constructor(connection) {
     this._traceCategories = Driver.traceCategories;
+    /**
+     * An event emitter that enforces mapping between Crdp event names and payload types.
+     * @type {CrdpEventEmitter}
+     */
     this._eventEmitter = new EventEmitter();
     this._connection = connection;
     // currently only used by WPT where just Page and Network are needed
@@ -63,7 +71,7 @@ class Driver {
      */
     this._monitoredUrl = null;
 
-    connection.on('notification', event => {
+    connection.on('protocolevent', event => {
       this._devtoolsLog.record(event);
       if (this._networkStatusMonitor) {
         this._networkStatusMonitor.dispatch(event.method, event.params);
@@ -124,49 +132,6 @@ class Driver {
     return this._connection.wsEndpoint();
   }
 
-  /**
-   * Bind listeners for protocol events
-   * @param {string} eventName
-   * @param {(event: any) => void} cb
-   */
-  on(eventName, cb) {
-    if (this._eventEmitter === null) {
-      throw new Error('connect() must be called before attempting to listen to events.');
-    }
-
-    // log event listeners being bound
-    log.formatProtocol('listen for event =>', {method: eventName}, 'verbose');
-    this._eventEmitter.on(eventName, cb);
-  }
-
-  /**
-   * Bind a one-time listener for protocol events. Listener is removed once it
-   * has been called.
-   * @param {string} eventName
-   * @param {(event: any) => void} cb
-   */
-  once(eventName, cb) {
-    if (this._eventEmitter === null) {
-      throw new Error('connect() must be called before attempting to listen to events.');
-    }
-    // log event listeners being bound
-    log.formatProtocol('listen once for event =>', {method: eventName}, 'verbose');
-    this._eventEmitter.once(eventName, cb);
-  }
-
-  /**
-   * Unbind event listeners
-   * @param {string} eventName
-   * @param {(event: any) => void} cb
-   */
-  off(eventName, cb) {
-    if (this._eventEmitter === null) {
-      throw new Error('connect() must be called before attempting to remove an event listener.');
-    }
-
-    this._eventEmitter.removeListener(eventName, cb);
-  }
-
   /**
    * Debounce enabling or disabling domains to prevent driver users from
    * stomping on each other. Maintains an internal count of the times a domain
@@ -495,7 +460,7 @@ class Driver {
     const checkForQuietExpression = `(${pageFunctions.checkTimeSinceLastLongTask.toString()})()`;
     /**
      * @param {Driver} driver
-     * @param {(value: void) => void} resolve
+     * @param {() => void} resolve
      */
     function checkForQuiet(driver, resolve) {
       if (cancelled) return;
@@ -893,7 +858,7 @@ class Driver {
   }
 
   /**
-   * @param {{additionalTraceCategories: string=}=} settings
+   * @param {{additionalTraceCategories?: string}=} settings
    * @return {Promise<void>}
    */
   beginTrace(settings) {
@@ -946,8 +911,8 @@ class Driver {
   endTrace() {
     return new Promise((resolve, reject) => {
       // When the tracing has ended this will fire with a stream handle.
-      this.once('Tracing.tracingComplete', streamHandle => {
-        this._readTraceFromStream(streamHandle)
+      this.once('Tracing.tracingComplete', completeEvent => {
+        this._readTraceFromStream(completeEvent)
             .then(traceContents => resolve(traceContents), reject);
       });
 
@@ -957,15 +922,15 @@ class Driver {
   }
 
   /**
-   * @param {LH.Crdp.Tracing.TracingCompleteEvent} streamHandle
+   * @param {LH.Crdp.Tracing.TracingCompleteEvent} traceCompleteEvent
    */
-  _readTraceFromStream(streamHandle) {
+  _readTraceFromStream(traceCompleteEvent) {
     return new Promise((resolve, reject) => {
       let isEOF = false;
       const parser = new TraceParser();
 
       const readArguments = {
-        handle: streamHandle.stream,
+        handle: traceCompleteEvent.stream,
       };
 
       /**
@@ -1204,4 +1169,45 @@ class Driver {
   }
 }
 
+// Declared outside class body because function expressions can be typed via more expressive @type
+/**
+ * Bind listeners for protocol events.
+ * @type {CrdpEventEmitter['on']}
+ */
+Driver.prototype.on = function on(eventName, cb) {
+  if (this._eventEmitter === null) {
+    throw new Error('connect() must be called before attempting to listen to events.');
+  }
+
+  // log event listeners being bound
+  log.formatProtocol('listen for event =>', {method: eventName}, 'verbose');
+  this._eventEmitter.on(eventName, cb);
+};
+
+/**
+ * Bind a one-time listener for protocol events. Listener is removed once it
+ * has been called.
+ * @type {CrdpEventEmitter['once']}
+ */
+Driver.prototype.once = function once(eventName, cb) {
+  if (this._eventEmitter === null) {
+    throw new Error('connect() must be called before attempting to listen to events.');
+  }
+  // log event listeners being bound
+  log.formatProtocol('listen once for event =>', {method: eventName}, 'verbose');
+  this._eventEmitter.once(eventName, cb);
+};
+
+/**
+ * Unbind event listener.
+ * @type {CrdpEventEmitter['removeListener']}
+ */
+Driver.prototype.off = function off(eventName, cb) {
+  if (this._eventEmitter === null) {
+    throw new Error('connect() must be called before attempting to remove an event listener.');
+  }
+
+  this._eventEmitter.removeListener(eventName, cb);
+};
+
 module.exports = Driver;