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.

Sign up for GitHub

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

Jump to bottom

core(css-usage): ignore removed stylesheets #12827

Merged
merged 6 commits into from
Aug 3, 2021
Merged

core(css-usage): ignore removed stylesheets #12827

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2c8b876
core(css-usage): ignore removed stylesheets
adamraine Jul 26, 2021
a8d4774
Update lighthouse-core/test/gather/gatherers/css-usage-test.js
adamraine Jul 27, 2021
f658c15
Update lighthouse-core/test/gather/gatherers/css-usage-test.js
adamraine Jul 27, 2021
44f8c82
Update lighthouse-core/gather/gatherers/css-usage.js
adamraine Aug 2, 2021
9d0ddb9
Merge branch 'master' into fr-fix-css
adamraine Aug 3, 2021
e985571
lint
adamraine Aug 3, 2021
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
7 changes: 7 additions & 0 deletions lighthouse-core/gather/gatherers/css-usage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ class CSSUsage extends FRGatherer {
this._stylesheets = [];
/** @param {LH.Crdp.CSS.StyleSheetAddedEvent} sheet */
this._onStylesheetAdded = sheet => this._stylesheets.push(sheet);
/** @param {LH.Crdp.CSS.StyleSheetRemovedEvent} sheet */
this._onStylesheetRemoved = sheet => {
const styleSheetId = sheet.styleSheetId;
adamraine marked this conversation as resolved.
Show resolved Hide resolved
this._stylesheets = this._stylesheets.filter(s => s.header.styleSheetId !== styleSheetId);
};
/**
* Initialize as undefined so we can assert results are fetched.
* @type {LH.Crdp.CSS.RuleUsage[]|undefined}
Expand All @@ -35,6 +40,7 @@ class CSSUsage extends FRGatherer {
async startCSSUsageTracking(context) {
const session = context.driver.defaultSession;
session.on('CSS.styleSheetAdded', this._onStylesheetAdded);
session.on('CSS.styleSheetRemoved', this._onStylesheetRemoved);

await session.sendCommand('DOM.enable');
await session.sendCommand('CSS.enable');
Expand All @@ -57,6 +63,7 @@ class CSSUsage extends FRGatherer {
const coverageResponse = await session.sendCommand('CSS.stopRuleUsageTracking');
this._ruleUsage = coverageResponse.ruleUsage;
session.off('CSS.styleSheetAdded', this._onStylesheetAdded);
session.off('CSS.styleSheetRemoved', this._onStylesheetRemoved);
}

/**
Expand Down
63 changes: 61 additions & 2 deletions lighthouse-core/test/gather/gatherers/css-usage-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
const CSSUsage = require('../../../gather/gatherers/css-usage.js');
const {defaultSettings} = require('../../../config/constants.js');
const {createMockDriver} = require('../../fraggle-rock/gather/mock-driver.js');
const {flushAllTimersAndMicrotasks} = require('../../test-utils.js');

jest.useFakeTimers();

describe('.getArtifact', () => {
it('gets CSS usage', async () => {
Expand All @@ -19,7 +22,8 @@ describe('.getArtifact', () => {
.mockEvent('CSS.styleSheetAdded', {header: {styleSheetId: '2'}});
driver.defaultSession.sendCommand
.mockResponse('DOM.enable')
.mockResponse('CSS.enable', undefined, 1) // Force events to emit
// @ts-expect-error - Force events to emit.
.mockResponse('CSS.enable', flushAllTimersAndMicrotasks)
.mockResponse('CSS.startRuleUsageTracking')
.mockResponse('CSS.getStyleSheetText', {text: 'CSS text 1'})
.mockResponse('CSS.getStyleSheetText', {text: 'CSS text 2'})
Expand Down Expand Up @@ -68,14 +72,69 @@ describe('.getArtifact', () => {
});
});

it('ignores removed stylesheets', async () => {
const driver = createMockDriver();
driver.defaultSession.on
.mockEvent('CSS.styleSheetAdded', {header: {styleSheetId: '1'}})
.mockEvent('CSS.styleSheetAdded', {header: {styleSheetId: '2'}})
.mockEvent('CSS.styleSheetRemoved', {styleSheetId: '1'});
driver.defaultSession.sendCommand
.mockResponse('DOM.enable')
.mockResponse('CSS.enable')
.mockResponse('CSS.startRuleUsageTracking')
.mockResponse('CSS.getStyleSheetText', {text: 'CSS text 2'})
.mockResponse('CSS.stopRuleUsageTracking', {
ruleUsage: [
{styleSheetId: '2', used: false},
],
})
.mockResponse('CSS.disable')
.mockResponse('DOM.disable');

/** @type {LH.Gatherer.FRTransitionalContext} */
const context = {
driver: driver.asDriver(),
url: 'https://example.com',
gatherMode: 'timespan',
computedCache: new Map(),
dependencies: {},
settings: defaultSettings,
};

const gatherer = new CSSUsage();
await gatherer.startInstrumentation(context);

// Force events to emit.
await flushAllTimersAndMicrotasks(1);

await gatherer.stopInstrumentation(context);
const artifact = await gatherer.getArtifact(context);

expect(artifact).toEqual({
stylesheets: [
{
header: {styleSheetId: '2'},
content: 'CSS text 2',
},
],
rules: [
{
styleSheetId: '2',
used: false,
},
],
});
});

it('dedupes stylesheets', async () => {
const driver = createMockDriver();
driver.defaultSession.on
.mockEvent('CSS.styleSheetAdded', {header: {styleSheetId: '1'}})
.mockEvent('CSS.styleSheetAdded', {header: {styleSheetId: '1'}});
driver.defaultSession.sendCommand
.mockResponse('DOM.enable')
.mockResponse('CSS.enable', undefined, 1) // Force events to emit
// @ts-expect-error - Force events to emit.
.mockResponse('CSS.enable', flushAllTimersAndMicrotasks)
.mockResponse('CSS.startRuleUsageTracking')
.mockResponse('CSS.getStyleSheetText', {text: 'CSS text 1'})
.mockResponse('CSS.getStyleSheetText', {text: 'CSS text 1'})
Expand Down