Repository: GoogleChrome/lighthouse
Pull request #8964 (2019-05-15 18:13:52)

core: refactor gather-runner
This is a refactor of `gather-runner.js` to
1) more clearly say what it's doing. The big outline at the top shouldn't be necessary; you should be able to just read `run()` (and now `runPass()`) to get an overview. `GatherRunner.beforePass`, `pass`, and `afterPass` now *only* call those methods on gatherers and it's a lot easier to see where things like recording a trace starts and stops.
2) make it easier to handle error cases. The code should be able to express what we're trying to do: at certain points (as the page is loading, after the page is done, between passes, etc) we need to check for errors, and then depending on the type of error take one of several actions (continue (allowing what gatherers can run to run), return error artifacts, don't run any more passes, cancel the entire LH run, etc). I've tried to add natural spots for these checks and more explicit error handling rather than trying to cram more things into the array-of-resolved-or-rejected-beforepass-pass-afterpass-promises model :) I believe #8340 and #8866 (and future error handling beyond those) should integrate well with these changes.

`Driver` commands and input/output should be *exactly* the same except:
* we go back to loading `about:blank` twice on the first pass
* some `log.time` groups have different `Driver` functions in them so the timing might change slightly
The first is not a big deal since after #6446 the second `about:blank` load is like 10ms, and the `timing` changes seem unavoidable since it's an explicit externalizing of internal implementation details.

This is a big PR touching a lot of stuff, so if reviewers prefer this PR can just show the motivating goal and be closed and I can split into a couple of PRs.


Patch:
diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
index 38fe38320..f238a1227 100644
--- a/lighthouse-core/gather/driver.js
+++ b/lighthouse-core/gather/driver.js
@@ -1476,15 +1476,20 @@ class Driver {
   }
 
   /**
-   * Emulate internet disconnection.
+   * Clear the network cache on disk and in memory.
    * @return {Promise<void>}
    */
-  cleanBrowserCaches() {
+  async cleanBrowserCaches() {
+    const status = {msg: 'Cleaning browser cache', id: 'lh:driver:cleanBrowserCaches'};
+    log.time(status);
+
     // Wipe entire disk cache
-    return this.sendCommand('Network.clearBrowserCache')
-      // Toggle 'Disable Cache' to evict the memory cache
-      .then(_ => this.sendCommand('Network.setCacheDisabled', {cacheDisabled: true}))
-      .then(_ => this.sendCommand('Network.setCacheDisabled', {cacheDisabled: false}));
+    await this.sendCommand('Network.clearBrowserCache');
+    // Toggle 'Disable Cache' to evict the memory cache
+    await this.sendCommand('Network.setCacheDisabled', {cacheDisabled: true});
+    await this.sendCommand('Network.setCacheDisabled', {cacheDisabled: false});
+
+    log.timeEnd(status);
   }
 
   /**
diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js
index f7efa385f..9c637cfcb 100644
--- a/lighthouse-core/gather/gather-runner.js
+++ b/lighthouse-core/gather/gather-runner.js
@@ -13,7 +13,7 @@ const URL = require('../lib/url-shim.js');
 const NetworkRecorder = require('../lib/network-recorder.js');
 const constants = require('../config/constants.js');
 
-const Driver = require('../gather/driver.js'); // eslint-disable-line no-unused-vars
+/** @typedef {import('../gather/driver.js')} Driver */
 
 /** @typedef {import('./gatherers/gatherer.js').PhaseResult} PhaseResult */
 /**
@@ -23,44 +23,9 @@ const Driver = require('../gather/driver.js'); // eslint-disable-line no-unused-
  * @typedef {Record<keyof LH.GathererArtifacts, Array<PhaseResult|Promise<PhaseResult>>>} GathererResults
  */
 /** @typedef {Array<[keyof GathererResults, GathererResults[keyof GathererResults]]>} GathererResultsEntries */
+
 /**
  * Class that drives browser to load the page and runs gatherer lifecycle hooks.
- * Execution sequence when GatherRunner.run() is called:
- *
- * 1. Setup
- *   A. driver.connect()
- *   B. GatherRunner.setupDriver()
- *     i. assertNoSameOriginServiceWorkerClients
- *     ii. retrieve and save userAgent
- *     iii. beginEmulation
- *     iv. enableRuntimeEvents/enableAsyncStacks
- *     v. evaluateScriptOnLoad rescue native Promise from potential polyfill
- *     vi. register a performance observer
- *     vii. register dialog dismisser
- *     viii. clearDataForOrigin
- *
- * 2. For each pass in the config:
- *   A. GatherRunner.beforePass()
- *     i. navigate to about:blank
- *     ii. Enable network request blocking for specified patterns
- *     iii. all gatherers' beforePass()
- *   B. GatherRunner.pass()
- *     i. cleanBrowserCaches() (if it's a perf run)
- *     ii. beginDevtoolsLog()
- *     iii. beginTrace (if requested)
- *     iv. GatherRunner.loadPage()
- *       a. navigate to options.url (and wait for onload)
- *     v. all gatherers' pass()
- *   C. GatherRunner.afterPass()
- *     i. endTrace (if requested) & endDevtoolsLog & endThrottling
- *     ii. all gatherers' afterPass()
- *
- * 3. Teardown
- *   A. clearDataForOrigin
- *   B. GatherRunner.disposeDriver()
- *   C. collect all artifacts and return them
- *     i. collectArtifacts() from completed passes on each gatherer
- *     ii. add trace and devtoolsLog data
  */
 class GatherRunner {
   /**
@@ -89,12 +54,22 @@ class GatherRunner {
    * @return {Promise<void>}
    */
   static async loadPage(driver, passContext) {
+    const gatherers = passContext.passConfig.gatherers;
+    const status = {
+      msg: 'Loading page & waiting for onload',
+      id: `lh:gather:loadPage-${passContext.passConfig.passName}`,
+      args: [gatherers.map(g => g.instance.name).join(', ')],
+    };
+    log.time(status);
+
     const finalUrl = await driver.gotoURL(passContext.url, {
       waitForFCP: passContext.passConfig.recordTrace,
       waitForLoad: true,
       passContext,
     });
     passContext.url = finalUrl;
+
+    log.timeEnd(status);
   }
 
   /**
@@ -118,14 +93,20 @@ class GatherRunner {
   }
 
   /**
+   * Reset browser state where needed and release the connection.
    * @param {Driver} driver
+   * @param {{requestedUrl: string, settings: LH.Config.Settings}} options
    * @return {Promise<void>}
    */
-  static async disposeDriver(driver) {
+  static async disposeDriver(driver, options) {
     const status = {msg: 'Disconnecting from browser...', id: 'lh:gather:disconnect'};
 
     log.time(status);
     try {
+      // If storage was cleared for the run, clear at the end so Lighthouse specifics aren't cached.
+      const resetStorage = !options.settings.disableStorageReset;
+      if (resetStorage) await driver.clearDataForOrigin(options.requestedUrl);
+
       await driver.disconnect();
     } catch (err) {
       // Ignore disconnecting error if browser was already closed.
@@ -143,7 +124,7 @@ class GatherRunner {
    * @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
    * @return {LHError|undefined}
    */
-  static getPageLoadError(url, networkRecords) {
+  static getNetworkError(url, networkRecords) {
     const mainRecord = networkRecords.find(record => {
       // record.url is actual request url, so needs to be compared without any URL fragment.
       return URL.equalWithExcludedFragments(record.url, url);
@@ -176,15 +157,33 @@ class GatherRunner {
   }
 
   /**
-   * Calls beforePass() on gatherers before tracing
-   * has started and before navigation to the target page.
+   * Returns an error if the page load should be considered failed, e.g. from a
+   * main document request failure, a security issue, etc.
+   * @param {LH.Gatherer.PassContext} passContext
+   * @param {LH.Gatherer.LoadData} loadData
+   */
+  static getPageLoadError(passContext, loadData) {
+    const networkError = GatherRunner.getNetworkError(passContext.url, loadData.networkRecords);
+
+    //  If the driver was offline, the load will fail without offline support. Ignore this case.
+    if (!passContext.driver.online) return;
+
+    return networkError;
+  }
+
+  /**
+   * Initialize network settings for the pass, e.g. throttling, blocked URLs,
+   * and manual request headers.
    * @param {LH.Gatherer.PassContext} passContext
-   * @param {Partial<GathererResults>} gathererResults
    * @return {Promise<void>}
    */
-  static async beforePass(passContext, gathererResults) {
-    const bpStatus = {msg: `Running beforePass methods`, id: `lh:gather:beforePass`};
-    log.time(bpStatus, 'verbose');
+  static async setupPassNetwork(passContext) {
+    const status = {msg: 'Setting up network for the pass trace', id: `lh:gather:setupPassNetwork`};
+    log.time(status);
+
+    const passConfig = passContext.passConfig;
+    await passContext.driver.setThrottling(passContext.settings, passConfig);
+
     const blockedUrls = (passContext.passConfig.blockedUrlPatterns || [])
       .concat(passContext.settings.blockedUrlPatterns || []);
 
@@ -194,6 +193,73 @@ class GatherRunner {
     await passContext.driver.blockUrlPatterns(blockedUrls);
     await passContext.driver.setExtraHTTPHeaders(passContext.settings.extraHeaders);
 
+    log.timeEnd(status);
+  }
+
+  /**
+   * Beging recording devtoolsLog and trace (if requested).
+   * @param {LH.Gatherer.PassContext} passContext
+   * @return {Promise<void>}
+   */
+  static async beginRecording(passContext) {
+    const status = {msg: 'Beginning devtoolsLog and trace', id: 'lh:gather:beginRecording'};
+    log.time(status);
+
+    const {driver, passConfig, settings} = passContext;
+
+    // Always record devtoolsLog
+    await driver.beginDevtoolsLog();
+
+    if (passConfig.recordTrace) {
+      await driver.beginTrace(settings);
+    }
+
+    log.timeEnd(status);
+  }
+
+  /**
+   * End recording devtoolsLog and trace (if requested), returning an
+   * `LH.Gatherer.LoadData` with the recorded data.
+   * @param {LH.Gatherer.PassContext} passContext
+   * @return {Promise<LH.Gatherer.LoadData>}
+   */
+  static async endRecording(passContext) {
+    const {driver, passConfig} = passContext;
+
+    let trace;
+    if (passConfig.recordTrace) {
+      const status = {msg: 'Retrieving trace', id: `lh:gather:getTrace`};
+      log.time(status);
+      trace = await driver.endTrace();
+      log.timeEnd(status);
+    }
+
+    const status = {
+      msg: 'Retrieving devtoolsLog & network records',
+      id: `lh:gather:getDevtoolsLog`,
+    };
+    log.time(status);
+    const devtoolsLog = driver.endDevtoolsLog();
+    const networkRecords = NetworkRecorder.recordsFromLogs(devtoolsLog);
+    log.timeEnd(status);
+
+    return {
+      networkRecords,
+      devtoolsLog,
+      trace,
+    };
+  }
+
+  /**
+   * Run beforePass() on gatherers.
+   * @param {LH.Gatherer.PassContext} passContext
+   * @param {Partial<GathererResults>} gathererResults
+   * @return {Promise<void>}
+   */
+  static async beforePass(passContext, gathererResults) {
+    const bpStatus = {msg: `Running beforePass methods`, id: `lh:gather:beforePass`};
+    log.time(bpStatus, 'verbose');
+
     for (const gathererDefn of passContext.passConfig.gatherers) {
       const gatherer = gathererDefn.instance;
       // Abuse the passContext to pass through gatherer options
@@ -212,41 +278,18 @@ class GatherRunner {
   }
 
   /**
-   * Navigates to requested URL and then runs pass() on gatherers while trace
-   * (if requested) is still being recorded.
+   * Run pass() on gatherers.
    * @param {LH.Gatherer.PassContext} passContext
    * @param {Partial<GathererResults>} gathererResults
    * @return {Promise<void>}
    */
   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 status = {
-      msg: 'Loading page & waiting for onload',
-      id: `lh:gather:loadPage-${passContext.passConfig.passName}`,
-      args: [gatherers.map(g => g.instance.name).join(', ')],
-    };
-    log.time(status);
-
-    // Clear disk & memory cache if it's a perf run
-    if (isPerfRun) await driver.cleanBrowserCaches();
-    // Always record devtoolsLog
-    await driver.beginDevtoolsLog();
-    // Begin tracing if requested by config.
-    if (recordTrace) await driver.beginTrace(settings);
-
-    // Navigate.
-    await GatherRunner.loadPage(driver, passContext);
-    log.timeEnd(status);
-
     const pStatus = {msg: `Running pass methods`, id: `lh:gather:pass`};
     log.time(pStatus, 'verbose');
+
     for (const gathererDefn of gatherers) {
       const gatherer = gathererDefn.instance;
       // Abuse the passContext to pass through gatherer options
@@ -263,60 +306,22 @@ class GatherRunner {
       gathererResults[gatherer.name] = gathererResult;
       await artifactPromise.catch(() => {});
     }
-    log.timeEnd(status);
+
     log.timeEnd(pStatus);
   }
 
   /**
-   * Ends tracing and collects trace data (if requested for this pass), and runs
-   * afterPass() on gatherers with trace data passed in. Promise resolves with
-   * object containing trace and network data.
+   * Run afterPass() on gatherers.
    * @param {LH.Gatherer.PassContext} passContext
+   * @param {LH.Gatherer.LoadData} loadData
    * @param {Partial<GathererResults>} gathererResults
-   * @return {Promise<LH.Gatherer.LoadData>}
+   * @return {Promise<void>}
    */
-  static async afterPass(passContext, gathererResults) {
-    const driver = passContext.driver;
+  static async afterPass(passContext, loadData, gathererResults) {
     const config = passContext.passConfig;
     const gatherers = config.gatherers;
 
-    let trace;
-    if (config.recordTrace) {
-      const status = {msg: 'Retrieving trace', id: `lh:gather:getTrace`};
-      log.time(status);
-      trace = await driver.endTrace();
-      log.timeEnd(status);
-    }
-
-    const status = {
-      msg: 'Retrieving devtoolsLog & network records',
-      id: `lh:gather:getDevtoolsLog`,
-    };
-    log.time(status);
-    const devtoolsLog = driver.endDevtoolsLog();
-    const networkRecords = NetworkRecorder.recordsFromLogs(devtoolsLog);
-    log.timeEnd(status);
-
-    let pageLoadError = GatherRunner.getPageLoadError(passContext.url, networkRecords);
-    // If the driver was offline, a page load error is expected, so do not save it.
-    if (!driver.online) pageLoadError = undefined;
-
-    if (pageLoadError) {
-      log.error('GatherRunner', pageLoadError.message, passContext.url);
-      passContext.LighthouseRunWarnings.push(pageLoadError.friendlyMessage);
-    }
-
-    // Expose devtoolsLog, networkRecords, and trace (if present) to gatherers
-    /** @type {LH.Gatherer.LoadData} */
-    const passData = {
-      networkRecords,
-      devtoolsLog,
-      trace,
-    };
-
     const apStatus = {msg: `Running afterPass methods`, id: `lh:gather:afterPass`};
-    // Disable throttling so the afterPass analysis isn't throttled
-    await driver.setThrottling(passContext.settings, {useThrottling: false});
     log.time(apStatus, 'verbose');
 
     for (const gathererDefn of gatherers) {
@@ -329,12 +334,8 @@ class GatherRunner {
 
       // Add gatherer options to the passContext.
       passContext.options = gathererDefn.options || {};
-
-      // If there was a pageLoadError, fail every afterPass with it rather than bail completely.
-      const artifactPromise = pageLoadError ?
-        Promise.reject(pageLoadError) :
-        // Wrap gatherer response in promise, whether rejected or not.
-        Promise.resolve().then(_ => gatherer.afterPass(passContext, passData));
+      const artifactPromise = Promise.resolve()
+        .then(_ => gatherer.afterPass(passContext, loadData));
 
       const gathererResult = gathererResults[gatherer.name] || [];
       gathererResult.push(artifactPromise);
@@ -343,9 +344,28 @@ class GatherRunner {
       log.timeEnd(status);
     }
     log.timeEnd(apStatus);
+  }
 
-    // Resolve on tracing data using passName from config.
-    return passData;
+  /**
+   * Generate a set of artfiacts for the given pass as if all the gatherers
+   * failed with the given pageLoadError.
+   * @param {LH.Gatherer.PassContext} passContext
+   * @param {LHError} pageLoadError
+   * @return {{pageLoadError: LHError, artifacts: Partial<LH.GathererArtifacts>}}
+   */
+  static generatePageLoadErrorArtifacts(passContext, pageLoadError) {
+    /** @type {Partial<Record<keyof LH.GathererArtifacts, LHError>>} */
+    const errorArtifacts = {};
+    for (const gathererDefn of passContext.passConfig.gatherers) {
+      const gatherer = gathererDefn.instance;
+      errorArtifacts[gatherer.name] = pageLoadError;
+    }
+
+    return {
+      pageLoadError,
+      // @ts-ignore - TODO(bckenny): figure out how to usefully type errored artifacts.
+      artifacts: errorArtifacts,
+    };
   }
 
   /**
@@ -354,17 +374,14 @@ class GatherRunner {
    * gatherer. If an error was rejected from a gatherer phase,
    * uses that error object as the artifact instead.
    * @param {Partial<GathererResults>} gathererResults
-   * @param {LH.BaseArtifacts} baseArtifacts
-   * @return {Promise<LH.Artifacts>}
+   * @return {Promise<{artifacts: Partial<LH.GathererArtifacts>}>}
    */
-  static async collectArtifacts(gathererResults, baseArtifacts) {
+  static async collectArtifacts(gathererResults) {
     /** @type {Partial<LH.GathererArtifacts>} */
     const gathererArtifacts = {};
 
     const resultsEntries = /** @type {GathererResultsEntries} */ (Object.entries(gathererResults));
     for (const [gathererName, phaseResultsPromises] of resultsEntries) {
-      if (gathererArtifacts[gathererName] !== undefined) continue;
-
       try {
         const phaseResults = await Promise.all(phaseResultsPromises);
         // Take last defined pass result as artifact.
@@ -382,21 +399,18 @@ class GatherRunner {
       }
     }
 
-    // Take only unique LighthouseRunWarnings.
-    baseArtifacts.LighthouseRunWarnings = Array.from(new Set(baseArtifacts.LighthouseRunWarnings));
-
-    // Take the timing entries we've gathered so far.
-    baseArtifacts.Timing = log.getTimeEntries();
-
-    // TODO(bckenny): correct Partial<LH.GathererArtifacts> at this point to drop cast.
-    return /** @type {LH.Artifacts} */ ({...baseArtifacts, ...gathererArtifacts});
+    return {
+      artifacts: gathererArtifacts,
+    };
   }
 
   /**
+   * Return an initialized but mostly empty set of base artifacts, to be
+   * populated as the run continues.
    * @param {{driver: Driver, requestedUrl: string, settings: LH.Config.Settings}} options
    * @return {Promise<LH.BaseArtifacts>}
    */
-  static async getBaseArtifacts(options) {
+  static async initializeBaseArtifacts(options) {
     const hostUserAgent = (await options.driver.getBrowserVersion()).userAgent;
 
     const {emulatedFormFactor} = options.settings;
@@ -422,6 +436,47 @@ class GatherRunner {
     };
   }
 
+  /**
+   * Populates the important base artifacts from a fully loaded test page.
+   * Currently must be run before `start-url` gatherer so that `WebAppManifest`
+   * will be available to it.
+   * @param {LH.Gatherer.PassContext} passContext
+   */
+  static async populateBaseArtifacts(passContext) {
+    const baseArtifacts = passContext.baseArtifacts;
+
+    // Copy redirected URL to artifact.
+    baseArtifacts.URL.finalUrl = passContext.url;
+
+    // Fetch the manifest, if it exists.
+    baseArtifacts.WebAppManifest = await GatherRunner.getWebAppManifest(passContext);
+
+    baseArtifacts.Stacks = await stacksGatherer(passContext);
+
+    // Find the NetworkUserAgent actually used in the devtoolsLogs.
+    const devtoolsLog = baseArtifacts.devtoolsLogs[passContext.passConfig.passName];
+    const userAgentEntry = devtoolsLog.find(entry =>
+      entry.method === 'Network.requestWillBeSent' &&
+      !!entry.params.request.headers['User-Agent']
+    );
+    if (userAgentEntry) {
+      // @ts-ignore - guaranteed to exist by the find above
+      baseArtifacts.NetworkUserAgent = userAgentEntry.params.request.headers['User-Agent'];
+    }
+  }
+
+  /**
+   * Finalize baseArtifacts after gathering is fully complete.
+   * @param {LH.BaseArtifacts} baseArtifacts
+   */
+  static finalizeBaseArtifacts(baseArtifacts) {
+    // Take only unique LighthouseRunWarnings.
+    baseArtifacts.LighthouseRunWarnings = Array.from(new Set(baseArtifacts.LighthouseRunWarnings));
+
+    // Take the timing entries we've gathered so far.
+    baseArtifacts.Timing = log.getTimeEntries();
+  }
+
   /**
    * Uses the debugger protocol to fetch the manifest from within the context of
    * the target page, reusing any credentials, emulation, etc, already established
@@ -441,91 +496,109 @@ class GatherRunner {
   }
 
   /**
-   * @param {Array<LH.Config.Pass>} passes
+   * @param {Array<LH.Config.Pass>} passConfigs
    * @param {{driver: Driver, requestedUrl: string, settings: LH.Config.Settings}} options
    * @return {Promise<LH.Artifacts>}
    */
-  static async run(passes, options) {
+  static async run(passConfigs, options) {
     const driver = options.driver;
 
-    /** @type {Partial<GathererResults>} */
-    const gathererResults = {};
+    /** @type {Partial<LH.GathererArtifacts>} */
+    const artifacts = {};
 
     try {
       await driver.connect();
-      const baseArtifacts = await GatherRunner.getBaseArtifacts(options);
       // In the devtools/extension case, we can't still be on the site while trying to clear state
       // So we first navigate to about:blank, then apply our emulation & setup
       await GatherRunner.loadBlank(driver);
+
+      const baseArtifacts = await GatherRunner.initializeBaseArtifacts(options);
       baseArtifacts.BenchmarkIndex = await options.driver.getBenchmarkIndex();
+
       await GatherRunner.setupDriver(driver, options);
 
-      // Run each pass
       let isFirstPass = true;
-      for (const passConfig of passes) {
+      for (const passConfig of passConfigs) {
+        /** @type {LH.Gatherer.PassContext} */
         const passContext = {
-          driver: options.driver,
-          // If the main document redirects, we'll update this to keep track
+          driver,
           url: options.requestedUrl,
           settings: options.settings,
           passConfig,
           baseArtifacts,
-          // *pass() functions and gatherers can push to this warnings array.
           LighthouseRunWarnings: baseArtifacts.LighthouseRunWarnings,
         };
-
-        await driver.setThrottling(options.settings, passConfig);
-        if (!isFirstPass) {
-          // Already on blank page if driver was just set up.
-          await GatherRunner.loadBlank(driver, passConfig.blankPage);
-        }
-        await GatherRunner.beforePass(passContext, gathererResults);
-        await GatherRunner.pass(passContext, gathererResults);
+        const passResults = await GatherRunner.runPass(passContext);
+        Object.assign(artifacts, passResults.artifacts);
 
         if (isFirstPass) {
-          // Fetch the manifest, if it exists. Currently must be fetched before gatherers' `afterPass`.
-          baseArtifacts.WebAppManifest = await GatherRunner.getWebAppManifest(passContext);
+          await GatherRunner.populateBaseArtifacts(passContext);
+          isFirstPass = false;
         }
+      }
 
-        const passData = await GatherRunner.afterPass(passContext, gathererResults);
+      await GatherRunner.disposeDriver(driver, options);
+      GatherRunner.finalizeBaseArtifacts(baseArtifacts);
+      return /** @type {LH.Artifacts} */ ({...baseArtifacts, ...artifacts}); // Cast to drop Partial<>.
+    } catch (err) {
+      // cleanup on error
+      GatherRunner.disposeDriver(driver, options);
+      throw err;
+    }
+  }
 
-        if (isFirstPass) {
-          baseArtifacts.Stacks = await stacksGatherer(passContext);
-        }
+  /**
+   * Returns whether this pass should be considered to be measuring performance.
+   * @param {LH.Gatherer.PassContext} passContext
+   * @return {boolean}
+   */
+  static isPerfPass(passContext) {
+    const {settings, passConfig} = passContext;
+    return !settings.disableStorageReset && passConfig.recordTrace && passConfig.useThrottling;
+  }
 
-        // Save devtoolsLog, but networkRecords are discarded and not added onto artifacts.
-        baseArtifacts.devtoolsLogs[passConfig.passName] = passData.devtoolsLog;
+  /**
+   * Starting from about:blank, load the page and run gatherers for this pass.
+   * @param {LH.Gatherer.PassContext} passContext
+   * @return {Promise<{artifacts: Partial<LH.GathererArtifacts>, pageLoadError?: LHError}>}
+   */
+  static async runPass(passContext) {
+    /** @type {Partial<GathererResults>} */
+    const gathererResults = {};
+    const {driver, passConfig} = passContext;
 
-        const userAgentEntry = passData.devtoolsLog.find(entry =>
-          entry.method === 'Network.requestWillBeSent' &&
-          !!entry.params.request.headers['User-Agent']
-        );
+    // Go to about:blank, set up, and run `beforePass()` on gatherers.
+    await GatherRunner.loadBlank(driver, passConfig.blankPage);
+    await GatherRunner.setupPassNetwork(passContext);
+    const isPerfPass = GatherRunner.isPerfPass(passContext);
+    if (isPerfPass) await driver.cleanBrowserCaches(); // Clear disk & memory cache if it's a perf run
+    await GatherRunner.beforePass(passContext, gathererResults);
 
-        if (userAgentEntry && !baseArtifacts.NetworkUserAgent) {
-          // @ts-ignore - guaranteed to exist by the find above
-          baseArtifacts.NetworkUserAgent = userAgentEntry.params.request.headers['User-Agent'];
-        }
+    // Navigate, start recording, and run `pass()` on gatherers.
+    await GatherRunner.beginRecording(passContext);
+    await GatherRunner.loadPage(driver, passContext);
+    await GatherRunner.pass(passContext, gathererResults);
+    const loadData = await GatherRunner.endRecording(passContext);
 
-        // If requested by config, save pass's trace.
-        if (passData.trace) {
-          baseArtifacts.traces[passConfig.passName] = passData.trace;
-        }
+    // Disable throttling so the afterPass analysis isn't throttled
+    await driver.setThrottling(passContext.settings, {useThrottling: false});
 
-        if (isFirstPass) {
-          // Copy redirected URL to artifact in the first pass only.
-          baseArtifacts.URL.finalUrl = passContext.url;
-          isFirstPass = false;
-        }
-      }
-      const resetStorage = !options.settings.disableStorageReset;
-      if (resetStorage) await driver.clearDataForOrigin(options.requestedUrl);
-      await GatherRunner.disposeDriver(driver);
-      return GatherRunner.collectArtifacts(gathererResults, baseArtifacts);
-    } catch (err) {
-      // cleanup on error
-      GatherRunner.disposeDriver(driver);
-      throw err;
+    // Save devtoolsLog and trace.
+    const baseArtifacts = passContext.baseArtifacts;
+    baseArtifacts.devtoolsLogs[passConfig.passName] = loadData.devtoolsLog;
+    if (loadData.trace) baseArtifacts.traces[passConfig.passName] = loadData.trace;
+
+    // If there were any load errors, treat all gatherers as if they errored.
+    const pageLoadError = GatherRunner.getPageLoadError(passContext, loadData);
+    if (pageLoadError) {
+      log.error('GatherRunner', pageLoadError.friendlyMessage, passContext.url);
+      passContext.LighthouseRunWarnings.push(pageLoadError.friendlyMessage);
+      return GatherRunner.generatePageLoadErrorArtifacts(passContext, pageLoadError);
     }
+
+    // If no error, run `afterPass()` on gatherers and return collected artifacts.
+    await GatherRunner.afterPass(passContext, loadData, gathererResults);
+    return GatherRunner.collectArtifacts(gathererResults);
   }
 }
 
diff --git a/types/gatherer.d.ts b/types/gatherer.d.ts
index 78e6c711c..21237d270 100644
--- a/types/gatherer.d.ts
+++ b/types/gatherer.d.ts
@@ -12,13 +12,14 @@ import Driver = require('../lighthouse-core/gather/driver');
 declare global {
   module LH.Gatherer {
     export interface PassContext {
+      /** The url of the currently loaded page. If the main document redirects, this will be updated to keep track. */
       url: string;
       driver: Driver;
       disableJavaScript?: boolean;
       passConfig: Config.Pass
       settings: Config.Settings;
       options?: object;
-      /** Push to this array to add top-level warnings to the LHR. */
+      /** Gatherers can push to this array to add top-level warnings to the LHR. */
       LighthouseRunWarnings: Array<string>;
       baseArtifacts: BaseArtifacts;
     }


Test patch:
diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js
index 36ab4f962..90b09daa4 100644
--- a/lighthouse-core/test/gather/gather-runner-test.js
+++ b/lighthouse-core/test/gather/gather-runner-test.js
@@ -102,7 +102,9 @@ describe('GatherRunner', function() {
     const passContext = {
       requestedUrl: url1,
       settings: {},
-      passConfig: {},
+      passConfig: {
+        gatherers: [],
+      },
     };
 
     return GatherRunner.loadPage(driver, passContext).then(_ => {
@@ -352,7 +354,7 @@ describe('GatherRunner', function() {
     });
   });
 
-  it('clears the disk & memory cache on a perf run', () => {
+  it('clears the disk & memory cache on a perf run', async () => {
     const asyncFunc = () => Promise.resolve();
     const tests = {
       calledCleanBrowserCaches: false,
@@ -366,8 +368,15 @@ describe('GatherRunner', function() {
       beginTrace: asyncFunc,
       gotoURL: asyncFunc,
       cleanBrowserCaches: createCheck('calledCleanBrowserCaches'),
+      setThrottling: asyncFunc,
+      blockUrlPatterns: asyncFunc,
+      setExtraHTTPHeaders: asyncFunc,
+      endTrace: asyncFunc,
+      endDevtoolsLog: () => [],
+      getBrowserVersion: async () => ({userAgent: ''}),
     };
     const passConfig = {
+      passName: 'default',
       recordTrace: true,
       useThrottling: true,
       gatherers: [],
@@ -375,9 +384,16 @@ describe('GatherRunner', function() {
     const settings = {
       disableStorageReset: false,
     };
-    return GatherRunner.pass({driver, passConfig, settings}, {TestGatherer: []}).then(_ => {
-      assert.equal(tests.calledCleanBrowserCaches, true);
-    });
+    const requestedUrl = 'https://example.com';
+    const passContext = {
+      driver,
+      passConfig,
+      settings,
+      baseArtifacts: await GatherRunner.initializeBaseArtifacts({driver, settings, requestedUrl}),
+    };
+
+    await GatherRunner.runPass(passContext, {TestGatherer: []});
+    assert.equal(tests.calledCleanBrowserCaches, true);
   });
 
   it('does not clear origin storage with flag --disable-storage-reset', () => {
@@ -420,7 +436,7 @@ describe('GatherRunner', function() {
       receivedUrlPatterns = params.urls;
     });
 
-    return GatherRunner.beforePass({
+    return GatherRunner.setupPassNetwork({
       driver,
       settings: {
         blockedUrlPatterns: ['http://*.evil.com', '.jpg', '.woff2'],
@@ -441,7 +457,7 @@ describe('GatherRunner', function() {
       receivedUrlPatterns = params.urls;
     });
 
-    return GatherRunner.beforePass({
+    return GatherRunner.setupPassNetwork({
       driver,
       settings: {},
       passConfig: {gatherers: []},
@@ -459,7 +475,7 @@ describe('GatherRunner', function() {
       'x-men': 'wolverine',
     };
 
-    return GatherRunner.beforePass({
+    return GatherRunner.setupPassNetwork({
       driver,
       settings: {
         extraHeaders: headers,
@@ -471,7 +487,7 @@ describe('GatherRunner', function() {
       ));
   });
 
-  it('tells the driver to begin tracing', () => {
+  it('tells the driver to begin tracing', async () => {
     let calledTrace = false;
     const driver = {
       beginTrace() {
@@ -494,9 +510,8 @@ describe('GatherRunner', function() {
     };
     const settings = {};
 
-    return GatherRunner.pass({driver, passConfig, settings}, {TestGatherer: []}).then(_ => {
-      assert.equal(calledTrace, true);
-    });
+    await GatherRunner.beginRecording({driver, passConfig, settings}, {TestGatherer: []});
+    assert.equal(calledTrace, true);
   });
 
   it('tells the driver to end tracing', () => {
@@ -518,13 +533,13 @@ describe('GatherRunner', function() {
       ],
     };
 
-    return GatherRunner.afterPass({url, driver, passConfig}, {TestGatherer: []}).then(passData => {
+    return GatherRunner.endRecording({url, driver, passConfig}).then(passData => {
       assert.equal(calledTrace, true);
       assert.equal(passData.trace, fakeTraceData);
     });
   });
 
-  it('tells the driver to begin devtoolsLog collection', () => {
+  it('tells the driver to begin devtoolsLog collection', async () => {
     let calledDevtoolsLogCollect = false;
     const driver = {
       beginDevtoolsLog() {
@@ -543,9 +558,8 @@ describe('GatherRunner', function() {
     };
     const settings = {};
 
-    return GatherRunner.pass({driver, passConfig, settings}, {TestGatherer: []}).then(_ => {
-      assert.equal(calledDevtoolsLogCollect, true);
-    });
+    await GatherRunner.beginRecording({driver, passConfig, settings}, {TestGatherer: []});
+    assert.equal(calledDevtoolsLogCollect, true);
   });
 
   it('tells the driver to end devtoolsLog collection', () => {
@@ -568,7 +582,7 @@ describe('GatherRunner', function() {
       ],
     };
 
-    return GatherRunner.afterPass({url, driver, passConfig}, {TestGatherer: []}).then(passData => {
+    return GatherRunner.endRecording({url, driver, passConfig}).then(passData => {
       assert.equal(calledDevtoolsLogCollect, true);
       assert.strictEqual(passData.devtoolsLog[0], fakeDevtoolsMessage);
     });
@@ -643,19 +657,19 @@ describe('GatherRunner', function() {
       });
   });
 
-  describe('#getPageLoadError', () => {
+  describe('#getNetworkError', () => {
     it('passes when the page is loaded', () => {
       const url = 'http://the-page.com';
       const mainRecord = new NetworkRequest();
       mainRecord.url = url;
-      assert.ok(!GatherRunner.getPageLoadError(url, [mainRecord]));
+      assert.ok(!GatherRunner.getNetworkError(url, [mainRecord]));
     });
 
     it('passes when the page is loaded, ignoring any fragment', () => {
       const url = 'http://example.com/#/page/list';
       const mainRecord = new NetworkRequest();
       mainRecord.url = 'http://example.com';
-      assert.ok(!GatherRunner.getPageLoadError(url, [mainRecord]));
+      assert.ok(!GatherRunner.getNetworkError(url, [mainRecord]));
     });
 
     it('fails when page fails to load', () => {
@@ -664,7 +678,7 @@ describe('GatherRunner', function() {
       mainRecord.url = url;
       mainRecord.failed = true;
       mainRecord.localizedFailDescription = 'foobar';
-      const error = GatherRunner.getPageLoadError(url, [mainRecord]);
+      const error = GatherRunner.getNetworkError(url, [mainRecord]);
       assert.equal(error.message, 'FAILED_DOCUMENT_REQUEST');
       assert.equal(error.code, 'FAILED_DOCUMENT_REQUEST');
       expect(error.friendlyMessage)
@@ -674,7 +688,7 @@ describe('GatherRunner', function() {
     it('fails when page times out', () => {
       const url = 'http://the-page.com';
       const records = [];
-      const error = GatherRunner.getPageLoadError(url, records);
+      const error = GatherRunner.getNetworkError(url, records);
       assert.equal(error.message, 'NO_DOCUMENT_REQUEST');
       assert.equal(error.code, 'NO_DOCUMENT_REQUEST');
       expect(error.friendlyMessage).toBeDisplayString(/^Lighthouse was unable to reliably load/);
@@ -685,7 +699,7 @@ describe('GatherRunner', function() {
       const mainRecord = new NetworkRequest();
       mainRecord.url = url;
       mainRecord.statusCode = 404;
-      const error = GatherRunner.getPageLoadError(url, [mainRecord]);
+      const error = GatherRunner.getNetworkError(url, [mainRecord]);
       assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST');
       assert.equal(error.code, 'ERRORED_DOCUMENT_REQUEST');
       expect(error.friendlyMessage)
@@ -697,7 +711,7 @@ describe('GatherRunner', function() {
       const mainRecord = new NetworkRequest();
       mainRecord.url = url;
       mainRecord.statusCode = 500;
-      const error = GatherRunner.getPageLoadError(url, [mainRecord]);
+      const error = GatherRunner.getNetworkError(url, [mainRecord]);
       assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST');
       assert.equal(error.code, 'ERRORED_DOCUMENT_REQUEST');
       expect(error.friendlyMessage)
@@ -710,7 +724,7 @@ describe('GatherRunner', function() {
       mainRecord.url = url;
       mainRecord.failed = true;
       mainRecord.localizedFailDescription = 'net::ERR_NAME_NOT_RESOLVED';
-      const error = GatherRunner.getPageLoadError(url, [mainRecord]);
+      const error = GatherRunner.getNetworkError(url, [mainRecord]);
       assert.equal(error.message, 'DNS_FAILURE');
       assert.equal(error.code, 'DNS_FAILURE');
       expect(error.friendlyMessage).toBeDisplayString(/^DNS servers could not resolve/);
@@ -929,7 +943,7 @@ describe('GatherRunner', function() {
         ],
       };
 
-      return GatherRunner.collectArtifacts(gathererResults, {}).then(artifacts => {
+      return GatherRunner.collectArtifacts(gathererResults, {}).then(({artifacts}) => {
         assert.strictEqual(artifacts.AfterGatherer, 97);
         assert.strictEqual(artifacts.PassGatherer, 284);
         assert.strictEqual(artifacts.SingleErrorGatherer, recoverableError);
@@ -937,20 +951,32 @@ describe('GatherRunner', function() {
       });
     });
 
-    it('produces a LighthouseRunWarnings artifact from array of warnings', () => {
-      const LighthouseRunWarnings = [
+    it('produces a deduped LighthouseRunWarnings artifact from array of warnings', async () => {
+      const runWarnings = [
         'warning0',
         'warning1',
         'warning2',
       ];
 
-      const baseArtifacts = {
-        LighthouseRunWarnings,
-      };
+      class WarningGatherer extends Gatherer {
+        afterPass(passContext) {
+          passContext.LighthouseRunWarnings.push(...runWarnings, ...runWarnings);
+          assert.strictEqual(passContext.LighthouseRunWarnings.length, runWarnings.length * 2);
+
+          return '';
+        }
+      }
 
-      return GatherRunner.collectArtifacts({}, baseArtifacts).then(artifacts => {
-        assert.deepStrictEqual(artifacts.LighthouseRunWarnings, LighthouseRunWarnings);
+      const passes = [{
+        gatherers: [{instance: new WarningGatherer()}],
+      }];
+      const artifacts = await GatherRunner.run(passes, {
+        driver: fakeDriver,
+        requestedUrl: 'https://example.com',
+        settings: {},
+        config: new Config({}),
       });
+      assert.deepStrictEqual(artifacts.LighthouseRunWarnings, runWarnings);
     });
 
     it('supports sync and async throwing of errors from gatherers', () => {