core(legacy-javascript): exclude header size for estimating wasted bytes

core/audits/byte-efficiency/byte-efficiency-audit.js

@@ -13,6 +13,7 @@
 import {LanternLargestContentfulPaint} from '../../computed/metrics/lantern-largest-contentful-paint.js';
 import {LanternFirstContentfulPaint} from '../../computed/metrics/lantern-first-contentful-paint.js';
 import {LCPImageRecord} from '../../computed/lcp-image-record.js';
+import {NetworkRequest} from '../../lib/network-request.js';
 
 const str_ = i18n.createIcuMessageFn(import.meta.url, {});
 
@@ -100,6 +101,65 @@
     }
   }
 
+  /**
+   * Estimates the number of bytes the content of this network record would have consumed on the network based on the
+   * uncompressed size (totalBytes). Uses the actual transfer size from the network record if applicable,
+   * minus the size of the response headers.
+   *
+   * This differs from `estimateTransferSize` only in that is subtracts the response headers from the estimate.
+   *
+   * @param {LH.Artifacts.NetworkRequest|undefined} networkRecord
+   * @param {number} totalBytes Uncompressed size of the resource
+   * @param {LH.Crdp.Network.ResourceType=} resourceType
+   * @return {number}
+   */
+  static estimateCompressedContentSize(networkRecord, totalBytes, resourceType) {
+    if (!networkRecord) {
+      // We don't know how many bytes this asset used on the network, but we can guess it was
+      // roughly the size of the content gzipped.
+      // See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/optimize-encoding-and-transfer for specific CSS/Script examples
+      // See https://discuss.httparchive.org/t/file-size-and-compression-savings/145 for fallback multipliers
+      switch (resourceType) {
+        case 'Stylesheet':
+          // Stylesheets tend to compress extremely well.
+          return Math.round(totalBytes * 0.2);
+        case 'Script':
+        case 'Document':
+          // Scripts and HTML compress fairly well too.
+          return Math.round(totalBytes * 0.33);
+        default:
+          // Otherwise we'll just fallback to the average savings in HTTPArchive
+          return Math.round(totalBytes * 0.5);
+      }
+    }
+
+    // Get the size of the response body on the network.
+    let contentTransferSize = networkRecord.transferSize || 0;
+    if (!NetworkRequest.isContentEncoded(networkRecord)) {
+      // This is not encoded, so we can use resourceSize directly.
+      // This would be equivalent to transfer size minus headers transfer size, but transfer size
+      // may also include bytes for SSL connection etc.
+      contentTransferSize = networkRecord.resourceSize;
+    } else if (networkRecord.responseHeadersTransferSize) {
+      // Subtract the size of the encoded headers.
+      contentTransferSize =
+        Math.max(0, contentTransferSize - networkRecord.responseHeadersTransferSize);
+    }
+
+    if (networkRecord.resourceType === resourceType) {
+      // This was a regular standalone asset, just use the transfer size.
+      return contentTransferSize;
+    } else {
+      // 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 resourceSize = networkRecord.resourceSize || 0;
+      // Get the compression ratio, if it's an invalid number, assume no compression.
+      const compressionRatio = Number.isFinite(resourceSize) && resourceSize > 0 ?
+        (contentTransferSize / resourceSize) : 1;
+      return Math.round(totalBytes * compressionRatio);
+    }
+  }
+
   /**
    * @param {LH.Artifacts} artifacts
    * @param {LH.Audit.Context} context

core/audits/byte-efficiency/legacy-javascript.js

@@ -391,34 +391,39 @@
   }
 
   /**
-   * Utility function to estimate transfer size and cache calculation.
+   * Utility function to estimate the ratio of the compression on the resource.
+   * This excludes the size of the response headers.
+   * Also caches the calculation.
    *
    * Note: duplicated-javascript does this exact thing. In the future, consider
-   * making a generic estimator on ByteEfficienyAudit.
-   * @param {Map<string, number>} transferRatioByUrl
+   * making a generic estimator on ByteEfficiencyAudit.
+   * @param {Map<string, number>} compressionRatioByUrl
    * @param {string} url
    * @param {LH.Artifacts} artifacts
    * @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
    */
-  static async estimateTransferRatioForScript(transferRatioByUrl, url, artifacts, networkRecords) {
-    let transferRatio = transferRatioByUrl.get(url);
-    if (transferRatio !== undefined) return transferRatio;
+  static async estimateCompressionRatioForContent(compressionRatioByUrl, url,
+      artifacts, networkRecords) {
+    let compressionRatio = compressionRatioByUrl.get(url);
+    if (compressionRatio !== undefined) return compressionRatio;
 
     const script = artifacts.Scripts.find(script => script.url === url);
 
-    if (!script || script.content === null) {
+    if (!script) {
       // Can't find content, so just use 1.
-      transferRatio = 1;
+      compressionRatio = 1;
     } else {
       const networkRecord = getRequestForScript(networkRecords, script);
-      const contentLength = script.length || 0;
-      const transferSize =
-        ByteEfficiencyAudit.estimateTransferSize(networkRecord, contentLength, 'Script');
-      transferRatio = transferSize / contentLength;
+      const contentLength = networkRecord?.resourceSize ?
+        networkRecord.resourceSize :
+        script.length || 0;
+      const compressedSize =
+        ByteEfficiencyAudit.estimateCompressedContentSize(networkRecord, contentLength, 'Script');
+      compressionRatio = compressedSize / contentLength;
     }
 
-    transferRatioByUrl.set(url, transferRatio);
-    return transferRatio;
+    compressionRatioByUrl.set(url, compressionRatio);
+    return compressionRatio;
   }
 
   /**
@@ -443,14 +448,14 @@
     ]);
 
     /** @type {Map<string, number>} */
-    const transferRatioByUrl = new Map();
+    const compressionRatioByUrl = new Map();
 
     const scriptToMatchResults =
       this.detectAcrossScripts(matcher, artifacts.Scripts, networkRecords, bundles);
     for (const [script, matches] of scriptToMatchResults.entries()) {
-      const transferRatio = await this.estimateTransferRatioForScript(
-        transferRatioByUrl, script.url, artifacts, networkRecords);
-      const wastedBytes = Math.round(this.estimateWastedBytes(matches) * transferRatio);
+      const compressionRatio = await this.estimateCompressionRatioForContent(
+        compressionRatioByUrl, script.url, artifacts, networkRecords);
+      const wastedBytes = Math.round(this.estimateWastedBytes(matches) * compressionRatio);
       /** @type {typeof items[number]} */
       const item = {
         url: script.url,

core/lib/network-request.js

@@ -135,6 +135,7 @@ class NetworkRequest {
 
     // Go read the comment on _updateTransferSizeForLightrider.
     this.transferSize = 0;
+    this.responseHeadersTransferSize = 0;
     this.resourceSize = 0;
     this.fromDiskCache = false;
     this.fromMemoryCache = false;
@@ -344,6 +345,7 @@ class NetworkRequest {
     this.responseHeadersEndTime = timestamp * 1000;
 
     this.transferSize = response.encodedDataLength;
+    this.responseHeadersTransferSize = response.encodedDataLength;
     if (typeof response.fromDiskCache === 'boolean') this.fromDiskCache = response.fromDiskCache;
     if (typeof response.fromPrefetchCache === 'boolean') {
       this.fromPrefetchCache = response.fromPrefetchCache;
@@ -600,6 +602,15 @@ class NetworkRequest {
     return reason === 'HSTS' && NetworkRequest.isSecureRequest(destination);
   }
 
+  /**
+   * Returns whether the network request was sent encoded.
+   * @param {NetworkRequest} record
+   * @return {boolean}
+   */
+  static isContentEncoded(record) {
+    return record.responseHeaders.some(item => item.name === 'Content-Encoding');
+  }
+
   /**
    * Resource size is almost always the right one to be using because of the below:
    *     `transferSize = resourceSize + headers.length`.

core/test/audits/byte-efficiency/byte-efficiency-audit-test.js

@@ -123,6 +123,57 @@ describe('Byte efficiency base audit', () => {
     });
   });
 
+  describe('#estimateCompressedContentSize', () => {
+    const estimate = ByteEfficiencyAudit.estimateCompressedContentSize;
+    const encoding = [{name: 'Content-Encoding'}];
+
+    it('should estimate by resource type compression ratio when no network info available', () => {
+      assert.equal(estimate(undefined, 1000, 'Stylesheet'), 200);
+      assert.equal(estimate(undefined, 1000, 'Script'), 330);
+      assert.equal(estimate(undefined, 1000, 'Document'), 330);
+      assert.equal(estimate(undefined, 1000, ''), 500);
+    });
+
+    it('should return transferSize when asset matches and is encoded', () => {
+      const resourceType = 'Stylesheet';
+      const result = estimate(
+        {transferSize: 1234, resourceType, responseHeaders: encoding},
+        10000, 'Stylesheet');
+      assert.equal(result, 1234);
+    });
+
+    it('should return resourceSize when asset matches and is not encoded', () => {
+      const resourceType = 'Stylesheet';
+      const result = estimate(
+        {transferSize: 1235, resourceSize: 1234, resourceType, responseHeaders: []},
+        10000, 'Stylesheet');
+      assert.equal(result, 1234);
+    });
+
+    // Ex: JS script embedded in HTML response.
+    it('should estimate by network compression ratio when asset does not match', () => {
+      const resourceType = 'Other';
+      const result = estimate(
+        {resourceSize: 2000, transferSize: 1000, resourceType, responseHeaders: encoding},
+        100);
+      assert.equal(result, 50);
+    });
+
+    it('should not error when missing resource size', () => {
+      const resourceType = 'Other';
+      const result = estimate({transferSize: 1000, resourceType, responseHeaders: []}, 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, responseHeaders: []},
+        100);
+      assert.equal(result, 100);
+    });
+  });
+
   it('should format details', async () => {
     const result = await ByteEfficiencyAudit.createAuditProduct({
       headings: baseHeadings,

core/test/audits/byte-efficiency/legacy-javascript-test.js

@@ -19,6 +19,7 @@ const getResult = scripts => {
     ...scripts.map(({url}, index) => ({
       requestId: String(index),
       url,
+      responseHeaders: [],
     })),
   ];
   const artifacts = {

core/test/computed/metrics/time-to-first-byte-test.js

@@ -44,6 +44,7 @@ function mockNetworkRecords() {
     transferSize: 300,
     url: requestedUrl,
     frameId: 'ROOT_FRAME',
+    responseHeaders: [{name: 'Content-Encoding'}],
   },
   {
     requestId: '2:redirect',
@@ -57,6 +58,7 @@ function mockNetworkRecords() {
     transferSize: 16_000,
     url: mainDocumentUrl,
     frameId: 'ROOT_FRAME',
+    responseHeaders: [{name: 'Content-Encoding'}],
   }];
 }
 

core/test/fixtures/user-flows/reports/sample-flow-result.json

@@ -3286,7 +3286,7 @@
               "items": [
                 {
                   "url": "https://www.mikescerealshack.co/_next/static/chunks/commons.49455e4fa8cc3f51203f.js",
-                  "wastedBytes": 57,
+                  "wastedBytes": 167,
                   "subItems": {
                     "type": "subitems",
                     "items": [
@@ -3306,7 +3306,7 @@
                 }
               ],
               "overallSavingsMs": 0,
-              "overallSavingsBytes": 57,
+              "overallSavingsBytes": 167,
               "sortedBy": [
                 "wastedBytes"
               ],
@@ -8034,7 +8034,7 @@
             "core/lib/i18n/i18n.js | displayValueByteSavings": [
               {
                 "values": {
-                  "wastedBytes": 57
+                  "wastedBytes": 167
                 },
                 "path": "audits[legacy-javascript].displayValue"
               }
@@ -20850,7 +20850,7 @@
             "scoreDisplayMode": "metricSavings",
             "numericValue": 0,
             "numericUnit": "millisecond",
-            "displayValue": "",
+            "displayValue": "Potential savings of 0 KiB",
             "metricSavings": {
               "FCP": 0,
               "LCP": 0
@@ -20884,7 +20884,7 @@
               "items": [
                 {
                   "url": "https://www.mikescerealshack.co/_next/static/chunks/commons.49455e4fa8cc3f51203f.js",
-                  "wastedBytes": 0,
+                  "wastedBytes": 167,
                   "subItems": {
                     "type": "subitems",
                     "items": [
@@ -20904,7 +20904,7 @@
                 }
               ],
               "overallSavingsMs": 0,
-              "overallSavingsBytes": 0,
+              "overallSavingsBytes": 167,
               "sortedBy": [
                 "wastedBytes"
               ],
@@ -25629,6 +25629,12 @@
                   "wastedBytes": 52102
                 },
                 "path": "audits[uses-responsive-images].displayValue"
+              },
+              {
+                "values": {
+                  "wastedBytes": 167
+                },
+                "path": "audits[legacy-javascript].displayValue"
               }
             ],
             "core/lib/i18n/i18n.js | columnResourceSize": [

core/test/network-records-to-devtools-log-test.js

@@ -57,7 +57,12 @@ describe('networkRecordsToDevtoolsLog', () => {
     const roundTripLogs = networkRecordsToDevtoolsLog(records, {skipVerification: true});
     const roundTripRecords = NetworkRecorder.recordsFromLogs(roundTripLogs);
 
-    expect(roundTripRecords).toEqual(records);
+    // First compare element-wise, as doing all at once results in too verbose an error message.
+    const len = Math.min(roundTripRecords.length, records.length);
+    for (let i = 0; i < len; i++) {
+      expect(roundTripRecords[i]).toEqual(records[i]);
+    }
+    expect(roundTripRecords.length).toEqual(records.length);
   });
 
   it('should roundtrip fake network records multiple times', () => {

core/test/network-records-to-devtools-log.js

@@ -288,8 +288,7 @@ function getResponseReceivedEvent(networkRecord, index, normalizedTiming) {
         connectionId: networkRecord.connectionId || 140,
         fromDiskCache: networkRecord.fromDiskCache || false,
         fromServiceWorker: networkRecord.fetchedViaServiceWorker || false,
-        encodedDataLength: networkRecord.transferSize === undefined ?
-          0 : networkRecord.transferSize,
+        encodedDataLength: networkRecord.responseHeadersTransferSize || networkRecord.transferSize,
         timing: {...normalizedTiming.timing},
         protocol: networkRecord.protocol || 'http/1.1',
       },

core/test/results/sample_v2.json

@@ -4975,7 +4975,7 @@
         "items": [
           {
             "url": "http://localhost:10200/dobetterweb/third_party/aggressive-promise-polyfill.js",
-            "wastedBytes": 26622,
+            "wastedBytes": 26585,
             "subItems": {
               "type": "subitems",
               "items": [
@@ -5005,7 +5005,7 @@
           }
         ],
         "overallSavingsMs": 450,
-        "overallSavingsBytes": 26622,
+        "overallSavingsBytes": 26585,
         "sortedBy": [
           "wastedBytes"
         ],
@@ -10473,7 +10473,7 @@
         },
         {
           "values": {
-            "wastedBytes": 26622
+            "wastedBytes": 26585
           },
           "path": "audits[legacy-javascript].displayValue"
         }