From db2265dd550c3d509be190ca2d3ac398d271e5da Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 28 Jun 2018 16:38:44 -0700 Subject: [PATCH] core: convert requestIds before sending to backend (#5580) --- lighthouse-core/gather/driver.js | 3 +++ .../gatherers/dobetterweb/optimized-images.js | 3 +++ lighthouse-core/lib/network-request.js | 10 ++++++++++ lighthouse-core/test/gather/driver-test.js | 2 +- .../gatherers/dobetterweb/optimized-images-test.js | 13 +++++++++++++ 5 files changed, 30 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index ad218c159ab1..89915e2e17dc 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -9,6 +9,7 @@ const NetworkRecorder = require('../lib/network-recorder'); const emulation = require('../lib/emulation'); const Element = require('../lib/element'); const LHError = require('../lib/errors'); +const NetworkRequest = require('../lib/network-request'); const EventEmitter = require('events').EventEmitter; const URL = require('../lib/url-shim'); const TraceParser = require('../lib/traces/trace-parser'); @@ -727,6 +728,8 @@ class Driver { * @return {Promise} */ getRequestContent(requestId, timeout = 1000) { + requestId = NetworkRequest.getRequestIdForBackend(requestId); + return new Promise((resolve, reject) => { // If this takes more than 1s, reject the Promise. // Why? Encoding issues can lead to hanging getResponseBody calls: https://github.com/GoogleChrome/lighthouse/pull/4718 diff --git a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js index d93138339c62..e0ccd65887e3 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js @@ -12,6 +12,7 @@ const Gatherer = require('../gatherer'); const URL = require('../../../lib/url-shim'); +const NetworkRequest = require('../../../lib/network-request'); const Sentry = require('../../../lib/sentry'); const Driver = require('../../driver.js'); // eslint-disable-line no-unused-vars @@ -114,6 +115,8 @@ class OptimizedImages extends Gatherer { * @return {Promise} */ _getEncodedResponse(driver, requestId, encoding) { + requestId = NetworkRequest.getRequestIdForBackend(requestId); + const quality = encoding === 'jpeg' ? JPEG_QUALITY : WEBP_QUALITY; const params = {requestId, encoding, quality, sizeOnly: true}; return driver.sendCommand('Audits.getEncodedResponse', params); diff --git a/lighthouse-core/lib/network-request.js b/lighthouse-core/lib/network-request.js index 42926ad31d0d..caac62ae1798 100644 --- a/lighthouse-core/lib/network-request.js +++ b/lighthouse-core/lib/network-request.js @@ -264,6 +264,16 @@ module.exports = class NetworkRequest { this._responseReceivedTime = Math.min(this.endTime, this._responseReceivedTime); } + /** + * Convert the requestId to backend-version by removing the `:redirect` portion + * + * @param {string} requestId + * @return {string} + */ + static getRequestIdForBackend(requestId) { + return requestId.replace(/(:redirect)+$/, ''); + } + /** * Based on DevTools NetworkManager. * @see https://github.com/ChromeDevTools/devtools-frontend/blob/3415ee28e86a3f4bcc2e15b652d22069938df3a6/front_end/sdk/NetworkManager.js#L285-L297 diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 929075564286..2b18e7573dbc 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -145,7 +145,7 @@ describe('Browser Driver', () => { }); it('throws if getRequestContent takes too long', () => { - return driverStub.getRequestContent(0, MAX_WAIT_FOR_PROTOCOL).then(_ => { + return driverStub.getRequestContent('', MAX_WAIT_FOR_PROTOCOL).then(_ => { assert.ok(false, 'long-running getRequestContent supposed to reject'); }, e => { assert.equal(e.code, 'REQUEST_CONTENT_TIMEOUT'); diff --git a/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js b/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js index 546c409e1808..e93bae9e264d 100644 --- a/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js +++ b/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js @@ -20,6 +20,7 @@ const fakeImageStats = { const traceData = { networkRecords: [ { + _requestId: '123.5', _url: 'http://google.com/image.jpg', _mimeType: 'image/jpeg', _resourceSize: 10000, @@ -28,6 +29,7 @@ const traceData = { finished: true, }, { + _requestId: '123.6:redirect', _url: 'http://google.com/transparent.png', _mimeType: 'image/png', _resourceSize: 11000, @@ -36,6 +38,7 @@ const traceData = { finished: true, }, { + _requestId: '123.5', _url: 'http://google.com/image.bmp', _mimeType: 'image/bmp', _resourceSize: 12000, @@ -44,6 +47,7 @@ const traceData = { finished: true, }, { + _requestId: '123.5', _url: 'http://google.com/image.bmp', _mimeType: 'image/bmp', _resourceSize: 12000, @@ -52,6 +56,7 @@ const traceData = { finished: true, }, { + _requestId: '123.5', _url: 'http://google.com/vector.svg', _mimeType: 'image/svg+xml', _resourceSize: 13000, @@ -60,6 +65,7 @@ const traceData = { finished: true, }, { + _requestId: '123.5', _url: 'http://gmail.com/image.jpg', _mimeType: 'image/jpeg', _resourceSize: 15000, @@ -68,6 +74,7 @@ const traceData = { finished: true, }, { + _requestId: '123.5', _url: 'data: image/jpeg ; base64 ,SgVcAT32587935321...', _mimeType: 'image/jpeg', _resourceType: 'Image', @@ -76,6 +83,7 @@ const traceData = { finished: true, }, { + _requestId: '123.5', _url: 'http://google.com/big-image.bmp', _mimeType: 'image/bmp', _resourceType: 'Image', @@ -84,6 +92,7 @@ const traceData = { finished: false, // ignore for not finishing }, { + _requestId: '123.5', _url: 'http://google.com/not-an-image.bmp', _mimeType: 'image/bmp', _resourceType: 'Document', // ignore for not really being an image @@ -162,7 +171,9 @@ describe('Optimized images', () => { }); it('supports Audits.getEncodedResponse', () => { + const calls = []; options.driver.sendCommand = (method, params) => { + calls.push({method, params}); const encodedSize = params.encoding === 'webp' ? 60 : 80; return Promise.resolve({encodedSize}); }; @@ -174,6 +185,8 @@ describe('Optimized images', () => { assert.equal(artifact[0].jpegSize, 80); // supports cross-origin assert.ok(/gmail.*image.jpg/.test(artifact[3].url)); + // strips the :redirect from requestId + assert.equal(calls[2].params.requestId, '123.6'); }); }); });