Skip to content

Commit

Permalink
core(fr): convert image-elements gatherer (#12474)
Browse files Browse the repository at this point in the history
  • Loading branch information
@adamraine
adamraine committed May 17, 2021
1 parent bc951c1 commit 10e6300
Show file tree
Hide file tree
Showing 6 changed files with 573 additions and 51 deletions.
3 changes: 3 additions & 0 deletions lighthouse-core/fraggle-rock/config/default-config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const artifacts = {
FormElements: '',
GlobalListeners: '',
IFrameElements: '',
ImageElements: '',
InstallabilityErrors: '',
JsUsage: '',
LinkElements: '',
Expand Down Expand Up @@ -68,6 +69,7 @@ const defaultConfig = {
{id: artifacts.FormElements, gatherer: 'form-elements'},
{id: artifacts.GlobalListeners, gatherer: 'global-listeners'},
{id: artifacts.IFrameElements, gatherer: 'iframe-elements'},
{id: artifacts.ImageElements, gatherer: 'image-elements'},
{id: artifacts.InstallabilityErrors, gatherer: 'installability-errors'},
{id: artifacts.JsUsage, gatherer: 'js-usage'},
{id: artifacts.LinkElements, gatherer: 'link-elements'},
Expand Down Expand Up @@ -109,6 +111,7 @@ const defaultConfig = {
artifacts.FormElements,
artifacts.GlobalListeners,
artifacts.IFrameElements,
artifacts.ImageElements,
artifacts.InstallabilityErrors,
artifacts.JsUsage,
artifacts.LinkElements,
Expand Down
139 changes: 92 additions & 47 deletions lighthouse-core/gather/gatherers/image-elements.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
'use strict';

const log = require('lighthouse-logger');
const Gatherer = require('./gatherer.js');
const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
const pageFunctions = require('../../lib/page-functions.js');
const Driver = require('../driver.js'); // eslint-disable-line no-unused-vars
const DevtoolsLog = require('./devtools-log.js');
const FontSize = require('./seo/font-size.js');
const NetworkRecords = require('../../computed/network-records.js');

/* global window, getElementsInDocument, Image, getNodeDetails, ShadowRoot */

Expand Down Expand Up @@ -221,26 +222,33 @@ function getEffectiveSizingRule({attributesStyle, inlineStyle, matchedCSSRules},
return null;
}

class ImageElements extends Gatherer {
class ImageElements extends FRGatherer {
/** @type {LH.Gatherer.GathererMeta<'DevtoolsLog'>} */
meta = {
supportedModes: ['navigation'],
dependencies: {DevtoolsLog: DevtoolsLog.symbol},
};

constructor() {
super();
/** @type {Map<string, {naturalWidth: number, naturalHeight: number}>} */
this._naturalSizeCache = new Map();
}

/**
* @param {Driver} driver
* @param {LH.Gatherer.FRTransitionalDriver} driver
* @param {LH.Artifacts.ImageElement} element
*/
async fetchElementWithSizeInformation(driver, element) {
const url = element.src;
if (this._naturalSizeCache.has(url)) {
Object.assign(element, this._naturalSizeCache.get(url));
return;
}

try {
// We don't want this to take forever, 250ms should be enough for images that are cached
driver.setNextProtocolTimeout(250);
driver.defaultSession.setNextProtocolTimeout(250);
const size = await driver.executionContext.evaluate(determineNaturalSize, {
args: [url],
});
Expand All @@ -256,18 +264,18 @@ class ImageElements extends Gatherer {
* Images might be sized via CSS. In order to compute unsized-images failures, we need to collect
* matched CSS rules to see if this is the case.
* @url http://go/dwoqq (googlers only)
* @param {Driver} driver
* @param {LH.Gatherer.FRProtocolSession} session
* @param {string} devtoolsNodePath
* @param {LH.Artifacts.ImageElement} element
*/
async fetchSourceRules(driver, devtoolsNodePath, element) {
async fetchSourceRules(session, devtoolsNodePath, element) {
try {
const {nodeId} = await driver.sendCommand('DOM.pushNodeByPathToFrontend', {
const {nodeId} = await session.sendCommand('DOM.pushNodeByPathToFrontend', {
path: devtoolsNodePath,
});
if (!nodeId) return;

const matchedRules = await driver.sendCommand('CSS.getMatchedStylesForNode', {
const matchedRules = await session.sendCommand('CSS.getMatchedStylesForNode', {
nodeId: nodeId,
});
const width = getEffectiveSizingRule(matchedRules, 'width');
Expand All @@ -284,13 +292,10 @@ class ImageElements extends Gatherer {
}

/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.LoadData} loadData
* @return {Promise<LH.Artifacts['ImageElements']>}
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
*/
async afterPass(passContext, loadData) {
const driver = passContext.driver;
const indexedNetworkRecords = loadData.networkRecords.reduce((map, record) => {
indexNetworkRecords(networkRecords) {
return networkRecords.reduce((map, record) => {
// An image response in newer formats is sometimes incorrectly marked as "application/octet-stream",
// so respect the extension too.
const isImage = /^image/.test(record.mimeType) || /\.(avif|webp)$/i.test(record.url);
Expand All @@ -301,43 +306,25 @@ class ImageElements extends Gatherer {
}

return map;
}, /** @type {Object<string, LH.Artifacts.NetworkRequest>} */ ({}));

const elements = await driver.executionContext.evaluate(collectImageElementInfo, {
args: [],
deps: [
pageFunctions.getElementsInDocumentString,
pageFunctions.getBoundingClientRectString,
pageFunctions.getNodeDetailsString,
getClientRect,
getPosition,
getHTMLImages,
getCSSImages,
],
});

await Promise.all([
driver.sendCommand('DOM.enable'),
driver.sendCommand('CSS.enable'),
driver.sendCommand('DOM.getDocument', {depth: -1, pierce: true}),
]);

// Sort (in-place) as largest images descending
elements.sort((a, b) => {
const aRecord = indexedNetworkRecords[a.src] || {};
const bRecord = indexedNetworkRecords[b.src] || {};
return bRecord.resourceSize - aRecord.resourceSize;
});
}, /** @type {Record<string, LH.Artifacts.NetworkRequest>} */ ({}));
}

/**
*
* @param {LH.Gatherer.FRTransitionalDriver} driver
* @param {LH.Artifacts.ImageElement[]} elements
* @param {Record<string, LH.Artifacts.NetworkRequest>} indexedNetworkRecords
*/
async collectExtraDetails(driver, elements, indexedNetworkRecords) {
// Don't do more than 5s of this expensive devtools protocol work. See #11289
let reachedGatheringBudget = false;
setTimeout(_ => (reachedGatheringBudget = true), 5000);
let skippedCount = 0;

for (const element of elements) {
// Pull some of our information directly off the network record.
const networkRecord = indexedNetworkRecords[element.src] || {};
element.mimeType = networkRecord.mimeType;
const networkRecord = indexedNetworkRecords[element.src];
element.mimeType = networkRecord && networkRecord.mimeType;

if (reachedGatheringBudget) {
skippedCount++;
Expand All @@ -346,7 +333,7 @@ class ImageElements extends Gatherer {

// Need source rules to determine if sized via CSS (for unsized-images).
if (!element.isInShadowDOM && !element.isCss) {
await this.fetchSourceRules(driver, element.node.devtoolsNodePath, element);
await this.fetchSourceRules(driver.defaultSession, element.node.devtoolsNodePath, element);
}
// 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.
Expand All @@ -359,14 +346,72 @@ class ImageElements extends Gatherer {
if (reachedGatheringBudget) {
log.warn('ImageElements', `Reached gathering budget of 5s. Skipped extra details for ${skippedCount}/${elements.length}`); // eslint-disable-line max-len
}
}

/**
* @param {LH.Gatherer.FRTransitionalContext} context
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
* @return {Promise<LH.Artifacts['ImageElements']>}
*/
async _getArtifact(context, networkRecords) {
const session = context.driver.defaultSession;
const executionContext = context.driver.executionContext;
const indexedNetworkRecords = this.indexNetworkRecords(networkRecords);

const elements = await executionContext.evaluate(collectImageElementInfo, {
args: [],
deps: [
pageFunctions.getElementsInDocumentString,
pageFunctions.getBoundingClientRectString,
pageFunctions.getNodeDetailsString,
getClientRect,
getPosition,
getHTMLImages,
getCSSImages,
],
});

await Promise.all([
driver.sendCommand('DOM.disable'),
driver.sendCommand('CSS.disable'),
session.sendCommand('DOM.enable'),
session.sendCommand('CSS.enable'),
session.sendCommand('DOM.getDocument', {depth: -1, pierce: true}),
]);

// Sort (in-place) as largest images descending.
elements.sort((a, b) => {
const aRecord = indexedNetworkRecords[a.src] || {};
const bRecord = indexedNetworkRecords[b.src] || {};
return bRecord.resourceSize - aRecord.resourceSize;
});

await this.collectExtraDetails(context.driver, elements, indexedNetworkRecords);

await Promise.all([
session.sendCommand('DOM.disable'),
session.sendCommand('CSS.disable'),
]);

return elements;
}

/**
* @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} context
* @return {Promise<LH.Artifacts['ImageElements']>}
*/
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<LH.Artifacts['ImageElements']>}
*/
async afterPass(passContext, loadData) {
return this._getArtifact({...passContext, dependencies: {}}, loadData.networkRecords);
}
}

module.exports = ImageElements;
2 changes: 1 addition & 1 deletion lighthouse-core/test/fraggle-rock/api-test-pptr.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(`107`);
expect(auditResults.length).toMatchInlineSnapshot(`112`);
expect(erroredAudits).toHaveLength(0);

const failedAuditIds = failedAudits.map(audit => audit.id);
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/test/fraggle-rock/config/config-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ describe('Fraggle Rock Config', () => {
});

it('should throw on invalid artifact definitions', () => {
const configJson = {artifacts: [{id: 'ImageElements', gatherer: 'image-elements'}]};
expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/ImageElements gatherer/);
const configJson = {artifacts: [{id: 'FullPageScreenshot', gatherer: 'full-page-screenshot'}]};
expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/FullPageScreenshot gatherer/);
});

it('should resolve navigation definitions', () => {
Expand Down
Loading

0 comments on commit 10e6300

Please sign in to comment.