Skip to content

Commit

Permalink
core(tsc): add type checking to use of CRDP events (#4886)
Browse files Browse the repository at this point in the history
  • Loading branch information
@brendankenny
brendankenny authored Mar 30, 2018
1 parent 73918f1 commit e26b2cf
Show file tree
Hide file tree
Showing 12 changed files with 577 additions and 86 deletions.
63 changes: 34 additions & 29 deletions lighthouse-core/gather/connections/connection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -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
Expand All @@ -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 */

/**
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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;
6 changes: 5 additions & 1 deletion lighthouse-core/gather/connections/extension.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
108 changes: 57 additions & 51 deletions lighthouse-core/gather/driver.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -893,7 +858,7 @@ class Driver {
}

/**
* @param {{additionalTraceCategories: string=}=} settings
* @param {{additionalTraceCategories?: string}=} settings
* @return {Promise<void>}
*/
beginTrace(settings) {
Expand Down Expand Up @@ -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);
});

Expand All @@ -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,
};

/**
Expand Down Expand Up @@ -1205,4 +1170,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;
Loading

0 comments on commit e26b2cf

Please sign in to comment.