From 663eb1ab80a21a9e816b9f4f42ddae55731d1ada Mon Sep 17 00:00:00 2001 From: Adam Raine <6752989+adamraine@users.noreply.github.com> Date: Tue, 18 May 2021 19:08:13 -0400 Subject: [PATCH] core(fr): convert response-compression gatherer (#12508) --- .../fraggle-rock/config/default-config.js | 3 + .../dobetterweb/response-compression.js | 47 ++- .../test/fraggle-rock/api-test-pptr.js | 2 +- .../test/fraggle-rock/gather/mock-driver.js | 9 +- .../dobetterweb/response-compression-test.js | 336 +++++++++--------- types/artifacts.d.ts | 1 - 6 files changed, 209 insertions(+), 189 deletions(-) diff --git a/lighthouse-core/fraggle-rock/config/default-config.js b/lighthouse-core/fraggle-rock/config/default-config.js index 2a2c0a05d644..97acd6034de0 100644 --- a/lighthouse-core/fraggle-rock/config/default-config.js +++ b/lighthouse-core/fraggle-rock/config/default-config.js @@ -34,6 +34,7 @@ const artifacts = { NetworkUserAgent: '', OptimizedImages: '', PasswordInputsWithPreventedPaste: '', + ResponseCompression: '', RobotsTxt: '', SourceMaps: '', Stacks: '', @@ -79,6 +80,7 @@ const defaultConfig = { {id: artifacts.NetworkUserAgent, gatherer: 'network-user-agent'}, {id: artifacts.OptimizedImages, gatherer: 'dobetterweb/optimized-images'}, {id: artifacts.PasswordInputsWithPreventedPaste, gatherer: 'dobetterweb/password-inputs-with-prevented-paste'}, + {id: artifacts.ResponseCompression, gatherer: 'dobetterweb/response-compression'}, {id: artifacts.RobotsTxt, gatherer: 'seo/robots-txt'}, {id: artifacts.SourceMaps, gatherer: 'source-maps'}, {id: artifacts.Stacks, gatherer: 'stacks'}, @@ -126,6 +128,7 @@ const defaultConfig = { artifacts.NetworkUserAgent, artifacts.OptimizedImages, artifacts.PasswordInputsWithPreventedPaste, + artifacts.ResponseCompression, artifacts.RobotsTxt, artifacts.SourceMaps, artifacts.Stacks, diff --git a/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js b/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js index f166c8776d96..c2f2be701612 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js @@ -10,17 +10,20 @@ */ 'use strict'; -const Gatherer = require('../gatherer.js'); +const FRGatherer = require('../../../fraggle-rock/gather/base-gatherer.js'); const URL = require('../../../lib/url-shim.js'); const Sentry = require('../../../lib/sentry.js'); const NetworkRequest = require('../../../lib/network-request.js'); const gzip = require('zlib').gzip; +const DevtoolsLog = require('../devtools-log.js'); +const {fetchResponseBodyFromCache} = require('../../driver/network.js'); +const NetworkRecords = require('../../../computed/network-records.js'); const CHROME_EXTENSION_PROTOCOL = 'chrome-extension:'; const compressionHeaders = ['content-encoding', 'x-original-content-encoding']; const compressionTypes = ['gzip', 'br', 'deflate']; const binaryMimeTypes = ['image', 'audio', 'video']; -/** @type {Array} */ +/** @type {LH.Crdp.Network.ResourceType[]} */ const textResourceTypes = [ NetworkRequest.TYPES.Document, NetworkRequest.TYPES.Script, @@ -30,9 +33,15 @@ const textResourceTypes = [ NetworkRequest.TYPES.EventSource, ]; -class ResponseCompression extends Gatherer { +class ResponseCompression extends FRGatherer { + /** @type {LH.Gatherer.GathererMeta<'DevtoolsLog'>} */ + meta = { + supportedModes: ['timespan', 'navigation'], + dependencies: {DevtoolsLog: DevtoolsLog.symbol}, + } + /** - * @param {Array} networkRecords + * @param {LH.Artifacts.NetworkRequest[]} networkRecords * @return {LH.Artifacts['ResponseCompression']} */ static filterUnoptimizedResponses(networkRecords) { @@ -77,17 +86,16 @@ class ResponseCompression extends Gatherer { } /** - * @param {LH.Gatherer.PassContext} passContext - * @param {LH.Gatherer.LoadData} loadData + * @param {LH.Gatherer.FRTransitionalContext} context + * @param {LH.Artifacts.NetworkRequest[]} networkRecords * @return {Promise} */ - afterPass(passContext, loadData) { - const networkRecords = loadData.networkRecords; + async _getArtifact(context, networkRecords) { + const session = context.driver.defaultSession; const textRecords = ResponseCompression.filterUnoptimizedResponses(networkRecords); - const driver = passContext.driver; return Promise.all(textRecords.map(record => { - return driver.getRequestContent(record.requestId).then(content => { + return fetchResponseBodyFromCache(session, record.requestId).then(content => { // if we don't have any content, gzipSize is already set to 0 if (!content) { return record; @@ -117,6 +125,25 @@ class ResponseCompression extends Gatherer { }); })); } + + /** + * @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} context + * @return {Promise} + */ + async getArtifact(context) { + const devtoolsLog = context.dependencies.DevtoolsLog; + const networkRecords = await NetworkRecords.request(devtoolsLog, context); + return this._getArtifact(context, networkRecords); + } + + /** + * @param {LH.Gatherer.PassContext} passContext + * @param {LH.Gatherer.LoadData} loadData + * @return {Promise} + */ + async afterPass(passContext, loadData) { + return this._getArtifact({...passContext, dependencies: {}}, loadData.networkRecords); + } } module.exports = ResponseCompression; diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js index cfa55deee7a2..4445de6d5ea1 100644 --- a/lighthouse-core/test/fraggle-rock/api-test-pptr.js +++ b/lighthouse-core/test/fraggle-rock/api-test-pptr.js @@ -159,7 +159,7 @@ describe('Fraggle Rock API', () => { const {lhr} = result; const {auditResults, failedAudits, erroredAudits} = getAuditsBreakdown(lhr); // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached. - expect(auditResults.length).toMatchInlineSnapshot(`112`); + expect(auditResults.length).toMatchInlineSnapshot(`113`); expect(erroredAudits).toHaveLength(0); const failedAuditIds = failedAudits.map(audit => audit.id); diff --git a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js index 51f945bac460..0db3a3f1dd84 100644 --- a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js +++ b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js @@ -150,14 +150,16 @@ function createMockContext() { } function mockDriverSubmodules() { - const navigationMock = {gotoURL: jest.fn(), - }; + const navigationMock = {gotoURL: jest.fn()}; const prepareMock = { prepareTargetForNavigationMode: jest.fn(), prepareTargetForIndividualNavigation: jest.fn(), }; const storageMock = {clearDataForOrigin: jest.fn()}; const emulationMock = {clearThrottling: jest.fn()}; + const networkMock = { + fetchResponseBodyFromCache: jest.fn(), + }; function reset() { navigationMock.gotoURL = jest.fn().mockResolvedValue({finalUrl: 'https://example.com', warnings: [], timedOut: false}); @@ -165,6 +167,7 @@ function mockDriverSubmodules() { prepareMock.prepareTargetForIndividualNavigation = jest.fn().mockResolvedValue({warnings: []}); storageMock.clearDataForOrigin = jest.fn(); emulationMock.clearThrottling = jest.fn(); + networkMock.fetchResponseBodyFromCache = jest.fn().mockResolvedValue(''); } /** @@ -178,6 +181,7 @@ function mockDriverSubmodules() { jest.mock('../../../gather/driver/navigation.js', () => new Proxy(navigationMock, {get})); jest.mock('../../../gather/driver/prepare.js', () => new Proxy(prepareMock, {get})); jest.mock('../../../gather/driver/storage.js', () => new Proxy(storageMock, {get})); + jest.mock('../../../gather/driver/network.js', () => new Proxy(networkMock, {get})); jest.mock('../../../lib/emulation.js', () => new Proxy(emulationMock, {get})); return { @@ -185,6 +189,7 @@ function mockDriverSubmodules() { prepareMock, storageMock, emulationMock, + networkMock, reset, }; } diff --git a/lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js b/lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js index 154068762328..f921584cc172 100644 --- a/lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js +++ b/lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js @@ -7,194 +7,180 @@ /* eslint-env jest */ -const ResponseCompression = - require('../../../../gather/gatherers/dobetterweb/response-compression.js'); -const assert = require('assert').strict; -const mockDriver = require('../../fake-driver.js'); +const { + createMockContext, + mockDriverSubmodules, +} = require('../../../fraggle-rock/gather/mock-driver.js'); +const mocks = mockDriverSubmodules(); +const ResponseCompression + = require('../../../../gather/gatherers/dobetterweb/response-compression.js'); -let options; -let responseCompression; -const traceData = { - networkRecords: [ - { - url: 'http://google.com/index.js', - statusCode: 200, - mimeType: 'text/javascript', - requestId: 0, - resourceSize: 9, - transferSize: 10, - resourceType: 'Script', - responseHeaders: [{ - name: 'Content-Encoding', - value: 'gzip', - }], - content: 'aaabbbccc', - finished: true, - }, - { - url: 'http://google.com/index.css', - statusCode: 200, - mimeType: 'text/css', - requestId: 1, - resourceSize: 6, - transferSize: 7, - resourceType: 'Stylesheet', - responseHeaders: [], - content: 'abcabc', - finished: true, - }, - { - url: 'http://google.com/index.json', - statusCode: 200, - mimeType: 'application/json', - requestId: 2, - resourceSize: 7, - transferSize: 8, - resourceType: 'XHR', - responseHeaders: [], - content: '1234567', - finished: true, - }, - { - url: 'http://google.com/index.json', - statusCode: 200, - mimeType: 'application/json', - requestId: 27, - resourceSize: 7, - transferSize: 8, - resourceType: 'XHR', - responseHeaders: [], - content: '1234567', - finished: true, - sessionId: 'oopif', // ignore for being from oopif - }, - { - url: 'http://google.com/index.json', - statusCode: 304, // ignore for being a cache not modified response - mimeType: 'application/json', - requestId: 22, - resourceSize: 7, - transferSize: 7, - resourceType: 'XHR', - responseHeaders: [], - content: '1234567', - finished: true, - }, - { - url: 'http://google.com/other.json', - statusCode: 200, - mimeType: 'application/json', - requestId: 23, - resourceSize: 7, - transferSize: 8, - resourceType: 'XHR', - responseHeaders: [], - content: '1234567', - finished: false, // ignore for not finishing - }, - { - url: 'http://google.com/index.jpg', - statusCode: 200, - mimeType: 'image/jpg', - requestId: 3, - resourceSize: 10, - transferSize: 10, - resourceType: 'Image', - responseHeaders: [], - content: 'aaaaaaaaaa', - finished: true, - }, - { - url: 'http://google.com/helloworld.mp4', - statusCode: 200, - mimeType: 'video/mp4', - requestId: 4, - resourceSize: 100, - transferSize: 100, - resourceType: 'Media', - responseHeaders: [], - content: 'bbbbbbbb', - finished: true, - }, - ], -}; +const networkRecords = [ + { + url: 'http://google.com/index.js', + statusCode: 200, + mimeType: 'text/javascript', + requestId: 0, + resourceSize: 9, + transferSize: 10, + resourceType: 'Script', + responseHeaders: [{ + name: 'Content-Encoding', + value: 'gzip', + }], + content: 'aaabbbccc', + finished: true, + }, + { + url: 'http://google.com/index.css', + statusCode: 200, + mimeType: 'text/css', + requestId: 1, + resourceSize: 6, + transferSize: 7, + resourceType: 'Stylesheet', + responseHeaders: [], + content: 'abcabc', + finished: true, + }, + { + url: 'http://google.com/index.json', + statusCode: 200, + mimeType: 'application/json', + requestId: 2, + resourceSize: 7, + transferSize: 8, + resourceType: 'XHR', + responseHeaders: [], + content: '1234567', + finished: true, + }, + { + url: 'http://google.com/index.json', + statusCode: 200, + mimeType: 'application/json', + requestId: 27, + resourceSize: 7, + transferSize: 8, + resourceType: 'XHR', + responseHeaders: [], + content: '1234567', + finished: true, + sessionId: 'oopif', // ignore for being from oopif + }, + { + url: 'http://google.com/index.json', + statusCode: 304, // ignore for being a cache not modified response + mimeType: 'application/json', + requestId: 22, + resourceSize: 7, + transferSize: 7, + resourceType: 'XHR', + responseHeaders: [], + content: '1234567', + finished: true, + }, + { + url: 'http://google.com/other.json', + statusCode: 200, + mimeType: 'application/json', + requestId: 23, + resourceSize: 7, + transferSize: 8, + resourceType: 'XHR', + responseHeaders: [], + content: '1234567', + finished: false, // ignore for not finishing + }, + { + url: 'http://google.com/index.jpg', + statusCode: 200, + mimeType: 'image/jpg', + requestId: 3, + resourceSize: 10, + transferSize: 10, + resourceType: 'Image', + responseHeaders: [], + content: 'aaaaaaaaaa', + finished: true, + }, + { + url: 'http://google.com/helloworld.mp4', + statusCode: 200, + mimeType: 'video/mp4', + requestId: 4, + resourceSize: 100, + transferSize: 100, + resourceType: 'Media', + responseHeaders: [], + content: 'bbbbbbbb', + finished: true, + }, +]; describe('Optimized responses', () => { - // Reset the Gatherer before each test. + let context; + /** @type {ResponseCompression} */ + let gatherer; beforeEach(() => { - responseCompression = new ResponseCompression(); - const driver = Object.assign({}, mockDriver, { - getRequestContent(id) { - return Promise.resolve(traceData.networkRecords[id].content); - }, + gatherer = new ResponseCompression(); + context = createMockContext(); + mocks.reset(); + mocks.networkMock.fetchResponseBodyFromCache.mockImplementation((_, id) => { + return Promise.resolve(networkRecords[id].content); }); - - options = { - url: 'http://google.com/', - driver, - }; }); - it('returns only text and non encoded responses', () => { - return responseCompression.afterPass(options, traceData) - .then(artifact => { - assert.equal(artifact.length, 2); - assert.ok(/index\.css$/.test(artifact[0].url)); - assert.ok(/index\.json$/.test(artifact[1].url)); - }); + it('returns only text and non encoded responses', async () => { + const artifact = await gatherer._getArtifact(context, networkRecords); + expect(artifact).toHaveLength(2); + expect(artifact[0].url).toMatch(/index\.css$/); + expect(artifact[1].url).toMatch(/index\.json$/); }); - it('computes sizes', () => { - return responseCompression.afterPass(options, traceData) - .then(artifact => { - assert.equal(artifact.length, 2); - assert.equal(artifact[0].resourceSize, 6); - assert.equal(artifact[0].gzipSize, 26); - }); + it('computes sizes', async () => { + const artifact = await gatherer._getArtifact(context, networkRecords); + expect(artifact).toHaveLength(2); + expect(artifact[0].resourceSize).toEqual(6); + expect(artifact[0].gzipSize).toEqual(26); }); - it('recovers from driver errors', () => { - options.driver.getRequestContent = () => Promise.reject(new Error('Failed')); - return responseCompression.afterPass(options, traceData) - .then(artifact => { - assert.equal(artifact.length, 2); - assert.equal(artifact[0].resourceSize, 6); - assert.equal(artifact[0].gzipSize, undefined); - }); + it('recovers from driver errors', async () => { + mocks.networkMock.fetchResponseBodyFromCache.mockRejectedValue(new Error('Failed')); + const artifact = await gatherer._getArtifact(context, networkRecords); + expect(artifact).toHaveLength(2); + expect(artifact[0].resourceSize).toEqual(6); + expect(artifact[0].gzipSize).toBeUndefined(); }); - it('ignores responses from installed Chrome extensions', () => { - const traceData = { - networkRecords: [ - { - url: 'chrome-extension://index.css', - mimeType: 'text/css', - requestId: 1, - resourceSize: 10, - transferSize: 10, - resourceType: 'Stylesheet', - responseHeaders: [], - content: 'aaaaaaaaaa', - finished: true, - }, - { - url: 'http://google.com/chrome-extension.css', - mimeType: 'text/css', - requestId: 1, - resourceSize: 123, - transferSize: 123, - resourceType: 'Stylesheet', - responseHeaders: [], - content: 'aaaaaaaaaa', - finished: true, - }, - ], - }; + it('ignores responses from installed Chrome extensions', async () => { + const networkRecords = [ + { + url: 'chrome-extension://index.css', + mimeType: 'text/css', + requestId: 1, + resourceSize: 10, + transferSize: 10, + resourceType: 'Stylesheet', + responseHeaders: [], + content: 'aaaaaaaaaa', + finished: true, + }, + { + url: 'http://google.com/chrome-extension.css', + mimeType: 'text/css', + requestId: 1, + resourceSize: 123, + transferSize: 123, + resourceType: 'Stylesheet', + responseHeaders: [], + content: 'aaaaaaaaaa', + finished: true, + }, + ]; - return responseCompression.afterPass(options, traceData) - .then(artifact => { - assert.equal(artifact.length, 1); - assert.equal(artifact[0].resourceSize, 123); - }); + const artifact = await gatherer._getArtifact(context, networkRecords); + expect(artifact).toHaveLength(1); + expect(artifact[0].resourceSize).toEqual(123); }); }); diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index cdf50ba89e05..df157bd4c6b0 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -26,7 +26,6 @@ declare global { | 'InspectorIssues' | 'Manifest' | 'MixedContent' - | 'ResponseCompression' | 'ScriptElements' | 'ServiceWorker' | 'TagsBlockingFirstPaint'