From ec71d815b0f1c091ae387026db16a37ad9f0e6a8 Mon Sep 17 00:00:00 2001 From: Artur Mizera Date: Mon, 24 Jul 2017 22:46:25 +0200 Subject: [PATCH 1/3] Ignoring other extension assets in request compression audit --- .../byte-efficiency/uses-request-compression.js | 7 ++++++- lighthouse-core/lib/url-shim.js | 9 +++++++++ .../uses-response-compression-test.js | 17 +++++++++++++++-- lighthouse-core/test/lib/url-shim-test.js | 7 +++++++ 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/audits/byte-efficiency/uses-request-compression.js b/lighthouse-core/audits/byte-efficiency/uses-request-compression.js index d4acfd082399..a260428fa03e 100644 --- a/lighthouse-core/audits/byte-efficiency/uses-request-compression.js +++ b/lighthouse-core/audits/byte-efficiency/uses-request-compression.js @@ -14,6 +14,7 @@ const URL = require('../../lib/url-shim'); const IGNORE_THRESHOLD_IN_BYTES = 1400; const IGNORE_THRESHOLD_IN_PERCENT = 0.1; +const CHROME_EXTENSION_PROTOCOL = 'chrome-extension:'; class ResponsesAreCompressed extends ByteEfficiencyAudit { /** @@ -40,8 +41,12 @@ class ResponsesAreCompressed extends ByteEfficiencyAudit { static audit_(artifacts) { const uncompressedResponses = artifacts.ResponseCompression; + function skipUnrelatedResponses(record) { + return URL.getProtocol(record.url) !== CHROME_EXTENSION_PROTOCOL; + } + const results = []; - uncompressedResponses.forEach(record => { + uncompressedResponses.filter(skipUnrelatedResponses).forEach(record => { const originalSize = record.resourceSize; const gzipSize = record.gzipSize; const gzipSavings = originalSize - gzipSize; diff --git a/lighthouse-core/lib/url-shim.js b/lighthouse-core/lib/url-shim.js index e33b5af4d1fc..b192cc4c6c7b 100644 --- a/lighthouse-core/lib/url-shim.js +++ b/lighthouse-core/lib/url-shim.js @@ -128,4 +128,13 @@ URL.equalWithExcludedFragments = function(url1, url2) { } }; +/** + * Returns protocol of the given URL + * @param {string} url + * @return {string} + */ +URL.getProtocol = function(url) { + return new URL(url).protocol; +}; + module.exports = URL; diff --git a/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js b/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js index 6a6caccfbb0a..c84934bf7207 100644 --- a/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js +++ b/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js @@ -10,9 +10,9 @@ const ResponsesAreCompressedAudit = require('../../../audits/byte-efficiency/uses-request-compression.js'); const assert = require('assert'); -function generateResponse(filename, type, originalSize, gzipSize) { +function generateResponse(filename, type, originalSize, gzipSize, schema = 'http:') { return { - url: `http://google.com/${filename}`, + url: `${schema}//google.com/${filename}`, mimeType: `${type}`, resourceSize: originalSize, gzipSize, @@ -45,4 +45,17 @@ describe('Page uses optimized responses', () => { assert.equal(auditResult.results.length, 1); }); + + it('skips responses from installed chrome extensions', () => { + const auditResult = ResponsesAreCompressedAudit.audit_({ + ResponseCompression: [ + generateResponse('index.css', 'text/css', 6 * KB_BYTES, 4.5 * KB_BYTES), // 1,5kb & 25% (hit) + generateResponse('chrome-extension.css', 'text/css', + 6 * KB_BYTES, 4.5 * KB_BYTES), // 1,5kb & 25% (hit) despite chrome-extension in asset's filename + generateResponse('extension_style.css', 'text/css', + 6 * KB_BYTES, 4.5 * KB_BYTES, 'chrome-extension:') // 1,5kb & 25% (hit) but comes from other extension + ] + }); + assert.equal(auditResult.results.length, 2); + }); }); diff --git a/lighthouse-core/test/lib/url-shim-test.js b/lighthouse-core/test/lib/url-shim-test.js index fedd66aa0dc1..7a52cb6e19a3 100644 --- a/lighthouse-core/test/lib/url-shim-test.js +++ b/lighthouse-core/test/lib/url-shim-test.js @@ -222,4 +222,11 @@ describe('URL Shim', () => { assert.ok(!URL.equalWithExcludedFragments('utter nonsense', 'http://example.com')); }); }); + + describe('getProtocol', () => { + it('returns protocol of the given url', () => { + assert.equal(URL.getProtocol('http://google.com'), 'http:'); + assert.equal(URL.getProtocol('chrome-extension://some_asset.css'), 'chrome-extension:'); + }); + }); }); From 36385b3620f4896582e463540b1e56743a66fdae Mon Sep 17 00:00:00 2001 From: Artur Mizera Date: Tue, 25 Jul 2017 00:39:06 +0200 Subject: [PATCH 2/3] Moved skipping extensions' assets to response compression gatherer --- .../uses-request-compression.js | 7 +--- .../dobetterweb/response-compression.js | 6 ++- lighthouse-core/lib/url-shim.js | 9 ----- .../uses-response-compression-test.js | 13 ------- .../dobetterweb/response-compression-test.js | 37 +++++++++++++++++++ lighthouse-core/test/lib/url-shim-test.js | 7 ---- 6 files changed, 43 insertions(+), 36 deletions(-) diff --git a/lighthouse-core/audits/byte-efficiency/uses-request-compression.js b/lighthouse-core/audits/byte-efficiency/uses-request-compression.js index a260428fa03e..d4acfd082399 100644 --- a/lighthouse-core/audits/byte-efficiency/uses-request-compression.js +++ b/lighthouse-core/audits/byte-efficiency/uses-request-compression.js @@ -14,7 +14,6 @@ const URL = require('../../lib/url-shim'); const IGNORE_THRESHOLD_IN_BYTES = 1400; const IGNORE_THRESHOLD_IN_PERCENT = 0.1; -const CHROME_EXTENSION_PROTOCOL = 'chrome-extension:'; class ResponsesAreCompressed extends ByteEfficiencyAudit { /** @@ -41,12 +40,8 @@ class ResponsesAreCompressed extends ByteEfficiencyAudit { static audit_(artifacts) { const uncompressedResponses = artifacts.ResponseCompression; - function skipUnrelatedResponses(record) { - return URL.getProtocol(record.url) !== CHROME_EXTENSION_PROTOCOL; - } - const results = []; - uncompressedResponses.filter(skipUnrelatedResponses).forEach(record => { + uncompressedResponses.forEach(record => { const originalSize = record.resourceSize; const gzipSize = record.gzipSize; const gzipSavings = originalSize - gzipSize; diff --git a/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js b/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js index c599f65465d3..65861cd31ac8 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js @@ -14,6 +14,7 @@ const Gatherer = require('../gatherer'); const gzip = require('zlib').gzip; const compressionTypes = ['gzip', 'br', 'deflate']; +const CHROME_EXTENSION_PROTOCOL = 'chrome-extension:'; class ResponseCompression extends Gatherer { /** @@ -25,7 +26,10 @@ class ResponseCompression extends Gatherer { networkRecords.forEach(record => { const isTextBasedResource = record.resourceType() && record.resourceType().isTextType(); - if (!isTextBasedResource || !record.resourceSize || !record.finished) { + const isChromeExtensionResource = record.url.startsWith(CHROME_EXTENSION_PROTOCOL); + + if (!isTextBasedResource || !record.resourceSize || !record.finished || + isChromeExtensionResource) { return; } diff --git a/lighthouse-core/lib/url-shim.js b/lighthouse-core/lib/url-shim.js index b192cc4c6c7b..e33b5af4d1fc 100644 --- a/lighthouse-core/lib/url-shim.js +++ b/lighthouse-core/lib/url-shim.js @@ -128,13 +128,4 @@ URL.equalWithExcludedFragments = function(url1, url2) { } }; -/** - * Returns protocol of the given URL - * @param {string} url - * @return {string} - */ -URL.getProtocol = function(url) { - return new URL(url).protocol; -}; - module.exports = URL; diff --git a/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js b/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js index c84934bf7207..5dde362f1339 100644 --- a/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js +++ b/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js @@ -45,17 +45,4 @@ describe('Page uses optimized responses', () => { assert.equal(auditResult.results.length, 1); }); - - it('skips responses from installed chrome extensions', () => { - const auditResult = ResponsesAreCompressedAudit.audit_({ - ResponseCompression: [ - generateResponse('index.css', 'text/css', 6 * KB_BYTES, 4.5 * KB_BYTES), // 1,5kb & 25% (hit) - generateResponse('chrome-extension.css', 'text/css', - 6 * KB_BYTES, 4.5 * KB_BYTES), // 1,5kb & 25% (hit) despite chrome-extension in asset's filename - generateResponse('extension_style.css', 'text/css', - 6 * KB_BYTES, 4.5 * KB_BYTES, 'chrome-extension:') // 1,5kb & 25% (hit) but comes from other extension - ] - }); - assert.equal(auditResult.results.length, 2); - }); }); diff --git a/lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js b/lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js index 21b6652c339f..a994c503fe4e 100644 --- a/lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js +++ b/lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js @@ -116,6 +116,43 @@ describe('Optimized responses', () => { }); }); + it('ignores responses from installed Chrome extensions', () => { + const traceData = { + networkRecords: [ + { + _url: 'chrome-extension://index.css', + _mimeType: 'text/css', + _requestId: 1, + _resourceSize: 10, + _resourceType: { + _isTextType: true, + }, + _responseHeaders: [], + content: 'aaaaaaaaaa', + finished: true, + }, + { + _url: 'http://google.com/chrome-extension.css', + _mimeType: 'text/css', + _requestId: 1, + _resourceSize: 123, + _resourceType: { + _isTextType: true, + }, + _responseHeaders: [], + content: 'aaaaaaaaaa', + finished: true, + } + ] + }; + + return responseCompression.afterPass(options, createNetworkRequests(traceData)) + .then(artifact => { + assert.equal(artifact.length, 1); + assert.equal(artifact[0].resourceSize, 123); + }); + }); + // Change into SDK.networkRequest when examples are ready function createNetworkRequests(traceData) { traceData.networkRecords = traceData.networkRecords.map(record => { diff --git a/lighthouse-core/test/lib/url-shim-test.js b/lighthouse-core/test/lib/url-shim-test.js index 7a52cb6e19a3..fedd66aa0dc1 100644 --- a/lighthouse-core/test/lib/url-shim-test.js +++ b/lighthouse-core/test/lib/url-shim-test.js @@ -222,11 +222,4 @@ describe('URL Shim', () => { assert.ok(!URL.equalWithExcludedFragments('utter nonsense', 'http://example.com')); }); }); - - describe('getProtocol', () => { - it('returns protocol of the given url', () => { - assert.equal(URL.getProtocol('http://google.com'), 'http:'); - assert.equal(URL.getProtocol('chrome-extension://some_asset.css'), 'chrome-extension:'); - }); - }); }); From 3844034cf50a3cd588112b9d70366cee2e9e2055 Mon Sep 17 00:00:00 2001 From: Artur Mizera Date: Tue, 25 Jul 2017 23:00:38 +0200 Subject: [PATCH 3/3] Reverted schema param --- .../audits/byte-efficiency/uses-response-compression-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js b/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js index 5dde362f1339..6a6caccfbb0a 100644 --- a/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js +++ b/lighthouse-core/test/audits/byte-efficiency/uses-response-compression-test.js @@ -10,9 +10,9 @@ const ResponsesAreCompressedAudit = require('../../../audits/byte-efficiency/uses-request-compression.js'); const assert = require('assert'); -function generateResponse(filename, type, originalSize, gzipSize, schema = 'http:') { +function generateResponse(filename, type, originalSize, gzipSize) { return { - url: `${schema}//google.com/${filename}`, + url: `http://google.com/${filename}`, mimeType: `${type}`, resourceSize: originalSize, gzipSize,