core(network-request): consider secondary headers for content encoded…

… check (#15708)
GoogleChrome · Jan 5, 2024 · 2d43de0 · 2d43de0
1 parent 680ec8c
commit 2d43de0
Show file tree

Hide file tree

Showing 8 changed files with 45 additions and 31 deletions.
diff --git a/core/gather/gatherers/dobetterweb/response-compression.js b/core/gather/gatherers/dobetterweb/response-compression.js
@@ -23,12 +23,6 @@ import {fetchResponseBodyFromCache} from '../../driver/network.js';
 import {NetworkRecords} from '../../../computed/network-records.js';
 
 const CHROME_EXTENSION_PROTOCOL = 'chrome-extension:';
-const compressionHeaders = [
-  'content-encoding',
-  'x-original-content-encoding',
-  'x-content-encoding-over-network',
-];
-const compressionTypes = ['gzip', 'br', 'deflate'];
 const binaryMimeTypes = ['image', 'audio', 'video'];
 /** @type {LH.Crdp.Network.ResourceType[]} */
 const textResourceTypes = [
@@ -71,12 +65,7 @@ class ResponseCompression extends BaseGatherer {
         return;
       }
 
-      const isContentEncoded = (record.responseHeaders || []).find(header =>
-        compressionHeaders.includes(header.name.toLowerCase()) &&
-        compressionTypes.includes(header.value)
-      );
-
-      if (!isContentEncoded) {
+      if (!NetworkRequest.isContentEncoded(record)) {
         unoptimizedResponses.push({
           requestId: record.requestId,
           url: record.url,

diff --git a/core/lib/network-request.js b/core/lib/network-request.js
@@ -616,8 +616,16 @@ class NetworkRequest {
   static isContentEncoded(record) {
     // FYI: older devtools logs (like our test fixtures) seems to be lower case, while modern logs
     // are Cased-Like-This.
-    const pattern = /^content-encoding$/i;
-    return record.responseHeaders.some(item => item.name.match(pattern));
+    const patterns = global.isLightrider ? [
+      /^x-original-content-encoding$/i,
+    ] : [
+      /^content-encoding$/i,
+      /^x-content-encoding-over-network$/i,
+    ];
+    const compressionTypes = ['gzip', 'br', 'deflate'];
+    return record.responseHeaders.some(header =>
+      patterns.some(p => header.name.match(p)) && compressionTypes.includes(header.value)
+    );
   }
 
   /**

diff --git a/core/test/audits/byte-efficiency/unminified-javascript-test.js b/core/test/audits/byte-efficiency/unminified-javascript-test.js
@@ -14,7 +14,7 @@ const KB = 1024;
 const resourceType = 'Script';
 describe('Page uses optimized responses', () => {
   it('fails when given unminified scripts', () => {
-    const responseHeaders = [{name: 'Content-Encoding'}];
+    const responseHeaders = [{name: 'Content-Encoding', value: 'gzip'}];
     const commonRecord = {resourceType, responseHeaders};
     const auditResult = UnminifiedJavascriptAudit.audit_({
       URL: {finalDisplayedUrl: 'https://www.example.com'},

diff --git a/core/test/audits/byte-efficiency/unused-css-rules-test.js b/core/test/audits/byte-efficiency/unused-css-rules-test.js
@@ -106,6 +106,7 @@ describe('Best Practices: unused css rules audit', () => {
           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
+          responseHeaders: [{name: 'Content-Encoding', value: 'gzip'}],
         },
         {
           url: 'file://a.html',

diff --git a/core/test/audits/byte-efficiency/unused-javascript-test.js b/core/test/audits/byte-efficiency/unused-javascript-test.js
@@ -30,7 +30,9 @@ function getScriptId(url) {
  * @param {LH.Crdp.Network.ResourceType} resourceType
  */
 function generateRecord(url, transferSize, resourceType) {
-  return {url, transferSize, resourceType, responseHeaders: [{name: 'Content-Encoding'}]};
+  return {url, transferSize, resourceType, responseHeaders: [
+    {name: 'Content-Encoding', value: 'gzip'},
+  ]};
 }
 
 /**

diff --git a/core/test/computed/metrics/time-to-first-byte-test.js b/core/test/computed/metrics/time-to-first-byte-test.js
@@ -44,7 +44,7 @@ function mockNetworkRecords() {
     transferSize: 300,
     url: requestedUrl,
     frameId: 'ROOT_FRAME',
-    responseHeaders: [{name: 'Content-Encoding'}],
+    responseHeaders: [{name: 'Content-Encoding', value: 'gzip'}],
   },
   {
     requestId: '2:redirect',
@@ -58,7 +58,7 @@ function mockNetworkRecords() {
     transferSize: 16_000,
     url: mainDocumentUrl,
     frameId: 'ROOT_FRAME',
-    responseHeaders: [{name: 'Content-Encoding'}],
+    responseHeaders: [{name: 'Content-Encoding', value: 'gzip'}],
   }];
 }
 

diff --git a/core/test/lib/network-recorder-test.js b/core/test/lib/network-recorder-test.js
@@ -9,6 +9,7 @@ import assert from 'assert/strict';
 import {NetworkRecorder} from '../../lib/network-recorder.js';
 import {networkRecordsToDevtoolsLog} from '../network-records-to-devtools-log.js';
 import {readJson} from '../test-utils.js';
+import {NetworkRequest} from '../../lib/network-request.js';
 
 const devtoolsLogItems = readJson('../fixtures/artifacts/perflog/defaultPass.devtoolslog.json', import.meta);
 const prefetchedScriptDevtoolsLog = readJson('../fixtures/prefetched-script.devtoolslog.json', import.meta);
@@ -165,20 +166,33 @@ describe('network recorder', function() {
     expect(records).toMatchObject([{url: 'http://example.com', networkRequestTime: 1, networkEndTime: 2}]);
   });
 
-  it('should use X-TotalFetchedSize in Lightrider for transferSize', () => {
-    global.isLightrider = true;
-    const records = NetworkRecorder.recordsFromLogs(lrRequestDevtoolsLog);
-    global.isLightrider = false;
+  describe('Lightrider', () => {
+    let records;
+    before(() => {
+      global.isLightrider = true;
+      records = NetworkRecorder.recordsFromLogs(lrRequestDevtoolsLog);
+    });
+
+    it('should use X-TotalFetchedSize in Lightrider for transferSize', () => {
+      expect(records.find(r => r.url === 'https://www.paulirish.com/'))
+      .toMatchObject({
+        resourceSize: 75221,
+        transferSize: 22889,
+      });
+      expect(records.find(r => r.url === 'https://www.paulirish.com/javascripts/modernizr-2.0.js'))
+      .toMatchObject({
+        resourceSize: 9736,
+        transferSize: 4730,
+      });
+    });
 
-    expect(records.find(r => r.url === 'https://www.paulirish.com/'))
-    .toMatchObject({
-      resourceSize: 75221,
-      transferSize: 22889,
+    it('should use respect X-Original-Content-Encoding', () => {
+      const record = records.find(r => r.url === 'https://www.paulirish.com/javascripts/modernizr-2.0.js');
+      expect(NetworkRequest.isContentEncoded(record)).toBe(true);
     });
-    expect(records.find(r => r.url === 'https://www.paulirish.com/javascripts/modernizr-2.0.js'))
-    .toMatchObject({
-      resourceSize: 9736,
-      transferSize: 4730,
+
+    after(() => {
+      global.isLightrider = false;
     });
   });
 

diff --git a/core/test/lib/script-helpers-test.js b/core/test/lib/script-helpers-test.js
@@ -46,7 +46,7 @@ describe('Script helpers', () => {
 
   describe('#estimateCompressedContentSize', () => {
     const estimate = estimateCompressedContentSize;
-    const encoding = [{name: 'Content-Encoding'}];
+    const encoding = [{name: 'Content-Encoding', value: 'gzip'}];
 
     it('should estimate by resource type compression ratio when no network info available', () => {
       assert.equal(estimate(undefined, 1000, 'Stylesheet'), 200);