Skip to content

Commit

Permalink
core(driver): request smaller trace in m71+ (#6117)
Browse files Browse the repository at this point in the history
  • Loading branch information
paulirish authored Oct 15, 2018
1 parent 7cb376f commit b878616
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 26 deletions.
23 changes: 17 additions & 6 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class Driver {
static get traceCategories() {
return [
'-*', // exclude default
'toplevel',
'disabled-by-default-lighthouse', // used instead of 'toplevel' in Chrome 71+
'v8.execute',
'blink.console',
'blink.user_timing',
Expand All @@ -102,11 +102,13 @@ class Driver {
}

/**
* @return {Promise<string>}
* @return {Promise<LH.Crdp.Browser.GetVersionResponse & {milestone: number}>}
*/
getUserAgent() {
// FIXME: use Browser.getVersion instead
return this.evaluateAsync('navigator.userAgent');
async getBrowserVersion() {
const version = await this.sendCommand('Browser.getVersion');
const match = version.product.match(/\/(\d+)/); // eg 'Chrome/71.0.3577.0'
const milestone = match ? parseInt(match[1]) : 0;
return Object.assign(version, {milestone});
}

/**
Expand Down Expand Up @@ -919,10 +921,19 @@ class Driver {
* @param {{additionalTraceCategories?: string|null}=} settings
* @return {Promise<void>}
*/
beginTrace(settings) {
async beginTrace(settings) {
const additionalCategories = (settings && settings.additionalTraceCategories &&
settings.additionalTraceCategories.split(',')) || [];
const traceCategories = this._traceCategories.concat(additionalCategories);

// In Chrome <71, gotta use the chatty 'toplevel' cat instead of our own.
// TODO(COMPAT): Once m71 ships to stable, drop this section
const milestone = (await this.getBrowserVersion()).milestone;
if (milestone < 71) {
const toplevelIndex = traceCategories.indexOf('disabled-by-default-lighthouse');
traceCategories.splice(toplevelIndex, 1, 'toplevel');
}

const uniqueCategories = Array.from(new Set(traceCategories));

// Check any domains that could interfere with or add overhead to the trace.
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ class GatherRunner {
return {
fetchTime: (new Date()).toJSON(),
LighthouseRunWarnings: [],
HostUserAgent: await options.driver.getUserAgent(),
HostUserAgent: (await options.driver.getBrowserVersion()).userAgent,
NetworkUserAgent: '', // updated later
BenchmarkIndex: 0, // updated later
traces: {},
Expand Down
21 changes: 13 additions & 8 deletions lighthouse-core/lib/traces/tracing-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,16 @@
// The ideal input response latency, the time between the input task and the
// first frame of the response.
const BASE_RESPONSE_LATENCY = 16;
// m65 and earlier
const SCHEDULABLE_TASK_TITLE = 'TaskQueueManager::ProcessTaskFromWorkQueue';
// m71+ We added RunTask to `disabled-by-default-lighthouse`
const SCHEDULABLE_TASK_TITLE_LH = 'RunTask';
// m69-70 DoWork is different and we now need RunTask, see https://bugs.chromium.org/p/chromium/issues/detail?id=871204#c11
const SCHEDULABLE_TASK_TITLE_ALT1 = 'ThreadControllerImpl::RunTask';
// In m66-68 refactored to this task title, https://crrev.com/c/883346
const SCHEDULABLE_TASK_TITLE_ALT1 = 'ThreadControllerImpl::DoWork';
// m69+ DoWork is different and we now need RunTask, see https://bugs.chromium.org/p/chromium/issues/detail?id=871204#c11
const SCHEDULABLE_TASK_TITLE_ALT2 = 'ThreadControllerImpl::RunTask';
const SCHEDULABLE_TASK_TITLE_ALT2 = 'ThreadControllerImpl::DoWork';
// m65 and earlier
const SCHEDULABLE_TASK_TITLE_ALT3 = 'TaskQueueManager::ProcessTaskFromWorkQueue';


const LHError = require('../lh-error');

class TraceProcessor {
Expand Down Expand Up @@ -234,9 +238,10 @@ class TraceProcessor {
* @return {boolean}
*/
static isScheduleableTask(evt) {
return evt.name === SCHEDULABLE_TASK_TITLE ||
evt.name === SCHEDULABLE_TASK_TITLE_ALT1 ||
evt.name === SCHEDULABLE_TASK_TITLE_ALT2;
return evt.name === SCHEDULABLE_TASK_TITLE_LH ||
evt.name === SCHEDULABLE_TASK_TITLE_ALT1 ||
evt.name === SCHEDULABLE_TASK_TITLE_ALT2 ||
evt.name === SCHEDULABLE_TASK_TITLE_ALT3;
}
}

Expand Down
42 changes: 38 additions & 4 deletions lighthouse-core/test/gather/driver-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
'use strict';

let sendCommandParams = [];
let sendCommandMockResponses = {};

const Driver = require('../../gather/driver.js');
const Connection = require('../../gather/connections/connection.js');
const Element = require('../../lib/element.js');
const assert = require('assert');
const EventEmitter = require('events').EventEmitter;
const {protocolGetVersionResponse} = require('./fake-driver');

const connection = new Connection();
const driverStub = new Driver(connection);
Expand Down Expand Up @@ -46,9 +48,23 @@ function createActiveWorker(id, url, controlledClients, status = 'activated') {
};
}

function createOnceMethodResponse(method, response) {
assert.deepEqual(!!sendCommandMockResponses[method], false, 'stub response already defined');
sendCommandMockResponses[method] = response;
}

connection.sendCommand = function(command, params) {
sendCommandParams.push({command, params});

const mockResponse = sendCommandMockResponses[command];
if (mockResponse) {
delete sendCommandMockResponses[command];
return Promise.resolve(mockResponse);
}

switch (command) {
case 'Browser.getVersion':
return Promise.resolve(protocolGetVersionResponse);
case 'DOM.getDocument':
return Promise.resolve({root: {nodeId: 249}});
case 'DOM.querySelector':
Expand Down Expand Up @@ -99,6 +115,7 @@ connection.sendCommand = function(command, params) {
describe('Browser Driver', () => {
beforeEach(() => {
sendCommandParams = [];
sendCommandMockResponses = {};
});

it('returns null when DOM.querySelector finds no node', () => {
Expand Down Expand Up @@ -232,16 +249,33 @@ describe('Browser Driver', () => {
});

it('will use requested additionalTraceCategories', () => {
return driverStub.beginTrace({additionalTraceCategories: 'v8,v8.execute,toplevel'}).then(() => {
return driverStub.beginTrace({additionalTraceCategories: 'loading,xtra_cat'}).then(() => {
const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
const categories = traceCmd.params.categories;
assert.ok(categories.includes('blink'), 'contains default categories');
assert.ok(categories.includes('v8.execute'), 'contains added categories');
assert.ok(categories.indexOf('toplevel') === categories.lastIndexOf('toplevel'),
assert.ok(categories.includes('blink.user_timing'), 'contains default categories');
assert.ok(categories.includes('xtra_cat'), 'contains added categories');
assert.ok(categories.indexOf('loading') === categories.lastIndexOf('loading'),
'de-dupes categories');
});
});

it('will adjust traceCategories based on chrome version', async () => {
// m70 doesn't have disabled-by-default-lighthouse, so 'toplevel' is used instead.
const m70ProtocolGetVersionResponse = Object.assign({}, protocolGetVersionResponse, {
product: 'Chrome/70.0.3577.0',
// eslint-disable-next-line max-len
userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3577.0 Safari/537.36',
});
createOnceMethodResponse('Browser.getVersion', m70ProtocolGetVersionResponse);

// eslint-disable-next-line max-len
await driverStub.beginTrace();
const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
const categories = traceCmd.params.categories;
assert.ok(categories.includes('toplevel'), 'contains old toplevel category');
assert.equal(categories.indexOf('disabled-by-default-lighthouse'), -1, 'excludes new cat');
});

it('should send the Network.setExtraHTTPHeaders command when there are extra-headers', () => {
return driverStub.setExtraHTTPHeaders({
'Cookie': 'monster',
Expand Down
19 changes: 16 additions & 3 deletions lighthouse-core/test/gather/fake-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,19 @@
*/
'use strict';

module.exports = {
getUserAgent() {
return Promise.resolve('Fake user agent');
// https://chromedevtools.github.io/devtools-protocol/tot/Browser#method-getVersion
const protocolGetVersionResponse = {
protocolVersion: '1.3',
product: 'Chrome/71.0.3577.0',
revision: '@fc334a55a70eec12fc77853c53979f81e8496c21',
// eslint-disable-next-line max-len
userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3577.0 Safari/537.36',
jsVersion: '7.1.314',
};

const fakeDriver = {
getBrowserVersion() {
return Promise.resolve(Object.assign({}, protocolGetVersionResponse, {milestone: 71}));
},
getBenchmarkIndex() {
return Promise.resolve(125.2);
Expand Down Expand Up @@ -72,3 +82,6 @@ module.exports = {
return Promise.resolve();
},
};

module.exports = fakeDriver;
module.exports.protocolGetVersionResponse = protocolGetVersionResponse;
7 changes: 3 additions & 4 deletions lighthouse-core/test/gather/gather-runner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ function getMockedEmulationDriver(emulationFn, netThrottleFn, cpuThrottleFn,
}
cleanBrowserCaches() {}
clearDataForOrigin() {}
getUserAgent() {
return Promise.resolve('Fake user agent');
}
};
const EmulationMock = class extends Connection {
sendCommand(command, params) {
Expand Down Expand Up @@ -126,7 +123,8 @@ describe('GatherRunner', function() {
const options = {url, driver, config, settings};

const results = await GatherRunner.run([], options);
expect(results.HostUserAgent).toEqual('Fake user agent');
expect(results.HostUserAgent).toEqual(fakeDriver.protocolGetVersionResponse.userAgent);
expect(results.HostUserAgent).toMatch(/Chrome\/\d+/);
});

it('collects network user agent as an artifact', async () => {
Expand Down Expand Up @@ -272,6 +270,7 @@ describe('GatherRunner', function() {
};
const createEmulationCheck = variable => (...args) => {
tests[variable] = args;

return true;
};
const driver = getMockedEmulationDriver(
Expand Down

0 comments on commit b878616

Please sign in to comment.