From df59493f90cf06d7657b26706c9b593167f9e7eb Mon Sep 17 00:00:00 2001
From: Paul Irish <paulirish@google.com>
Date: Wed, 26 Sep 2018 12:37:57 -0700
Subject: [PATCH 1/8] core(driver): request smaller trace in m71+

---
 lighthouse-core/gather/driver.js           | 15 +++++++++++++--
 lighthouse-core/gather/gather-runner.js    |  8 ++++----
 lighthouse-core/test/gather/driver-test.js | 17 +++++++++++++++--
 typings/gatherer.d.ts                      |  1 +
 4 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
index f96df0b09ae0..07717411d4ac 100644
--- a/lighthouse-core/gather/driver.js
+++ b/lighthouse-core/gather/driver.js
@@ -916,13 +916,24 @@ class Driver {
   }
 
   /**
-   * @param {{additionalTraceCategories?: string|null}=} settings
+   * @param {LH.Gatherer.PassContext} passContext
    * @return {Promise<void>}
    */
-  beginTrace(settings) {
+  beginTrace(passContext) {
+    const settings = passContext.settings;
     const additionalCategories = (settings && settings.additionalTraceCategories &&
         settings.additionalTraceCategories.split(',')) || [];
     const traceCategories = this._traceCategories.concat(additionalCategories);
+
+    // In Chrome 71+, we trade the chatty 'toplevel' cat for our own, reducing overall trace size
+    // TODO(COMPAT): Once m71 ships to stable, change the traceCategories for real
+    const reChrome71 = /Chrome\/(Headless)?7[1-9]/;
+    if (passContext.hostUserAgent && reChrome71.test(passContext.hostUserAgent)) {
+      traceCategories.forEach((cat, idx) => {
+        if (cat === 'toplevel') traceCategories[idx] = 'disabled-by-default-lighthouse';
+      });
+    }
+
     const uniqueCategories = Array.from(new Set(traceCategories));
 
     // Check any domains that could interfere with or add overhead to the trace.
diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js
index 918aa0e13553..3af0c0a046ff 100644
--- a/lighthouse-core/gather/gather-runner.js
+++ b/lighthouse-core/gather/gather-runner.js
@@ -211,11 +211,10 @@ class GatherRunner {
   static async pass(passContext, gathererResults) {
     const driver = passContext.driver;
     const config = passContext.passConfig;
-    const settings = passContext.settings;
     const gatherers = config.gatherers;
-
     const recordTrace = config.recordTrace;
-    const isPerfRun = !settings.disableStorageReset && recordTrace && config.useThrottling;
+    const isPerfRun = !passContext.settings.disableStorageReset && recordTrace &&
+        config.useThrottling;
 
     const gatherernames = gatherers.map(g => g.instance.name).join(', ');
     const status = 'Loading page & waiting for onload';
@@ -226,7 +225,7 @@ class GatherRunner {
     // Always record devtoolsLog
     await driver.beginDevtoolsLog();
     // Begin tracing if requested by config.
-    if (recordTrace) await driver.beginTrace(settings);
+    if (recordTrace) await driver.beginTrace(passContext);
 
     // Navigate.
     await GatherRunner.loadPage(driver, passContext);
@@ -402,6 +401,7 @@ class GatherRunner {
           // If the main document redirects, we'll update this to keep track
           url: options.requestedUrl,
           settings: options.settings,
+          hostUserAgent: baseArtifacts.HostUserAgent,
           passConfig,
           // *pass() functions and gatherers can push to this warnings array.
           LighthouseRunWarnings: baseArtifacts.LighthouseRunWarnings,
diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js
index 0f04bdc230c4..cddc07a3df53 100644
--- a/lighthouse-core/test/gather/driver-test.js
+++ b/lighthouse-core/test/gather/driver-test.js
@@ -224,7 +224,7 @@ describe('Browser Driver', () => {
   });
 
   it('will request default traceCategories', () => {
-    return driverStub.beginTrace().then(() => {
+    return driverStub.beginTrace({settings: {}}).then(() => {
       const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
       const categories = traceCmd.params.categories;
       assert.ok(categories.includes('devtools.timeline'), 'contains devtools.timeline');
@@ -232,7 +232,8 @@ describe('Browser Driver', () => {
   });
 
   it('will use requested additionalTraceCategories', () => {
-    return driverStub.beginTrace({additionalTraceCategories: 'v8,v8.execute,toplevel'}).then(() => {
+    const passContext = {settings: {additionalTraceCategories: 'v8,v8.execute,toplevel'}};
+    return driverStub.beginTrace(passContext).then(() => {
       const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
       const categories = traceCmd.params.categories;
       assert.ok(categories.includes('blink'), 'contains default categories');
@@ -242,6 +243,18 @@ describe('Browser Driver', () => {
     });
   });
 
+
+  it('will adjust traceCategories based on host user agent', () => {
+    // eslint-disable-next-line max-len
+    const passContext = {hostUserAgent: 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/Headless71.0.3561.0 Safari/537.36'};
+    return driverStub.beginTrace(passContext).then(() => {
+      const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
+      const categories = traceCmd.params.categories;
+      assert.ok(categories.includes('disabled-by-default-lighthouse'), 'contains new categories');
+      assert.equal(categories.indexOf('toplevel'), -1, 'excludes toplevel');
+    });
+  });
+
   it('should send the Network.setExtraHTTPHeaders command when there are extra-headers', () => {
     return driverStub.setExtraHTTPHeaders({
       'Cookie': 'monster',
diff --git a/typings/gatherer.d.ts b/typings/gatherer.d.ts
index db4755b36939..f0a9fc1efadc 100644
--- a/typings/gatherer.d.ts
+++ b/typings/gatherer.d.ts
@@ -18,6 +18,7 @@ declare global {
       passConfig: Config.Pass
       settings: Config.Settings;
       options?: object;
+      hostUserAgent: string;
       /** Push to this array to add top-level warnings to the LHR. */
       LighthouseRunWarnings: Array<string>;
     }

From 805e7142ae55736ea149eab555125c5db3bc3218 Mon Sep 17 00:00:00 2001
From: Paul Irish <paulirish@google.com>
Date: Thu, 11 Oct 2018 19:38:31 -0700
Subject: [PATCH 2/8] feedback

---
 lighthouse-core/gather/driver.js        | 27 +++++++++++++------------
 lighthouse-core/gather/gather-runner.js |  2 +-
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
index 07717411d4ac..7558dba60bec 100644
--- a/lighthouse-core/gather/driver.js
+++ b/lighthouse-core/gather/driver.js
@@ -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',
@@ -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+)/);
+    const milestone = match ? parseInt(match[1]) : 0;
+    return Object.assign(version, {milestone});
   }
 
   /**
@@ -919,19 +921,18 @@ class Driver {
    * @param {LH.Gatherer.PassContext} passContext
    * @return {Promise<void>}
    */
-  beginTrace(passContext) {
+  async beginTrace(passContext) {
     const settings = passContext.settings;
     const additionalCategories = (settings && settings.additionalTraceCategories &&
         settings.additionalTraceCategories.split(',')) || [];
     const traceCategories = this._traceCategories.concat(additionalCategories);
 
-    // In Chrome 71+, we trade the chatty 'toplevel' cat for our own, reducing overall trace size
-    // TODO(COMPAT): Once m71 ships to stable, change the traceCategories for real
-    const reChrome71 = /Chrome\/(Headless)?7[1-9]/;
-    if (passContext.hostUserAgent && reChrome71.test(passContext.hostUserAgent)) {
-      traceCategories.forEach((cat, idx) => {
-        if (cat === 'toplevel') traceCategories[idx] = 'disabled-by-default-lighthouse';
-      });
+    // 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; // eg 'Chrome/71.0.3577.0'
+    if (milestone < 71) {
+      const toplevelIndex = traceCategories.indexOf('disabled-by-default-lighthouse');
+      traceCategories.splice(toplevelIndex, 1, 'toplevel');
     }
 
     const uniqueCategories = Array.from(new Set(traceCategories));
diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js
index 3af0c0a046ff..57966a2eb32e 100644
--- a/lighthouse-core/gather/gather-runner.js
+++ b/lighthouse-core/gather/gather-runner.js
@@ -365,7 +365,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: {},

From f0535bc08ee597fd4b80d46e8a19b36c19ddc957 Mon Sep 17 00:00:00 2001
From: Paul Irish <paulirish@google.com>
Date: Thu, 11 Oct 2018 21:39:05 -0700
Subject: [PATCH 3/8] fix tests with new createOnceMethodResponse() method

---
 lighthouse-core/test/gather/driver-test.js | 40 ++++++++++++++++++----
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js
index cddc07a3df53..aa2eb3913003 100644
--- a/lighthouse-core/test/gather/driver-test.js
+++ b/lighthouse-core/test/gather/driver-test.js
@@ -6,6 +6,7 @@
 'use strict';
 
 let sendCommandParams = [];
+let sendCommandStubResponses = {};
 
 const Driver = require('../../gather/driver.js');
 const Connection = require('../../gather/connections/connection.js');
@@ -46,9 +47,27 @@ function createActiveWorker(id, url, controlledClients, status = 'activated') {
   };
 }
 
+function createOnceMethodResponse(method, response) {
+  sendCommandStubResponses[method] = response;
+}
+
 connection.sendCommand = function(command, params) {
   sendCommandParams.push({command, params});
+
+  if (sendCommandStubResponses[command]) {
+    return Promise.resolve(sendCommandStubResponses[command]);
+  }
+
   switch (command) {
+    case 'Browser.getVersion':
+      return Promise.resolve({
+        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',
+      });
     case 'DOM.getDocument':
       return Promise.resolve({root: {nodeId: 249}});
     case 'DOM.querySelector':
@@ -232,26 +251,33 @@ describe('Browser Driver', () => {
   });
 
   it('will use requested additionalTraceCategories', () => {
-    const passContext = {settings: {additionalTraceCategories: 'v8,v8.execute,toplevel'}};
+    const passContext = {settings: {additionalTraceCategories: 'v8,v8.execute,loading'}};
     return driverStub.beginTrace(passContext).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.indexOf('loading') === categories.lastIndexOf('loading'),
           'de-dupes categories');
     });
   });
 
+  it('will adjust traceCategories based on chrome version', () => {
+    createOnceMethodResponse('Browser.getVersion', {
+      protocolVersion: '1.3',
+      product: 'Chrome/70.0.3577.0', // m70 doesn't have disabled-by-default-lighthouse
+      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',
+    });
 
-  it('will adjust traceCategories based on host user agent', () => {
     // eslint-disable-next-line max-len
-    const passContext = {hostUserAgent: 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/Headless71.0.3561.0 Safari/537.36'};
-    return driverStub.beginTrace(passContext).then(() => {
+    return driverStub.beginTrace({settings: {}}).then(() => {
       const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
       const categories = traceCmd.params.categories;
-      assert.ok(categories.includes('disabled-by-default-lighthouse'), 'contains new categories');
-      assert.equal(categories.indexOf('toplevel'), -1, 'excludes toplevel');
+      assert.ok(categories.includes('toplevel'), 'contains old toplevel category');
+      assert.equal(categories.indexOf('disabled-by-default-lighthouse'), -1, 'excludes new cat');
     });
   });
 

From 49caf216e899ac57841cff1b855aa4349219b407 Mon Sep 17 00:00:00 2001
From: Paul Irish <paulirish@google.com>
Date: Thu, 11 Oct 2018 21:40:21 -0700
Subject: [PATCH 4/8] drop the passcontext changes

---
 lighthouse-core/gather/gather-runner.js | 8 ++++----
 typings/gatherer.d.ts                   | 1 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js
index 57966a2eb32e..b5e62d3ee8ca 100644
--- a/lighthouse-core/gather/gather-runner.js
+++ b/lighthouse-core/gather/gather-runner.js
@@ -211,10 +211,11 @@ class GatherRunner {
   static async pass(passContext, gathererResults) {
     const driver = passContext.driver;
     const config = passContext.passConfig;
+    const settings = passContext.settings;
     const gatherers = config.gatherers;
+
     const recordTrace = config.recordTrace;
-    const isPerfRun = !passContext.settings.disableStorageReset && recordTrace &&
-        config.useThrottling;
+    const isPerfRun = !settings.disableStorageReset && recordTrace && config.useThrottling;
 
     const gatherernames = gatherers.map(g => g.instance.name).join(', ');
     const status = 'Loading page & waiting for onload';
@@ -225,7 +226,7 @@ class GatherRunner {
     // Always record devtoolsLog
     await driver.beginDevtoolsLog();
     // Begin tracing if requested by config.
-    if (recordTrace) await driver.beginTrace(passContext);
+    if (recordTrace) await driver.beginTrace(settings);
 
     // Navigate.
     await GatherRunner.loadPage(driver, passContext);
@@ -401,7 +402,6 @@ class GatherRunner {
           // If the main document redirects, we'll update this to keep track
           url: options.requestedUrl,
           settings: options.settings,
-          hostUserAgent: baseArtifacts.HostUserAgent,
           passConfig,
           // *pass() functions and gatherers can push to this warnings array.
           LighthouseRunWarnings: baseArtifacts.LighthouseRunWarnings,
diff --git a/typings/gatherer.d.ts b/typings/gatherer.d.ts
index f0a9fc1efadc..db4755b36939 100644
--- a/typings/gatherer.d.ts
+++ b/typings/gatherer.d.ts
@@ -18,7 +18,6 @@ declare global {
       passConfig: Config.Pass
       settings: Config.Settings;
       options?: object;
-      hostUserAgent: string;
       /** Push to this array to add top-level warnings to the LHR. */
       LighthouseRunWarnings: Array<string>;
     }

From 651401f2fcf335a178d6401c17b0a897f8a803ae Mon Sep 17 00:00:00 2001
From: Paul Irish <paulirish@google.com>
Date: Sat, 13 Oct 2018 15:21:02 -0700
Subject: [PATCH 5/8] update tracing processor.

---
 .../lib/traces/tracing-processor.js           | 21 ++++++++++++-------
 lighthouse-core/test/gather/fake-driver.js    |  4 ++--
 .../test/gather/gather-runner-test.js         |  4 ++--
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/lighthouse-core/lib/traces/tracing-processor.js b/lighthouse-core/lib/traces/tracing-processor.js
index 399d7b127862..21f9ac3371f5 100644
--- a/lighthouse-core/lib/traces/tracing-processor.js
+++ b/lighthouse-core/lib/traces/tracing-processor.js
@@ -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 {
@@ -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;
   }
 }
 
diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js
index 0055bbe6c4cf..8e52b437dc3a 100644
--- a/lighthouse-core/test/gather/fake-driver.js
+++ b/lighthouse-core/test/gather/fake-driver.js
@@ -6,8 +6,8 @@
 'use strict';
 
 module.exports = {
-  getUserAgent() {
-    return Promise.resolve('Fake user agent');
+  getBrowserVersion() {
+    return Promise.resolve({userAgent: 'Fake user agent'});
   },
   getBenchmarkIndex() {
     return Promise.resolve(125.2);
diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js
index 76084d924b5d..849e5439c7eb 100644
--- a/lighthouse-core/test/gather/gather-runner-test.js
+++ b/lighthouse-core/test/gather/gather-runner-test.js
@@ -53,8 +53,8 @@ function getMockedEmulationDriver(emulationFn, netThrottleFn, cpuThrottleFn,
     }
     cleanBrowserCaches() {}
     clearDataForOrigin() {}
-    getUserAgent() {
-      return Promise.resolve('Fake user agent');
+    getBrowserVersion() {
+      return Promise.resolve({userAgent: 'Fake user agent'});
     }
   };
   const EmulationMock = class extends Connection {

From b7ac629a9761b88678bc3b7e6c635adf502fef96 Mon Sep 17 00:00:00 2001
From: Paul Irish <paulirish@google.com>
Date: Sat, 13 Oct 2018 15:32:27 -0700
Subject: [PATCH 6/8] mock

---
 lighthouse-core/gather/driver.js           |  5 ++---
 lighthouse-core/test/gather/driver-test.js | 17 +++++++++++------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
index 7558dba60bec..dd7fd3494eee 100644
--- a/lighthouse-core/gather/driver.js
+++ b/lighthouse-core/gather/driver.js
@@ -918,11 +918,10 @@ class Driver {
   }
 
   /**
-   * @param {LH.Gatherer.PassContext} passContext
+   * @param {{additionalTraceCategories?: string|null}=} settings
    * @return {Promise<void>}
    */
-  async beginTrace(passContext) {
-    const settings = passContext.settings;
+  async beginTrace(settings) {
     const additionalCategories = (settings && settings.additionalTraceCategories &&
         settings.additionalTraceCategories.split(',')) || [];
     const traceCategories = this._traceCategories.concat(additionalCategories);
diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js
index aa2eb3913003..ba121cf6700b 100644
--- a/lighthouse-core/test/gather/driver-test.js
+++ b/lighthouse-core/test/gather/driver-test.js
@@ -6,7 +6,7 @@
 'use strict';
 
 let sendCommandParams = [];
-let sendCommandStubResponses = {};
+let sendCommandMockResponses = {};
 
 const Driver = require('../../gather/driver.js');
 const Connection = require('../../gather/connections/connection.js');
@@ -48,14 +48,17 @@ function createActiveWorker(id, url, controlledClients, status = 'activated') {
 }
 
 function createOnceMethodResponse(method, response) {
-  sendCommandStubResponses[method] = response;
+  assert.deepEqual(!!sendCommandMockResponses[method], false, 'stub response already defined');
+  sendCommandMockResponses[method] = response;
 }
 
 connection.sendCommand = function(command, params) {
   sendCommandParams.push({command, params});
 
-  if (sendCommandStubResponses[command]) {
-    return Promise.resolve(sendCommandStubResponses[command]);
+  const mockResponse = sendCommandMockResponses[command];
+  if (mockResponse) {
+    delete sendCommandMockResponses[command];
+    return Promise.resolve(mockResponse);
   }
 
   switch (command) {
@@ -118,6 +121,7 @@ connection.sendCommand = function(command, params) {
 describe('Browser Driver', () => {
   beforeEach(() => {
     sendCommandParams = [];
+    sendCommandMockResponses = {};
   });
 
   it('returns null when DOM.querySelector finds no node', () => {
@@ -263,12 +267,13 @@ describe('Browser Driver', () => {
   });
 
   it('will adjust traceCategories based on chrome version', () => {
+    // m70 doesn't have disabled-by-default-lighthouse, so 'toplevel' is used instead.
     createOnceMethodResponse('Browser.getVersion', {
       protocolVersion: '1.3',
-      product: 'Chrome/70.0.3577.0', // m70 doesn't have disabled-by-default-lighthouse
+      product: 'Chrome/70.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',
+      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',
       jsVersion: '7.1.314',
     });
 

From 6f501bc0299bfc39dd7bdb16a84f51cb878d6e22 Mon Sep 17 00:00:00 2001
From: Paul Irish <paulirish@google.com>
Date: Sat, 13 Oct 2018 15:49:53 -0700
Subject: [PATCH 7/8] DRY up browserVersion mock usage

---
 lighthouse-core/test/gather/driver-test.js    | 27 +++++++------------
 lighthouse-core/test/gather/fake-driver.js    | 16 +++++++++--
 .../test/gather/gather-runner-test.js         |  7 +++--
 3 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js
index ba121cf6700b..b8fc2534ca1b 100644
--- a/lighthouse-core/test/gather/driver-test.js
+++ b/lighthouse-core/test/gather/driver-test.js
@@ -13,6 +13,7 @@ const Connection = require('../../gather/connections/connection.js');
 const Element = require('../../lib/element.js');
 const assert = require('assert');
 const EventEmitter = require('events').EventEmitter;
+const {browserVersion} = require('./fake-driver');
 
 const connection = new Connection();
 const driverStub = new Driver(connection);
@@ -63,14 +64,7 @@ connection.sendCommand = function(command, params) {
 
   switch (command) {
     case 'Browser.getVersion':
-      return Promise.resolve({
-        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',
-      });
+      return Promise.resolve(browserVersion);
     case 'DOM.getDocument':
       return Promise.resolve({root: {nodeId: 249}});
     case 'DOM.querySelector':
@@ -247,7 +241,7 @@ describe('Browser Driver', () => {
   });
 
   it('will request default traceCategories', () => {
-    return driverStub.beginTrace({settings: {}}).then(() => {
+    return driverStub.beginTrace().then(() => {
       const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
       const categories = traceCmd.params.categories;
       assert.ok(categories.includes('devtools.timeline'), 'contains devtools.timeline');
@@ -255,12 +249,11 @@ describe('Browser Driver', () => {
   });
 
   it('will use requested additionalTraceCategories', () => {
-    const passContext = {settings: {additionalTraceCategories: 'v8,v8.execute,loading'}};
-    return driverStub.beginTrace(passContext).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.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');
     });
@@ -268,17 +261,15 @@ describe('Browser Driver', () => {
 
   it('will adjust traceCategories based on chrome version', () => {
     // m70 doesn't have disabled-by-default-lighthouse, so 'toplevel' is used instead.
-    createOnceMethodResponse('Browser.getVersion', {
-      protocolVersion: '1.3',
+    const m70BrowserVersion = Object.assign({}, browserVersion, {
       product: 'Chrome/70.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/70.0.3577.0 Safari/537.36',
-      jsVersion: '7.1.314',
     });
+    createOnceMethodResponse('Browser.getVersion', m70BrowserVersion);
 
     // eslint-disable-next-line max-len
-    return driverStub.beginTrace({settings: {}}).then(() => {
+    return driverStub.beginTrace().then(() => {
       const traceCmd = sendCommandParams.find(obj => obj.command === 'Tracing.start');
       const categories = traceCmd.params.categories;
       assert.ok(categories.includes('toplevel'), 'contains old toplevel category');
diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js
index 8e52b437dc3a..2954581758ca 100644
--- a/lighthouse-core/test/gather/fake-driver.js
+++ b/lighthouse-core/test/gather/fake-driver.js
@@ -5,9 +5,18 @@
  */
 'use strict';
 
-module.exports = {
+const browserVersion = {
+  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({userAgent: 'Fake user agent'});
+    return Promise.resolve(browserVersion);
   },
   getBenchmarkIndex() {
     return Promise.resolve(125.2);
@@ -72,3 +81,6 @@ module.exports = {
     return Promise.resolve();
   },
 };
+
+module.exports = fakeDriver;
+module.exports.browserVersion = browserVersion;
diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js
index 849e5439c7eb..5a3834fb6225 100644
--- a/lighthouse-core/test/gather/gather-runner-test.js
+++ b/lighthouse-core/test/gather/gather-runner-test.js
@@ -53,9 +53,6 @@ function getMockedEmulationDriver(emulationFn, netThrottleFn, cpuThrottleFn,
     }
     cleanBrowserCaches() {}
     clearDataForOrigin() {}
-    getBrowserVersion() {
-      return Promise.resolve({userAgent: 'Fake user agent'});
-    }
   };
   const EmulationMock = class extends Connection {
     sendCommand(command, params) {
@@ -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.browserVersion.userAgent);
+    expect(results.HostUserAgent).toMatch(/Chrome\/\d+/);
   });
 
   it('collects network user agent as an artifact', async () => {
@@ -272,6 +270,7 @@ describe('GatherRunner', function() {
     };
     const createEmulationCheck = variable => (...args) => {
       tests[variable] = args;
+
       return true;
     };
     const driver = getMockedEmulationDriver(

From c3e15ca59db5af5cd279b39fb301b3a80a2731ca Mon Sep 17 00:00:00 2001
From: Paul Irish <paulirish@google.com>
Date: Sun, 14 Oct 2018 13:31:55 -0700
Subject: [PATCH 8/8] Feedback

---
 lighthouse-core/gather/driver.js              |  4 ++--
 lighthouse-core/test/gather/driver-test.js    | 21 +++++++++----------
 lighthouse-core/test/gather/fake-driver.js    |  7 ++++---
 .../test/gather/gather-runner-test.js         |  2 +-
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
index dd7fd3494eee..f38b3850b523 100644
--- a/lighthouse-core/gather/driver.js
+++ b/lighthouse-core/gather/driver.js
@@ -106,7 +106,7 @@ class Driver {
    */
   async getBrowserVersion() {
     const version = await this.sendCommand('Browser.getVersion');
-    const match = version.product.match(/\/(\d+)/);
+    const match = version.product.match(/\/(\d+)/); // eg 'Chrome/71.0.3577.0'
     const milestone = match ? parseInt(match[1]) : 0;
     return Object.assign(version, {milestone});
   }
@@ -928,7 +928,7 @@ class Driver {
 
     // 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; // eg 'Chrome/71.0.3577.0'
+    const milestone = (await this.getBrowserVersion()).milestone;
     if (milestone < 71) {
       const toplevelIndex = traceCategories.indexOf('disabled-by-default-lighthouse');
       traceCategories.splice(toplevelIndex, 1, 'toplevel');
diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js
index b8fc2534ca1b..a8af8272f31c 100644
--- a/lighthouse-core/test/gather/driver-test.js
+++ b/lighthouse-core/test/gather/driver-test.js
@@ -13,7 +13,7 @@ const Connection = require('../../gather/connections/connection.js');
 const Element = require('../../lib/element.js');
 const assert = require('assert');
 const EventEmitter = require('events').EventEmitter;
-const {browserVersion} = require('./fake-driver');
+const {protocolGetVersionResponse} = require('./fake-driver');
 
 const connection = new Connection();
 const driverStub = new Driver(connection);
@@ -64,7 +64,7 @@ connection.sendCommand = function(command, params) {
 
   switch (command) {
     case 'Browser.getVersion':
-      return Promise.resolve(browserVersion);
+      return Promise.resolve(protocolGetVersionResponse);
     case 'DOM.getDocument':
       return Promise.resolve({root: {nodeId: 249}});
     case 'DOM.querySelector':
@@ -259,22 +259,21 @@ describe('Browser Driver', () => {
     });
   });
 
-  it('will adjust traceCategories based on chrome version', () => {
+  it('will adjust traceCategories based on chrome version', async () => {
     // m70 doesn't have disabled-by-default-lighthouse, so 'toplevel' is used instead.
-    const m70BrowserVersion = Object.assign({}, browserVersion, {
+    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', m70BrowserVersion);
+    createOnceMethodResponse('Browser.getVersion', m70ProtocolGetVersionResponse);
 
     // eslint-disable-next-line max-len
-    return driverStub.beginTrace().then(() => {
-      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');
-    });
+    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', () => {
diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js
index 2954581758ca..f90ed3b0f032 100644
--- a/lighthouse-core/test/gather/fake-driver.js
+++ b/lighthouse-core/test/gather/fake-driver.js
@@ -5,7 +5,8 @@
  */
 'use strict';
 
-const browserVersion = {
+// https://chromedevtools.github.io/devtools-protocol/tot/Browser#method-getVersion
+const protocolGetVersionResponse = {
   protocolVersion: '1.3',
   product: 'Chrome/71.0.3577.0',
   revision: '@fc334a55a70eec12fc77853c53979f81e8496c21',
@@ -16,7 +17,7 @@ const browserVersion = {
 
 const fakeDriver = {
   getBrowserVersion() {
-    return Promise.resolve(browserVersion);
+    return Promise.resolve(Object.assign({}, protocolGetVersionResponse, {milestone: 71}));
   },
   getBenchmarkIndex() {
     return Promise.resolve(125.2);
@@ -83,4 +84,4 @@ const fakeDriver = {
 };
 
 module.exports = fakeDriver;
-module.exports.browserVersion = browserVersion;
+module.exports.protocolGetVersionResponse = protocolGetVersionResponse;
diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js
index 5a3834fb6225..8ab6da53e655 100644
--- a/lighthouse-core/test/gather/gather-runner-test.js
+++ b/lighthouse-core/test/gather/gather-runner-test.js
@@ -123,7 +123,7 @@ describe('GatherRunner', function() {
     const options = {url, driver, config, settings};
 
     const results = await GatherRunner.run([], options);
-    expect(results.HostUserAgent).toEqual(fakeDriver.browserVersion.userAgent);
+    expect(results.HostUserAgent).toEqual(fakeDriver.protocolGetVersionResponse.userAgent);
     expect(results.HostUserAgent).toMatch(/Chrome\/\d+/);
   });