Repository: GoogleChrome/lighthouse
Pull request #7062 (2019-01-18 23:00:46)

core: add FCP on 3G audit to JSON
**Summary**
Paul tells me we need a FCP value for 3G in the LR response. This uses some rough 3G numbers based on WPT's "Regular 3G" profile and the definition of 3G in [network information API spec effective connection type 3G](https://wicg.github.io/netinfo/#dfn-effective-connection-types).



Patch:
diff --git a/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js b/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js
index 62d1bde7d..5e1e17e65 100644
--- a/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js
+++ b/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js
@@ -103,14 +103,12 @@ class RenderBlockingResources extends Audit {
     for (const resource of artifacts.TagsBlockingFirstPaint) {
       // Ignore any resources that finished after observed FCP (they're clearly not render-blocking)
       if (resource.endTime * 1000 > fcpTsInMs) continue;
-      // TODO(phulce): beacon these occurences to Sentry to improve FCP graph
+      // TODO: beacon to Sentry, https://github.com/GoogleChrome/lighthouse/issues/7041
       if (!nodesByUrl[resource.tag.url]) continue;
 
       const {node, nodeTiming} = nodesByUrl[resource.tag.url];
 
       // Mark this node and all its dependents as deferrable
-      // TODO(phulce): make this slightly more surgical
-      // i.e. the referenced font asset won't become inlined just because you inline the CSS
       node.traverse(node => deferredNodeIds.add(node.id));
 
       // "wastedMs" is the download time of the network request, responseReceived - requestSent
@@ -190,7 +188,6 @@ class RenderBlockingResources extends Audit {
   static async computeWastedCSSBytes(artifacts, context) {
     const wastedBytesByUrl = new Map();
     try {
-      // TODO(phulce): pull this out into computed artifact
       const results = await UnusedCSS.audit(artifacts, context);
       // @ts-ignore - TODO(bckenny): details types.
       for (const item of results.details.items) {
diff --git a/lighthouse-core/audits/content-width.js b/lighthouse-core/audits/content-width.js
index 7de0b62fb..3f4f06a6b 100644
--- a/lighthouse-core/audits/content-width.js
+++ b/lighthouse-core/audits/content-width.js
@@ -34,7 +34,6 @@ class ContentWidth extends Audit {
     const windowWidth = artifacts.ViewportDimensions.outerWidth;
     const widthsMatch = viewportWidth === windowWidth;
 
-    // TODO(phulce): refactor this `isMobile` boolean to be on context
     const isMobileHost = userAgent.includes('Android') || userAgent.includes('Mobile');
     const isMobile = context.settings.emulatedFormFactor === 'mobile' ||
       (context.settings.emulatedFormFactor !== 'desktop' && isMobileHost);
diff --git a/lighthouse-core/audits/metrics/first-contentful-paint-3g.js b/lighthouse-core/audits/metrics/first-contentful-paint-3g.js
new file mode 100644
index 000000000..a61adb2a8
--- /dev/null
+++ b/lighthouse-core/audits/metrics/first-contentful-paint-3g.js
@@ -0,0 +1,68 @@
+/**
+ * @license Copyright 2019 Google Inc. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+'use strict';
+
+const Audit = require('../audit.js');
+const regular3G = require('../../config/constants.js').throttling.mobileRegluar3G;
+const i18n = require('../../lib/i18n/i18n.js');
+const FCP = require('./first-contentful-paint.js');
+const ComputedFcp = require('../../computed/metrics/first-contentful-paint.js');
+
+const i18nFilename = require.resolve('./first-contentful-paint.js');
+const str_ = i18n.createMessageInstanceIdFn(i18nFilename, FCP.UIStrings);
+
+class FirstContentfulPaint3G extends Audit {
+  /**
+   * @return {LH.Audit.Meta}
+   */
+  static get meta() {
+    return {
+      id: 'first-contentful-paint-3g',
+      title: str_(FCP.UIStrings.title),
+      description: str_(FCP.UIStrings.description),
+      scoreDisplayMode: Audit.SCORING_MODES.NUMERIC,
+      requiredArtifacts: ['traces', 'devtoolsLogs'],
+    };
+  }
+
+  /**
+   * @return {LH.Audit.ScoreOptions}
+   */
+  static get defaultOptions() {
+    return {
+      // 75th and 95th percentiles HTTPArchive on Fast 3G -> multiply by 1.5 for RTT differential -> median and PODR
+      // https://bigquery.cloud.google.com/table/httparchive:lighthouse.2018_04_01_mobile?pli=1
+      scorePODR: 3000,
+      scoreMedian: 6000,
+    };
+  }
+
+  /**
+   * @param {LH.Artifacts} artifacts
+   * @param {LH.Audit.Context} context
+   * @return {Promise<LH.Audit.Product>}
+   */
+  static async audit(artifacts, context) {
+    const trace = artifacts.traces[Audit.DEFAULT_PASS];
+    const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
+    /** @type {LH.Config.Settings} */
+    const settings = {...context.settings, throttlingMethod: 'simulate', throttling: regular3G};
+    const metricComputationData = {trace, devtoolsLog, settings};
+    const metricResult = await ComputedFcp.request(metricComputationData, context);
+
+    return {
+      score: Audit.computeLogNormalScore(
+        metricResult.timing,
+        context.options.scorePODR,
+        context.options.scoreMedian
+      ),
+      rawValue: metricResult.timing,
+      displayValue: str_(i18n.UIStrings.seconds, {timeInMs: metricResult.timing}),
+    };
+  }
+}
+
+module.exports = FirstContentfulPaint3G;
diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js
index 54cf15322..5671e998c 100644
--- a/lighthouse-core/config/config.js
+++ b/lighthouse-core/config/config.js
@@ -184,7 +184,6 @@ function cleanFlagsForSettings(flags = {}) {
   return settings;
 }
 
-// TODO(phulce): disentangle this merge function
 /**
  * More widely typed than exposed merge() function, below.
  * @param {Object<string, any>|Array<any>|undefined|null} base
diff --git a/lighthouse-core/config/constants.js b/lighthouse-core/config/constants.js
index 46d19987f..4aea3ef77 100644
--- a/lighthouse-core/config/constants.js
+++ b/lighthouse-core/config/constants.js
@@ -16,6 +16,8 @@ const DEVTOOLS_THROUGHPUT_ADJUSTMENT_FACTOR = 0.9;
 const throttling = {
   DEVTOOLS_RTT_ADJUSTMENT_FACTOR,
   DEVTOOLS_THROUGHPUT_ADJUSTMENT_FACTOR,
+  // These values align with WebPageTest's definition of "Fast 3G"
+  // But offer similar charateristics to roughly the 75th percentile of 4G connections.
   mobileSlow4G: {
     rttMs: 150,
     throughputKbps: 1.6 * 1024,
@@ -24,6 +26,17 @@ const throttling = {
     uploadThroughputKbps: 750 * DEVTOOLS_THROUGHPUT_ADJUSTMENT_FACTOR,
     cpuSlowdownMultiplier: 4,
   },
+  // These values partially align with WebPageTest's definition of "Regular 3G".
+  // These values are meant to roughly align with Chrome UX report's 3G definition which are based
+  // on HTTP RTT of 300-1400ms and downlink throughput of <700kbps.
+  mobileRegluar3G: {
+    rttMs: 300,
+    throughputKbps: 700,
+    requestLatencyMs: 300 * DEVTOOLS_RTT_ADJUSTMENT_FACTOR,
+    downloadThroughputKbps: 700 * DEVTOOLS_THROUGHPUT_ADJUSTMENT_FACTOR,
+    uploadThroughputKbps: 700 * DEVTOOLS_THROUGHPUT_ADJUSTMENT_FACTOR,
+    cpuSlowdownMultiplier: 4,
+  },
 };
 
 /** @type {LH.Config.Settings} */
diff --git a/lighthouse-core/config/lr-desktop-config.js b/lighthouse-core/config/lr-desktop-config.js
index 11fc64485..8e0aa8f28 100644
--- a/lighthouse-core/config/lr-desktop-config.js
+++ b/lighthouse-core/config/lr-desktop-config.js
@@ -18,7 +18,7 @@ const config = {
       throughputKbps: 10 * 1024,
       cpuSlowdownMultiplier: 1,
     },
-    // Skip the h2 audit so it doesn't lie to us. See #6539
+    // Skip the h2 audit so it doesn't lie to us. See https://github.com/GoogleChrome/lighthouse/issues/6539
     skipAudits: ['uses-http2'],
   },
   audits: [
diff --git a/lighthouse-core/config/lr-mobile-config.js b/lighthouse-core/config/lr-mobile-config.js
index 264663542..2c689ad1b 100644
--- a/lighthouse-core/config/lr-mobile-config.js
+++ b/lighthouse-core/config/lr-mobile-config.js
@@ -10,9 +10,20 @@ const config = {
   extends: 'lighthouse:default',
   settings: {
     maxWaitForLoad: 35 * 1000,
-    // Skip the h2 audit so it doesn't lie to us. See #6539
+    // Skip the h2 audit so it doesn't lie to us. See https://github.com/GoogleChrome/lighthouse/issues/6539
     skipAudits: ['uses-http2'],
   },
+  audits: [
+    'metrics/first-contentful-paint-3g',
+  ],
+  // @ts-ignore TODO(bckenny): type extended Config where e.g. category.title isn't required
+  categories: {
+    performance: {
+      auditRefs: [
+        {id: 'first-contentful-paint-3g', weight: 0},
+      ],
+    },
+  },
 };
 
 module.exports = config;
diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
index 60f59ee30..b2e26267f 100644
--- a/lighthouse-core/gather/driver.js
+++ b/lighthouse-core/gather/driver.js
@@ -1245,7 +1245,6 @@ class Driver {
    * @return {Promise<void>}
    */
   async beginEmulation(settings) {
-    // TODO(phulce): remove this flag on next breaking change
     if (!settings.disableDeviceEmulation) {
       if (settings.emulatedFormFactor === 'mobile') {
         await emulation.enableNexus5X(this);
diff --git a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js
index 67484a2fd..6ca6a9a16 100644
--- a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js
+++ b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js
@@ -129,8 +129,6 @@ class OptimizedImages extends Gatherer {
    * @return {Promise<?{fromProtocol: boolean, originalSize: number, jpegSize: number, webpSize: number}>}
    */
   calculateImageStats(driver, networkRecord) {
-    // TODO(phulce): remove this dance of trying _getEncodedResponse with a fallback when Audits
-    // domain hits stable in Chrome 62
     return Promise.resolve(networkRecord.requestId).then(requestId => {
       if (this._getEncodedResponseUnsupported) return;
       return this._getEncodedResponse(driver, requestId, 'jpeg').then(jpegData => {
@@ -153,7 +151,7 @@ class OptimizedImages extends Gatherer {
     }).then(result => {
       if (result) return result;
 
-      // Take the slower fallback path if getEncodedResponse isn't available yet
+      // Take the slower fallback path if getEncodedResponse didn't work
       // CORS canvas tainting doesn't support cross-origin images, so skip them early
       if (!networkRecord.isSameOrigin && !networkRecord.isBase64DataUri) return null;
 
diff --git a/lighthouse-core/gather/gatherers/link-elements.js b/lighthouse-core/gather/gatherers/link-elements.js
index 4a74e3abe..9283265e0 100644
--- a/lighthouse-core/gather/gatherers/link-elements.js
+++ b/lighthouse-core/gather/gatherers/link-elements.js
@@ -16,7 +16,6 @@ class LinkElements extends Gatherer {
   async afterPass(passContext) {
     const driver = passContext.driver;
 
-    // TODO(phulce): merge this with the main resource header values too
     // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize
     // the values like access from JavaScript does.
     return driver.evaluateAsync(`(() => {
diff --git a/lighthouse-core/lib/dependency-graph/simulator/network-analyzer.js b/lighthouse-core/lib/dependency-graph/simulator/network-analyzer.js
index ef2187df5..7f2dd6b0d 100644
--- a/lighthouse-core/lib/dependency-graph/simulator/network-analyzer.js
+++ b/lighthouse-core/lib/dependency-graph/simulator/network-analyzer.js
@@ -289,8 +289,6 @@ class NetworkAnalyzer {
         );
       }
 
-      // TODO(phulce): compute the maximum number of parallel requests (N) and ensure we have at
-      // least N requests that required new connections
       const firstRecord = originRecords.reduce((a, b) => (a.startTime > b.startTime ? b : a));
       connectionWasReused.set(firstRecord.requestId, false);
     }
@@ -448,7 +446,6 @@ class NetworkAnalyzer {
    * @return {LH.Artifacts.NetworkRequest}
    */
   static findMainDocument(records) {
-    // TODO(phulce): handle more edge cases like client redirects, or plumb through finalUrl
     const documentRequests = records.filter(record => record.resourceType ===
         NetworkRequest.TYPES.Document);
     // The main document is the earliest document request, using position in networkRecords array to break ties.
diff --git a/lighthouse-core/lib/dependency-graph/simulator/simulator.js b/lighthouse-core/lib/dependency-graph/simulator/simulator.js
index fe46d21c2..9c6651520 100644
--- a/lighthouse-core/lib/dependency-graph/simulator/simulator.js
+++ b/lighthouse-core/lib/dependency-graph/simulator/simulator.js
@@ -106,7 +106,6 @@ class Simulator {
     this._cachedNodeListByStartTime = [];
     // NOTE: We don't actually need *all* of these sets, but the clarity that each node progresses
     // through the system is quite nice.
-    // TODO(phulce): consider refactoring this so that it's easier to follow
     for (const state of Object.values(NodeState)) {
       this._nodes[state] = new Set();
     }
diff --git a/lighthouse-core/lib/network-recorder.js b/lighthouse-core/lib/network-recorder.js
index 4de1df7ff..4264e34eb 100644
--- a/lighthouse-core/lib/network-recorder.js
+++ b/lighthouse-core/lib/network-recorder.js
@@ -221,7 +221,7 @@ class NetworkRecorder extends EventEmitter {
       return;
     }
 
-    // TODO(phulce): log these to sentry?
+    // TODO: beacon to Sentry, https://github.com/GoogleChrome/lighthouse/issues/7041
     if (!data.redirectResponse) {
       return;
     }
@@ -231,7 +231,6 @@ class NetworkRecorder extends EventEmitter {
     const modifiedData = {
       ...data,
       // Copy over the initiator as well to match DevTools behavior
-      // TODO(phulce): abandon this DT hack and update Lantern graph to handle it
       initiator: originalRequest.initiator,
       requestId: `${originalRequest.requestId}:redirect`,
     };
diff --git a/lighthouse-core/lib/network-request.js b/lighthouse-core/lib/network-request.js
index 883a851df..8c5fc69ec 100644
--- a/lighthouse-core/lib/network-request.js
+++ b/lighthouse-core/lib/network-request.js
@@ -51,7 +51,6 @@ const RESOURCE_TYPES = {
 module.exports = class NetworkRequest {
   constructor() {
     this.requestId = '';
-    // TODO(phulce): remove default DevTools connectionId
     this.connectionId = '0';
     this.connectionReused = false;
 
diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js
index cb9fb5f02..e08294dc1 100644
--- a/lighthouse-core/runner.js
+++ b/lighthouse-core/runner.js
@@ -227,7 +227,6 @@ class Runner {
       const normalizedGatherSettings = Object.assign({}, artifacts.settings, overrides);
       const normalizedAuditSettings = Object.assign({}, settings, overrides);
 
-      // TODO(phulce): allow change of throttling method to `simulate`
       if (!isDeepEqual(normalizedGatherSettings, normalizedAuditSettings)) {
         throw new Error('Cannot change settings between gathering and auditing');
       }
diff --git a/lighthouse-core/scripts/assert-golden-lhr-unchanged.sh b/lighthouse-core/scripts/assert-golden-lhr-unchanged.sh
index 99bbc750b..7f2908446 100644
--- a/lighthouse-core/scripts/assert-golden-lhr-unchanged.sh
+++ b/lighthouse-core/scripts/assert-golden-lhr-unchanged.sh
@@ -28,7 +28,6 @@ trap teardown EXIT
 
 colorText "Generating a fresh LHR..." "$purple"
 set -x
-# TODO(phulce): add a lantern LHR-differ
 node "$lhroot_path/lighthouse-cli" -A="$lhroot_path/lighthouse-core/test/results/artifacts" --throttling-method=devtools --quiet --output=json --output-path="$freshLHRPath"
 set +x
 
diff --git a/lighthouse-viewer/app/src/lighthouse-report-viewer.js b/lighthouse-viewer/app/src/lighthouse-report-viewer.js
index 0e77a6a3a..e59551dfb 100644
--- a/lighthouse-viewer/app/src/lighthouse-report-viewer.js
+++ b/lighthouse-viewer/app/src/lighthouse-report-viewer.js
@@ -138,6 +138,11 @@ class LighthouseReportViewer {
   // TODO: Really, `json` should really have type `unknown` and
   // we can have _validateReportJson verify that it's an LH.Result
   _replaceReportHtml(json) {
+    // Allow users to view the runnerResult
+    if ('lhr' in json) {
+      json = /** @type {LH.RunnerResult} */ (json).lhr;
+    }
+
     this._validateReportJson(json);
 
     // Redirect to old viewer if a v2 report. v3 and v4 both handled by v4 viewer.


Test patch:
diff --git a/lighthouse-cli/test/smokehouse/byte-config.js b/lighthouse-cli/test/smokehouse/byte-config.js
index 78c58b7fb..6718ab526 100644
--- a/lighthouse-cli/test/smokehouse/byte-config.js
+++ b/lighthouse-cli/test/smokehouse/byte-config.js
@@ -22,8 +22,6 @@ module.exports = {
       'unused-css-rules',
       'unused-javascript',
     ],
-
-    // TODO(phulce): re-write testers to work with faster lantern loading
     throttlingMethod: 'devtools',
   },
 };
diff --git a/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js b/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js
index 0822e727c..d4a379638 100644
--- a/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js
+++ b/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js
@@ -5,9 +5,6 @@
  */
 'use strict';
 
-// TODO(phulce): assert more score values once Lantern can tie more trace events back to images.
-// See https://github.com/GoogleChrome/lighthouse/issues/4600.
-
 /**
  * Expected Lighthouse audit values for byte efficiency tests
  */
diff --git a/lighthouse-core/test/audits/metrics/first-contentful-paint-3g-test.js b/lighthouse-core/test/audits/metrics/first-contentful-paint-3g-test.js
new file mode 100644
index 000000000..b569abe2f
--- /dev/null
+++ b/lighthouse-core/test/audits/metrics/first-contentful-paint-3g-test.js
@@ -0,0 +1,36 @@
+/**
+ * @license Copyright 2019 Google Inc. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+'use strict';
+
+const FCP3G = require('../../../audits/metrics/first-contentful-paint-3g.js');
+const options = FCP3G.defaultOptions;
+
+const pwaTrace = require('../../fixtures/traces/progressive-app-m60.json');
+const pwaDevtoolsLog = require('../../fixtures/traces/progressive-app-m60.devtools.log.json');
+
+/* eslint-env jest */
+
+describe('Performance: first-contentful-paint-3g audit', () => {
+  it('evaluates valid input correctly', async () => {
+    const artifacts = {
+      traces: {
+        [FCP3G.DEFAULT_PASS]: pwaTrace,
+      },
+      devtoolsLogs: {
+        [FCP3G.DEFAULT_PASS]: pwaDevtoolsLog,
+      },
+    };
+
+    const result = await FCP3G.audit(artifacts, {settings: {}, options, computedCache: new Map()});
+    // Use InlineSnapshot here so changes to Lantern coefficients can be easily updated en masse
+    expect({score: result.score, value: Math.round(result.rawValue)}).toMatchInlineSnapshot(`
+Object {
+  "score": 1,
+  "value": 1661,
+}
+`);
+  });
+});