diff --git a/lighthouse-cli/test/smokehouse/byte-config.js b/lighthouse-cli/test/smokehouse/byte-config.js index 97c6d0b19a3e..d97fc6694500 100644 --- a/lighthouse-cli/test/smokehouse/byte-config.js +++ b/lighthouse-cli/test/smokehouse/byte-config.js @@ -9,13 +9,14 @@ * Config file for running PWA smokehouse audits. */ module.exports = { - extends: 'lighthouse:default', + extends: 'lighthouse:full', settings: { onlyAudits: [ 'offscreen-images', 'uses-webp-images', 'uses-optimized-images', 'uses-responsive-images', + 'unused-css-rules', ] } }; diff --git a/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js b/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js index d22a9b540ab8..e29582a83320 100644 --- a/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js +++ b/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js @@ -13,17 +13,17 @@ module.exports = [ initialUrl: 'http://localhost:10200/byte-efficiency/tester.html', url: 'http://localhost:10200/byte-efficiency/tester.html', audits: { - // TODO: re-enable once CSS protocol has stabilized - // 'unused-css-rules': { - // score: false, - // extendedInfo: { - // value: { - // results: { - // length: 2 - // } - // } - // } - // }, + 'unused-css-rules': { + score: '<100', + extendedInfo: { + value: { + wastedKb: 26, + results: { + length: 2 + } + } + } + }, 'offscreen-images': { score: '<100', extendedInfo: { diff --git a/lighthouse-core/audits/byte-efficiency/unused-css-rules.js b/lighthouse-core/audits/byte-efficiency/unused-css-rules.js index 5a575afbfff9..5a29f376e9aa 100644 --- a/lighthouse-core/audits/byte-efficiency/unused-css-rules.js +++ b/lighthouse-core/audits/byte-efficiency/unused-css-rules.js @@ -6,7 +6,6 @@ 'use strict'; const ByteEfficiencyAudit = require('./byte-efficiency-audit'); -const URL = require('../../lib/url-shim'); const PREVIEW_LENGTH = 100; @@ -34,15 +33,14 @@ class UnusedCSSRules extends ByteEfficiencyAudit { */ static indexStylesheetsById(styles, networkRecords) { const indexedNetworkRecords = networkRecords - .filter(record => record._resourceType && record._resourceType._name === 'stylesheet') .reduce((indexed, record) => { indexed[record.url] = record; return indexed; }, {}); + return styles.reduce((indexed, stylesheet) => { indexed[stylesheet.header.styleSheetId] = Object.assign({ - used: [], - unused: [], + usedRules: [], networkRecord: indexedNetworkRecords[stylesheet.header.sourceURL], }, stylesheet); return indexed; @@ -50,14 +48,11 @@ class UnusedCSSRules extends ByteEfficiencyAudit { } /** - * Counts the number of unused rules and adds count information to sheets. + * Adds used rules to their corresponding stylesheet. * @param {!Array.<{styleSheetId: string, used: boolean}>} rules The output of the CSSUsage gatherer. * @param {!Object} indexedStylesheets Stylesheet information indexed by id. - * @return {number} The number of unused rules. */ - static countUnusedRules(rules, indexedStylesheets) { - let unused = 0; - + static indexUsedRules(rules, indexedStylesheets) { rules.forEach(rule => { const stylesheetInfo = indexedStylesheets[rule.styleSheetId]; @@ -66,14 +61,48 @@ class UnusedCSSRules extends ByteEfficiencyAudit { } if (rule.used) { - stylesheetInfo.used.push(rule); - } else { - unused++; - stylesheetInfo.unused.push(rule); + stylesheetInfo.usedRules.push(rule); } }); + } + + /** + * @param {!Object} stylesheetInfo + * @return {{wastedBytes: number, totalBytes: number, wastedPercent: number}} + */ + static computeUsage(stylesheetInfo) { + let usedUncompressedBytes = 0; + const totalUncompressedBytes = stylesheetInfo.content.length; + + for (const usedRule of stylesheetInfo.usedRules) { + usedUncompressedBytes += usedRule.endOffset - usedRule.startOffset; + } + + const networkRecord = stylesheetInfo.networkRecord; + let totalTransferredBytes; + if (!networkRecord) { + // We don't know how many bytes this sheet used on the network, but we can guess it was + // roughly the size of the content gzipped. + // See https://discuss.httparchive.org/t/file-size-and-compression-savings/145 for multipliers + totalTransferredBytes = Math.round(totalUncompressedBytes * .5); + } else if (networkRecord._resourceType && networkRecord._resourceType._name === 'stylesheet') { + // This was a regular standalone stylesheet, just use the transfer size. + totalTransferredBytes = networkRecord.transferSize; + } else { + // This was a stylesheet that was inlined in a different resource type (e.g. HTML document). + // Use the compression ratio of the resource to estimate the total transferred bytes. + const compressionRatio = (networkRecord._transferSize / networkRecord._resourceSize) || 1; + totalTransferredBytes = Math.round(compressionRatio * totalUncompressedBytes); + } + + const percentUnused = (totalUncompressedBytes - usedUncompressedBytes) / totalUncompressedBytes; + const wastedBytes = Math.round(percentUnused * totalTransferredBytes); - return unused; + return { + wastedBytes, + wastedPercent: percentUnused * 100, + totalBytes: totalTransferredBytes, + }; } /** @@ -115,40 +144,21 @@ class UnusedCSSRules extends ByteEfficiencyAudit { /** * @param {!Object} stylesheetInfo The stylesheetInfo object. * @param {string} pageUrl The URL of the page, used to identify inline styles. - * @return {{url: string, label: string, code: string}} The result for the details object. + * @return {?{url: string, wastedBytes: number, totalBytes: number}} */ static mapSheetToResult(stylesheetInfo, pageUrl) { - const numUsed = stylesheetInfo.used.length; - const numUnused = stylesheetInfo.unused.length; - - if ((numUsed === 0 && numUnused === 0) || stylesheetInfo.isDuplicate) { + if (stylesheetInfo.isDuplicate) { return null; } let url = stylesheetInfo.header.sourceURL; if (!url || url === pageUrl) { const contentPreview = UnusedCSSRules.determineContentPreview(stylesheetInfo.content); - url = '*inline*```' + contentPreview + '```'; - } else { - url = URL.getURLDisplayName(url); + url = {type: 'code', text: contentPreview}; } - // If we don't know for sure how many bytes this sheet used on the network, - // we can guess it was roughly the size of the content gzipped. - const totalBytes = stylesheetInfo.networkRecord ? - stylesheetInfo.networkRecord.transferSize : - Math.round(stylesheetInfo.content.length / 3); - - const percentUnused = numUnused / (numUsed + numUnused); - const wastedBytes = Math.round(percentUnused * totalBytes); - - return { - url, - numUnused, - wastedBytes, - wastedPercent: percentUnused * 100, - totalBytes, - }; + const usage = UnusedCSSRules.computeUsage(stylesheetInfo); + return Object.assign({url}, usage); } /** @@ -163,21 +173,21 @@ class UnusedCSSRules extends ByteEfficiencyAudit { const devtoolsLogs = artifacts.devtoolsLogs[ByteEfficiencyAudit.DEFAULT_PASS]; return artifacts.requestNetworkRecords(devtoolsLogs).then(networkRecords => { const indexedSheets = UnusedCSSRules.indexStylesheetsById(styles, networkRecords); - UnusedCSSRules.countUnusedRules(usage, indexedSheets); - const results = Object.keys(indexedSheets).map(sheetId => { - return UnusedCSSRules.mapSheetToResult(indexedSheets[sheetId], pageUrl); - }).filter(sheet => sheet && sheet.wastedBytes > 1024); + UnusedCSSRules.indexUsedRules(usage, indexedSheets); + + const results = Object.keys(indexedSheets) + .map(sheetId => UnusedCSSRules.mapSheetToResult(indexedSheets[sheetId], pageUrl)) + .filter(sheet => sheet && sheet.wastedBytes > 1024); const headings = [ {key: 'url', itemType: 'url', text: 'URL'}, - {key: 'numUnused', itemType: 'url', text: 'Unused Rules'}, {key: 'totalKb', itemType: 'text', text: 'Original'}, {key: 'potentialSavings', itemType: 'text', text: 'Potential Savings'}, ]; return { results, - headings + headings, }; }); } diff --git a/lighthouse-core/config/default.js b/lighthouse-core/config/default.js index 8ee2bb791127..d273c674d8b1 100644 --- a/lighthouse-core/config/default.js +++ b/lighthouse-core/config/default.js @@ -24,7 +24,6 @@ module.exports = { 'manifest', 'chrome-console-messages', 'image-usage', - // 'css-usage', 'accessibility', 'dobetterweb/all-event-listeners', 'dobetterweb/anchors-with-no-rel-noopener', @@ -123,7 +122,6 @@ module.exports = { 'accessibility/video-caption', 'accessibility/video-description', 'byte-efficiency/total-byte-weight', - // 'byte-efficiency/unused-css-rules', 'byte-efficiency/offscreen-images', 'byte-efficiency/uses-webp-images', 'byte-efficiency/uses-optimized-images', @@ -229,7 +227,6 @@ module.exports = { {id: 'estimated-input-latency', weight: 1, group: 'perf-metric'}, {id: 'link-blocking-first-paint', weight: 0, group: 'perf-hint'}, {id: 'script-blocking-first-paint', weight: 0, group: 'perf-hint'}, - // {id: 'unused-css-rules', weight: 0}, {id: 'uses-responsive-images', weight: 0, group: 'perf-hint'}, {id: 'offscreen-images', weight: 0, group: 'perf-hint'}, {id: 'uses-optimized-images', weight: 0, group: 'perf-hint'}, diff --git a/lighthouse-core/config/full-config.js b/lighthouse-core/config/full-config.js index b4f293ae1573..2a7de014abb8 100644 --- a/lighthouse-core/config/full-config.js +++ b/lighthouse-core/config/full-config.js @@ -12,17 +12,24 @@ module.exports = { passName: 'extraPass', gatherers: [ 'styles', + 'css-usage', ] }, ], audits: [ + 'byte-efficiency/unused-css-rules', 'dobetterweb/no-old-flexbox', ], categories: { + 'performance': { + audits: [ + {id: 'unused-css-rules', weight: 0, group: 'perf-hint'}, + ], + }, 'best-practices': { audits: [ {id: 'no-old-flexbox', weight: 1}, - ] - } + ], + }, }, }; diff --git a/lighthouse-core/gather/gatherers/css-usage.js b/lighthouse-core/gather/gatherers/css-usage.js index 1f00d1b76120..cd6d00133fcc 100644 --- a/lighthouse-core/gather/gatherers/css-usage.js +++ b/lighthouse-core/gather/gatherers/css-usage.js @@ -14,15 +14,7 @@ class CSSUsage extends Gatherer { beforePass(options) { return options.driver.sendCommand('DOM.enable') .then(_ => options.driver.sendCommand('CSS.enable')) - .then(_ => options.driver.sendCommand('CSS.startRuleUsageTracking')) - .catch(err => { - // TODO(phulce): Remove this once CSS usage hits stable - if (/startRuleUsageTracking/.test(err.message)) { - throw new Error('CSS Usage tracking requires Chrome \u2265 56'); - } - - throw err; - }); + .then(_ => options.driver.sendCommand('CSS.startRuleUsageTracking')); } afterPass(options) { diff --git a/lighthouse-core/test/audits/byte-efficiency/unused-css-rules-test.js b/lighthouse-core/test/audits/byte-efficiency/unused-css-rules-test.js index b1a3b4cf61d6..8ca992d3fb49 100644 --- a/lighthouse-core/test/audits/byte-efficiency/unused-css-rules-test.js +++ b/lighthouse-core/test/audits/byte-efficiency/unused-css-rules-test.js @@ -98,25 +98,21 @@ describe('Best Practices: unused css rules audit', () => { baseSheet = { header: {sourceURL: baseUrl}, content: 'dummy', - used: [{dummy: 1}], - unused: [], + usedRules: [], }; }); it('correctly computes potentialSavings', () => { - assert.equal(map({used: [], unused: [1, 2]}).wastedPercent, 100); - assert.equal(map({used: [1, 2], unused: [1, 2]}).wastedPercent, 50); - assert.equal(map({used: [1, 2], unused: []}).wastedPercent, 0); + assert.equal(map({usedRules: []}).wastedPercent, 100); + assert.equal(map({usedRules: [{startOffset: 0, endOffset: 3}]}).wastedPercent, 40); + assert.equal(map({usedRules: [{startOffset: 0, endOffset: 5}]}).wastedPercent, 0); }); it('correctly computes url', () => { - assert.equal(map({header: {sourceURL: ''}}).url, '*inline*```dummy```'); - assert.equal(map({header: {sourceURL: 'a'}}, 'http://g.co/a').url, '*inline*```dummy```'); - assert.equal(map({header: {sourceURL: 'foobar'}}).url, '/foobar'); - }); - - it('does not give content preview when url is present', () => { - assert.ok(!/dummy/.test(map({header: {sourceURL: 'foobar'}}).url)); + const expectedPreview = {type: 'code', text: 'dummy'}; + assert.deepEqual(map({header: {sourceURL: ''}}).url, expectedPreview); + assert.deepEqual(map({header: {sourceURL: 'a'}}, 'http://g.co/a').url, expectedPreview); + assert.equal(map({header: {sourceURL: 'foobar'}}).url, 'http://g.co/foobar'); }); }); @@ -175,13 +171,8 @@ describe('Best Practices: unused css rules audit', () => { requestNetworkRecords, URL: {finalUrl: ''}, CSSUsage: [ - {styleSheetId: 'a', used: true}, - {styleSheetId: 'a', used: false}, - {styleSheetId: 'a', used: false}, - {styleSheetId: 'a', used: false}, - {styleSheetId: 'b', used: true}, - {styleSheetId: 'b', used: false}, - {styleSheetId: 'c', used: false}, + {styleSheetId: 'a', used: true, startOffset: 0, endOffset: 11}, // 44 * 1 / 4 + {styleSheetId: 'b', used: true, startOffset: 0, endOffset: 3075}, // 2050 * 3 / 2 ], Styles: [ { @@ -200,7 +191,7 @@ describe('Best Practices: unused css rules audit', () => { }).then(result => { assert.equal(result.results.length, 2); assert.equal(result.results[0].totalBytes, 10 * 1024); - assert.equal(result.results[1].totalBytes, 2050); + assert.equal(result.results[1].totalBytes, 3075); assert.equal(result.results[0].wastedPercent, 75); assert.equal(result.results[1].wastedPercent, 50); }); @@ -212,10 +203,7 @@ describe('Best Practices: unused css rules audit', () => { requestNetworkRecords, URL: {finalUrl: ''}, CSSUsage: [ - {styleSheetId: 'a', used: true}, - {styleSheetId: 'a', used: true}, - {styleSheetId: 'a', used: false}, - {styleSheetId: 'b', used: false}, + {styleSheetId: 'a', used: true, startOffset: 0, endOffset: 33}, // 44 * 3 / 4 ], Styles: [ { @@ -239,12 +227,8 @@ describe('Best Practices: unused css rules audit', () => { requestNetworkRecords, URL: {finalUrl: ''}, CSSUsage: [ - {styleSheetId: 'a', used: true}, - {styleSheetId: 'a', used: true}, - {styleSheetId: 'a', used: false}, - {styleSheetId: 'b', used: true}, - {styleSheetId: 'b', used: false}, - {styleSheetId: 'b', used: false}, + {styleSheetId: 'a', used: true, startOffset: 0, endOffset: 8000}, // 4000 * 3 / 2 + {styleSheetId: 'b', used: true, startOffset: 0, endOffset: 500}, // 500 * 3 / 3 ], Styles: [ { @@ -256,21 +240,21 @@ describe('Best Practices: unused css rules audit', () => { content: `${generate('123', 500)}` }, { - header: {styleSheetId: 'c', sourceURL: 'c.css'}, + header: {styleSheetId: 'c', sourceURL: 'file://c.css'}, content: '@import url(http://googlefonts.com?myfont)' }, { - header: {styleSheetId: 'd', sourceURL: 'd.css'}, + header: {styleSheetId: 'd', sourceURL: 'file://d.css'}, content: '/* nothing to see here */' }, { - header: {styleSheetId: 'e', sourceURL: 'e.css'}, + header: {styleSheetId: 'e', sourceURL: 'file://e.css'}, content: ' ' }, ] }).then(result => { assert.equal(result.results.length, 1); - assert.equal(result.results[0].numUnused, 1); + assert.equal(Math.floor(result.results[0].wastedPercent), 33); }); }); });