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: split up CSSUsage artifact #15952

Merged
merged 4 commits into from
Apr 17, 2024
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
4 changes: 3 additions & 1 deletion core/audits/byte-efficiency/render-blocking-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ class RenderBlockingResources extends Audit {
guidanceLevel: 2,
// TODO: look into adding an `optionalArtifacts` property that captures the non-required nature
// of CSSUsage
requiredArtifacts: ['URL', 'traces', 'devtoolsLogs', 'CSSUsage', 'GatherContext', 'Stacks'],
requiredArtifacts:
['URL', 'traces', 'devtoolsLogs', 'Stylesheets', 'CSSUsage', 'GatherContext', 'Stacks'],
};
}

Expand Down Expand Up @@ -263,6 +264,7 @@ class RenderBlockingResources extends Audit {
const wastedBytesByUrl = new Map();
try {
const unusedCssItems = await UnusedCSS.request({
Stylesheets: artifacts.Stylesheets,
CSSUsage: artifacts.CSSUsage,
devtoolsLog: artifacts.devtoolsLogs[Audit.DEFAULT_PASS],
}, context);
Expand Down
4 changes: 2 additions & 2 deletions core/audits/byte-efficiency/unminified-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
description: str_(UIStrings.description),
scoreDisplayMode: ByteEfficiencyAudit.SCORING_MODES.METRIC_SAVINGS,
guidanceLevel: 3,
requiredArtifacts: ['CSSUsage', 'devtoolsLogs', 'traces', 'URL', 'GatherContext'],
requiredArtifacts: ['Stylesheets', 'devtoolsLogs', 'traces', 'URL', 'GatherContext'],
};
}

Expand Down Expand Up @@ -85,7 +85,7 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
*/
static audit_(artifacts, networkRecords) {
const items = [];
for (const stylesheet of artifacts.CSSUsage.stylesheets) {
for (const stylesheet of artifacts.Stylesheets) {
const networkRecord = networkRecords
.find(record => record.url === stylesheet.header.sourceURL);
if (!stylesheet.content) continue;
Expand Down
4 changes: 3 additions & 1 deletion core/audits/byte-efficiency/unused-css-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
description: str_(UIStrings.description),
scoreDisplayMode: ByteEfficiencyAudit.SCORING_MODES.METRIC_SAVINGS,
guidanceLevel: 1,
requiredArtifacts: ['CSSUsage', 'URL', 'devtoolsLogs', 'traces', 'GatherContext'],
requiredArtifacts:
['Stylesheets', 'CSSUsage', 'URL', 'devtoolsLogs', 'traces', 'GatherContext'],
};
}

Expand All @@ -46,6 +47,7 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
*/
static async audit_(artifacts, _, context) {
const unusedCssItems = await UnusedCSS.request({
Stylesheets: artifacts.Stylesheets,
CSSUsage: artifacts.CSSUsage,
devtoolsLog: artifacts.devtoolsLogs[ByteEfficiencyAudit.DEFAULT_PASS],
}, context);
Expand Down
4 changes: 2 additions & 2 deletions core/audits/font-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class FontDisplay extends Audit {
description: str_(UIStrings.description),
supportedModes: ['navigation'],
guidanceLevel: 3,
requiredArtifacts: ['devtoolsLogs', 'CSSUsage', 'URL'],
requiredArtifacts: ['devtoolsLogs', 'Stylesheets', 'URL'],
scoreDisplayMode: Audit.SCORING_MODES.METRIC_SAVINGS,
};
}
Expand All @@ -67,7 +67,7 @@ class FontDisplay extends Audit {
const failingURLs = new Set();

// Go through all the stylesheets to find all @font-face declarations
for (const stylesheet of artifacts.CSSUsage.stylesheets) {
for (const stylesheet of artifacts.Stylesheets) {
// Eliminate newlines so we can more easily scan through with a regex
const newlinesStripped = stylesheet.content.replace(/(\r|\n)+/g, ' ');
// Find the @font-faces
Expand Down
11 changes: 6 additions & 5 deletions core/computed/unused-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,21 +134,22 @@ class UnusedCSS {
}

/**
* @param {{CSSUsage: LH.Artifacts['CSSUsage'], devtoolsLog: LH.DevtoolsLog}} data
* @param {{Stylesheets: LH.Artifacts['Stylesheets'], CSSUsage: LH.Artifacts['CSSUsage'], devtoolsLog: LH.DevtoolsLog}} data
* @param {LH.Artifacts.ComputedContext} context
* @return {Promise<LH.Audit.ByteEfficiencyItem[]>}
*/
static async compute_(data, context) {
const {CSSUsage, devtoolsLog} = data;
const {CSSUsage, Stylesheets, devtoolsLog} = data;
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
const indexedSheets = UnusedCSS.indexStylesheetsById(CSSUsage.stylesheets, networkRecords);
UnusedCSS.indexUsedRules(CSSUsage.rules, indexedSheets);
const indexedSheets = UnusedCSS.indexStylesheetsById(Stylesheets, networkRecords);
UnusedCSS.indexUsedRules(CSSUsage, indexedSheets);

const items = Object.keys(indexedSheets)
.map(sheetId => UnusedCSS.mapSheetToResult(indexedSheets[sheetId]));
return items;
}
}

const UnusedCSSComputed = makeComputedArtifact(UnusedCSS, ['CSSUsage', 'devtoolsLog']);
const UnusedCSSComputed = makeComputedArtifact(UnusedCSS,
['Stylesheets', 'CSSUsage', 'devtoolsLog']);
export {UnusedCSSComputed as UnusedCSS};
1 change: 1 addition & 0 deletions core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ const defaultConfig = {
{id: 'Scripts', gatherer: 'scripts'},
{id: 'SourceMaps', gatherer: 'source-maps'},
{id: 'Stacks', gatherer: 'stacks'},
{id: 'Stylesheets', gatherer: 'stylesheets'},
{id: 'TraceElements', gatherer: 'trace-elements'},
{id: 'ViewportDimensions', gatherer: 'viewport-dimensions'},

Expand Down
128 changes: 9 additions & 119 deletions core/gather/gatherers/css-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,145 +8,35 @@
* @fileoverview Tracks unused CSS rules.
*/

import log from 'lighthouse-logger';

import BaseGatherer from '../base-gatherer.js';
import {Sentry} from '../../lib/sentry.js';

class CSSUsage extends BaseGatherer {
constructor() {
super();
/** @type {LH.Gatherer.ProtocolSession|undefined} */
this._session = undefined;
/** @type {Map<string, Promise<LH.Artifacts.CSSStyleSheetInfo|Error>>} */
this._sheetPromises = new Map();
/**
* Initialize as undefined so we can assert results are fetched.
* @type {LH.Crdp.CSS.RuleUsage[]|undefined}
*/
this._ruleUsage = undefined;
this._onStylesheetAdded = this._onStylesheetAdded.bind(this);
}

/** @type {LH.Gatherer.GathererMeta} */
meta = {
supportedModes: ['snapshot', 'timespan', 'navigation'],
supportedModes: ['snapshot', 'navigation'],
};

/**
* @param {LH.Crdp.CSS.StyleSheetAddedEvent} event
*/
_onStylesheetAdded(event) {
if (!this._session) throw new Error('Session not initialized');
const styleSheetId = event.header.styleSheetId;
const sheetPromise = this._session.sendCommand('CSS.getStyleSheetText', {styleSheetId})
.then(content => ({
header: event.header,
content: content.text,
}))
.catch(/** @param {Error} err */ (err) => {
log.warn(
'CSSUsage',
`Error fetching content of stylesheet with URL "${event.header.sourceURL}"`
);
Sentry.captureException(err, {
tags: {
gatherer: 'CSSUsage',
},
extra: {
url: event.header.sourceURL,
},
level: 'error',
});
return err;
});
this._sheetPromises.set(styleSheetId, sheetPromise);
}

/**
* @param {LH.Gatherer.Context} context
* @return {Promise<LH.Artifacts['CSSUsage']>}
*/
async startInstrumentation(context) {
async getArtifact(context) {
const session = context.driver.defaultSession;
this._session = session;

// Calling `CSS.enable` will emit events for stylesheets currently on the page.
// We want to ignore these events in navigation mode because they are not relevant to the
// navigation that is about to happen. Adding the event listener *after* calling `CSS.enable`
// ensures that the events for pre-existing stylesheets are ignored.
const isNavigation = context.gatherMode === 'navigation';
if (!isNavigation) {
session.on('CSS.styleSheetAdded', this._onStylesheetAdded);
}
const executionContext = context.driver.executionContext;

await session.sendCommand('DOM.enable');
await session.sendCommand('CSS.enable');
await session.sendCommand('CSS.startRuleUsageTracking');

if (isNavigation) {
session.on('CSS.styleSheetAdded', this._onStylesheetAdded);
}
}


/**
* @param {LH.Gatherer.Context} context
*/
async stopInstrumentation(context) {
const session = context.driver.defaultSession;
const coverageResponse = await session.sendCommand('CSS.stopRuleUsageTracking');
this._ruleUsage = coverageResponse.ruleUsage;
session.off('CSS.styleSheetAdded', this._onStylesheetAdded);

// Ensure we finish fetching all stylesheet contents before disabling the CSS domain
await Promise.all(this._sheetPromises.values());
// Force style to recompute.
// Doesn't appear to be necessary in newer versions of Chrome.
await executionContext.evaluateAsync('getComputedStyle(document.body)');

const {ruleUsage} = await session.sendCommand('CSS.stopRuleUsageTracking');
await session.sendCommand('CSS.disable');
await session.sendCommand('DOM.disable');
}

/**
* @param {LH.Gatherer.Context} context
* @return {Promise<LH.Artifacts['CSSUsage']>}
*/
async getArtifact(context) {
const executionContext = context.driver.executionContext;

if (context.gatherMode === 'snapshot') {
await this.startInstrumentation(context);

// Force style to recompute.
// Doesn't appear to be necessary in newer versions of Chrome.
await executionContext.evaluateAsync('getComputedStyle(document.body)');

await this.stopInstrumentation(context);
}

/** @type {Map<string, LH.Artifacts.CSSStyleSheetInfo>} */
const dedupedStylesheets = new Map();
const sheets = await Promise.all(this._sheetPromises.values());

for (const sheet of sheets) {
// Erroneous sheets will be reported via sentry and the log.
// We can ignore them here without throwing a fatal error.
if (sheet instanceof Error) {
continue;
}

// Exclude empty stylesheets.
if (sheet.header.length === 0) {
continue;
}

dedupedStylesheets.set(sheet.content, sheet);
}

if (!this._ruleUsage) throw new Error('Issue collecting rule usages');

return {
rules: this._ruleUsage,
stylesheets: Array.from(dedupedStylesheets.values()),
};
return ruleUsage;
}
}

Expand Down
Loading
Loading