diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 299a8c78e211..bcb475ced925 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -353,6 +353,10 @@ class Driver { * @return {Promise<*>} */ async _evaluateInContext(expression, contextId) { + // Use a higher than default timeout if the user hasn't specified a specific timeout. + // Otherwise, use whatever was requested. + const timeout = this._nextProtocolTimeout === DEFAULT_PROTOCOL_TIMEOUT ? + 60000 : this._nextProtocolTimeout; const evaluationParams = { // We need to explicitly wrap the raw expression for several purposes: // 1. Ensure that the expression will be a native Promise and not a polyfill/non-Promise. @@ -372,11 +376,11 @@ class Driver { includeCommandLineAPI: true, awaitPromise: true, returnByValue: true, - timeout: 60000, + timeout, contextId, }; - this.setNextProtocolTimeout(60000); + this.setNextProtocolTimeout(timeout); const response = await this.sendCommand('Runtime.evaluate', evaluationParams); if (response.exceptionDetails) { // An error occurred before we could even create a Promise, should be *very* rare diff --git a/lighthouse-core/gather/gatherers/image-elements.js b/lighthouse-core/gather/gatherers/image-elements.js index ebb01908b896..25e8eb10c2c7 100644 --- a/lighthouse-core/gather/gatherers/image-elements.js +++ b/lighthouse-core/gather/gatherers/image-elements.js @@ -87,8 +87,8 @@ function collectImageElementInfo() { displayedHeight: element.clientHeight, clientRect: getClientRect(element), // CSS Images do not expose natural size, we'll determine the size later - naturalWidth: Number.MAX_VALUE, - naturalHeight: Number.MAX_VALUE, + naturalWidth: 0, + naturalHeight: 0, isCss: true, isPicture: false, usesObjectFit: false, @@ -130,12 +130,14 @@ class ImageElements extends Gatherer { async fetchElementWithSizeInformation(driver, element) { const url = JSON.stringify(element.src); try { + // We don't want this to take forever, 250ms should be enough for images that are cached + driver.setNextProtocolTimeout(250); /** @type {{naturalWidth: number, naturalHeight: number}} */ const size = await driver.evaluateAsync(`(${determineNaturalSize.toString()})(${url})`); return Object.assign(element, size); } catch (_) { // determineNaturalSize fails on invalid images, which we treat as non-visible - return Object.assign(element, {naturalWidth: 0, naturalHeight: 0}); + return element; } } @@ -147,7 +149,9 @@ class ImageElements extends Gatherer { async afterPass(passContext, loadData) { const driver = passContext.driver; const indexedNetworkRecords = loadData.networkRecords.reduce((map, record) => { - if (/^image/.test(record.mimeType) && record.finished) { + // The network record is only valid for size information if it finished with a successful status + // code that indicates a complete resource response. + if (/^image/.test(record.mimeType) && record.finished && record.statusCode === 200) { map[record.url] = record; } @@ -163,6 +167,9 @@ class ImageElements extends Gatherer { const elements = await driver.evaluateAsync(expression); const imageUsage = []; + const top50Images = Object.values(indexedNetworkRecords) + .sort((a, b) => b.resourceSize - a.resourceSize) + .slice(0, 50); for (let element of elements) { // Pull some of our information directly off the network record. const networkRecord = indexedNetworkRecords[element.src] || {}; @@ -177,8 +184,13 @@ class ImageElements extends Gatherer { // Images within `picture` behave strangely and natural size information isn't accurate, // CSS images have no natural size information at all. Try to get the actual size if we can. - // Additional fetch is expensive; don't bother if we don't have a networkRecord for the image. - if ((element.isPicture || element.isCss) && networkRecord) { + // Additional fetch is expensive; don't bother if we don't have a networkRecord for the image, + // or it's not in the top 50 largest images. + if ( + (element.isPicture || element.isCss) && + networkRecord && + top50Images.includes(networkRecord) + ) { element = await this.fetchElementWithSizeInformation(driver, element); } diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 817dac10cb9c..5befd18dd8b7 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -31,13 +31,14 @@ function createMockSendCommandFn() { const mockFn = jest.fn().mockImplementation(command => { const indexOfResponse = mockResponses.findIndex(entry => entry.command === command); if (indexOfResponse === -1) throw new Error(`${command} unimplemented`); - const {response} = mockResponses[indexOfResponse]; + const {response, delay} = mockResponses[indexOfResponse]; mockResponses.splice(indexOfResponse, 1); + if (delay) return new Promise(resolve => setTimeout(() => resolve(response), delay)); return Promise.resolve(response); }); - mockFn.mockResponse = (command, response) => { - mockResponses.push({command, response}); + mockFn.mockResponse = (command, response, delay) => { + mockResponses.push({command, response, delay}); return mockFn; }; @@ -260,6 +261,34 @@ describe('.evaluateAsync', () => { connectionStub.sendCommand.findInvocation('Runtime.evaluate'); }); + it('uses a high default timeout', async () => { + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Runtime.evaluate', {result: {value: 2}}, 65000); + + const evaluatePromise = makePromiseInspectable(driver.evaluateAsync('1 + 1')); + jest.advanceTimersByTime(30000); + await flushAllTimersAndMicrotasks(); + expect(evaluatePromise).not.toBeDone(); + + jest.advanceTimersByTime(30000); + await flushAllTimersAndMicrotasks(); + expect(evaluatePromise).toBeDone(); + await expect(evaluatePromise).rejects.toBeTruthy(); + }); + + it('uses the specific timeout given', async () => { + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Runtime.evaluate', {result: {value: 2}}, 10000); + + driver.setNextProtocolTimeout(5000); + const evaluatePromise = makePromiseInspectable(driver.evaluateAsync('1 + 1')); + + jest.advanceTimersByTime(5001); + await flushAllTimersAndMicrotasks(); + expect(evaluatePromise).toBeDone(); + await expect(evaluatePromise).rejects.toBeTruthy(); + }); + it('evaluates an expression in isolation', async () => { connectionStub.sendCommand = createMockSendCommandFn() .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: 1337}}}) diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 9dca79bb4160..9fe2deb61d7f 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -81,7 +81,7 @@ declare global { HTMLWithoutJavaScript: {bodyText: string, hasNoScript: boolean}; /** Whether the page ended up on an HTTPS page after attempting to load the HTTP version. */ HTTPRedirect: {value: boolean}; - /** Information on size and loading for all the images in the page. */ + /** Information on size and loading for all the images in the page. Natural size information for `picture` and CSS images is only available if the image was one of the largest 50 images. */ ImageElements: Artifacts.ImageElement[]; /** Information on JS libraries and versions used by the page. */ JSLibraries: {name: string, version: string, npmPkgName: string}[];