support collecting network records per pass

brendankenny · brendankenny · commit 4ef3c98bfd7d · 2016-08-24T17:03:48.000-07:00
diff --git a/lighthouse-core/audits/audit.js b/lighthouse-core/audits/audit.js
@@ -16,14 +16,14 @@
  */
 'use strict';
 
-const DEFAULT_TRACE = 'defaultPass';
+const DEFAULT_PASS = 'defaultPass';
 
 class Audit {
   /**
    * @return {!String}
    */
-  static get DEFAULT_TRACE() {
-    return DEFAULT_TRACE;
+  static get DEFAULT_PASS() {
+    return DEFAULT_PASS;
   }
 
   /**
diff --git a/lighthouse-core/audits/critical-request-chains.js b/lighthouse-core/audits/critical-request-chains.js
@@ -40,7 +40,8 @@ class CriticalRequestChains extends Audit {
    * @return {!AuditResult} The score from the audit, ranging from 0-100.
    */
   static audit(artifacts) {
-    return artifacts.requestCriticalRequestChains(artifacts.networkRecords).then(chains => {
+    const networkRecord = artifacts.networkRecords[Audit.DEFAULT_PASS];
+    return artifacts.requestCriticalRequestChains(networkRecord).then(chains => {
       let chainCount = 0;
       function walk(node, depth) {
         const children = Object.keys(node);
diff --git a/lighthouse-core/audits/estimated-input-latency.js b/lighthouse-core/audits/estimated-input-latency.js
@@ -79,7 +79,7 @@ class EstimatedInputLatency extends Audit {
    * @return {!Promise<!AuditResult>} The score from the audit, ranging from 0-100.
    */
   static audit(artifacts) {
-    const trace = artifacts.traces[this.DEFAULT_TRACE];
+    const trace = artifacts.traces[this.DEFAULT_PASS];
 
     return artifacts.requestSpeedline(trace)
       .then(speedline => EstimatedInputLatency.calculate(speedline, trace))
diff --git a/lighthouse-core/audits/first-meaningful-paint.js b/lighthouse-core/audits/first-meaningful-paint.js
@@ -53,7 +53,7 @@ class FirstMeaningfulPaint extends Audit {
    */
   static audit(artifacts) {
     return new Promise((resolve, reject) => {
-      const traceContents = artifacts.traces[this.DEFAULT_TRACE].traceEvents;
+      const traceContents = artifacts.traces[this.DEFAULT_PASS].traceEvents;
       if (!traceContents || !Array.isArray(traceContents)) {
         throw new Error(FAILURE_MESSAGE);
       }
diff --git a/lighthouse-core/audits/screenshots.js b/lighthouse-core/audits/screenshots.js
@@ -38,7 +38,7 @@ class Screenshots extends Audit {
    * @return {!Promise<!AuditResult>}
    */
   static audit(artifacts) {
-    const trace = artifacts.traces[this.DEFAULT_TRACE];
+    const trace = artifacts.traces[this.DEFAULT_PASS];
     if (typeof trace === 'undefined') {
       return Promise.resolve(Screenshots.generateAuditResult({
         rawValue: -1,
diff --git a/lighthouse-core/audits/speed-index-metric.js b/lighthouse-core/audits/speed-index-metric.js
@@ -47,7 +47,7 @@ class SpeedIndexMetric extends Audit {
    * @return {!Promise<!AuditResult>} The score from the audit, ranging from 0-100.
    */
   static audit(artifacts) {
-    const trace = artifacts.traces[this.DEFAULT_TRACE];
+    const trace = artifacts.traces[this.DEFAULT_PASS];
     if (typeof trace === 'undefined') {
       return SpeedIndexMetric.generateAuditResult({
         rawValue: -1,
diff --git a/lighthouse-core/audits/time-to-interactive.js b/lighthouse-core/audits/time-to-interactive.js
@@ -57,7 +57,7 @@ class TTIMetric extends Audit {
    * @return {!Promise<!AuditResult>} The score from the audit, ranging from 0-100.
    */
   static audit(artifacts) {
-    const trace = artifacts.traces[Audit.DEFAULT_TRACE];
+    const trace = artifacts.traces[Audit.DEFAULT_PASS];
     const pendingSpeedline = artifacts.requestSpeedline(trace);
     const pendingFMP = FMPMetric.audit(artifacts);
 
@@ -74,7 +74,7 @@ class TTIMetric extends Audit {
 
       // Process the trace
       const tracingProcessor = new TracingProcessor();
-      const trace = artifacts.traces[Audit.DEFAULT_TRACE];
+      const trace = artifacts.traces[Audit.DEFAULT_PASS];
       const model = tracingProcessor.init(trace);
       const endOfTraceTime = model.bounds.max;
 
diff --git a/lighthouse-core/audits/user-timings.js b/lighthouse-core/audits/user-timings.js
@@ -129,8 +129,8 @@ class UserTimings extends Audit {
   static audit(artifacts) {
     return new Promise((resolve, reject) => {
       const traceContents =
-        artifacts.traces[this.DEFAULT_TRACE] &&
-        artifacts.traces[this.DEFAULT_TRACE].traceEvents;
+        artifacts.traces[this.DEFAULT_PASS] &&
+        artifacts.traces[this.DEFAULT_PASS].traceEvents;
       if (!traceContents || !Array.isArray(traceContents)) {
         throw new Error(FAILURE_MESSAGE);
       }
diff --git a/lighthouse-core/closure/typedefs/Artifacts.js b/lighthouse-core/closure/typedefs/Artifacts.js
@@ -38,6 +38,9 @@ Artifacts.prototype.HTTPS;
 /** @type {!Array<!Object>} */
 Artifacts.prototype.traces;
 
+/** @type {!Object<!Array>} */
+Artifacts.prototype.networkRecords;
+
 /** @type {!ManifestNode<(!Manifest|undefined)>} */
 Artifacts.prototype.Manifest;
 
diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js
@@ -22,6 +22,7 @@ const recordsFromLogs = require('../lib/network-recorder').recordsFromLogs;
 const GatherRunner = require('../gather/gather-runner');
 const log = require('../lib/log');
 const path = require('path');
+const Audit = require('../audits/audit');
 
 // cleanTrace is run to remove duplicate TracingStartedInPage events,
 // and to change TracingStartedInBrowser events into TracingStartedInPage.
@@ -122,6 +123,21 @@ function validatePasses(passes, audits, rootPath) {
       }
     });
   });
+
+  // Log if multiple passes require trace or network data and could overwrite one another.
+  const usedNames = new Set();
+  passes.forEach((pass, index) => {
+    if (!pass.network && !pass.trace) {
+      return;
+    }
+
+    const passName = pass.traceName || Audit.DEFAULT_PASS;
+    if (usedNames.has(passName)) {
+      log.warn('config', `passes[${index}] may overwrite trace or network ` +
+          `data of earlier pass without a unique traceName (repeated name: ${passName}.`);
+    }
+    usedNames.add(passName);
+  });
 }
 
 function getGatherersNeededByAudits(audits) {
@@ -238,8 +254,21 @@ function expandArtifacts(artifacts) {
       artifacts.traces[key] = trace;
     });
   }
+
   if (artifacts.performanceLog) {
-    artifacts.networkRecords = recordsFromLogs(require(artifacts.performanceLog));
+    if (typeof artifacts.performanceLog === 'string') {
+      // Support older format of a single performance log.
+      const log = require(artifacts.performanceLog);
+      artifacts.networkRecords = {
+        [Audit.DEFAULT_PASS]: recordsFromLogs(log)
+      };
+    } else {
+      artifacts.networkRecords = {};
+      Object.keys(artifacts.performanceLog).forEach(key => {
+        const log = require(artifacts.performanceLog[key]);
+        artifacts.networkRecords[key] = recordsFromLogs(log);
+      });
+    }
   }
 
   return artifacts;
diff --git a/lighthouse-core/gather/drivers/driver.js b/lighthouse-core/gather/drivers/driver.js
@@ -213,8 +213,6 @@ class Driver {
    * If our main document URL redirects, we will update options.url accordingly
    * As such, options.url will always represent the post-redirected URL.
    * options.initialUrl is the pre-redirect URL that things started with
-   *
-   * Caveat: only works when network recording enabled for a pass
    */
   enableUrlUpdateIfRedirected(opts) {
     this._networkRecorder.on('requestloaded', redirectRequest => {
diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js
@@ -37,11 +37,11 @@ const path = require('path');
  *   B. GatherRunner.pass()
  *     i. GatherRunner.loadPage()
  *       a. navigate to about:blank
- *       b. beginTrace & beginNetworkCollect (if requested)
+ *       b. beginTrace (if requested) & beginNetworkCollect
  *       c. navigate to options.url (and wait for onload)
  *     ii. all gatherer's pass()
  *   C. GatherRunner.afterPass()
- *     i. endTrace & endNetworkCollect (if requested)
+ *     i. endTrace (if requested) & endNetworkCollect
  *     ii. all gatherer's afterPass()
  *
  * 3. Teardown
@@ -57,9 +57,10 @@ class GatherRunner {
     return driver.gotoURL('about:blank')
       // Wait a bit for about:blank to "take hold" before switching back to the page.
       .then(_ => new Promise((resolve, reject) => setTimeout(resolve, 300)))
-      // Begin tracing and network recording if required.
+      // Begin tracing if required.
       .then(_ => options.config.trace && driver.beginTrace())
-      .then(_ => options.config.network && driver.beginNetworkCollect(options))
+      // Begin network recording.
+      .then(_ => driver.beginNetworkCollect(options))
       // Navigate.
       .then(_ => driver.gotoURL(options.url, {
         waitForLoad: true,
@@ -135,7 +136,8 @@ class GatherRunner {
     const driver = options.driver;
     const config = options.config;
     const gatherers = config.gatherers;
-    const loadData = {traces: {}};
+    const loadData = {};
+
     let pass = Promise.resolve();
 
     if (config.trace) {
@@ -144,32 +146,24 @@ class GatherRunner {
         return driver.endTrace();
       }).then(traceContents => {
         // Before Chrome 54.0.2816 (codereview.chromium.org/2161583004),
-        // traceContents was an array of trace events. After this point,
-        // traceContents is an object with a traceEvents property. Normalize
-        // to new format.
-        if (Array.isArray(traceContents)) {
-          traceContents = {
-            traceEvents: traceContents
-          };
-        }
-
-        const traceName = config.traceName || Audit.DEFAULT_TRACE;
-        loadData.traces[traceName] = traceContents;
-        loadData.traceEvents = traceContents.traceEvents;
+        // traceContents was an array of trace events; after, traceContents is
+        // an object with a traceEvents property. Normalize to object form.
+        loadData.trace = Array.isArray(traceContents) ? {
+          traceEvents: traceContents
+        } : traceContents;
         log.verbose('statusEnd', 'Retrieving trace');
       });
     }
 
-    if (config.network) {
-      const status = 'Retrieving network records';
-      pass = pass.then(_ => {
-        log.log('status', status);
-        return driver.endNetworkCollect();
-      }).then(networkRecords => {
-        loadData.networkRecords = networkRecords;
-        log.verbose('statusEnd', status);
-      });
-    }
+    const status = 'Retrieving network records';
+    pass = pass.then(_ => {
+      log.log('status', status);
+      return driver.endNetworkCollect();
+    }).then(networkRecords => {
+      // Network records only given to gatherers if requested by config.
+      config.network && (loadData.networkRecords = networkRecords);
+      log.verbose('statusEnd', status);
+    });
 
     pass = gatherers.reduce((chain, gatherer) => {
       const status = `Retrieving: ${gatherer.name}`;
@@ -182,13 +176,16 @@ class GatherRunner {
       });
     }, pass);
 
-    // Resolve on loadData.
+    // Resolve on tracing data using traceName from config.
     return pass.then(_ => loadData);
   }
 
   static run(passes, options) {
     const driver = options.driver;
-    const tracingData = {traces: {}};
+    const tracingData = {
+      traces: {},
+      networkRecords: {}
+    };
 
     if (typeof options.url !== 'string' || options.url.length === 0) {
       return Promise.reject(new Error('You must provide a url to the driver'));
@@ -227,10 +224,13 @@ class GatherRunner {
             .then(_ => GatherRunner.beforePass(runOptions))
             .then(_ => GatherRunner.pass(runOptions))
             .then(_ => GatherRunner.afterPass(runOptions))
-            .then(loadData => {
-              // Merge pass trace and network data into tracingData.
-              config.trace && Object.assign(tracingData.traces, loadData.traces);
-              config.network && (tracingData.networkRecords = loadData.networkRecords);
+            .then(passData => {
+              // If requested by config, merge trace and network data for this
+              // pass into tracingData.
+              const passName = config.traceName || Audit.DEFAULT_PASS;
+              config.trace && (tracingData.traces[passName] = passData.trace);
+              config.network && (tracingData.networkRecords[passName] = passData.networkRecords);
+
               if (passIndex === 0) {
                 urlAfterRedirects = runOptions.url;
               }
diff --git a/lighthouse-core/gather/gatherers/gatherer.js b/lighthouse-core/gather/gatherers/gatherer.js
@@ -52,7 +52,7 @@ class Gatherer {
    * executed, and — if generated in this pass — the trace is ended. The trace
    * and record of network activity are provided in `loadData`.
    * @param {!Object} options
-   * @param {{networkRecords: !Array, traceEvents: !Array} loadData
+   * @param {{networkRecords: !Array, trace: {traceEvents: !Array}} loadData
    */
   afterPass(options, loadData) { }
 
diff --git a/lighthouse-core/lib/log.js b/lighthouse-core/lib/log.js
@@ -39,13 +39,23 @@ function _log(title, logargs) {
 }
 
 class Emitter extends EventEmitter {
-  // issueStatus fires off all status updates
-  // listen with `require('lib/log').events.addListener('status', callback)`
+  /**
+   * Fires off all status updates. Listen with
+   * `require('lib/log').events.addListener('status', callback)`
+   */
   issueStatus(title, args) {
     if (title === 'status' || title === 'statusEnd') {
       this.emit(title, args);
     }
   }
+
+  /**
+   * Fires off all warnings. Listen with
+   * `require('lib/log').events.addListener('warning', callback)`
+   */
+  issueWarning(args) {
+    this.emit('warning', args);
+  }
 }
 
 module.exports = {
@@ -57,6 +67,7 @@ module.exports = {
   },
 
   warn(title) {
+    this.events.issueWarning(arguments);
     return _log(`${title}:warn`, arguments);
   },
 
diff --git a/lighthouse-core/test/audits/critical-request-chains-test.js b/lighthouse-core/test/audits/critical-request-chains-test.js
@@ -50,6 +50,9 @@ const CriticalRequestChains = {
 };
 
 const mockArtifacts = {
+  networkRecords: {
+    [Audit.DEFAULT_PASS]: []
+  },
   requestCriticalRequestChains: function() {
     return Promise.resolve(CriticalRequestChains);
   }
diff --git a/lighthouse-core/test/audits/estimated-input-latency-test.js b/lighthouse-core/test/audits/estimated-input-latency-test.js
@@ -26,7 +26,7 @@ let computedArtifacts = GatherRunner.instantiateComputedArtifacts();
 function generateArtifactsWithTrace(trace) {
   return Object.assign(computedArtifacts, {
     traces: {
-      [Audit.DEFAULT_TRACE]: trace
+      [Audit.DEFAULT_PASS]: trace
     }
   });
 }
diff --git a/lighthouse-core/test/audits/first-meaningful-paint-test.js b/lighthouse-core/test/audits/first-meaningful-paint-test.js
@@ -38,7 +38,7 @@ describe('Performance: first-meaningful-paint audit', () => {
 
     it('processes a valid trace file', done => {
       assert.doesNotThrow(_ => {
-        Audit.audit({traces: {[Audit.DEFAULT_TRACE]: {traceEvents}}})
+        Audit.audit({traces: {[Audit.DEFAULT_PASS]: {traceEvents}}})
           .then(response => {
             fmpResult = response;
             done();
diff --git a/lighthouse-core/test/audits/time-to-interactive-test.js b/lighthouse-core/test/audits/time-to-interactive-test.js
@@ -28,7 +28,7 @@ describe('Performance: time-to-interactive audit', () => {
   it('scores a -1 with invalid trace data', () => {
     return Audit.audit({
       traces: {
-        [Audit.DEFAULT_TRACE]: {
+        [Audit.DEFAULT_PASS]: {
           traceEvents: '[{"pid": 15256,"tid": 1295,"t'
         }
       },
@@ -44,7 +44,7 @@ describe('Performance: time-to-interactive audit', () => {
   it('evaluates valid input correctly', () => {
     let artifacts = mockArtifacts;
     artifacts.traces = {
-      [Audit.DEFAULT_TRACE]: {
+      [Audit.DEFAULT_PASS]: {
         traceEvents: pwaTrace
       }
     };
diff --git a/lighthouse-core/test/audits/user-timing-test.js b/lighthouse-core/test/audits/user-timing-test.js
@@ -29,7 +29,7 @@ describe('Performance: user-timings audit', () => {
   });
 
   it('evaluates valid input correctly', () => {
-    return Audit.audit({traces: {[Audit.DEFAULT_TRACE]: {traceEvents}}})
+    return Audit.audit({traces: {[Audit.DEFAULT_PASS]: {traceEvents}}})
       .then(response => {
         assert.equal(response.score, 2);
         assert.ok(!Number.isNaN(response.extendedInfo.value[0].startTime));
diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js
diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js
diff --git a/lighthouse-core/test/lib/asset-saver-test.js b/lighthouse-core/test/lib/asset-saver-test.js
diff --git a/lighthouse-core/test/runner-test.js b/lighthouse-core/test/runner-test.js

Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ class FirstMeaningfulPaint extends Audit {`
`53`	`53`	`*/`
`54`	`54`	`static audit(artifacts) {`
`55`	`55`	`return new Promise((resolve, reject) => {`
`56`		`- const traceContents = artifacts.traces[this.DEFAULT_TRACE].traceEvents;`
	`56`	`+ const traceContents = artifacts.traces[this.DEFAULT_PASS].traceEvents;`
`57`	`57`	`if (!traceContents \|\| !Array.isArray(traceContents)) {`
`58`	`58`	`throw new Error(FAILURE_MESSAGE);`
`59`	`59`	`}`