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(fr): add settings to context #12574

Merged
merged 6 commits into from
Jun 1, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -23,6 +23,7 @@ const artifacts = {
EmbeddedContent: '',
FontSize: '',
FormElements: '',
FullPageScreenshot: '',
GlobalListeners: '',
IFrameElements: '',
ImageElements: '',
Expand Down Expand Up @@ -71,6 +72,7 @@ const defaultConfig = {
{id: artifacts.EmbeddedContent, gatherer: 'seo/embedded-content'},
{id: artifacts.FontSize, gatherer: 'seo/font-size'},
{id: artifacts.FormElements, gatherer: 'form-elements'},
{id: artifacts.FullPageScreenshot, gatherer: 'full-page-screenshot'},
{id: artifacts.GlobalListeners, gatherer: 'global-listeners'},
{id: artifacts.IFrameElements, gatherer: 'iframe-elements'},
{id: artifacts.ImageElements, gatherer: 'image-elements'},
Expand Down Expand Up @@ -121,6 +123,7 @@ const defaultConfig = {
artifacts.EmbeddedContent,
artifacts.FontSize,
artifacts.FormElements,
artifacts.FullPageScreenshot,
artifacts.GlobalListeners,
artifacts.IFrameElements,
artifacts.ImageElements,
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 @@ -171,6 +171,7 @@ async function _navigation(navigationContext) {
computedCache: navigationContext.computedCache,
artifactDefinitions: navigationContext.navigation.artifacts,
artifactState,
settings: navigationContext.config.settings,
};

const setupResult = await _setupNavigation(navigationContext);
Expand Down
12 changes: 11 additions & 1 deletion lighthouse-core/fraggle-rock/gather/runner-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* @property {LH.Gatherer.FRGatherPhase} phase
* @property {LH.Gatherer.GatherMode} gatherMode
* @property {Map<string, LH.ArbitraryEqualityMap>} computedCache
* @property {LH.Config.Settings} settings
*/

/** @typedef {Record<string, Promise<any>>} IntermediateArtifacts */
Expand Down Expand Up @@ -61,7 +62,15 @@ const phaseToPriorPhase = {
* @param {CollectPhaseArtifactOptions} options
*/
async function collectPhaseArtifacts(options) {
const {driver, artifactDefinitions, artifactState, phase, gatherMode, computedCache} = options;
const {
driver,
artifactDefinitions,
artifactState,
phase,
gatherMode,
computedCache,
settings,
} = options;
const priorPhase = phaseToPriorPhase[phase];
const priorPhaseArtifacts = (priorPhase && artifactState[priorPhase]) || {};

Expand All @@ -80,6 +89,7 @@ async function collectPhaseArtifacts(options) {
driver,
dependencies,
computedCache,
settings,
});
});

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 @@ -40,6 +40,7 @@ async function snapshot(options) {
artifactDefinitions,
artifactState,
computedCache,
settings: config.settings,
});

const artifacts = await awaitArtifacts(artifactState);
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 @@ -36,6 +36,7 @@ async function startTimespan(options) {
artifactState,
computedCache,
gatherMode: 'timespan',
settings: config.settings,
};

await collectPhaseArtifacts({phase: 'startInstrumentation', ...phaseOptions});
Expand Down
61 changes: 33 additions & 28 deletions lighthouse-core/gather/gatherers/full-page-screenshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@

/* globals window document getBoundingClientRect */

const Gatherer = require('./gatherer.js');
const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
const emulation = require('../../lib/emulation.js');
const pageFunctions = require('../../lib/page-functions.js');

/** @typedef {import('../driver.js')} Driver */

// JPEG quality setting
// Exploration and examples of reports using different quality settings: https://docs.google.com/document/d/1ZSffucIca9XDW2eEwfoevrk-OTl7WQFeMf0CgeJAA8M/edit#
const FULL_PAGE_SCREENSHOT_QUALITY = 30;
Expand All @@ -24,28 +22,34 @@ function snakeCaseToCamelCase(str) {
return str.replace(/(-\w)/g, m => m[1].toUpperCase());
}

class FullPageScreenshot extends Gatherer {
class FullPageScreenshot extends FRGatherer {
/** @type {LH.Gatherer.GathererMeta} */
meta = {
supportedModes: ['snapshot', 'timespan', 'navigation'],
}

/**
* @param {Driver} driver
* @param {LH.Gatherer.FRTransitionalContext} context
* @return {Promise<number>}
* @see https://bugs.chromium.org/p/chromium/issues/detail?id=770769
*/
async getMaxScreenshotHeight(driver) {
return await driver.executionContext.evaluate(pageFunctions.getMaxTextureSize, {
async getMaxScreenshotHeight(context) {
return await context.driver.executionContext.evaluate(pageFunctions.getMaxTextureSize, {
args: [],
useIsolation: true,
deps: [],
});
}

/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.FRTransitionalContext} context
* @param {LH.Config.Settings} settings
* @return {Promise<LH.Artifacts.FullPageScreenshot['screenshot']>}
*/
async _takeScreenshot(passContext) {
const driver = passContext.driver;
const maxScreenshotHeight = await this.getMaxScreenshotHeight(driver);
const metrics = await driver.sendCommand('Page.getLayoutMetrics');
async _takeScreenshot(context, settings) {
const session = context.driver.defaultSession;
const maxScreenshotHeight = await this.getMaxScreenshotHeight(context);
const metrics = await session.sendCommand('Page.getLayoutMetrics');

// Width should match emulated width, without considering content overhang.
// Both layoutViewport and visualViewport capture this. visualViewport accounts
Expand All @@ -56,9 +60,9 @@ class FullPageScreenshot extends Gatherer {
const width = Math.min(metrics.layoutViewport.clientWidth, maxScreenshotHeight);
const height = Math.min(metrics.contentSize.height, maxScreenshotHeight);

await driver.sendCommand('Emulation.setDeviceMetricsOverride', {
await session.sendCommand('Emulation.setDeviceMetricsOverride', {
// If we're gathering with mobile screenEmulation on (overlay scrollbars, etc), continue to use that for this screenshot.
mobile: passContext.settings.screenEmulation.mobile,
mobile: settings.screenEmulation.mobile,
height,
width,
deviceScaleFactor: 1,
Expand All @@ -70,7 +74,7 @@ class FullPageScreenshot extends Gatherer {
// The lower in the page, the more likely (footer elements especially).
// https://github.com/GoogleChrome/lighthouse/issues/11118

const result = await driver.sendCommand('Page.captureScreenshot', {
const result = await session.sendCommand('Page.captureScreenshot', {
format: 'jpeg',
quality: FULL_PAGE_SCREENSHOT_QUALITY,
});
Expand All @@ -90,10 +94,10 @@ class FullPageScreenshot extends Gatherer {
* `getNodeDetails` maintains a collection of DOM objects in the page, which we can iterate
* to re-collect the bounding client rectangle.
* @see pageFunctions.getNodeDetails
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.FRTransitionalContext} context
* @return {Promise<LH.Artifacts.FullPageScreenshot['nodes']>}
*/
async _resolveNodes(passContext) {
async _resolveNodes(context) {
function resolveNodes() {
/** @type {LH.Artifacts.FullPageScreenshot['nodes']} */
const nodes = {};
Expand All @@ -113,7 +117,7 @@ class FullPageScreenshot extends Gatherer {
* @param {{useIsolation: boolean}} _
*/
function resolveNodesInPage({useIsolation}) {
return passContext.driver.executionContext.evaluate(resolveNodes, {
return context.driver.executionContext.evaluate(resolveNodes, {
args: [],
useIsolation,
deps: [pageFunctions.getBoundingClientRectString],
Expand All @@ -129,26 +133,27 @@ class FullPageScreenshot extends Gatherer {
}

/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.FRTransitionalContext} context
* @return {Promise<LH.Artifacts['FullPageScreenshot']>}
*/
async afterPass(passContext) {
const {driver} = passContext;
const executionContext = driver.executionContext;
async getArtifact(context) {
const session = context.driver.defaultSession;
const executionContext = context.driver.executionContext;
const settings = context.settings;

// In case some other program is controlling emulation, try to remember what the device looks
// like now and reset after gatherer is done.
const lighthouseControlsEmulation = !passContext.settings.screenEmulation.disabled;
const lighthouseControlsEmulation = !settings.screenEmulation.disabled;

try {
return {
screenshot: await this._takeScreenshot(passContext),
nodes: await this._resolveNodes(passContext),
screenshot: await this._takeScreenshot(context, settings),
nodes: await this._resolveNodes(context),
};
} finally {
// Revert resized page.
if (lighthouseControlsEmulation) {
await emulation.emulate(driver.defaultSession, passContext.settings);
await emulation.emulate(session, settings);
} else {
// Best effort to reset emulation to what it was.
// https://github.com/GoogleChrome/lighthouse/pull/10716#discussion_r428970681
Expand Down Expand Up @@ -179,8 +184,8 @@ class FullPageScreenshot extends Gatherer {
useIsolation: true,
deps: [snakeCaseToCamelCase],
});
await driver.sendCommand('Emulation.setDeviceMetricsOverride', {
mobile: passContext.settings.formFactor === 'mobile',
await session.sendCommand('Emulation.setDeviceMetricsOverride', {
mobile: settings.formFactor === 'mobile',
...observedDeviceMetrics,
});
}
Expand Down
6 changes: 3 additions & 3 deletions lighthouse-core/test/fraggle-rock/api-test-pptr.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,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(`73`);
expect(auditResults.length).toMatchInlineSnapshot(`74`);

expect(erroredAudits).toHaveLength(0);
expect(failedAudits.map(audit => audit.id)).toContain('label');
Expand All @@ -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(`27`);
expect(auditResults.length).toMatchInlineSnapshot(`28`);

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(`113`);
expect(auditResults.length).toMatchInlineSnapshot(`114`);
expect(erroredAudits).toHaveLength(0);

const failedAuditIds = failedAudits.map(audit => audit.id);
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/test/fraggle-rock/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ describe('Fraggle Rock Config', () => {
});

it('should throw on invalid artifact definitions', () => {
const configJson = {artifacts: [{id: 'FullPageScreenshot', gatherer: 'full-page-screenshot'}]};
expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/FullPageScreenshot gatherer/);
const configJson = {artifacts: [{id: 'ScriptElements', gatherer: 'script-elements'}]};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are running out of candidates for this hehe

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we are 😎

expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/ScriptElements gatherer/);
});

it('should resolve navigation definitions', () => {
Expand Down
8 changes: 7 additions & 1 deletion lighthouse-core/test/fraggle-rock/gather/mock-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const {
createMockOnceFn,
createMockSendCommandFn,
} = require('../../gather/mock-commands.js');
const {defaultSettings} = require('../../../config/constants.js');

/**
* @fileoverview Mock fraggle rock driver for testing.
Expand Down Expand Up @@ -134,6 +135,7 @@ function createMockContext() {
gatherMode: 'navigation',
computedCache: new Map(),
dependencies: {},
settings: defaultSettings,

/** @return {LH.Gatherer.FRTransitionalContext} */
asContext() {
Expand All @@ -156,7 +158,10 @@ function mockDriverSubmodules() {
prepareTargetForIndividualNavigation: jest.fn(),
};
const storageMock = {clearDataForOrigin: jest.fn()};
const emulationMock = {clearThrottling: jest.fn()};
const emulationMock = {
clearThrottling: jest.fn(),
emulate: jest.fn(),
};
const networkMock = {
fetchResponseBodyFromCache: jest.fn(),
};
Expand All @@ -167,6 +172,7 @@ function mockDriverSubmodules() {
prepareMock.prepareTargetForIndividualNavigation = jest.fn().mockResolvedValue({warnings: []});
storageMock.clearDataForOrigin = jest.fn();
emulationMock.clearThrottling = jest.fn();
emulationMock.emulate = jest.fn();
networkMock.fetchResponseBodyFromCache = jest.fn().mockResolvedValue('');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

const helpers = require('../../../fraggle-rock/gather/runner-helpers.js');
const Gatherer = require('../../../fraggle-rock/gather/base-gatherer.js');
const {defaultSettings} = require('../../../config/constants.js');
const {createMockDriver, createMockGathererInstance} = require('./mock-driver.js');

/* eslint-env jest */
Expand Down Expand Up @@ -130,6 +131,7 @@ describe('collectPhaseArtifacts', () => {
phase,
gatherMode: /** @type {any} */ (gatherMode),
computedCache: new Map(),
settings: defaultSettings,
});
expect(artifactState[phase]).toEqual({
Timespan: expect.any(Promise),
Expand All @@ -152,6 +154,7 @@ describe('collectPhaseArtifacts', () => {
gatherMode: 'navigation',
phase: 'getArtifact',
computedCache: new Map(),
settings: defaultSettings,
});
expect(await artifactState.getArtifact.Snapshot).toEqual({type: 'snapshot'});
expect(await artifactState.getArtifact.Timespan).toEqual({type: 'timespan'});
Expand All @@ -173,6 +176,7 @@ describe('collectPhaseArtifacts', () => {
gatherMode: 'navigation',
phase: 'getArtifact',
computedCache: new Map(),
settings: defaultSettings,
});
expect(artifactState.getArtifact).toEqual({
Dependency: expect.any(Promise),
Expand Down Expand Up @@ -204,6 +208,7 @@ describe('collectPhaseArtifacts', () => {
gatherMode: 'navigation',
phase: 'getArtifact',
computedCache: new Map(),
settings: defaultSettings,
});
expect(artifactState.getArtifact).toEqual({
Snapshot: expect.any(Promise),
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/test/gather/gatherers/css-usage-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
/* eslint-env jest */

const CSSUsage = require('../../../gather/gatherers/css-usage.js');
const {defaultSettings} = require('../../../config/constants.js');
const {createMockDriver} = require('../../fraggle-rock/gather/mock-driver.js');

describe('.getArtifact', () => {
Expand Down Expand Up @@ -38,6 +39,7 @@ describe('.getArtifact', () => {
gatherMode: 'snapshot',
computedCache: new Map(),
dependencies: {},
settings: defaultSettings,
};
const gatherer = new CSSUsage();
const artifact = await gatherer.getArtifact(context);
Expand Down Expand Up @@ -92,6 +94,7 @@ describe('.getArtifact', () => {
gatherMode: 'snapshot',
computedCache: new Map(),
dependencies: {},
settings: defaultSettings,
};
const gatherer = new CSSUsage();
const artifact = await gatherer.getArtifact(context);
Expand Down
Loading