Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(driver): don't clear indexedb, websql, or localstorage before run #11438

Merged
merged 17 commits into from
Sep 29, 2020
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',
adamraine marked this conversation as resolved.
Show resolved Hide resolved
'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}
adamraine marked this conversation as resolved.
Show resolved Hide resolved
);
}
}

/**
* 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(
adamraine marked this conversation as resolved.
Show resolved Hide resolved
`"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;
}
}