Skip to content

Commit

Permalink
core(css-usage): ignore removed stylesheets (#12827)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine authored Aug 3, 2021
1 parent cbc5f31 commit d350438
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 2 deletions.
9 changes: 9 additions & 0 deletions lighthouse-core/gather/gatherers/css-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ 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 => {
// We can't fetch the content of removed stylesheets, so we ignore them completely.
// TODO(FR-COMPAT): Refactor use of CSSUsage artifact to not *always* rely on the content of stylesheets.
const styleSheetId = sheet.styleSheetId;
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 +42,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 +65,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
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

0 comments on commit d350438

Please sign in to comment.