Skip to content

Commit

Permalink
core(fr): add computed cache to pass context (#12427)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine authored and paulirish committed May 12, 2021
1 parent 5c74780 commit ccc1a33
Show file tree
Hide file tree
Showing 19 changed files with 175 additions and 43 deletions.
3 changes: 3 additions & 0 deletions lighthouse-core/fraggle-rock/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const artifacts = {
IFrameElements: '',
InstallabilityErrors: '',
JsUsage: '',
LinkElements: '',
MetaElements: '',
NetworkUserAgent: '',
PasswordInputsWithPreventedPaste: '',
Expand Down Expand Up @@ -68,6 +69,7 @@ const defaultConfig = {
{id: artifacts.IFrameElements, gatherer: 'iframe-elements'},
{id: artifacts.InstallabilityErrors, gatherer: 'installability-errors'},
{id: artifacts.JsUsage, gatherer: 'js-usage'},
{id: artifacts.LinkElements, gatherer: 'link-elements'},
{id: artifacts.MetaElements, gatherer: 'meta-elements'},
{id: artifacts.NetworkUserAgent, gatherer: 'network-user-agent'},
{id: artifacts.PasswordInputsWithPreventedPaste, gatherer: 'dobetterweb/password-inputs-with-prevented-paste'},
Expand Down Expand Up @@ -107,6 +109,7 @@ const defaultConfig = {
artifacts.IFrameElements,
artifacts.InstallabilityErrors,
artifacts.JsUsage,
artifacts.LinkElements,
artifacts.MetaElements,
artifacts.NetworkUserAgent,
artifacts.PasswordInputsWithPreventedPaste,
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/fraggle-rock/gather/base-gatherer.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ class FRGatherer {
* @return {Promise<LH.Gatherer.PhaseResultNonPromise>}
*/
async afterPass(passContext, loadData) {
if ('dependencies' in this.meta) {
throw Error('Gatherer with dependencies should override afterPass');
}
await this.stopSensitiveInstrumentation({...passContext, dependencies: {}});
await this.stopInstrumentation({...passContext, dependencies: {}});
return this.getArtifact({...passContext, dependencies: {}});
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/fraggle-rock/gather/navigation-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ async function navigation(options) {
{
url: requestedUrl,
config,
computedCache: new Map(),
}
);
}
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/fraggle-rock/gather/runner-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ async function collectPhaseArtifacts(options) {
gatherMode,
driver,
dependencies,
computedCache: new Map(),
});
});

Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/fraggle-rock/gather/snapshot-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ async function snapshot(options) {
{
url,
config,
computedCache: new Map(),
}
);
}
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/fraggle-rock/gather/timespan-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ async function startTimespan(options) {
{
url: finalUrl,
config,
computedCache: new Map(),
}
);
},
Expand Down
8 changes: 5 additions & 3 deletions lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
'use strict';

const log = require('lighthouse-logger');
const NetworkRecords = require('../computed/network-records.js');
const {getPageLoadError} = require('../lib/navigation-error.js');
const NetworkRecorder = require('../lib/network-recorder.js');
const emulation = require('../lib/emulation.js');
const constants = require('../config/constants.js');
const i18n = require('../lib/i18n/i18n.js');
Expand Down Expand Up @@ -55,6 +55,7 @@ const SLOW_CPU_BENCHMARK_INDEX_THRESHOLD = 1000;
const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

/** @typedef {import('../gather/driver.js')} Driver */
/** @typedef {import('../lib/arbitrary-equality-map.js')} ArbitraryEqualityMap */

/**
* Each entry in each gatherer result array is the output of a gatherer phase:
Expand Down Expand Up @@ -294,7 +295,7 @@ class GatherRunner {
};
log.time(status);
const devtoolsLog = driver.endDevtoolsLog();
const networkRecords = NetworkRecorder.recordsFromLogs(devtoolsLog);
const networkRecords = await NetworkRecords.request(devtoolsLog, passContext);
log.timeEnd(status);

return {
Expand Down Expand Up @@ -524,7 +525,7 @@ class GatherRunner {

/**
* @param {Array<LH.Config.Pass>} passConfigs
* @param {{driver: Driver, requestedUrl: string, settings: LH.Config.Settings}} options
* @param {{driver: Driver, requestedUrl: string, settings: LH.Config.Settings, computedCache: Map<string, ArbitraryEqualityMap>}} options
* @return {Promise<LH.Artifacts>}
*/
static async run(passConfigs, options) {
Expand Down Expand Up @@ -554,6 +555,7 @@ class GatherRunner {
settings: options.settings,
passConfig,
baseArtifacts,
computedCache: options.computedCache,
LighthouseRunWarnings: baseArtifacts.LighthouseRunWarnings,
};
const passResults = await GatherRunner.runPass(passContext);
Expand Down
64 changes: 48 additions & 16 deletions lighthouse-core/gather/gatherers/link-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
'use strict';

const LinkHeader = require('http-link-header');
const Gatherer = require('./gatherer.js');
const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
const {URL} = require('../../lib/url-shim.js');
const NetworkRecords = require('../../computed/network-records.js');
const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js');
const pageFunctions = require('../../lib/page-functions.js');
const DevtoolsLog = require('./devtools-log.js');

/* globals HTMLLinkElement getNodeDetails */

Expand Down Expand Up @@ -79,15 +81,28 @@ function getLinkElementsInDOM() {
}
/* c8 ignore stop */

class LinkElements extends Gatherer {
class LinkElements extends FRGatherer {
constructor() {
super();
/**
* This needs to be in the constructor.
* https://github.com/GoogleChrome/lighthouse/issues/12134
* @type {LH.Gatherer.GathererMeta<'DevtoolsLog'>}
*/
this.meta = {
supportedModes: ['timespan', 'navigation'],
dependencies: {DevtoolsLog: DevtoolsLog.symbol},
};
}

/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.FRTransitionalContext} context
* @return {Promise<LH.Artifacts['LinkElements']>}
*/
static getLinkElementsInDOM(passContext) {
static getLinkElementsInDOM(context) {
// We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize
// the values like access from JavaScript does.
return passContext.driver.executionContext.evaluate(getLinkElementsInDOM, {
return context.driver.executionContext.evaluate(getLinkElementsInDOM, {
args: [],
useIsolation: true,
deps: [
Expand All @@ -98,14 +113,13 @@ class LinkElements extends Gatherer {
}

/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.LoadData} loadData
* @param {LH.Gatherer.FRTransitionalContext} context
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
* @return {LH.Artifacts['LinkElements']}
*/
static getLinkElementsInHeaders(passContext, loadData) {
const finalUrl = passContext.url;
const records = loadData.networkRecords;
const mainDocument = NetworkAnalyzer.findMainDocument(records, finalUrl);
static getLinkElementsInHeaders(context, networkRecords) {
const finalUrl = context.url;
const mainDocument = NetworkAnalyzer.findMainDocument(networkRecords, finalUrl);

/** @type {LH.Artifacts['LinkElements']} */
const linkElements = [];
Expand All @@ -131,13 +145,13 @@ class LinkElements extends Gatherer {
}

/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.LoadData} loadData
* @param {LH.Gatherer.FRTransitionalContext} context
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
* @return {Promise<LH.Artifacts['LinkElements']>}
*/
async afterPass(passContext, loadData) {
const fromDOM = await LinkElements.getLinkElementsInDOM(passContext);
const fromHeaders = LinkElements.getLinkElementsInHeaders(passContext, loadData);
async _getArtifact(context, networkRecords) {
const fromDOM = await LinkElements.getLinkElementsInDOM(context);
const fromHeaders = LinkElements.getLinkElementsInHeaders(context, networkRecords);
const linkElements = fromDOM.concat(fromHeaders);

for (const link of linkElements) {
Expand All @@ -147,6 +161,24 @@ class LinkElements extends Gatherer {

return linkElements;
}

/**
* @param {LH.Gatherer.PassContext} context
* @param {LH.Gatherer.LoadData} loadData
* @return {Promise<LH.Artifacts['LinkElements']>}
*/
async afterPass(context, loadData) {
return this._getArtifact({...context, dependencies: {}}, loadData.networkRecords);
}

/**
* @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} context
* @return {Promise<LH.Artifacts['LinkElements']>}
*/
async getArtifact(context) {
const records = await NetworkRecords.request(context.dependencies.DevtoolsLog, context);
return this._getArtifact(context, records);
}
}

module.exports = LinkElements;
3 changes: 2 additions & 1 deletion lighthouse-core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ async function lighthouse(url, flags = {}, configJSON, userConnection) {
log.setLevel(flags.logLevel);

const config = generateConfig(configJSON, flags);
const options = {url, config};
const computedCache = new Map();
const options = {url, config, computedCache};
const connection = userConnection || new ChromeProtocol(flags.port, flags.hostname);

// kick off a lighthouse run
Expand Down
13 changes: 8 additions & 5 deletions lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@ const generateReport = require('./report/report-generator.js').generateReport;
const LHError = require('./lib/lh-error.js');

/** @typedef {import('./gather/connections/connection.js')} Connection */
/** @typedef {import('./lib/arbitrary-equality-map.js')} ArbitraryEqualityMap */
/** @typedef {LH.Config.Config} Config */

class Runner {
/**
* @template {LH.Config.Config | LH.Config.FRConfig} TConfig
* @param {(runnerData: {requestedUrl: string, config: TConfig, driverMock?: Driver}) => Promise<LH.Artifacts>} gatherFn
* @param {{config: TConfig, url?: string, driverMock?: Driver}} runOpts
* @param {{config: TConfig, computedCache: Map<string, ArbitraryEqualityMap>, url?: string, driverMock?: Driver}} runOpts
* @return {Promise<LH.RunnerResult|undefined>}
*/
static async run(gatherFn, runOpts) {
Expand Down Expand Up @@ -100,7 +101,7 @@ class Runner {
throw new Error('No audits to evaluate.');
}
const auditResultsById = await Runner._runAudits(settings, runOpts.config.audits, artifacts,
lighthouseRunWarnings);
lighthouseRunWarnings, runOpts.computedCache);

// LHR construction phase
const resultsStatus = {msg: 'Generating results...', id: 'lh:runner:generate'};
Expand Down Expand Up @@ -217,7 +218,7 @@ class Runner {
/**
* Establish connection, load page and collect all required artifacts
* @param {string} requestedUrl
* @param {{config: Config, driverMock?: Driver}} runnerOpts
* @param {{config: Config, computedCache: Map<string, ArbitraryEqualityMap>, driverMock?: Driver}} runnerOpts
* @param {Connection} connection
* @return {Promise<LH.Artifacts>}
*/
Expand All @@ -230,6 +231,7 @@ class Runner {
driver,
requestedUrl,
settings: runnerOpts.config.settings,
computedCache: runnerOpts.computedCache,
};
const artifacts = await GatherRunner.run(runnerOpts.config.passes, gatherOpts);
return artifacts;
Expand All @@ -241,9 +243,10 @@ class Runner {
* @param {Array<LH.Config.AuditDefn>} audits
* @param {LH.Artifacts} artifacts
* @param {Array<string | LH.IcuMessage>} runWarnings
* @param {Map<string, ArbitraryEqualityMap>} computedCache
* @return {Promise<Record<string, LH.RawIcu<LH.Audit.Result>>>}
*/
static async _runAudits(settings, audits, artifacts, runWarnings) {
static async _runAudits(settings, audits, artifacts, runWarnings, computedCache) {
const status = {msg: 'Analyzing and running audits...', id: 'lh:runner:auditing'};
log.time(status);

Expand All @@ -267,7 +270,7 @@ class Runner {
// Members of LH.Audit.Context that are shared across all audits.
const sharedAuditContext = {
settings,
computedCache: new Map(),
computedCache,
};

// Run each audit sequentially
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/test/fraggle-rock/api-test-pptr.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ describe('Fraggle Rock API', () => {

const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr);
// TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
expect(auditResults.length).toMatchInlineSnapshot(`24`);
expect(auditResults.length).toMatchInlineSnapshot(`25`);

expect(erroredAudits).toHaveLength(0);
expect(failedAudits.map(audit => audit.id)).toContain('errors-in-console');
Expand Down Expand Up @@ -159,7 +159,7 @@ describe('Fraggle Rock API', () => {
const {lhr} = result;
const {auditResults, failedAudits, erroredAudits} = getAuditsBreakdown(lhr);
// TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
expect(auditResults.length).toMatchInlineSnapshot(`106`);
expect(auditResults.length).toMatchInlineSnapshot(`107`);
expect(erroredAudits).toHaveLength(0);

const failedAuditIds = failedAudits.map(audit => audit.id);
Expand Down
15 changes: 15 additions & 0 deletions lighthouse-core/test/fraggle-rock/gather/base-gatherer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,20 @@ describe('BaseGatherer', () => {
expect(gatherer.stopInstrumentation).toHaveBeenCalled();
expect(gatherer.stopSensitiveInstrumentation).toHaveBeenCalled();
});

it('throws if no override and has dependencies', async () => {
class MyGatherer extends Gatherer {
/** @type {LH.Gatherer.GathererMeta<'Trace'>} */
meta = {supportedModes: ['timespan'], dependencies: {Trace: Symbol('Trace')}};
stopInstrumentation = jest.fn();
stopSensitiveInstrumentation = jest.fn();
}

const gatherer = new MyGatherer();
const afterPassPromise = gatherer.afterPass(fakeParam, fakeParam);
await expect(afterPassPromise).rejects.toThrowError(/Gatherer with dependencies/);
expect(gatherer.stopInstrumentation).not.toHaveBeenCalled();
expect(gatherer.stopSensitiveInstrumentation).not.toHaveBeenCalled();
});
});
});

0 comments on commit ccc1a33

Please sign in to comment.