Skip to content

Commit

Permalink
core: single network monitor kept on Driver (#15055)
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark authored Jul 21, 2023
1 parent 7587605 commit 42ffe53
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 10 deletions.
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
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
4 changes: 3 additions & 1 deletion core/test/gather/mock-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from './mock-commands.js';
import * as constants from '../../config/constants.js';
import {fnAny} from '../test-utils.js';
import {NetworkMonitor} from '../../gather/driver/network-monitor.js';

/** @typedef {import('../../gather/driver.js').Driver} Driver */
/** @typedef {import('../../gather/driver/execution-context.js')} ExecutionContext */
Expand Down Expand Up @@ -141,7 +142,7 @@ function createMockTargetManager(session) {
on: createMockOnFn(),
off: fnAny(),

/** @return {import('../../gather/driver/target-manager.js')} */
/** @return {LH.Gatherer.Driver['targetManager']} */
asTargetManager() {
// @ts-expect-error - We'll rely on the tests passing to know this matches.
return this;
Expand All @@ -168,6 +169,7 @@ function createMockDriver() {
fetcher: {
fetchResource: fnAny(),
},
networkMonitor: new NetworkMonitor(targetManager.asTargetManager()),

/** @return {Driver} */
asDriver() {
Expand Down
2 changes: 2 additions & 0 deletions types/gatherer.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {NetworkNode as _NetworkNode} from '../core/lib/dependency-graph/network-
import {CPUNode as _CPUNode} from '../core/lib/dependency-graph/cpu-node.js';
import {Simulator as _Simulator} from '../core/lib/dependency-graph/simulator/simulator.js';
import {ExecutionContext} from '../core/gather/driver/execution-context.js';
import {NetworkMonitor} from '../core/gather/driver/network-monitor.js';
import {Fetcher} from '../core/gather/fetcher.js';
import {ArbitraryEqualityMap} from '../core/lib/arbitrary-equality-map.js';

Expand Down Expand Up @@ -48,6 +49,7 @@ declare module Gatherer {
on(event: 'protocolevent', callback: (payload: Protocol.RawEventMessage) => void): void
off(event: 'protocolevent', callback: (payload: Protocol.RawEventMessage) => void): void
};
networkMonitor: NetworkMonitor;
}

interface Context<TDependencies extends DependencyKey = DefaultDependenciesKey> {
Expand Down

0 comments on commit 42ffe53

Please sign in to comment.