Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions lighthouse-core/gather/connections/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,16 @@ class Connection {
* Call protocol methods
* @param {!string} method
* @param {!Object} params
* @param {{silent: boolean}=} cmdOpts
* @return {!Promise}
*/
sendCommand(method, params = {}) {
sendCommand(method, params = {}, cmdOpts = {}) {
log.formatProtocol('method => browser', {method, params}, 'verbose');
const id = ++this._lastCommandId;
const message = JSON.stringify({id, method, params});
this.sendRawMessage(message);
return new Promise((resolve, reject) => {
this._callbacks.set(id, {resolve, reject, method});
this._callbacks.set(id, {resolve, reject, method, options: cmdOpts});
});
}

Expand Down Expand Up @@ -98,7 +99,8 @@ class Connection {

return callback.resolve(Promise.resolve().then(_ => {
if (object.error) {
log.formatProtocol('method <= browser ERR', {method: callback.method}, 'error');
const logLevel = callback.options && callback.options.silent ? 'verbose' : 'error';
log.formatProtocol('method <= browser ERR', {method: callback.method}, logLevel);
throw new Error(`Protocol error (${callback.method}): ${object.error.message}`);
}

Expand Down
19 changes: 17 additions & 2 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,10 @@ class Driver {
* Call protocol methods
* @param {!string} method
* @param {!Object} params
* @param {{silent: boolean}=} cmdOpts
* @return {!Promise}
*/
sendCommand(method, params) {
sendCommand(method, params, cmdOpts) {
const domainCommand = /^(\w+)\.(enable|disable)$/.exec(method);
if (domainCommand) {
const enable = domainCommand[2] === 'enable';
Expand All @@ -198,7 +199,7 @@ class Driver {
}
}

return this._connection.sendCommand(method, params);
return this._connection.sendCommand(method, params, cmdOpts);
}

/**
Expand Down Expand Up @@ -710,9 +711,23 @@ class Driver {

// Enable Page domain to wait for Page.loadEventFired
return this.sendCommand('Page.enable')
// ensure tracing is stopped before we can start
// see https://github.com/GoogleChrome/lighthouse/issues/1091
.then(_ => this.endTraceIfStarted())
.then(_ => this.sendCommand('Tracing.start', tracingOpts));
}

endTraceIfStarted() {
return new Promise((resolve) => {
const traceCallback = () => resolve();
this.once('Tracing.tracingComplete', traceCallback);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why listen here? what about

this.sendCommand('Tracing.end').then(resolve, resolve);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nah it moves on too quickly and still fails fatally with tracing already started, this seemed preferable to an arbitrary timeout

return this.sendCommand('Tracing.end', undefined, {silent: true}).catch(() => {
this.off('Tracing.tracingComplete', traceCallback);
traceCallback();
});
});
}

endTrace() {
return new Promise((resolve, reject) => {
// When the tracing has ended this will fire with a stream handle.
Expand Down
17 changes: 17 additions & 0 deletions lighthouse-core/test/gather/driver-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ connection.sendCommand = function(command, params) {
case 'ServiceWorker.enable':
case 'ServiceWorker.disable':
return Promise.resolve();
case 'Tracing.end':
return Promise.reject(new Error('tracing not started'));
default:
throw Error(`Stub not implemented: ${command}`);
}
Expand Down Expand Up @@ -186,6 +188,21 @@ describe('Browser Driver', () => {
});
});

it('waits for tracingComplete when tracing already started', () => {
const fakeConnection = new Connection();
const fakeDriver = new Driver(fakeConnection);
const commands = [];
fakeConnection.sendCommand = evt => {
commands.push(evt);
return Promise.resolve();
};

fakeDriver.once = createOnceStub({'Tracing.tracingComplete': {}});
return fakeDriver.beginTrace().then(() => {
assert.deepEqual(commands, ['Page.enable', 'Tracing.end', 'Tracing.start']);
});
});

it('will request default traceCategories', () => {
return driverStub.beginTrace().then(() => {
const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
Expand Down