From 540fd3b854bc063eedec1e214bd55ac3b9d73084 Mon Sep 17 00:00:00 2001 From: Adam Raine <6752989+adamraine@users.noreply.github.com> Date: Tue, 29 Sep 2020 19:33:30 -0400 Subject: [PATCH] core(driver): don't clear indexedb, websql, or localstorage before run (#11438) --- lighthouse-core/audits/multi-check-audit.js | 2 +- lighthouse-core/gather/driver.js | 54 +++++++++++++++-- lighthouse-core/gather/gather-runner.js | 13 ++++- lighthouse-core/lib/i18n/locales/en-US.json | 3 + lighthouse-core/lib/i18n/locales/en-XL.json | 3 + lighthouse-core/test/gather/driver-test.js | 58 +++++++++++++++++++ lighthouse-core/test/gather/fake-driver.js | 3 + .../test/gather/gather-runner-test.js | 4 ++ types/jest.d.ts | 2 +- 9 files changed, 133 insertions(+), 9 deletions(-) diff --git a/lighthouse-core/audits/multi-check-audit.js b/lighthouse-core/audits/multi-check-audit.js index 5973e152358b..a617f4c1e253 100644 --- a/lighthouse-core/audits/multi-check-audit.js +++ b/lighthouse-core/audits/multi-check-audit.js @@ -53,7 +53,7 @@ class MultiCheckAudit extends Audit { if (result.failures.length > 0) { return { score: 0, - // TODO(#7238): make this i18n-able. + // TODO(#11495): make this i18n-able. explanation: `Failures: ${result.failures.join(',\n')}.`, details, }; diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index b1bfca21f3fe..44a39dfe2000 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -12,6 +12,7 @@ const LHElement = require('../lib/lh-element.js'); const LHError = require('../lib/lh-error.js'); const NetworkRequest = require('../lib/network-request.js'); const EventEmitter = require('events').EventEmitter; +const i18n = require('../lib/i18n/i18n.js'); const URL = require('../lib/url-shim.js'); const constants = require('../config/constants.js'); @@ -24,6 +25,24 @@ const pageFunctions = require('../lib/page-functions.js'); // eslint-disable-next-line no-unused-vars const Connection = require('./connections/connection.js'); +const UIStrings = { + /** + * @description A warning that previously-saved data may have affected the measured performance and instructions on how to avoid the problem. "locations" will be a list of possible types of data storage locations, e.g. "IndexedDB", "Local Storage", or "Web SQL". + * @example {IndexedDB, Local Storage} locations + */ + warningData: `{locationCount, plural, + =1 {There may be stored data affecting loading performance in this location: {locations}. ` + + `Audit this page in an incognito window to prevent those resources ` + + `from affecting your scores.} + other {There may be stored data affecting loading ` + + `performance in these locations: {locations}. ` + + `Audit this page in an incognito window to prevent those resources ` + + `from affecting your scores.} + }`, +}; + +const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); + // Controls how long to wait after FCP before continuing const DEFAULT_PAUSE_AFTER_FCP = 0; // Controls how long to wait after onLoad before continuing @@ -1445,16 +1464,15 @@ class Driver { async clearDataForOrigin(url) { const origin = new URL(url).origin; - // Clear all types of storage except cookies, so the user isn't logged out. + // Clear some types of storage. + // Cookies are not cleared, so the user isn't logged out. + // indexeddb, websql, and localstorage are not cleared to prevent loss of potentially important data. // https://chromedevtools.github.io/debugger-protocol-viewer/tot/Storage/#type-StorageType const typesToClear = [ 'appcache', // 'cookies', 'file_systems', - 'indexeddb', - 'local_storage', 'shader_cache', - 'websql', 'service_workers', 'cache_storage', ].join(','); @@ -1477,6 +1495,33 @@ class Driver { } } + /** + * @param {string} url + * @return {Promise} + */ + async getImportantStorageWarning(url) { + const usageData = await this.sendCommand('Storage.getUsageAndQuota', { + origin: url, + }); + /** @type {Record} */ + const storageTypeNames = { + local_storage: 'Local Storage', + indexeddb: 'IndexedDB', + websql: 'Web SQL', + }; + const locations = usageData.usageBreakdown + .filter(usage => usage.usage) + .map(usage => storageTypeNames[usage.storageType] || '') + .filter(Boolean); + if (locations.length) { + // TODO(#11495): Use Intl.ListFormat with Node 12 + return str_( + UIStrings.warningData, + {locations: locations.join(', '), locationCount: locations.length} + ); + } + } + /** * Cache native functions/objects inside window * so we are sure polyfills do not overwrite the native implementations @@ -1549,3 +1594,4 @@ class Driver { } module.exports = Driver; +module.exports.UIStrings = UIStrings; diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index 6efc7eeba59e..87114a72b64f 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -105,9 +105,10 @@ class GatherRunner { /** * @param {Driver} driver * @param {{requestedUrl: string, settings: LH.Config.Settings}} options + * @param {(string | LH.IcuMessage)[]} LighthouseRunWarnings * @return {Promise} */ - static async setupDriver(driver, options) { + static async setupDriver(driver, options, LighthouseRunWarnings) { const status = {msg: 'Initializing…', id: 'lh:gather:setupDriver'}; log.time(status); const resetStorage = !options.settings.disableStorageReset; @@ -119,7 +120,13 @@ class GatherRunner { await driver.registerPerformanceObserver(); await driver.dismissJavaScriptDialogs(); await driver.registerRequestIdleCallbackWrap(options.settings); - if (resetStorage) await driver.clearDataForOrigin(options.requestedUrl); + if (resetStorage) { + const warning = await driver.getImportantStorageWarning(options.requestedUrl); + if (warning) { + LighthouseRunWarnings.push(warning); + } + await driver.clearDataForOrigin(options.requestedUrl); + } log.timeEnd(status); } @@ -654,7 +661,7 @@ class GatherRunner { const baseArtifacts = await GatherRunner.initializeBaseArtifacts(options); baseArtifacts.BenchmarkIndex = await options.driver.getBenchmarkIndex(); - await GatherRunner.setupDriver(driver, options); + await GatherRunner.setupDriver(driver, options, baseArtifacts.LighthouseRunWarnings); let isFirstPass = true; for (const passConfig of passConfigs) { diff --git a/lighthouse-core/lib/i18n/locales/en-US.json b/lighthouse-core/lib/i18n/locales/en-US.json index aed013730111..9a1b313eafc3 100644 --- a/lighthouse-core/lib/i18n/locales/en-US.json +++ b/lighthouse-core/lib/i18n/locales/en-US.json @@ -1598,6 +1598,9 @@ "lighthouse-core/config/default-config.js | seoMobileGroupTitle": { "message": "Mobile Friendly" }, + "lighthouse-core/gather/driver.js | warningData": { + "message": "{locationCount, plural,\n =1 {There may be stored data affecting loading performance in this location: {locations}. Audit this page in an incognito window to prevent those resources from affecting your scores.}\n other {There may be stored data affecting loading performance in these locations: {locations}. Audit this page in an incognito window to prevent those resources from affecting your scores.}\n }" + }, "lighthouse-core/gather/gather-runner.js | warningRedirected": { "message": "The page may not be loading as expected because your test URL ({requested}) was redirected to {final}. Try testing the second URL directly." }, diff --git a/lighthouse-core/lib/i18n/locales/en-XL.json b/lighthouse-core/lib/i18n/locales/en-XL.json index 85dce06544a9..6465291c837d 100644 --- a/lighthouse-core/lib/i18n/locales/en-XL.json +++ b/lighthouse-core/lib/i18n/locales/en-XL.json @@ -1598,6 +1598,9 @@ "lighthouse-core/config/default-config.js | seoMobileGroupTitle": { "message": "M̂ób̂íl̂é F̂ŕîén̂d́l̂ý" }, + "lighthouse-core/gather/driver.js | warningData": { + "message": "{locationCount, plural,\n =1 {T̂h́êŕê ḿâý b̂é ŝt́ôŕêd́ d̂át̂á âf́f̂éĉt́îńĝ ĺôád̂ín̂ǵ p̂ér̂f́ôŕm̂án̂ćê ín̂ t́ĥíŝ ĺôćât́îón̂: {locations}. Áûd́ît́ t̂h́îś p̂áĝé îń âń îńĉóĝńît́ô ẃîńd̂óŵ t́ô ṕr̂év̂én̂t́ t̂h́ôśê ŕêśôúr̂ćêś f̂ŕôḿ âf́f̂éĉt́îńĝ ýôúr̂ śĉór̂éŝ.}\n other {T́ĥér̂é m̂áŷ b́ê śt̂ór̂éd̂ d́ât́â áf̂f́êćt̂ín̂ǵ l̂óâd́îńĝ ṕêŕf̂ór̂ḿâńĉé îń t̂h́êśê ĺôćât́îón̂ś: {locations}. Âúd̂ít̂ t́ĥíŝ ṕâǵê ín̂ án̂ ín̂ćôǵn̂ít̂ó ŵín̂d́ôẃ t̂ó p̂ŕêv́êńt̂ t́ĥóŝé r̂éŝóûŕĉéŝ f́r̂óm̂ áf̂f́êćt̂ín̂ǵ ŷóûŕ ŝćôŕêś.}\n }" + }, "lighthouse-core/gather/gather-runner.js | warningRedirected": { "message": "T̂h́ê ṕâǵê ḿâý n̂ót̂ b́ê ĺôád̂ín̂ǵ âś êx́p̂éĉt́êd́ b̂éĉáûśê ýôúr̂ t́êśt̂ ÚR̂Ĺ ({requested}) ŵáŝ ŕêd́îŕêćt̂éd̂ t́ô {final}. T́r̂ý t̂éŝt́îńĝ t́ĥé ŝéĉón̂d́ ÛŔL̂ d́îŕêćt̂ĺŷ." }, diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 168a7453860a..4dbe76b1f78b 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -913,6 +913,64 @@ describe('.goOnline', () => { }); }); +describe('.clearDataForOrigin', () => { + it('only clears data from certain locations', async () => { + let foundStorageTypes; + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Storage.clearDataForOrigin', ({storageTypes}) => { + foundStorageTypes = storageTypes; + }); + await driver.clearDataForOrigin('https://example.com'); + // Should not see cookies, websql, indexeddb, or local_storage. + // Cookies are not cleared to preserve login. + // websql, indexeddb, and local_storage are not cleared to preserve important user data. + expect(foundStorageTypes).toMatchInlineSnapshot( + `"appcache,file_systems,shader_cache,service_workers,cache_storage"` + ); + }); +}); + +describe('.getImportantDataWarning', () => { + it('properly returns warning', async () => { + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Storage.getUsageAndQuota', {usageBreakdown: [ + {storageType: 'local_storage', usage: 5}, + {storageType: 'indexeddb', usage: 5}, + {storageType: 'websql', usage: 0}, + {storageType: 'appcache', usage: 5}, + {storageType: 'cookies', usage: 5}, + {storageType: 'file_systems', usage: 5}, + {storageType: 'shader_cache', usage: 5}, + {storageType: 'service_workers', usage: 5}, + {storageType: 'cache_storage', usage: 0}, + ]}); + const warning = await driver.getImportantStorageWarning('https://example.com'); + expect(warning).toBeDisplayString( + 'There may be stored data affecting loading performance in ' + + 'these locations: Local Storage, IndexedDB. ' + + 'Audit this page in an incognito window to prevent those resources ' + + 'from affecting your scores.' + ); + }); + + it('only warn for certain locations', async () => { + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Storage.getUsageAndQuota', {usageBreakdown: [ + {storageType: 'local_storage', usage: 0}, + {storageType: 'indexeddb', usage: 0}, + {storageType: 'websql', usage: 0}, + {storageType: 'appcache', usage: 5}, + {storageType: 'cookies', usage: 5}, + {storageType: 'file_systems', usage: 5}, + {storageType: 'shader_cache', usage: 5}, + {storageType: 'service_workers', usage: 5}, + {storageType: 'cache_storage', usage: 5}, + ]}); + const warning = await driver.getImportantStorageWarning('https://example.com'); + expect(warning).toBeUndefined(); + }); +}); + describe('Domain.enable/disable State', () => { it('dedupes (simple)', async () => { connectionStub.sendCommand = createMockSendCommandFn() diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js index 4f3f338b1786..db166317525b 100644 --- a/lighthouse-core/test/gather/fake-driver.js +++ b/lighthouse-core/test/gather/fake-driver.js @@ -62,6 +62,9 @@ function makeFakeDriver({protocolGetVersionResponse}) { }, cleanBrowserCaches() {}, clearDataForOrigin() {}, + getImportantStorageWarning() { + return Promise.resolve(undefined); + }, cacheNatives() { return Promise.resolve(); }, diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index 498a86bd0584..977047718c6d 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -111,6 +111,9 @@ class EmulationDriver extends Driver { registerRequestIdleCallbackWrap() { return Promise.resolve(); } + getImportantStorageWarning() { + return Promise.resolve(undefined); + } } const fakeDriver = require('./fake-driver.js'); @@ -415,6 +418,7 @@ describe('GatherRunner', function() { clearDataForOrigin: createCheck('calledClearStorage'), blockUrlPatterns: asyncFunc, setExtraHTTPHeaders: asyncFunc, + getImportantStorageWarning: asyncFunc, }; return GatherRunner.setupDriver(driver, {settings: {}}).then(_ => { diff --git a/types/jest.d.ts b/types/jest.d.ts index 96e551108499..c23c5a7bf491 100644 --- a/types/jest.d.ts +++ b/types/jest.d.ts @@ -20,6 +20,6 @@ declare namespace jest { /** * Asserts that an i18n string (using en-US) matches an expected pattern. */ - toBeDisplayString: (pattern: RegExp) => R; + toBeDisplayString: (pattern: RegExp | string) => R; } }