Skip to content

Commit

Permalink
Merge branch 'main' into rm-fr-legacy-names
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine committed Jul 21, 2023
2 parents 1ce9956 + f60f4e4 commit 63750b7
Show file tree
Hide file tree
Showing 16 changed files with 109 additions and 172 deletions.
2 changes: 1 addition & 1 deletion cli/test/smokehouse/lighthouse-runners/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {Worker, isMainThread, parentPort, workerData} from 'worker_threads';
import {once} from 'events';

import puppeteer from 'puppeteer-core';
import ChromeLauncher from 'chrome-launcher';
import * as ChromeLauncher from 'chrome-launcher';

import {LH_ROOT} from '../../../../root.js';
import {loadArtifacts, saveArtifacts} from '../../../../core/lib/asset-saver.js';
Expand Down
2 changes: 1 addition & 1 deletion core/config/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const throttling = {
DEVTOOLS_RTT_ADJUSTMENT_FACTOR,
DEVTOOLS_THROUGHPUT_ADJUSTMENT_FACTOR,
// These values align with WebPageTest's definition of "Fast 3G"
// But offer similar charateristics to roughly the 75th percentile of 4G connections.
// But offer similar characteristics to roughly the 75th percentile of 4G connections.
mobileSlow4G: {
rttMs: 150,
throughputKbps: 1.6 * 1024,
Expand Down
12 changes: 11 additions & 1 deletion core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import log from 'lighthouse-logger';
import {ExecutionContext} from './driver/execution-context.js';
import {TargetManager} from './driver/target-manager.js';
import {Fetcher} from './fetcher.js';
import {NetworkMonitor} from './driver/network-monitor.js';

/** @return {*} */
const throwNotConnectedFn = () => {
Expand Down Expand Up @@ -37,6 +38,8 @@ class Driver {
this._page = page;
/** @type {TargetManager|undefined} */
this._targetManager = undefined;
/** @type {NetworkMonitor|undefined} */
this._networkMonitor = undefined;
/** @type {ExecutionContext|undefined} */
this._executionContext = undefined;
/** @type {Fetcher|undefined} */
Expand All @@ -51,7 +54,6 @@ class Driver {
return this._executionContext;
}

/** @return {Fetcher} */
get fetcher() {
if (!this._fetcher) return throwNotConnectedFn();
return this._fetcher;
Expand All @@ -62,6 +64,11 @@ class Driver {
return this._targetManager;
}

get networkMonitor() {
if (!this._networkMonitor) return throwNotConnectedFn();
return this._networkMonitor;
}

/** @return {Promise<string>} */
async url() {
return this._page.url();
Expand All @@ -75,6 +82,8 @@ class Driver {
const cdpSession = await this._page.target().createCDPSession();
this._targetManager = new TargetManager(cdpSession);
await this._targetManager.enable();
this._networkMonitor = new NetworkMonitor(this._targetManager);
await this._networkMonitor.enable();
this.defaultSession = this._targetManager.rootSession();
this._executionContext = new ExecutionContext(this.defaultSession);
this._fetcher = new Fetcher(this.defaultSession);
Expand All @@ -85,6 +94,7 @@ class Driver {
async disconnect() {
if (this.defaultSession === throwingSession) return;
await this._targetManager?.disable();
await this._networkMonitor?.disable();
await this.defaultSession.dispose();
}
}
Expand Down
5 changes: 1 addition & 4 deletions core/gather/driver/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import log from 'lighthouse-logger';

import {NetworkMonitor} from './network-monitor.js';
import {waitForFullyLoaded, waitForFrameNavigated, waitForUserToContinue} from './wait-for-condition.js'; // eslint-disable-line max-len
import * as constants from '../../config/constants.js';
import * as i18n from '../../lib/i18n/i18n.js';
Expand Down Expand Up @@ -89,10 +88,9 @@ async function gotoURL(driver, requestor, options) {
log.time(status);

const session = driver.defaultSession;
const networkMonitor = new NetworkMonitor(driver.targetManager);
const networkMonitor = driver.networkMonitor;

// Enable the events and network monitor needed to track navigation progress.
await networkMonitor.enable();
await session.sendCommand('Page.enable');
await session.sendCommand('Page.setLifecycleEventsEnabled', {enabled: true});

Expand Down Expand Up @@ -144,7 +142,6 @@ async function gotoURL(driver, requestor, options) {

// Bring `Page.navigate` errors back into the promise chain. See https://github.com/GoogleChrome/lighthouse/pull/6739.
await waitForNavigationTriggered;
await networkMonitor.disable();

if (options.debugNavigation) {
await waitForUserToContinue(driver);
Expand Down
1 change: 1 addition & 0 deletions core/gather/driver/network-monitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {EventEmitter} from 'events';

import log from 'lighthouse-logger';

import * as LH from '../../../types/lh.js';
import {NetworkRecorder} from '../../lib/network-recorder.js';
import {NetworkRequest} from '../../lib/network-request.js';
import UrlUtils from '../../lib/url-utils.js';
Expand Down
5 changes: 1 addition & 4 deletions core/gather/gatherers/full-page-screenshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import BaseGatherer from '../base-gatherer.js';
import * as emulation from '../../lib/emulation.js';
import {pageFunctions} from '../../lib/page-functions.js';
import {NetworkMonitor} from '../driver/network-monitor.js';
import {waitForNetworkIdle} from '../driver/wait-for-condition.js';

// JPEG quality setting
Expand Down Expand Up @@ -89,15 +88,14 @@ class FullPageScreenshot extends BaseGatherer {
const height = Math.min(fullHeight, MAX_WEBP_SIZE);

// Setup network monitor before we change the viewport.
const networkMonitor = new NetworkMonitor(context.driver.targetManager);
const networkMonitor = context.driver.networkMonitor;
const waitForNetworkIdleResult = waitForNetworkIdle(session, networkMonitor, {
pretendDCLAlreadyFired: true,
networkQuietThresholdMs: 1000,
busyEvent: 'network-critical-busy',
idleEvent: 'network-critical-idle',
isIdle: recorder => recorder.isCriticalIdle(),
});
await networkMonitor.enable();

await session.sendCommand('Emulation.setDeviceMetricsOverride', {
mobile: deviceMetrics.mobile,
Expand All @@ -113,7 +111,6 @@ class FullPageScreenshot extends BaseGatherer {
waitForNetworkIdleResult.promise,
]);
waitForNetworkIdleResult.cancel();
await networkMonitor.disable();

// Now that new resources are (probably) fetched, wait long enough for a layout.
await context.driver.executionContext.evaluate(waitForDoubleRaf, {args: []});
Expand Down
7 changes: 3 additions & 4 deletions core/gather/gatherers/scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@ function isLighthouseRuntimeEvaluateScript(script) {
* @fileoverview Gets JavaScript file contents.
*/
class Scripts extends BaseGatherer {
static symbol = Symbol('Scripts');

/** @type {LH.Gatherer.GathererMeta} */
meta = {
symbol: Scripts.symbol,
supportedModes: ['timespan', 'navigation'],
};

Expand Down Expand Up @@ -91,10 +94,6 @@ class Scripts extends BaseGatherer {

session.off('Debugger.scriptParsed', this.onScriptParsed);

// Without this line the Debugger domain will be off due
// to overlapped enabled/disable calls in other gatherers.
await session.sendCommand('Debugger.enable');

// If run on a mobile device, be sensitive to memory limitations and only
// request one at a time.
this._scriptContents = await runInSeriesOrParallel(
Expand Down
67 changes: 18 additions & 49 deletions core/gather/gatherers/source-maps.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,18 @@

import SDK from '../../lib/cdt/SDK.js';
import BaseGatherer from '../base-gatherer.js';
import Scripts from './scripts.js';

/**
* @fileoverview Gets JavaScript source maps.
*/
class SourceMaps extends BaseGatherer {
/** @type {LH.Gatherer.GathererMeta} */
/** @type {LH.Gatherer.GathererMeta<'Scripts'>} */
meta = {
supportedModes: ['timespan', 'navigation'],
dependencies: {Scripts: Scripts.symbol},
};

constructor() {
super();
/** @type {LH.Crdp.Debugger.ScriptParsedEvent[]} */
this._scriptParsedEvents = [];
this.onScriptParsed = this.onScriptParsed.bind(this);
}

/**
* @param {LH.Gatherer.Driver} driver
* @param {string} sourceMapUrl
Expand All @@ -45,15 +40,6 @@ class SourceMaps extends BaseGatherer {
return SDK.SourceMap.parseSourceMap(buffer.toString());
}

/**
* @param {LH.Crdp.Debugger.ScriptParsedEvent} event
*/
onScriptParsed(event) {
if (event.sourceMapURL) {
this._scriptParsedEvents.push(event);
}
}

/**
* @param {string} url
* @param {string} base
Expand All @@ -69,27 +55,27 @@ class SourceMaps extends BaseGatherer {

/**
* @param {LH.Gatherer.Driver} driver
* @param {LH.Crdp.Debugger.ScriptParsedEvent} event
* @param {LH.Artifacts.Script} script
* @return {Promise<LH.Artifacts.SourceMap>}
*/
async _retrieveMapFromScriptParsedEvent(driver, event) {
if (!event.sourceMapURL) {
async _retrieveMapFromScript(driver, script) {
if (!script.sourceMapURL) {
throw new Error('precondition failed: event.sourceMapURL should exist');
}

// `sourceMapURL` is simply the URL found in either a magic comment or an x-sourcemap header.
// It has not been resolved to a base url.
const isSourceMapADataUri = event.sourceMapURL.startsWith('data:');
const scriptUrl = event.url;
const isSourceMapADataUri = script.sourceMapURL.startsWith('data:');
const scriptUrl = script.name;
const rawSourceMapUrl = isSourceMapADataUri ?
event.sourceMapURL :
this._resolveUrl(event.sourceMapURL, event.url);
script.sourceMapURL :
this._resolveUrl(script.sourceMapURL, script.name);

if (!rawSourceMapUrl) {
return {
scriptId: event.scriptId,
scriptId: script.scriptId,
scriptUrl,
errorMessage: `Could not resolve map url: ${event.sourceMapURL}`,
errorMessage: `Could not resolve map url: ${script.sourceMapURL}`,
};
}

Expand All @@ -109,14 +95,14 @@ class SourceMaps extends BaseGatherer {
map.sections = map.sections.filter(section => section.map);
}
return {
scriptId: event.scriptId,
scriptId: script.scriptId,
scriptUrl,
sourceMapUrl,
map,
};
} catch (err) {
return {
scriptId: event.scriptId,
scriptId: script.scriptId,
scriptUrl,
sourceMapUrl,
errorMessage: err.toString(),
Expand All @@ -125,30 +111,13 @@ class SourceMaps extends BaseGatherer {
}

/**
* @param {LH.Gatherer.Context} context
*/
async startSensitiveInstrumentation(context) {
const session = context.driver.defaultSession;
session.on('Debugger.scriptParsed', this.onScriptParsed);
await session.sendCommand('Debugger.enable');
}

/**
* @param {LH.Gatherer.Context} context
*/
async stopSensitiveInstrumentation(context) {
const session = context.driver.defaultSession;
await session.sendCommand('Debugger.disable');
session.off('Debugger.scriptParsed', this.onScriptParsed);
}

/**
* @param {LH.Gatherer.Context} context
* @param {LH.Gatherer.Context<'Scripts'>} context
* @return {Promise<LH.Artifacts['SourceMaps']>}
*/
async getArtifact(context) {
const eventProcessPromises = this._scriptParsedEvents
.map((event) => this._retrieveMapFromScriptParsedEvent(context.driver, event));
const eventProcessPromises = context.dependencies.Scripts
.filter(script => script.sourceMapURL)
.map(script => this._retrieveMapFromScript(context.driver, script));
return Promise.all(eventProcessPromises);
}
}
Expand Down
1 change: 1 addition & 0 deletions core/lib/network-recorder.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {EventEmitter} from 'events';

import log from 'lighthouse-logger';

import * as LH from '../../types/lh.js';
import {NetworkRequest} from './network-request.js';
import {PageDependencyGraph} from '../computed/page-dependency-graph.js';

Expand Down
8 changes: 8 additions & 0 deletions core/test/gather/driver/navigation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ describe('.gotoURL', () => {

it('will track redirects through gotoURL load with warning', async () => {
mockDriver.defaultSession.on = mockDriver.defaultSession.once;
await driver.networkMonitor.enable();

const url = 'http://example.com';

Expand Down Expand Up @@ -91,6 +92,7 @@ describe('.gotoURL', () => {

it('backfills requestedUrl when using a callback requestor', async () => {
mockDriver.defaultSession.on = mockDriver.defaultSession.once;
await driver.networkMonitor.enable();

const requestor = () => Promise.resolve();

Expand All @@ -110,6 +112,7 @@ describe('.gotoURL', () => {

it('throws if no navigations found using a callback requestor', async () => {
mockDriver.defaultSession.on = mockDriver.defaultSession.once;
await driver.networkMonitor.enable();

const requestor = () => Promise.resolve();

Expand All @@ -129,6 +132,7 @@ describe('.gotoURL', () => {

it('does not add warnings when URLs are equal', async () => {
mockDriver.defaultSession.on = mockDriver.defaultSession.once;
await driver.networkMonitor.enable();

const url = 'https://www.example.com';

Expand All @@ -145,6 +149,7 @@ describe('.gotoURL', () => {

it('waits for Page.frameNavigated', async () => {
mockDriver.defaultSession.on = mockDriver.defaultSession.once;
await driver.networkMonitor.enable();

const url = 'https://www.example.com';

Expand All @@ -163,6 +168,7 @@ describe('.gotoURL', () => {

it('waits for page load', async () => {
mockDriver.defaultSession.on = mockDriver.defaultSession.once;
await driver.networkMonitor.enable();

const url = 'https://www.example.com';

Expand Down Expand Up @@ -192,6 +198,7 @@ describe('.gotoURL', () => {

it('waits for page FCP', async () => {
mockDriver.defaultSession.on = mockDriver.defaultSession.once;
await driver.networkMonitor.enable();

const url = 'https://www.example.com';

Expand Down Expand Up @@ -226,6 +233,7 @@ describe('.gotoURL', () => {

it('throws when asked to wait for FCP without waiting for load', async () => {
mockDriver.defaultSession.on = mockDriver.defaultSession.once;
await driver.networkMonitor.enable();

const url = 'https://www.example.com';

Expand Down
Loading

0 comments on commit 63750b7

Please sign in to comment.