Repository: GoogleChrome/lighthouse
Pull request #9731 (2019-09-24 22:53:49)

core(unused-css-rules): no more inifinity savings
**Summary**
Described by brendan already in https://github.com/GoogleChrome/lighthouse/issues/9684#issuecomment-532381611

**Related Issues/PRs**
fixes https://github.com/GoogleChrome/lighthouse/issues/9684 (and https://github.com/GoogleChrome/lighthouse/issues/9614 which started this for me 😄 )


patrickhulce: > Did you find any clues on what's going on with those network requests? Is the protocol wrong on size or wrong on not from cache?

Oh! Yeah they were all `rel=prefetch` links that the server responded to a 304 Not Modified for. So the were initiated only with the only purpose of populating the cache but the cache was already up to date, I guess a DevTools size of 0 makes sense in that case? 

Best I can tell, in Gatsby after a service worker is activated, the page prefetches all of the resources that were already on the page. I'm not sure why.

@wardpeet do you happen to know why Gatsby does this?
connorjclark: ```
DevTools size of 0 makes sense in that case
```

doesn't it also make sense for any case with an empty response body? like, an empty .js file? jw b/c I think you mentioned that zero should be impossible

Patch:
diff --git a/.travis.yml b/.travis.yml
index 1c1c165e8..69ce7695c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -39,6 +39,7 @@ script:
   - yarn test-viewer
   - yarn test-lantern
   - yarn i18n:checks
+  - yarn dogfood-lhci
 before_cache:
   # nyc, jest and other projects store files in here. They mess up the travis build cache.
   - rm -rf ./node_modules/.cache/
diff --git a/docs/throttling.md b/docs/throttling.md
index c29cf95a1..4b24c1d7c 100644
--- a/docs/throttling.md
+++ b/docs/throttling.md
@@ -20,6 +20,8 @@ Within web performance testing, there are four typical styles of throttling:
 
 Lighthouse, by default, uses simulated throttling as it provides both quick evaluation and minimized variance. However, some may want to experiment with more accurate throttling...
 
+[Learn more about these throttling types and how they behave in in different scenarios](https://www.debugbear.com/blog/network-throttling-methods).
+
 ## How do I get packet-level throttling?
 
 This Performance Calendar article, [Testing with Realistic Networking Conditions](https://calendar.perfplanet.com/2016/testing-with-realistic-networking-conditions/), has a good explanation of packet-level traffic shaping (which applies across TCP/UDP/ICMP) and recommendations.
@@ -36,7 +38,7 @@ go get github.com/tylertreat/comcast
 # Ensure your $GOPATH/bin is in your $PATH (https://github.com/golang/go/wiki/GOPATH)
 
 # To use the recommended throttling values:
-comcast --latency=150 --target-bw=1600 --dry-run
+comcast --latency=150 --target-bw=1638 --dry-run
 
 # To disable throttling
 # comcast --stop
diff --git a/docs/variability.md b/docs/variability.md
index dce77de3b..72f9a721b 100644
--- a/docs/variability.md
+++ b/docs/variability.md
@@ -83,3 +83,4 @@ The median Lighthouse score of 5 runs is twice as stable as 1 run, and tools lik
 
 - [Lighthouse Variability and Accuracy Analysis](https://docs.google.com/document/d/1BqtL-nG53rxWOI5RO0pItSRPowZVnYJ_gBEQCJ5EeUE/edit?usp=sharing)
 - [Throttling documentation](./throttling.md)
+- [Why is my Lighthouse score different from PageSpeed Insights?](https://www.debugbear.com/blog/why-is-my-lighthouse-score-different-from-pagespeed-insights)
\ No newline at end of file
diff --git a/lighthouse-core/audits/apple-touch-icon.js b/lighthouse-core/audits/apple-touch-icon.js
index eb5ad8486..43086ae53 100644
--- a/lighthouse-core/audits/apple-touch-icon.js
+++ b/lighthouse-core/audits/apple-touch-icon.js
@@ -20,7 +20,7 @@ const UIStrings = {
   /** Description of a Lighthouse audit that tells the user that having an apple-touch-icon allows websites to include an app icon to their installed progressive web apps, similar to a native app. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. "apple-touch-icon" is an HTML attribute value and should not be translated. */
   description: 'For ideal appearance on iOS when users add a progressive web app to the home ' +
     'screen, define an `apple-touch-icon`. It must point to a non-transparent 192px (or 180px) ' +
-    'square PNG. [Learn More](https://developers.google.com/web/fundamentals/design-and-ux/browser-customization/).',
+    'square PNG. [Learn More](https://web.dev/apple-touch-icon/).',
   /** Warning that HTML attribute `apple-touch-icon-precomposed` should not be used in favor of `apple-touch-icon`.  "apple-touch-icon-precomposed" and "apple-touch-icon" are HTML attribute values and should not be translated. */
   precomposedWarning: '`apple-touch-icon-precomposed` is out of date; ' +
     '`apple-touch-icon` is preferred.',
diff --git a/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js b/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
index 26cc33c16..333fdd05e 100644
--- a/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
+++ b/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
@@ -93,8 +93,10 @@ class UnusedBytes extends Audit {
       // This was an asset that was inlined in a different resource type (e.g. HTML document).
       // Use the compression ratio of the resource to estimate the total transferred bytes.
       const transferSize = networkRecord.transferSize || 0;
-      const resourceSize = networkRecord.resourceSize;
-      const compressionRatio = resourceSize !== undefined ? (transferSize / resourceSize) : 1;
+      const resourceSize = networkRecord.resourceSize || 0;
+      // Get the compression ratio, if it's an invalid number, assume no compression.
+      const compressionRatio = Number.isFinite(resourceSize) && resourceSize > 0 ?
+        (transferSize / resourceSize) : 1;
       return Math.round(totalBytes * compressionRatio);
     }
   }
diff --git a/lighthouse-core/audits/byte-efficiency/unused-css-rules.js b/lighthouse-core/audits/byte-efficiency/unused-css-rules.js
index d2b7344d7..2bb3d3a6b 100644
--- a/lighthouse-core/audits/byte-efficiency/unused-css-rules.js
+++ b/lighthouse-core/audits/byte-efficiency/unused-css-rules.js
@@ -47,6 +47,10 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
    */
   static indexStylesheetsById(styles, networkRecords) {
     const indexedNetworkRecords = networkRecords
+        // Some phantom network records appear with a 0 resourceSize that aren't real.
+        // A network record that has no size data is just as good as no network record at all for our
+        // purposes, so we'll just filter them out. https://github.com/GoogleChrome/lighthouse/issues/9684#issuecomment-532381611
+        .filter(record => record.resourceSize > 0)
         .reduce((indexed, record) => {
           indexed[record.url] = record;
           return indexed;
diff --git a/lighthouse-core/lib/i18n/locales/en-US.json b/lighthouse-core/lib/i18n/locales/en-US.json
index eabd53c07..072110054 100644
--- a/lighthouse-core/lib/i18n/locales/en-US.json
+++ b/lighthouse-core/lib/i18n/locales/en-US.json
@@ -318,7 +318,7 @@
     "message": "`<video>` elements contain a `<track>` element with `[kind=\"description\"]`"
   },
   "lighthouse-core/audits/apple-touch-icon.js | description": {
-    "message": "For ideal appearance on iOS when users add a progressive web app to the home screen, define an `apple-touch-icon`. It must point to a non-transparent 192px (or 180px) square PNG. [Learn More](https://developers.google.com/web/fundamentals/design-and-ux/browser-customization/)."
+    "message": "For ideal appearance on iOS when users add a progressive web app to the home screen, define an `apple-touch-icon`. It must point to a non-transparent 192px (or 180px) square PNG. [Learn More](https://web.dev/apple-touch-icon/)."
   },
   "lighthouse-core/audits/apple-touch-icon.js | failureTitle": {
     "message": "Does not provide a valid `apple-touch-icon`"
diff --git a/lighthouse-core/lib/i18n/locales/en-XL.json b/lighthouse-core/lib/i18n/locales/en-XL.json
index 88ec3d321..618f5118c 100644
--- a/lighthouse-core/lib/i18n/locales/en-XL.json
+++ b/lighthouse-core/lib/i18n/locales/en-XL.json
@@ -318,7 +318,7 @@
     "message": "`<video>` êĺêḿêńt̂ś ĉón̂t́âín̂ á `<track>` êĺêḿêńt̂ ẃît́ĥ `[kind=\"description\"]`"
   },
   "lighthouse-core/audits/apple-touch-icon.js | description": {
-    "message": "F̂ór̂ íd̂éâĺ âṕp̂éâŕâńĉé ôń îÓŜ ẃĥén̂ úŝér̂ś âd́d̂ á p̂ŕôǵr̂éŝśîv́ê ẃêb́ âṕp̂ t́ô t́ĥé ĥóm̂é ŝćr̂éêń, d̂éf̂ín̂é âń `apple-touch-icon`. Ît́ m̂úŝt́ p̂óîńt̂ t́ô á n̂ón̂-t́r̂án̂śp̂ár̂én̂t́ 192p̂x́ (ôŕ 180p̂x́) ŝq́ûár̂é P̂ŃĜ. [Ĺêár̂ń M̂ór̂é](https://developers.google.com/web/fundamentals/design-and-ux/browser-customization/)."
+    "message": "F̂ór̂ íd̂éâĺ âṕp̂éâŕâńĉé ôń îÓŜ ẃĥén̂ úŝér̂ś âd́d̂ á p̂ŕôǵr̂éŝśîv́ê ẃêb́ âṕp̂ t́ô t́ĥé ĥóm̂é ŝćr̂éêń, d̂éf̂ín̂é âń `apple-touch-icon`. Ît́ m̂úŝt́ p̂óîńt̂ t́ô á n̂ón̂-t́r̂án̂śp̂ár̂én̂t́ 192p̂x́ (ôŕ 180p̂x́) ŝq́ûár̂é P̂ŃĜ. [Ĺêár̂ń M̂ór̂é](https://web.dev/apple-touch-icon/)."
   },
   "lighthouse-core/audits/apple-touch-icon.js | failureTitle": {
     "message": "D̂óêś n̂ót̂ ṕr̂óv̂íd̂é â v́âĺîd́ `apple-touch-icon`"
diff --git a/lighthouse-core/scripts/dogfood-lhci.sh b/lighthouse-core/scripts/dogfood-lhci.sh
new file mode 100755
index 000000000..af064aa99
--- /dev/null
+++ b/lighthouse-core/scripts/dogfood-lhci.sh
@@ -0,0 +1,52 @@
+#!/bin/bash
+
+set -euox pipefail
+
+# This script requires LHCI_CANARY_SERVER_URL and LHCI_CANARY_SERVER_TOKEN variables to be set.
+
+if [[ -z "$LHCI_CANARY_SERVER_TOKEN" ]]; then
+  echo "No server token available, skipping.";
+  exit 0;
+fi
+
+if [[ "$TRAVIS_NODE_VERSION" != "10" ]]; then
+  echo "Not running dogfood script on node versions other than 10";
+  exit 0;
+fi
+
+
+SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+LH_ROOT_DIR="$SCRIPT_DIR/../.."
+
+# Testing lhci takes time and the server ain't massive, we'll only run the tests if we touched files that affect the report.
+CHANGED_FILES=""
+if [[ "$CI" ]]; then
+  CHANGED_FILES=$(git --no-pager diff --name-only "$TRAVIS_COMMIT_RANGE")
+else
+  CHANGED_FILES=$(git --no-pager diff --name-only master)
+fi
+
+printf "Determined the following files have been touched:\n\n$CHANGED_FILES\n\n"
+
+if ! echo "$CHANGED_FILES" | grep -E 'report|lhci' > /dev/null; then
+  echo "No report files affected, skipping lhci checks."
+  exit 0
+fi
+
+# Generate an HTML report and copy into static-server directory for serving.
+yarn now-build
+cp ./dist/now/english/index.html ./lighthouse-cli/test/fixtures/lhci.report.html
+
+# Install LHCI
+npm install -g @lhci/cli@next
+# Start up a test server.
+yarn static-server &
+# Wait for the server to start before hitting it with data.
+sleep 10
+# Collect our LHCI results.
+lhci collect --url=http://localhost:10200/lhci.report.html
+# Upload the results to our canary server.
+lhci upload --serverBaseUrl="$LHCI_CANARY_SERVER_URL" --token="$LHCI_CANARY_SERVER_TOKEN"
+
+# Kill the static server from earlier.
+kill $!
diff --git a/package.json b/package.json
index e58dabd87..56c82ebe1 100644
--- a/package.json
+++ b/package.json
@@ -49,6 +49,7 @@
     "deploy-viewer": "yarn build-viewer --deploy",
     "now-build": "node lighthouse-core/scripts/build-report-for-autodeployment.js",
     "bundlesize": "bundlesize",
+    "dogfood-lhci": "./lighthouse-core/scripts/dogfood-lhci.sh",
     "timing-trace": "node lighthouse-core/scripts/generate-timing-trace.js",
     "changelog": "conventional-changelog --config ./build/changelog-generator/index.js --infile changelog.md --same-file",
     "type-check": "tsc -p . && tsc -p lighthouse-viewer/",
diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json
index ddaefce7f..c20f2fc2b 100644
--- a/proto/sample_v2_round_trip.json
+++ b/proto/sample_v2_round_trip.json
@@ -16,7 +16,7 @@
             "title": "Uses Application Cache"
         },
         "apple-touch-icon": {
-            "description": "For ideal appearance on iOS when users add a progressive web app to the home screen, define an `apple-touch-icon`. It must point to a non-transparent 192px (or 180px) square PNG. [Learn More](https://developers.google.com/web/fundamentals/design-and-ux/browser-customization/).",
+            "description": "For ideal appearance on iOS when users add a progressive web app to the home screen, define an `apple-touch-icon`. It must point to a non-transparent 192px (or 180px) square PNG. [Learn More](https://web.dev/apple-touch-icon/).",
             "id": "apple-touch-icon",
             "score": 0.0,
             "scoreDisplayMode": "binary",
diff --git a/types/smokehouse.d.ts b/types/smokehouse.d.ts
index 7cc91133e..aff855384 100644
--- a/types/smokehouse.d.ts
+++ b/types/smokehouse.d.ts
@@ -19,7 +19,7 @@
     diff?: Difference | null;
   }
 
-  export type ExpectedLHR = Pick<LH.Result, 'audits' | 'finalUrl' | 'requestedUrl' | 'runtimeError'>
+  export type ExpectedLHR = Pick<LH.Result, 'audits' | 'finalUrl' | 'requestedUrl' | 'runWarnings' | 'runtimeError'>
 
   export type ExpectedRunnerResult = {
     lhr: ExpectedLHR,


Test patch:
diff --git a/lighthouse-cli/test/smokehouse/error-expectations.js b/lighthouse-cli/test/smokehouse/error-expectations.js
index a77071d2f..b462fd665 100644
--- a/lighthouse-cli/test/smokehouse/error-expectations.js
+++ b/lighthouse-cli/test/smokehouse/error-expectations.js
@@ -20,6 +20,7 @@ module.exports = [
       requestedUrl: 'http://localhost:10200/infinite-loop.html',
       finalUrl: 'http://localhost:10200/infinite-loop.html',
       runtimeError: {code: 'PAGE_HUNG'},
+      runWarnings: ['Lighthouse was unable to reliably load the URL you requested because the page stopped responding.'],
       audits: {
         'first-contentful-paint': {
           scoreDisplayMode: 'error',
@@ -42,6 +43,7 @@ module.exports = [
       requestedUrl: 'https://expired.badssl.com',
       finalUrl: 'https://expired.badssl.com/',
       runtimeError: {code: 'INSECURE_DOCUMENT_REQUEST'},
+      runWarnings: ['The URL you have provided does not have a valid security certificate. net::ERR_CERT_DATE_INVALID'],
       audits: {
         'first-contentful-paint': {
           scoreDisplayMode: 'error',
diff --git a/lighthouse-cli/test/smokehouse/seo/expectations.js b/lighthouse-cli/test/smokehouse/seo/expectations.js
index 5544b6091..18c66eef5 100644
--- a/lighthouse-cli/test/smokehouse/seo/expectations.js
+++ b/lighthouse-cli/test/smokehouse/seo/expectations.js
@@ -270,6 +270,7 @@ module.exports = [
         code: 'ERRORED_DOCUMENT_REQUEST',
         message: /Status code: 403/,
       },
+      runWarnings: ['Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 403)'],
       audits: {
         'http-status-code': {
           score: null,
diff --git a/lighthouse-cli/test/smokehouse/smokehouse-report.js b/lighthouse-cli/test/smokehouse/smokehouse-report.js
index 37fe6d26a..421764e8d 100644
--- a/lighthouse-cli/test/smokehouse/smokehouse-report.js
+++ b/lighthouse-cli/test/smokehouse/smokehouse-report.js
@@ -143,6 +143,10 @@ function collateResults(actual, expected) {
   const runtimeErrorAssertion = makeComparison('runtimeError', actual.lhr.runtimeError,
       expected.lhr.runtimeError);
 
+  // Same for warnings.
+  const runWarningsAssertion = makeComparison('runWarnings', actual.lhr.runWarnings,
+      expected.lhr.runWarnings || []);
+
   /** @type {Smokehouse.Comparison[]} */
   let artifactAssertions = [];
   if (expected.artifacts) {
@@ -179,6 +183,7 @@ function collateResults(actual, expected) {
       equal: actual.lhr.finalUrl === expected.lhr.finalUrl,
     },
     runtimeErrorAssertion,
+    runWarningsAssertion,
     ...artifactAssertions,
     ...auditAssertions,
   ];
diff --git a/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js b/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js
index b6656de78..103ee8187 100644
--- a/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js
+++ b/lighthouse-core/test/audits/byte-efficiency/byte-efficiency-audit-test.js
@@ -77,6 +77,12 @@ describe('Byte efficiency base audit', () => {
       const result = estimate({transferSize: 1000, resourceType}, 100);
       assert.equal(result, 100);
     });
+
+    it('should not error when resource size is 0', () => {
+      const resourceType = 'Other';
+      const result = estimate({transferSize: 1000, resourceSize: 0, resourceType}, 100);
+      assert.equal(result, 100);
+    });
   });
 
   it('should format details', () => {
diff --git a/lighthouse-core/test/audits/byte-efficiency/unused-css-rules-test.js b/lighthouse-core/test/audits/byte-efficiency/unused-css-rules-test.js
index ff3c7c236..be8b64b28 100644
--- a/lighthouse-core/test/audits/byte-efficiency/unused-css-rules-test.js
+++ b/lighthouse-core/test/audits/byte-efficiency/unused-css-rules-test.js
@@ -122,6 +122,7 @@ describe('Best Practices: unused css rules audit', () => {
       {
         url: 'file://a.css',
         transferSize: 100 * 1024,
+        resourceSize: 100 * 1024,
         resourceType: 'Stylesheet',
       },
     ];
@@ -191,6 +192,36 @@ describe('Best Practices: unused css rules audit', () => {
       });
     });
 
+    it('handles phantom network records without size data', async () => {
+      const result = await UnusedCSSAudit.audit_(getArtifacts({
+        CSSUsage: {rules: [
+          {styleSheetId: 'a', used: true, startOffset: 0, endOffset: 60000}, // 40000 * 3 * 50% = 60000
+        ], stylesheets: [
+          {
+            header: {styleSheetId: 'a', sourceURL: 'file://a.html'},
+            content: `${generate('123', 40000)}`, // stylesheet size of 40000 * 3 uncompressed bytes
+          },
+        ]},
+      }), [
+        {
+          url: 'file://a.html',
+          transferSize: 100 * 1024 * 0.5, // compression ratio of 0.5
+          resourceSize: 100 * 1024,
+          resourceType: 'Document', // this is a document so it'll use the compressionRatio but not the raw size
+        },
+        {
+          url: 'file://a.html',
+          transferSize: 0,
+          resourceSize: 0,
+          resourceType: 'Document',
+        },
+      ]);
+
+      expect(result.items).toMatchObject([
+        {totalBytes: 40000 * 3 * 0.5, wastedPercent: 50},
+      ]);
+    });
+
     it('does not include empty or small sheets', () => {
       return UnusedCSSAudit.audit_(getArtifacts({
         CSSUsage: {rules: [
diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json
index a36375242..3ea69984e 100644
--- a/lighthouse-core/test/results/sample_v2.json
+++ b/lighthouse-core/test/results/sample_v2.json
@@ -488,7 +488,7 @@
     "apple-touch-icon": {
       "id": "apple-touch-icon",
       "title": "Does not provide a valid `apple-touch-icon`",
-      "description": "For ideal appearance on iOS when users add a progressive web app to the home screen, define an `apple-touch-icon`. It must point to a non-transparent 192px (or 180px) square PNG. [Learn More](https://developers.google.com/web/fundamentals/design-and-ux/browser-customization/).",
+      "description": "For ideal appearance on iOS when users add a progressive web app to the home screen, define an `apple-touch-icon`. It must point to a non-transparent 192px (or 180px) square PNG. [Learn More](https://web.dev/apple-touch-icon/).",
       "score": 0,
       "scoreDisplayMode": "binary",
       "warnings": []