Skip to content

Commit

Permalink
core: convert requestIds before sending to backend (#5580)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce committed Jun 28, 2018
1 parent 3474c39 commit db2265d
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 1 deletion.
3 changes: 3 additions & 0 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -727,6 +728,8 @@ class Driver {
* @return {Promise<string>}
*/
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -114,6 +115,8 @@ class OptimizedImages extends Gatherer {
* @return {Promise<LH.Crdp.Audits.GetEncodedResponseResponse>}
*/
_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);
Expand Down
10 changes: 10 additions & 0 deletions lighthouse-core/lib/network-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/gather/driver-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const fakeImageStats = {
const traceData = {
networkRecords: [
{
_requestId: '123.5',
_url: 'http://google.com/image.jpg',
_mimeType: 'image/jpeg',
_resourceSize: 10000,
Expand All @@ -28,6 +29,7 @@ const traceData = {
finished: true,
},
{
_requestId: '123.6:redirect',
_url: 'http://google.com/transparent.png',
_mimeType: 'image/png',
_resourceSize: 11000,
Expand All @@ -36,6 +38,7 @@ const traceData = {
finished: true,
},
{
_requestId: '123.5',
_url: 'http://google.com/image.bmp',
_mimeType: 'image/bmp',
_resourceSize: 12000,
Expand All @@ -44,6 +47,7 @@ const traceData = {
finished: true,
},
{
_requestId: '123.5',
_url: 'http://google.com/image.bmp',
_mimeType: 'image/bmp',
_resourceSize: 12000,
Expand All @@ -52,6 +56,7 @@ const traceData = {
finished: true,
},
{
_requestId: '123.5',
_url: 'http://google.com/vector.svg',
_mimeType: 'image/svg+xml',
_resourceSize: 13000,
Expand All @@ -60,6 +65,7 @@ const traceData = {
finished: true,
},
{
_requestId: '123.5',
_url: 'http://gmail.com/image.jpg',
_mimeType: 'image/jpeg',
_resourceSize: 15000,
Expand All @@ -68,6 +74,7 @@ const traceData = {
finished: true,
},
{
_requestId: '123.5',
_url: 'data: image/jpeg ; base64 ,SgVcAT32587935321...',
_mimeType: 'image/jpeg',
_resourceType: 'Image',
Expand All @@ -76,6 +83,7 @@ const traceData = {
finished: true,
},
{
_requestId: '123.5',
_url: 'http://google.com/big-image.bmp',
_mimeType: 'image/bmp',
_resourceType: 'Image',
Expand All @@ -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
Expand Down Expand Up @@ -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});
};
Expand All @@ -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');
});
});
});

0 comments on commit db2265d

Please sign in to comment.