Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: convert requestIds before sending to backend #5580

Merged
merged 1 commit into from
Jun 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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');
});
});
});