Skip to content

Commit

Permalink
core(driver): don't clear indexedb, websql, or localstorage before run (
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine committed Sep 29, 2020
1 parent 5ba5e51 commit 540fd3b
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 9 deletions.
2 changes: 1 addition & 1 deletion lighthouse-core/audits/multi-check-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
54 changes: 50 additions & 4 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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
Expand Down Expand Up @@ -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(',');
Expand All @@ -1477,6 +1495,33 @@ class Driver {
}
}

/**
* @param {string} url
* @return {Promise<LH.IcuMessage | undefined>}
*/
async getImportantStorageWarning(url) {
const usageData = await this.sendCommand('Storage.getUsageAndQuota', {
origin: url,
});
/** @type {Record<string, string>} */
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
Expand Down Expand Up @@ -1549,3 +1594,4 @@ class Driver {
}

module.exports = Driver;
module.exports.UIStrings = UIStrings;
13 changes: 10 additions & 3 deletions lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,10 @@ class GatherRunner {
/**
* @param {Driver} driver
* @param {{requestedUrl: string, settings: LH.Config.Settings}} options
* @param {(string | LH.IcuMessage)[]} LighthouseRunWarnings
* @return {Promise<void>}
*/
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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/lib/i18n/locales/en-US.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions lighthouse-core/lib/i18n/locales/en-XL.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

58 changes: 58 additions & 0 deletions lighthouse-core/test/gather/driver-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/test/gather/fake-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ function makeFakeDriver({protocolGetVersionResponse}) {
},
cleanBrowserCaches() {},
clearDataForOrigin() {},
getImportantStorageWarning() {
return Promise.resolve(undefined);
},
cacheNatives() {
return Promise.resolve();
},
Expand Down
4 changes: 4 additions & 0 deletions lighthouse-core/test/gather/gather-runner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ class EmulationDriver extends Driver {
registerRequestIdleCallbackWrap() {
return Promise.resolve();
}
getImportantStorageWarning() {
return Promise.resolve(undefined);
}
}

const fakeDriver = require('./fake-driver.js');
Expand Down Expand Up @@ -415,6 +418,7 @@ describe('GatherRunner', function() {
clearDataForOrigin: createCheck('calledClearStorage'),
blockUrlPatterns: asyncFunc,
setExtraHTTPHeaders: asyncFunc,
getImportantStorageWarning: asyncFunc,
};

return GatherRunner.setupDriver(driver, {settings: {}}).then(_ => {
Expand Down
2 changes: 1 addition & 1 deletion types/jest.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

0 comments on commit 540fd3b

Please sign in to comment.