Skip to content

Commit

Permalink
move driver event handling to base class
Browse files Browse the repository at this point in the history 
remove custom event handler/listener logic from extension driver

move cli's custom logger to base class
  • Loading branch information
@brendankenny
brendankenny committed Jul 30, 2016
1 parent 999d3bf commit 07b3ccb
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 131 deletions.
63 changes: 5 additions & 58 deletions lighthouse-core/gather/drivers/cri.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class CriDriver extends Driver {
chromeRemoteInterface({port: port, chooseTab: tab}, chrome => {
this._tab = tab;
this._chrome = chrome;
this.beginLogging();
this._eventEmitter = chrome;
this.enableRuntimeEvents().then(_ => {
resolve();
});
Expand Down Expand Up @@ -79,57 +79,11 @@ class CriDriver extends Driver {
}
this._tab = null;
this._chrome = null;
this._eventEmitter = null;
this.url = null;
});
}

beginLogging() {
// log events received
this._chrome.on('event', req => _log('<=', req, 'verbose'));
}

/**
* Bind listeners for protocol events
* @param {!string} eventName
* @param {function(...)} cb
*/
on(eventName, cb) {
if (this._chrome === null) {
throw new Error('connect() must be called before attempting to listen to events.');
}
// log event listeners being bound
_log('listen for event =>', {method: eventName});
this._chrome.on(eventName, cb);
}

/**
* Bind a one-time listener for protocol events. Listener is removed once it
* has been called.
* @param {!string} eventName
* @param {function(...)} cb
*/
once(eventName, cb) {
if (this._chrome === null) {
throw new Error('connect() must be called before attempting to listen to events.');
}
// log event listeners being bound
_log('listen once for event =>', {method: eventName});
this._chrome.once(eventName, cb);
}

/**
* Unbind event listeners
* @param {!string} eventName
* @param {function(...)} cb
*/
off(eventName, cb) {
if (this._chrome === null) {
throw new Error('connect() must be called before attempting to remove an event listener.');
}

this._chrome.removeListener(eventName, cb);
}

/**
* Call protocol methods
* @param {!string} command
Expand All @@ -142,25 +96,18 @@ class CriDriver extends Driver {
}

return new Promise((resolve, reject) => {
_log('method => browser', {method: command, params: params});
this.formattedLog('method => browser', {method: command, params: params});

this._chrome.send(command, params, (err, result) => {
if (err) {
_log('method <= browser ERR', {method: command, params: result}, 'error');
this.formattedLog('method <= browser ERR', {method: command, params: result}, 'error');
return reject(result);
}
_log('method <= browser OK', {method: command, params: result});
this.formattedLog('method <= browser OK', {method: command, params: result});
resolve(result);
});
});
}
}

function _log(prefix, data, level) {
const columns = (typeof process === 'undefined') ? Infinity : process.stdout.columns;
const maxLength = columns - data.method.length - prefix.length - 18;
const snippet = data.params ? JSON.stringify(data.params).substr(0, maxLength) : '';
log[level ? level : 'log'](prefix, data.method, snippet);
}

module.exports = CriDriver;
49 changes: 43 additions & 6 deletions lighthouse-core/gather/drivers/driver.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ const NetworkRecorder = require('../../lib/network-recorder');
const emulation = require('../../lib/emulation');
const Element = require('../../lib/element');

const log = require('../../lib/log.js');

class Driver {

constructor() {
Expand All @@ -28,6 +30,7 @@ class Driver {
this._chrome = null;
this._traceEvents = [];
this._traceCategories = Driver.traceCategories;
this._eventEmitter = null;
}

get url() {
Expand Down Expand Up @@ -64,6 +67,19 @@ class Driver {
return this.sendCommand('Security.enable');
}

/**
* A simple formatting utility for event logging.
* @param {string} prefix
* @param {!Object} data A JSON-serializable object of event data to log.
* @param {string=} level Optional logging level. Defaults to 'log'.
*/
formattedLog(prefix, data, level) {
const columns = (!process || process.browser) ? Infinity : process.stdout.columns;
const maxLength = columns - data.method.length - prefix.length - 18;
const snippet = data.params ? JSON.stringify(data.params).substr(0, maxLength) : '';
log[level ? level : 'log'](prefix, data.method, snippet);
}

/**
* @return {!Promise<null>}
*/
Expand All @@ -77,24 +93,45 @@ class Driver {

/**
* Bind listeners for protocol events
* @param {!string} eventName
* @param {function(...)} cb
*/
on() {
return Promise.reject(new Error('Not implemented'));
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
this.formattedLog('listen for event =>', {method: eventName});
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 {function(...)} cb
*/
once() {
return Promise.reject(new Error('Not implemented'));
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
this.formattedLog('listen once for event =>', {method: eventName});
this._eventEmitter.once(eventName, cb);
}

/**
* Unbind event listeners
* @param {!string} eventName
* @param {function(...)} cb
*/
off() {
return Promise.reject(new Error('Not implemented'));
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);
}

/**
Expand Down
76 changes: 9 additions & 67 deletions lighthouse-core/gather/drivers/extension.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,21 @@

const Driver = require('./driver.js');
const log = require('../../lib/log.js');
const EventEmitter = require('events').EventEmitter;

/* globals chrome */

class ExtensionDriver extends Driver {

constructor() {
super();
this._listeners = {};
this._tabId = null;
this._debuggerConnected = false;
chrome.debugger.onEvent.addListener(this._onEvent.bind(this));
this.handleUnexpectedDetach();

this._eventEmitter = new EventEmitter();
chrome.debugger.onEvent.addListener((_, method, params) =>
this._eventEmitter.emit(method, params));
}

connect() {
Expand Down Expand Up @@ -75,53 +78,6 @@ class ExtensionDriver extends Driver {
);
}

/**
* Bind listeners for protocol events
* @param {!string} eventName
* @param {function(...)} cb
*/
on(eventName, cb) {
if (typeof this._listeners[eventName] === 'undefined') {
this._listeners[eventName] = [];
}
// log event listeners being bound
log.log('listen for event =>', eventName);
this._listeners[eventName].push(cb);
}

/**
* Bind a one-time listener for protocol events. Listener is removed once it
* has been called.
* @param {!string} eventName
* @param {function(...)} cb
*/
once(eventName, cb) {
// Create a replacement listener that will immediately remove itself after calling cb once.
const cbGuard = function() {
cb(...arguments);
this.off(eventName, cbGuard);
}.bind(this);

this.on(eventName, cbGuard);
}

_onEvent(source, method, params) {
if (this._listeners[method] === undefined) {
return;
}

// Copy array of listeners so all listeners are called, even if some removed
// during loop over listeners. Consistent with node's removeListener behavior:
// https://nodejs.org/api/events.html#events_emitter_removelistener_eventname_listener
const listenersCopy = Array.from(this._listeners[method]);
listenersCopy.forEach(cb => {
// despite best efforts, this still sometimes loses the context of the instance,
// which means sendCommand will not have this._tabId. Luckily, it doesn't need it
// for trace-based commands like IO.read.
cb.call(this, params);
});
}

/**
* Call protocol methods
* @param {!string} command
Expand All @@ -130,22 +86,22 @@ class ExtensionDriver extends Driver {
*/
sendCommand(command, params) {
return new Promise((resolve, reject) => {
log.log('method => browser', command, params);
this.formattedLog('method => browser', {method: command, params: params});
if (!this._tabId) {
log.error('No tabId set for sendCommand');
}
chrome.debugger.sendCommand({tabId: this._tabId}, command, params, result => {
if (chrome.runtime.lastError) {
log.error('method <= browser', command, result);
this.formattedLog('method <= browser ERR', {method: command, params: result}, 'error');
return reject(chrome.runtime.lastError);
}

if (result.wasThrown) {
log.log('error', 'method <= browser', command, result);
this.formattedLog('method <= browser ERR', {method: command, params: result}, 'error');
return reject(result.exceptionDetails);
}

log.log('method <= browser OK', command, result);
this.formattedLog('method <= browser OK', {method: command, params: result});
resolve(result);
});
});
Expand Down Expand Up @@ -207,20 +163,6 @@ class ExtensionDriver extends Driver {
throw new Error('Lighthouse detached from browser: ' + detachReason);
});
}

off(eventName, cb) {
if (typeof this._listeners[eventName] === 'undefined') {
console.warn(`Unable to remove listener ${eventName}; no such listener found.`);
return;
}

const callbackIndex = this._listeners[eventName].indexOf(cb);
if (callbackIndex === -1) {
return;
}

this._listeners[eventName].splice(callbackIndex, 1);
}
}

module.exports = ExtensionDriver;

0 comments on commit 07b3ccb

Please sign in to comment.