From 06a655c05cf558c2dba4fb9e31a48f80c69cccd6 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Fri, 10 Jun 2022 11:54:41 -0500 Subject: [PATCH 1/7] core: make targetManager accessible on driver --- lighthouse-core/fraggle-rock/gather/driver.js | 18 +- .../fraggle-rock/gather/session.js | 37 +---- lighthouse-core/gather/driver.js | 39 +++-- lighthouse-core/gather/driver/navigation.js | 2 +- .../gather/driver/network-monitor.js | 71 ++------ .../gather/driver/target-manager.js | 155 +++++++++++++----- .../gather/gatherers/devtools-log.js | 20 +-- .../gather/gatherers/full-page-screenshot.js | 2 +- types/gatherer.d.ts | 7 +- types/protocol.d.ts | 34 ++-- 10 files changed, 193 insertions(+), 192 deletions(-) diff --git a/lighthouse-core/fraggle-rock/gather/driver.js b/lighthouse-core/fraggle-rock/gather/driver.js index 064401e1223c..00def9d384d4 100644 --- a/lighthouse-core/fraggle-rock/gather/driver.js +++ b/lighthouse-core/fraggle-rock/gather/driver.js @@ -6,9 +6,9 @@ 'use strict'; const log = require('lighthouse-logger'); -const ProtocolSession = require('./session.js'); const ExecutionContext = require('../../gather/driver/execution-context.js'); const Fetcher = require('../../gather/fetcher.js'); +const TargetManager = require('../../gather/driver/target-manager.js'); /** @return {*} */ const throwNotConnectedFn = () => { @@ -24,8 +24,6 @@ const throwingSession = { on: throwNotConnectedFn, once: throwNotConnectedFn, off: throwNotConnectedFn, - addProtocolMessageListener: throwNotConnectedFn, - removeProtocolMessageListener: throwNotConnectedFn, sendCommand: throwNotConnectedFn, dispose: throwNotConnectedFn, }; @@ -37,6 +35,8 @@ class Driver { */ constructor(page) { this._page = page; + /** @type {TargetManager|undefined} */ + this._targetManager = undefined; /** @type {ExecutionContext|undefined} */ this._executionContext = undefined; /** @type {Fetcher|undefined} */ @@ -57,6 +57,11 @@ class Driver { return this._fetcher; } + get targetManager() { + if (!this._targetManager) return throwNotConnectedFn(); + return this._targetManager; + } + /** @return {Promise} */ async url() { return this._page.url(); @@ -67,8 +72,10 @@ class Driver { if (this.defaultSession !== throwingSession) return; const status = {msg: 'Connecting to browser', id: 'lh:driver:connect'}; log.time(status); - const session = await this._page.target().createCDPSession(); - this.defaultSession = new ProtocolSession(session); + const cdpSession = await this._page.target().createCDPSession(); + this._targetManager = new TargetManager(cdpSession); + await this._targetManager.enable(); + this.defaultSession = this._targetManager.rootSession(); this._executionContext = new ExecutionContext(this.defaultSession); this._fetcher = new Fetcher(this.defaultSession); log.timeEnd(status); @@ -77,6 +84,7 @@ class Driver { /** @return {Promise} */ async disconnect() { if (this.defaultSession === throwingSession) return; + this._targetManager?.disable(); await this.defaultSession.dispose(); } } diff --git a/lighthouse-core/fraggle-rock/gather/session.js b/lighthouse-core/fraggle-rock/gather/session.js index 5477cdcc45a5..d7f0116113c6 100644 --- a/lighthouse-core/fraggle-rock/gather/session.js +++ b/lighthouse-core/fraggle-rock/gather/session.js @@ -21,15 +21,10 @@ class ProtocolSession { this._targetInfo = undefined; /** @type {number|undefined} */ this._nextProtocolTimeout = undefined; - /** @type {WeakMap} */ - this._callbackMap = new WeakMap(); } - sessionId() { - return this._targetInfo && this._targetInfo.type === 'iframe' ? - // TODO: use this._session.id() for real session id. - this._targetInfo.targetId : - undefined; + id() { + return this._cdpSession.id(); } /** @param {LH.Crdp.Target.TargetInfo} targetInfo */ @@ -78,34 +73,6 @@ class ProtocolSession { this._cdpSession.once(eventName, /** @type {*} */ (callback)); } - /** - * Bind to puppeteer's '*' event that fires for *any* protocol event, - * and wrap it with data about the protocol message instead of just the event. - * @param {(payload: LH.Protocol.RawEventMessage) => void} callback - */ - addProtocolMessageListener(callback) { - /** - * @param {any} method - * @param {any} event - */ - const listener = (method, event) => callback({ - method, - params: event, - sessionId: this.sessionId(), - }); - this._callbackMap.set(callback, listener); - this._cdpSession.on('*', /** @type {*} */ (listener)); - } - - /** - * @param {(payload: LH.Protocol.RawEventMessage) => void} callback - */ - removeProtocolMessageListener(callback) { - const listener = this._callbackMap.get(callback); - if (!listener) return; - this._cdpSession.off('*', /** @type {*} */ (listener)); - } - /** * Bind listeners for protocol events. * @template {keyof LH.CrdpEvents} E diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index c540c7b1e1fe..c9ebb483e8f3 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -96,6 +96,29 @@ class Driver { this.evaluate = this.executionContext.evaluate.bind(this.executionContext); /** @private @deprecated Only available for plugin backcompat. */ this.evaluateAsync = this.executionContext.evaluateAsync.bind(this.executionContext); + + // A shim for sufficient coverage of targetManager functionality. + this.targetManager = { + rootSession: () => { + return this.defaultSession; + }, + /** + * Bind to *any* protocol event. + * @param {'protocolevent'} event + * @param {(payload: LH.Protocol.RawEventMessage) => void} callback + */ + on: (event, callback) => { + this._connection.on('protocolevent', callback); + }, + /** + * Unbind to *any* protocol event. + * @param {'protocolevent'} event + * @param {(payload: LH.Protocol.RawEventMessage) => void} callback + */ + off: (event, callback) => { + this._connection.off('protocolevent', callback); + }, + }; } /** @deprecated - Not available on Fraggle Rock driver. */ @@ -187,22 +210,6 @@ class Driver { this._eventEmitter.removeListener(eventName, cb); } - /** - * Bind to *any* protocol event. - * @param {(payload: LH.Protocol.RawEventMessage) => void} callback - */ - addProtocolMessageListener(callback) { - this._connection.on('protocolevent', callback); - } - - /** - * Unbind to *any* protocol event. - * @param {(payload: LH.Protocol.RawEventMessage) => void} callback - */ - removeProtocolMessageListener(callback) { - this._connection.off('protocolevent', callback); - } - /** @param {LH.Crdp.Target.TargetInfo} targetInfo */ setTargetInfo(targetInfo) { // eslint-disable-line no-unused-vars // OOPIF handling in legacy driver is implicit. diff --git a/lighthouse-core/gather/driver/navigation.js b/lighthouse-core/gather/driver/navigation.js index b3e78f3bc960..25a77d6abc70 100644 --- a/lighthouse-core/gather/driver/navigation.js +++ b/lighthouse-core/gather/driver/navigation.js @@ -89,7 +89,7 @@ async function gotoURL(driver, requestor, options) { log.time(status); const session = driver.defaultSession; - const networkMonitor = new NetworkMonitor(driver.defaultSession); + const networkMonitor = new NetworkMonitor(driver.targetManager); // Enable the events and network monitor needed to track navigation progress. await networkMonitor.enable(); diff --git a/lighthouse-core/gather/driver/network-monitor.js b/lighthouse-core/gather/driver/network-monitor.js index 3afc3c618ea5..9274217de17a 100644 --- a/lighthouse-core/gather/driver/network-monitor.js +++ b/lighthouse-core/gather/driver/network-monitor.js @@ -15,38 +15,34 @@ const {EventEmitter} = require('events'); const NetworkRecorder = require('../../lib/network-recorder.js'); const NetworkRequest = require('../../lib/network-request.js'); const URL = require('../../lib/url-shim.js'); -const TargetManager = require('./target-manager.js'); /** @typedef {import('../../lib/network-recorder.js').NetworkRecorderEvent} NetworkRecorderEvent */ /** @typedef {'network-2-idle'|'network-critical-idle'|'networkidle'|'networkbusy'|'network-critical-busy'|'network-2-busy'} NetworkMonitorEvent_ */ /** @typedef {NetworkRecorderEvent|NetworkMonitorEvent_} NetworkMonitorEvent */ -/** @typedef {Record & Record & {protocolmessage: [LH.Protocol.RawEventMessage]}} NetworkMonitorEventMap */ +/** @typedef {Record & Record} NetworkMonitorEventMap */ /** @typedef {LH.Protocol.StrictEventEmitter} NetworkMonitorEmitter */ /** @implements {NetworkMonitorEmitter} */ class NetworkMonitor { /** @type {NetworkRecorder|undefined} */ _networkRecorder = undefined; - /** @type {TargetManager|undefined} */ - _targetManager = undefined; /** @type {Array} */ _frameNavigations = []; - /** @param {LH.Gatherer.FRProtocolSession} session */ - constructor(session) { - this._session = session; + // TODO(FR-COMPAT): switch to real TargetManager when legacy removed. + /** @param {LH.Gatherer.FRTransitionalDriver['targetManager']} targetManager */ + constructor(targetManager) { + /** @type {LH.Gatherer.FRTransitionalDriver['targetManager']} */ + this._targetManager = targetManager; - this._onTargetAttached = this._onTargetAttached.bind(this); - - /** @type {Map} */ - this._sessions = new Map(); + /** @type {LH.Gatherer.FRProtocolSession} */ + this._session = targetManager.rootSession(); /** @param {LH.Crdp.Page.FrameNavigatedEvent} event */ this._onFrameNavigated = event => this._frameNavigations.push(event.frame); /** @param {LH.Protocol.RawEventMessage} event */ this._onProtocolMessage = event => { - this.emit('protocolmessage', event); if (!this._networkRecorder) return; this._networkRecorder.dispatch(event); }; @@ -69,31 +65,14 @@ class NetworkMonitor { this.removeAllListeners = emitter.removeAllListeners.bind(emitter); } - /** - * @param {{target: {targetId: string}, session: LH.Gatherer.FRProtocolSession}} session - */ - async _onTargetAttached({session, target}) { - const targetId = target.targetId; - - this._sessions.set(targetId, session); - session.addProtocolMessageListener(this._onProtocolMessage); - - await session.sendCommand('Network.enable'); - } - /** * @return {Promise} */ async enable() { - if (this._targetManager) return; + if (this._networkRecorder) return; this._frameNavigations = []; - this._sessions = new Map(); this._networkRecorder = new NetworkRecorder(); - /** @type {LH.Puppeteer.CDPSession} */ - // @ts-expect-error - temporarily reach in to get CDPSession - const rootCdpSession = this._session._cdpSession; - this._targetManager = new TargetManager(rootCdpSession); /** * Reemit the same network recorder events. @@ -109,46 +88,20 @@ class NetworkMonitor { this._networkRecorder.on('requestfinished', reEmit('requestfinished')); this._session.on('Page.frameNavigated', this._onFrameNavigated); - await this._session.sendCommand('Page.enable'); - - // Legacy driver does its own target management. - // @ts-expect-error - const isLegacyRunner = Boolean(this._session._domainEnabledCounts); - if (isLegacyRunner) { - this._session.addProtocolMessageListener(this._onProtocolMessage); - } else { - this._targetManager.addTargetAttachedListener(this._onTargetAttached); - await this._targetManager.enable(); - } + this._targetManager.on('protocolevent', this._onProtocolMessage); } /** * @return {Promise} */ async disable() { - if (!this._targetManager) return; + if (!this._networkRecorder) return; this._session.off('Page.frameNavigated', this._onFrameNavigated); - - // Legacy driver does its own target management. - // @ts-expect-error - const isLegacyRunner = Boolean(this._session._domainEnabledCounts); - if (isLegacyRunner) { - this._session.removeProtocolMessageListener(this._onProtocolMessage); - } else { - this._targetManager.removeTargetAttachedListener(this._onTargetAttached); - - for (const session of this._sessions.values()) { - session.removeProtocolMessageListener(this._onProtocolMessage); - } - - await this._targetManager.disable(); - } + this._targetManager.off('protocolevent', this._onProtocolMessage); this._frameNavigations = []; this._networkRecorder = undefined; - this._targetManager = undefined; - this._sessions = new Map(); } /** @return {Promise<{requestedUrl?: string, mainDocumentUrl?: string}>} */ diff --git a/lighthouse-core/gather/driver/target-manager.js b/lighthouse-core/gather/driver/target-manager.js index 1909465f3302..b1f70fa584b9 100644 --- a/lighthouse-core/gather/driver/target-manager.js +++ b/lighthouse-core/gather/driver/target-manager.js @@ -5,70 +5,125 @@ */ 'use strict'; -/** - * @fileoverview This class tracks multiple targets (the page itself and its OOPIFs) and allows consumers to - * listen for protocol events before each target is resumed. - */ - +const EventEmitter = require('events').EventEmitter; const log = require('lighthouse-logger'); const ProtocolSession = require('../../fraggle-rock/gather/session.js'); -/** @typedef {{target: LH.Crdp.Target.TargetInfo, session: LH.Gatherer.FRProtocolSession}} TargetWithSession */ +/** + * @typedef {{ + * target: LH.Crdp.Target.TargetInfo, + * cdpSession: LH.Puppeteer.CDPSession, + * session: LH.Gatherer.FRProtocolSession, + * protocolListener: (event: unknown) => void, + * }} TargetWithSession + */ + +// Add protocol event types to EventEmitter. +/** @typedef {{'protocolevent': [LH.Protocol.RawEventMessage]}} ProtocolEventRecord */ +/** @typedef {new () => LH.Protocol.StrictEventEmitter} CrdpEventMessageEmitter */ +const ProtocolEventEmitter = /** @type {CrdpEventMessageEmitter} */ (EventEmitter); -class TargetManager { +/** + * Tracks targets (the page itself, its iframes, their iframes, etc) as they + * appear and allows listeners to the flattened protocol events from all targets. + */ +class TargetManager extends ProtocolEventEmitter { /** @param {LH.Puppeteer.CDPSession} cdpSession */ constructor(cdpSession) { + super(); + this._enabled = false; this._rootCdpSession = cdpSession; - /** @type {Array<(targetWithSession: TargetWithSession) => Promise|void>} */ - this._listeners = []; + /** + * A map of target id to target/session information. Used to ensure unique + * attached targets. + * @type {Map} + * */ + this._targetIdToTargets = new Map(); this._onSessionAttached = this._onSessionAttached.bind(this); + this._onFrameNavigated = this._onFrameNavigated.bind(this); + } - /** @type {Map} */ - this._targetIdToTargets = new Map(); + /** + * @param {LH.Crdp.Page.FrameNavigatedEvent} frameNavigatedEvent + */ + async _onFrameNavigated(frameNavigatedEvent) { + // Child frames are handled in `_onSessionAttached`. + if (frameNavigatedEvent.frame.parentId) return; + + // It's not entirely clear when this is necessary, but when the page switches processes on + // navigating from about:blank to the `requestedUrl`, resetting `setAutoAttach` has been + // necessary in the past. + await this._rootCdpSession.send('Target.setAutoAttach', { + autoAttach: true, + flatten: true, + waitForDebuggerOnStart: true, + }); + } - /** @param {LH.Crdp.Page.FrameNavigatedEvent} event */ - this._onFrameNavigated = async event => { - // Child frames are handled in `_onSessionAttached`. - if (event.frame.parentId) return; + /** + * @param {string} sessionId + * @return {LH.Gatherer.FRProtocolSession} + */ + _findSession(sessionId) { + for (const {session, cdpSession} of this._targetIdToTargets.values()) { + if (cdpSession.id() === sessionId) return session; + } - // It's not entirely clear when this is necessary, but when the page switches processes on - // navigating from about:blank to the `requestedUrl`, resetting `setAutoAttach` has been - // necessary in the past. - await this._rootCdpSession.send('Target.setAutoAttach', { - autoAttach: true, - flatten: true, - waitForDebuggerOnStart: true, - }); - }; + throw new Error(`session not found`); + } + + /** + * Returns the root session. + * @return {LH.Gatherer.FRProtocolSession} + */ + rootSession() { + const rootSessionId = this._rootCdpSession.id(); + return this._findSession(rootSessionId); } /** * @param {LH.Puppeteer.CDPSession} cdpSession */ async _onSessionAttached(cdpSession) { - const session = new ProtocolSession(cdpSession); + const newSession = new ProtocolSession(cdpSession); try { - const target = await session.sendCommand('Target.getTargetInfo').catch(() => null); + const target = await newSession.sendCommand('Target.getTargetInfo').catch(() => null); const targetType = target?.targetInfo?.type; const hasValidTargetType = targetType === 'page' || targetType === 'iframe'; + // TODO: should detach from target in this case? + // See pptr: https://github.com/puppeteer/puppeteer/blob/733cbecf487c71483bee8350e37030edb24bc021/src/common/Page.ts#L495-L526 if (!target || !hasValidTargetType) return; + // No need to continue if target has already been seen. const targetId = target.targetInfo.targetId; - session.setTargetInfo(target.targetInfo); if (this._targetIdToTargets.has(targetId)) return; + newSession.setTargetInfo(target.targetInfo); const targetName = target.targetInfo.url || target.targetInfo.targetId; log.verbose('target-manager', `target ${targetName} attached`); - const targetWithSession = {target: target.targetInfo, session}; + const trueProtocolListener = this._getProtocolEventListener(newSession.id()); + /** @type {(event: unknown) => void} */ + // @ts-expect-error - pptr currently typed only for single arg emits. + const protocolListener = trueProtocolListener; + cdpSession.on('*', protocolListener); + + const targetWithSession = { + target: target.targetInfo, + cdpSession, + session: newSession, + protocolListener, + }; this._targetIdToTargets.set(targetId, targetWithSession); - for (const listener of this._listeners) await listener(targetWithSession); - await session.sendCommand('Target.setAutoAttach', { + // We want to receive information about network requests from iframes, so enable the Network domain. + await newSession.sendCommand('Network.enable'); + // We also want to receive information about subtargets of subtargets, so make sure we autoattach recursively. + await newSession.sendCommand('Target.setAutoAttach', { autoAttach: true, flatten: true, waitForDebuggerOnStart: true, @@ -80,10 +135,30 @@ class TargetManager { throw err; } finally { // Resume the target if it was paused, but if it's unnecessary, we don't care about the error. - await session.sendCommand('Runtime.runIfWaitingForDebugger').catch(() => {}); + await newSession.sendCommand('Runtime.runIfWaitingForDebugger').catch(() => {}); } } + /** + * Returns a listener for all protocol events from session, and augments the + * event with the sessionId. + * @param {string} sessionId + */ + _getProtocolEventListener(sessionId) { + /** + * @template {keyof LH.Protocol.RawEventMessageRecord} EventName + * @param {EventName} method + * @param {LH.Protocol.RawEventMessageRecord[EventName]['params']} params + */ + const onProtocolEvent = (method, params) => { + // Cast because tsc 4.7 still can't quite track the dependent parameters. + const payload = /** @type {LH.Protocol.RawEventMessage} */ ({method, params, sessionId}); + this.emit('protocolevent', payload); + }; + + return onProtocolEvent; + } + /** * @return {Promise} */ @@ -115,23 +190,13 @@ class TargetManager { // No need to remove listener if connection is already closed. this._rootCdpSession.connection()?.off('sessionattached', this._onSessionAttached); + for (const {cdpSession, protocolListener} of this._targetIdToTargets.values()) { + cdpSession.off('*', protocolListener); + } + this._enabled = false; this._targetIdToTargets = new Map(); } - - /** - * @param {(targetWithSession: TargetWithSession) => Promise|void} listener - */ - addTargetAttachedListener(listener) { - this._listeners.push(listener); - } - - /** - * @param {(targetWithSession: TargetWithSession) => Promise|void} listener - */ - removeTargetAttachedListener(listener) { - this._listeners = this._listeners.filter(entry => entry !== listener); - } } module.exports = TargetManager; diff --git a/lighthouse-core/gather/gatherers/devtools-log.js b/lighthouse-core/gather/gatherers/devtools-log.js index deddc186495b..684c98fdf026 100644 --- a/lighthouse-core/gather/gatherers/devtools-log.js +++ b/lighthouse-core/gather/gatherers/devtools-log.js @@ -11,9 +11,10 @@ * This protocol log can be used to recreate the network records using lib/network-recorder.js. */ -const NetworkMonitor = require('../driver/network-monitor.js'); const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js'); +/** @typedef {import('../driver/target-manager.js')} TargetManager */ + class DevtoolsLog extends FRGatherer { static symbol = Symbol('DevtoolsLog'); @@ -26,9 +27,6 @@ class DevtoolsLog extends FRGatherer { constructor() { super(); - /** @type {NetworkMonitor|undefined} */ - this._networkMonitor = undefined; - this._messageLog = new DevtoolsMessageLog(/^(Page|Network|Target|Runtime)\./); /** @param {LH.Protocol.RawEventMessage} e */ @@ -42,16 +40,16 @@ class DevtoolsLog extends FRGatherer { this._messageLog.reset(); this._messageLog.beginRecording(); - this._networkMonitor = new NetworkMonitor(driver.defaultSession); - this._networkMonitor.on('protocolmessage', this._onProtocolMessage); - this._networkMonitor.enable(); + driver.targetManager.on('protocolevent', this._onProtocolMessage); + await driver.defaultSession.sendCommand('Page.enable'); } - async stopSensitiveInstrumentation() { - if (!this._networkMonitor) return; + /** + * @param {LH.Gatherer.FRTransitionalContext} passContext + */ + async stopSensitiveInstrumentation({driver}) { this._messageLog.endRecording(); - this._networkMonitor.disable(); - this._networkMonitor.off('protocolmessage', this._onProtocolMessage); + driver.targetManager.off('protocolevent', this._onProtocolMessage); } /** diff --git a/lighthouse-core/gather/gatherers/full-page-screenshot.js b/lighthouse-core/gather/gatherers/full-page-screenshot.js index f9031d6fab29..58cd23f89405 100644 --- a/lighthouse-core/gather/gatherers/full-page-screenshot.js +++ b/lighthouse-core/gather/gatherers/full-page-screenshot.js @@ -89,7 +89,7 @@ class FullPageScreenshot extends FRGatherer { const height = Math.min(fullHeight, maxTextureSize); // Setup network monitor before we change the viewport. - const networkMonitor = new NetworkMonitor(session); + const networkMonitor = new NetworkMonitor(context.driver.targetManager); const waitForNetworkIdleResult = waitForNetworkIdle(session, networkMonitor, { pretendDCLAlreadyFired: true, networkQuietThresholdMs: 1000, diff --git a/types/gatherer.d.ts b/types/gatherer.d.ts index b7fe07c562e2..05519f49dcbb 100644 --- a/types/gatherer.d.ts +++ b/types/gatherer.d.ts @@ -28,8 +28,6 @@ declare module Gatherer { setNextProtocolTimeout(ms: number): void; on(event: TEvent, callback: (...args: LH.CrdpEvents[TEvent]) => void): void; once(event: TEvent, callback: (...args: LH.CrdpEvents[TEvent]) => void): void; - addProtocolMessageListener(callback: (payload: Protocol.RawEventMessage) => void): void - removeProtocolMessageListener(callback: (payload: Protocol.RawEventMessage) => void): void off(event: TEvent, callback: (...args: LH.CrdpEvents[TEvent]) => void): void; sendCommand(method: TMethod, ...params: LH.CrdpCommands[TMethod]['paramsType']): Promise; dispose(): Promise; @@ -41,6 +39,11 @@ declare module Gatherer { executionContext: ExecutionContext; fetcher: Fetcher; url: () => Promise; + targetManager: { + rootSession(): FRProtocolSession; + on(event: 'protocolevent', callback: (payload: Protocol.RawEventMessage) => void): void + off(event: 'protocolevent', callback: (payload: Protocol.RawEventMessage) => void): void + }; } /** The limited context interface shared between pre and post Fraggle Rock Lighthouse. */ diff --git a/types/protocol.d.ts b/types/protocol.d.ts index 2ca8612c3985..a27e92b14b5a 100644 --- a/types/protocol.d.ts +++ b/types/protocol.d.ts @@ -5,6 +5,23 @@ */ declare module Protocol { + /** + * An intermediate type, used to create a record of all possible Crdp raw event + * messages, keyed on method. e.g. { + * 'Domain.method1Name': {method: 'Domain.method1Name', params: EventPayload1}, + * 'Domain.method2Name': {method: 'Domain.method2Name', params: EventPayload2}, + * } + */ + type RawEventMessageRecord = { + [K in keyof LH.CrdpEvents]: { + method: K, + // Drop [] for `undefined` (so a JS value is valid). + params: LH.CrdpEvents[K] extends [] ? undefined: LH.CrdpEvents[K][number] + // If sessionId is not set, it means the event was from the root target. + sessionId?: string; + }; + } + /** * Union of raw (over the wire) message format of all possible Crdp events, * of the form `{method: 'Domain.event', params: eventPayload}`. @@ -53,21 +70,4 @@ declare module Protocol { } } -/** - * An intermediate type, used to create a record of all possible Crdp raw event - * messages, keyed on method. e.g. { - * 'Domain.method1Name': {method: 'Domain.method1Name', params: EventPayload1}, - * 'Domain.method2Name': {method: 'Domain.method2Name', params: EventPayload2}, - * } - */ -type RawEventMessageRecord = { - [K in keyof LH.CrdpEvents]: { - method: K, - // Drop [] for `undefined` (so a JS value is valid). - params: LH.CrdpEvents[K] extends [] ? undefined: LH.CrdpEvents[K][number] - // If sessionId is not set, it means the event was from the root target. - sessionId?: string; - }; -} - export default Protocol; From ccc846f1a2d1fc052c97b150e209faf450066b23 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Mon, 13 Jun 2022 19:06:46 -0500 Subject: [PATCH 2/7] feedback --- lighthouse-core/gather/driver.js | 3 ++- lighthouse-core/gather/driver/target-manager.js | 8 ++++---- lighthouse-core/gather/gatherers/devtools-log.js | 2 -- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index c9ebb483e8f3..06b7b481c49d 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -97,7 +97,8 @@ class Driver { /** @private @deprecated Only available for plugin backcompat. */ this.evaluateAsync = this.executionContext.evaluateAsync.bind(this.executionContext); - // A shim for sufficient coverage of targetManager functionality. + // A shim for sufficient coverage of targetManager functionality. Exposes the target + // management that legacy driver already handles (see this._handleTargetAttached). this.targetManager = { rootSession: () => { return this.defaultSession; diff --git a/lighthouse-core/gather/driver/target-manager.js b/lighthouse-core/gather/driver/target-manager.js index b1f70fa584b9..472e02ad5a5f 100644 --- a/lighthouse-core/gather/driver/target-manager.js +++ b/lighthouse-core/gather/driver/target-manager.js @@ -20,8 +20,8 @@ const ProtocolSession = require('../../fraggle-rock/gather/session.js'); // Add protocol event types to EventEmitter. /** @typedef {{'protocolevent': [LH.Protocol.RawEventMessage]}} ProtocolEventRecord */ -/** @typedef {new () => LH.Protocol.StrictEventEmitter} CrdpEventMessageEmitter */ -const ProtocolEventEmitter = /** @type {CrdpEventMessageEmitter} */ (EventEmitter); +/** @typedef {new () => LH.Protocol.StrictEventEmitter} ProtocolEventMessageEmitter */ +const ProtocolEventEmitter = /** @type {ProtocolEventMessageEmitter} */ (EventEmitter); /** * Tracks targets (the page itself, its iframes, their iframes, etc) as they @@ -39,7 +39,7 @@ class TargetManager extends ProtocolEventEmitter { * A map of target id to target/session information. Used to ensure unique * attached targets. * @type {Map} - * */ + */ this._targetIdToTargets = new Map(); this._onSessionAttached = this._onSessionAttached.bind(this); @@ -72,7 +72,7 @@ class TargetManager extends ProtocolEventEmitter { if (cdpSession.id() === sessionId) return session; } - throw new Error(`session not found`); + throw new Error(`session ${sessionId} not found`); } /** diff --git a/lighthouse-core/gather/gatherers/devtools-log.js b/lighthouse-core/gather/gatherers/devtools-log.js index 684c98fdf026..7a6b8a08ddb7 100644 --- a/lighthouse-core/gather/gatherers/devtools-log.js +++ b/lighthouse-core/gather/gatherers/devtools-log.js @@ -13,8 +13,6 @@ const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js'); -/** @typedef {import('../driver/target-manager.js')} TargetManager */ - class DevtoolsLog extends FRGatherer { static symbol = Symbol('DevtoolsLog'); From b3a55b21cefdd0dd61ac9404ddef131b18292c40 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Thu, 16 Jun 2022 14:00:49 -0500 Subject: [PATCH 3/7] target-manager-test --- .../test/fraggle-rock/gather/mock-driver.js | 6 +- .../test/gather/driver/target-manager-test.js | 76 +++++++++++++------ lighthouse-core/test/gather/mock-commands.js | 2 + 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js index cf64d0bc82d1..ee81ac20db21 100644 --- a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js +++ b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js @@ -43,10 +43,14 @@ function createMockSession() { }; } -function createMockCdpSession() { +/** + * @param {string} sessionId + */ +function createMockCdpSession(sessionId = 'DEFAULT_ID') { const connection = createMockCdpConnection(); return { + id: () => sessionId, send: createMockSendCommandFn({useSessionId: false}), once: createMockOnceFn(), on: createMockOnFn(), diff --git a/lighthouse-core/test/gather/driver/target-manager-test.js b/lighthouse-core/test/gather/driver/target-manager-test.js index 33288b372d87..fd8e76fb5ba4 100644 --- a/lighthouse-core/test/gather/driver/target-manager-test.js +++ b/lighthouse-core/test/gather/driver/target-manager-test.js @@ -37,18 +37,19 @@ describe('TargetManager', () => { beforeEach(() => { sessionMock = createMockCdpSession(); - sessionMock.send + sendMock = sessionMock.send; + sendMock .mockResponse('Page.enable') .mockResponse('Runtime.runIfWaitingForDebugger'); - sendMock = sessionMock.send; targetManager = new TargetManager(sessionMock.asCdpSession()); targetInfo = createTargetInfo(); }); describe('.enable()', () => { it('should autoattach to root session', async () => { - sessionMock.send + sendMock .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Network.enable') .mockResponse('Target.setAutoAttach'); await targetManager.enable(); @@ -57,14 +58,17 @@ describe('TargetManager', () => { }); it('should autoattach to further unique sessions', async () => { - sessionMock.send + sendMock .mockResponse('Target.getTargetInfo', {targetInfo}) // original, attach .mockResponse('Target.getTargetInfo', {targetInfo}) // duplicate, no attach .mockResponse('Target.getTargetInfo', {targetInfo: {...targetInfo, targetId: '1'}}) // unique, attach .mockResponse('Target.getTargetInfo', {targetInfo: {...targetInfo, targetId: '2'}}) // unique, attach + .mockResponse('Network.enable') .mockResponse('Target.setAutoAttach') + .mockResponse('Network.enable') .mockResponse('Target.setAutoAttach') + .mockResponse('Network.enable') .mockResponse('Target.setAutoAttach') .mockResponse('Runtime.runIfWaitingForDebugger') @@ -94,47 +98,70 @@ describe('TargetManager', () => { await sessionListener(sessionMock); expect(sendMock.findAllInvocations('Target.getTargetInfo')).toHaveLength(4); expect(sendMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(3); + + // Four resumes because in finally clause, so runs regardless of uniqueness. + expect(sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger')).toHaveLength(4); }); it('should ignore non-frame targets', async () => { targetInfo.type = 'worker'; - sessionMock.send + sendMock .mockResponse('Target.getTargetInfo', {targetInfo}) .mockResponse('Target.setAutoAttach'); await targetManager.enable(); const invocations = sendMock.findAllInvocations('Target.setAutoAttach'); expect(invocations).toHaveLength(0); + + // Should still be resumed. + expect(sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger')).toHaveLength(1); }); - it('should fire listeners before target attached', async () => { - sessionMock.send + it('should listen to target before resuming', async () => { + sessionMock.on = /** @type {typeof sessionMock.on} */ (fnAny() + .mockImplementation(/** @param {string} eventName */ (eventName) => { + // Intercept listener for all protocol events and ensure target is still paused. + if (eventName === '*') { + expect(sendMock.findAllInvocations('Target.getTargetInfo')).toHaveLength(1); + expect(sendMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(0); + expect(sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger')).toHaveLength(0); + } + })); + + sendMock .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Network.enable') .mockResponse('Target.setAutoAttach'); - targetManager.addTargetAttachedListener(fnAny().mockImplementation(() => { - const setAutoAttachCalls = sessionMock.send.mock.calls - .filter(call => call[0] === 'Target.setAutoAttach'); - expect(setAutoAttachCalls).toHaveLength(0); - })); await targetManager.enable(); + + expect(sendMock.findAllInvocations('Target.getTargetInfo')).toHaveLength(1); + expect(sendMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(1); + expect(sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger')).toHaveLength(1); }); - it('should handle target closed gracefully', async () => { - sessionMock.send.mockResponse('Target.getTargetInfo', {targetInfo}); + it('should gracefully handle a target closing while attaching', async () => { const targetClosedError = new Error('Target closed'); - targetManager.addTargetAttachedListener(fnAny().mockRejectedValue(targetClosedError)); + sendMock + .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Network.enable') + .mockResponse('Target.setAutoAttach', () => Promise.reject(targetClosedError)); await targetManager.enable(); }); - it('should throw other listener errors', async () => { - sessionMock.send.mockResponse('Target.getTargetInfo', {targetInfo}); - const targetClosedError = new Error('Fatal error'); - targetManager.addTargetAttachedListener(fnAny().mockRejectedValue(targetClosedError)); + it('should throw other protocol errors while attaching', async () => { + const fatalError = new Error('Fatal error'); + sendMock + .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Network.enable') + .mockResponse('Target.setAutoAttach', () => Promise.reject(fatalError)); await expect(targetManager.enable()).rejects.toMatchObject({message: 'Fatal error'}); + + // Should still attempt to resume target. + expect(sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger')).toHaveLength(1); }); it('should resume the target when finished', async () => { - sessionMock.send.mockResponse('Target.getTargetInfo', {}); + sendMock.mockResponse('Target.getTargetInfo', {}); await targetManager.enable(); const invocations = sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger'); @@ -142,8 +169,9 @@ describe('TargetManager', () => { }); it('should autoattach on main frame navigation', async () => { - sessionMock.send + sendMock .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Network.enable') .mockResponse('Target.setAutoAttach') .mockResponse('Target.setAutoAttach'); await targetManager.enable(); @@ -156,8 +184,9 @@ describe('TargetManager', () => { }); it('should not autoattach on subframe navigation', async () => { - sessionMock.send + sendMock .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Network.enable') .mockResponse('Target.setAutoAttach') .mockResponse('Target.setAutoAttach'); await targetManager.enable(); @@ -170,8 +199,9 @@ describe('TargetManager', () => { }); it('should be idempotent', async () => { - sessionMock.send + sendMock .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Network.enable') .mockResponse('Target.setAutoAttach'); await targetManager.enable(); await targetManager.enable(); diff --git a/lighthouse-core/test/gather/mock-commands.js b/lighthouse-core/test/gather/mock-commands.js index a1b57d6dcfd8..bdab2fc518a7 100644 --- a/lighthouse-core/test/gather/mock-commands.js +++ b/lighthouse-core/test/gather/mock-commands.js @@ -30,6 +30,8 @@ import {fnAny} from '../test-utils.js'; * - `findInvocation` which asserts that `sendCommand` was invoked with the given command and * returns the protocol message argument. * + * To mock an error response, use `send.mockResponse('Command', () => Promise.reject(error))`. + * * There are two variants of sendCommand, one that expects a sessionId as the second positional * argument (legacy Lighthouse `Connection.sendCommand`) and one that does not (Fraggle Rock * `ProtocolSession.sendCommand`). From f78707ed666bd6ee4dcd883aa94fb6e1944bb271 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Thu, 16 Jun 2022 16:38:30 -0500 Subject: [PATCH 4/7] move protocolevent tests from session to target-manager --- .../test/fraggle-rock/gather/session-test.js | 91 ------------- .../test/gather/driver/target-manager-test.js | 122 +++++++++++++++++- 2 files changed, 118 insertions(+), 95 deletions(-) diff --git a/lighthouse-core/test/fraggle-rock/gather/session-test.js b/lighthouse-core/test/fraggle-rock/gather/session-test.js index c2db350d25e0..2772e7d7bf75 100644 --- a/lighthouse-core/test/fraggle-rock/gather/session-test.js +++ b/lighthouse-core/test/fraggle-rock/gather/session-test.js @@ -31,58 +31,6 @@ describe('ProtocolSession', () => { session = new ProtocolSession(puppeteerSession); }); - describe('ProtocolSession', () => { - it('should emit a copy of events on "*"', () => { - session = new ProtocolSession(puppeteerSession); - - const regularListener = fnAny(); - const allListener = fnAny(); - - session.on('Network.dataReceived', regularListener); - session.addProtocolMessageListener(allListener); - puppeteerSession.emit('Network.dataReceived', 1); - puppeteerSession.emit('Bar', 1); - - expect(regularListener).toHaveBeenCalledTimes(1); - expect(allListener).toHaveBeenCalledTimes(2); - expect(allListener).toHaveBeenCalledWith({method: 'Network.dataReceived', params: 1}); - expect(allListener).toHaveBeenCalledWith({method: 'Bar', params: 1}); - }); - - it('should not fire duplicate events', () => { - session = new ProtocolSession(puppeteerSession); - session = new ProtocolSession(puppeteerSession); - - const regularListener = fnAny(); - const allListener = fnAny(); - - session.on('Network.dataReceived', regularListener); - session.addProtocolMessageListener(allListener); - puppeteerSession.emit('Network.dataReceived', 1); - puppeteerSession.emit('Bar', 1); - - expect(regularListener).toHaveBeenCalledTimes(1); - expect(allListener).toHaveBeenCalledTimes(2); - }); - - it('should include sessionId for iframes', () => { - session = new ProtocolSession(puppeteerSession); - - const listener = fnAny(); - const targetInfo = {title: '', url: '', attached: true, canAccessOpener: false}; - - session.addProtocolMessageListener(listener); - session.setTargetInfo({targetId: 'page', type: 'page', ...targetInfo}); - puppeteerSession.emit('Foo', 1); - session.setTargetInfo({targetId: 'iframe', type: 'iframe', ...targetInfo}); - puppeteerSession.emit('Bar', 1); - - expect(listener).toHaveBeenCalledTimes(2); - expect(listener).toHaveBeenCalledWith({method: 'Foo', params: 1}); - expect(listener).toHaveBeenCalledWith({method: 'Bar', params: 1, sessionId: 'iframe'}); - }); - }); - /** @type {Array<'on'|'off'|'once'>} */ const delegateMethods = ['on', 'once', 'off']; for (const method of delegateMethods) { @@ -111,45 +59,6 @@ describe('ProtocolSession', () => { }); }); - describe('.addProtocolMessageListener', () => { - it('should listen for any event', () => { - session = new ProtocolSession(puppeteerSession); - - const regularListener = fnAny(); - const allListener = fnAny(); - - session.on('Page.frameNavigated', regularListener); - session.addProtocolMessageListener(allListener); - - puppeteerSession.emit('Page.frameNavigated'); - puppeteerSession.emit('Debugger.scriptParsed', {script: 'details'}); - - expect(regularListener).toHaveBeenCalledTimes(1); - expect(regularListener).toHaveBeenCalledWith(undefined); - expect(allListener).toHaveBeenCalledTimes(2); - expect(allListener).toHaveBeenCalledWith({method: 'Page.frameNavigated', params: undefined}); - expect(allListener).toHaveBeenCalledWith({ - method: 'Debugger.scriptParsed', - params: {script: 'details'}, - }); - }); - }); - - describe('.removeProtocolMessageListener', () => { - it('should stop listening for any event', () => { - session = new ProtocolSession(puppeteerSession); - - const allListener = fnAny(); - - session.addProtocolMessageListener(allListener); - puppeteerSession.emit('Page.frameNavigated'); - expect(allListener).toHaveBeenCalled(); - session.removeProtocolMessageListener(allListener); - puppeteerSession.emit('Page.frameNavigated'); - expect(allListener).toHaveBeenCalledTimes(1); - }); - }); - describe('.sendCommand', () => { it('delegates to puppeteer', async () => { const send = puppeteerSession.send = fnAny().mockResolvedValue(123); diff --git a/lighthouse-core/test/gather/driver/target-manager-test.js b/lighthouse-core/test/gather/driver/target-manager-test.js index fd8e76fb5ba4..f450dbedef50 100644 --- a/lighthouse-core/test/gather/driver/target-manager-test.js +++ b/lighthouse-core/test/gather/driver/target-manager-test.js @@ -4,16 +4,16 @@ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -import {jest} from '@jest/globals'; +import {EventEmitter} from 'events'; + +import {CDPSession} from 'puppeteer-core/lib/cjs/puppeteer/common/Connection.js'; import TargetManager from '../../../gather/driver/target-manager.js'; import {createMockCdpSession} from '../../fraggle-rock/gather/mock-driver.js'; +import {createMockSendCommandFn} from '../../gather/mock-commands.js'; import {fnAny} from '../../test-utils.js'; -jest.useFakeTimers(); - /** - * * @param {{type?: string, targetId?: string}} [overrides] * @return {LH.Crdp.Target.TargetInfo} */ @@ -220,4 +220,118 @@ describe('TargetManager', () => { expect(sessionMock.connection().off).toHaveBeenCalled(); }); }); + + describe('protocolevent emit', () => { + /** @param {string} sessionId */ + function createCdpSession(sessionId) { + class MockCdpConnection extends EventEmitter { + constructor() { + super(); + + this._rawSend = fnAny(); + } + } + + const mockCdpConnection = new MockCdpConnection(); + /** @type {LH.Puppeteer.CDPSession} */ + // @ts-expect-error - close enough to the real thing. + const cdpSession = new CDPSession(mockCdpConnection, '', sessionId); + return cdpSession; + } + + it('should listen for and re-emit protocol events across sessions', async () => { + const rootSession = createCdpSession('root'); + + const rootTargetInfo = createTargetInfo(); + // Still mock command responses at session level. + rootSession.send = createMockSendCommandFn({useSessionId: false}) + .mockResponse('Page.enable') + .mockResponse('Target.getTargetInfo', {targetInfo: rootTargetInfo}) + .mockResponse('Network.enable') + .mockResponse('Target.setAutoAttach') + .mockResponse('Runtime.runIfWaitingForDebugger'); + + const targetManager = new TargetManager(rootSession); + await targetManager.enable(); + + // Attach an iframe session. + const iframeSession = createCdpSession('iframe'); + const iframeTargetInfo = createTargetInfo({type: 'iframe', targetId: 'iframe'}); + // Still mock command responses at session level. + iframeSession.send = createMockSendCommandFn({useSessionId: false}) + .mockResponse('Target.getTargetInfo', {targetInfo: iframeTargetInfo}) + .mockResponse('Network.enable') + .mockResponse('Target.setAutoAttach') + .mockResponse('Runtime.runIfWaitingForDebugger'); + + const rootConnection = rootSession.connection(); + if (!rootConnection) throw new Error('no connection'); + rootConnection.emit('sessionattached', iframeSession); + + // Wait for iframe session to be attached. + await new Promise(resolve => setTimeout(resolve, 0)); + + const rootListener = fnAny(); + const iframeListener = fnAny(); + const allListener = fnAny(); + rootSession.on('DOM.documentUpdated', rootListener); + iframeSession.on('Animation.animationCreated', iframeListener); + targetManager.on('protocolevent', allListener); + + // @ts-expect-error - types for _onMessage are wrong. + rootSession._onMessage({method: 'DOM.documentUpdated'}); + // @ts-expect-error - types for _onMessage are wrong. + rootSession._onMessage({method: 'Debugger.scriptParsed', params: {script: 'details'}}); + // @ts-expect-error - types for _onMessage are wrong. + iframeSession._onMessage({method: 'Animation.animationCreated', params: {id: 'animated'}}); + + expect(rootListener).toHaveBeenCalledTimes(1); + expect(rootListener).toHaveBeenCalledWith(undefined); + + expect(iframeListener).toHaveBeenCalledTimes(1); + expect(iframeListener).toHaveBeenCalledWith({id: 'animated'}); + + expect(allListener).toHaveBeenCalledTimes(3); + expect(allListener).toHaveBeenCalledWith({ + method: 'DOM.documentUpdated', + params: undefined, + sessionId: 'root', + }); + expect(allListener).toHaveBeenCalledWith({ + method: 'Debugger.scriptParsed', + params: {script: 'details'}, + sessionId: 'root', + }); + expect(allListener).toHaveBeenCalledWith({ + method: 'Animation.animationCreated', + params: {id: 'animated'}, + sessionId: 'iframe', + }); + }); + + it('should stop listening for protocol events', async () => { + const rootSession = createCdpSession('root'); + // Still mock command responses at session level. + rootSession.send = createMockSendCommandFn({useSessionId: false}) + .mockResponse('Page.enable') + .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Network.enable') + .mockResponse('Target.setAutoAttach') + .mockResponse('Runtime.runIfWaitingForDebugger'); + + const targetManager = new TargetManager(rootSession); + await targetManager.enable(); + + const allListener = fnAny(); + targetManager.on('protocolevent', allListener); + + // @ts-expect-error - types for _onMessage are wrong. + rootSession._onMessage({method: 'DOM.documentUpdated'}); + expect(allListener).toHaveBeenCalled(); + targetManager.off('protocolevent', allListener); + // @ts-expect-error - types for _onMessage are wrong. + rootSession._onMessage({method: 'DOM.documentUpdated'}); + expect(allListener).toHaveBeenCalledTimes(1); + }); + }); }); From a3bc0c7bc191518beddbd5a321bc411535ebe8da Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Fri, 17 Jun 2022 12:55:38 -0500 Subject: [PATCH 5/7] network-monitor-test --- .../gather/driver/network-monitor-test.js | 260 +++++++++--------- 1 file changed, 133 insertions(+), 127 deletions(-) diff --git a/lighthouse-core/test/gather/driver/network-monitor-test.js b/lighthouse-core/test/gather/driver/network-monitor-test.js index a7430ae94a44..4c74d0d8ea0a 100644 --- a/lighthouse-core/test/gather/driver/network-monitor-test.js +++ b/lighthouse-core/test/gather/driver/network-monitor-test.js @@ -4,40 +4,24 @@ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -import {jest} from '@jest/globals'; - -import {mockDriverSubmodules} from '../../fraggle-rock/gather/mock-driver.js'; -// import NetworkMonitor from '../../../gather/driver/network-monitor.js'; +import {createMockCdpSession} from '../../fraggle-rock/gather/mock-driver.js'; +import NetworkMonitor from '../../../gather/driver/network-monitor.js'; import NetworkRequest from '../../../lib/network-request.js'; import networkRecordsToDevtoolsLog from '../../network-records-to-devtools-log.js'; -import {fnAny, mockCommands} from '../../test-utils.js'; - -const mocks = mockDriverSubmodules(); - -jest.useFakeTimers(); +import TargetManager from '../../../gather/driver/target-manager.js'; -// This can be removed when FR becomes the default. -const createMockSendCommandFn = - mockCommands.createMockSendCommandFn.bind(null, {useSessionId: false}); - -// Some imports needs to be done dynamically, so that their dependencies will be mocked. -// See: https://jestjs.io/docs/ecmascript-modules#differences-between-esm-and-commonjs -// https://github.com/facebook/jest/issues/10025 -/** @typedef {import('../../../gather/driver/network-monitor.js')} NetworkMonitor */ -/** @type {typeof import('../../../gather/driver/network-monitor.js')} */ -let NetworkMonitor; - -beforeAll(async () => { - NetworkMonitor = (await import('../../../gather/driver/network-monitor.js')).default; -}); - -const tscErr = new Error('Typecheck constrait failed'); +const tscErr = new Error('Typecheck constraint failed'); describe('NetworkMonitor', () => { - /** @type {ReturnType} */ - let sendCommandMock; - /** @type {LH.Gatherer.FRProtocolSession & {dispatch: (event: LH.Protocol.RawEventMessage | undefined) => void}} */ - let sessionMock; + /** @type {ReturnType} */ + let rootCdpSessionMock; + /** + * Dispatch events on the root CDPSession. + * @type {(event: {method: string, params: unknown}) => void} + */ + let rootDispatch; + /** @type {TargetManager} */ + let targetManager; /** @type {Array} */ let devtoolsLog; /** @type {NetworkMonitor} */ @@ -45,35 +29,68 @@ describe('NetworkMonitor', () => { /** @type {string[]} */ let statusLog = []; - function createMockSession() { - /** @type {any} */ - const session = {}; - session.off = fnAny(); - session.removeProtocolMessageListener = fnAny(); - - const on = (session.on = fnAny()); - const addProtocolMessageListener = (session.addProtocolMessageListener = fnAny()); - sendCommandMock = session.sendCommand = createMockSendCommandFn() + /** + * Mock a Puppeteer CDPSession, including the commands needed to attach to the + * target manager. + * @param {{id: string, targetType: 'page'|'iframe'|'other'}} args + */ + function mockAttachingSession({id, targetType}) { + const cdpSessionMock = createMockCdpSession(id); + cdpSessionMock.send .mockResponse('Page.enable') - .mockResponse('Network.enable'); - /** @type {(event: LH.Protocol.RawEventMessage) => void} */ - session.dispatch = event => { - const relevantOnListeners = on.mock.calls.filter(call => call[0] === event.method); - for (const call of relevantOnListeners) { - if (typeof call[1] === 'function') call[1](event.params); + .mockResponse('Target.getTargetInfo', {targetInfo: {type: targetType, targetId: id}}) + .mockResponse('Network.enable') + .mockResponse('Target.setAutoAttach') + .mockResponse('Runtime.runIfWaitingForDebugger'); + + return cdpSessionMock; + } + + /** @param {ReturnType} cdpSession */ + function createSessionDispatch(cdpSession) { + const on = cdpSession.on; + + /** @param {{method: string, params: unknown}} args */ + function dispatch({method, params}) { + // Get live listeners in case they change. + const eventListeners = on.mock.calls.filter(call => call[0] === method).map(call => call[1]); + for (const listener of eventListeners) { + listener(params); } - for (const call of addProtocolMessageListener.mock.calls) { - if (typeof call[0] === 'function') call[0](event); + const starListeners = on.mock.calls.filter(call => call[0] === '*').map(call => call[1]); + for (const listener of starListeners) { + listener(method, params); } - }; - return session; + } + + return dispatch; + } + + /** Use requestIdPrefix to ensure unique requests if used more than once in a test. */ + function getDevtoolsLog(requestIdPrefix = '36185') { + const log = networkRecordsToDevtoolsLog([ + {url: 'http://example.com', priority: 'VeryHigh', requestId: `${requestIdPrefix}.1`}, + {url: 'http://example.com/xhr', priority: 'High', requestId: `${requestIdPrefix}.2`}, + {url: 'http://example.com/css', priority: 'VeryHigh', requestId: `${requestIdPrefix}.3`}, + {url: 'http://example.com/offscreen', priority: 'Low', requestId: `${requestIdPrefix}.4`}, + ]); + + // Bring the starting events forward in the log. + const startEvents = log.filter(m => m.method === 'Network.requestWillBeSent'); + const restEvents = log.filter(m => !startEvents.includes(m)); + return [...startEvents, ...restEvents]; } beforeEach(async () => { - sessionMock = createMockSession(); + rootCdpSessionMock = mockAttachingSession({id: 'root', targetType: 'page'}); + + rootDispatch = createSessionDispatch(rootCdpSessionMock); + targetManager = new TargetManager(rootCdpSessionMock.asCdpSession()); + await targetManager.enable(); + monitor = new NetworkMonitor(targetManager); + statusLog = []; - monitor = new NetworkMonitor(sessionMock); monitor.on('networkbusy', () => statusLog.push('networkbusy')); monitor.on('networkidle', () => statusLog.push('networkidle')); monitor.on('network-2-busy', () => statusLog.push('network-2-busy')); @@ -81,90 +98,67 @@ describe('NetworkMonitor', () => { monitor.on('network-critical-busy', () => statusLog.push('network-critical-busy')); monitor.on('network-critical-idle', () => statusLog.push('network-critical-idle')); - mocks.targetManagerMock.enable.mockImplementation(async () => { - for (const call of mocks.targetManagerMock.addTargetAttachedListener.mock.calls) { - await call[0]({target: {type: 'page', targetId: 'page'}, session: sessionMock}); - } - }); - - const log = networkRecordsToDevtoolsLog([ - {url: 'http://example.com', priority: 'VeryHigh'}, - {url: 'http://example.com/xhr', priority: 'High'}, - {url: 'http://example.com/css', priority: 'VeryHigh'}, - {url: 'http://example.com/offscreen', priority: 'Low'}, - ]); - - const startEvents = log.filter(m => m.method === 'Network.requestWillBeSent'); - const restEvents = log.filter(m => !startEvents.includes(m)); - devtoolsLog = [...startEvents, ...restEvents]; - }); - - afterEach(() => { - mocks.targetManagerMock.reset(); + devtoolsLog = getDevtoolsLog(); }); describe('.enable() / .disable()', () => { it('should not record anything when disabled', async () => { - for (const message of devtoolsLog) sessionMock.dispatch(message); + for (const message of devtoolsLog) rootDispatch(message); expect(statusLog).toEqual([]); }); it('should record once enabled', async () => { await monitor.enable(); - for (const message of devtoolsLog) sessionMock.dispatch(message); - expect(sessionMock.on).toHaveBeenCalled(); - expect(sessionMock.addProtocolMessageListener).toHaveBeenCalled(); - expect(mocks.targetManagerMock.enable).toHaveBeenCalled(); + for (const message of devtoolsLog) rootDispatch(message); expect(statusLog.length).toBeGreaterThan(0); }); it('should not record after enable/disable cycle', async () => { - sendCommandMock.mockResponse('Network.disable'); await monitor.enable(); await monitor.disable(); - for (const message of devtoolsLog) sessionMock.dispatch(message); - expect(sessionMock.on).toHaveBeenCalled(); - expect(sessionMock.off).toHaveBeenCalled(); - expect(sessionMock.addProtocolMessageListener).toHaveBeenCalled(); - expect(sessionMock.removeProtocolMessageListener).toHaveBeenCalled(); - expect(mocks.targetManagerMock.enable).toHaveBeenCalled(); - expect(mocks.targetManagerMock.disable).toHaveBeenCalled(); + for (const message of devtoolsLog) rootDispatch(message); expect(statusLog).toEqual([]); }); it('should listen on every unique target', async () => { await monitor.enable(); - expect(mocks.targetManagerMock.addTargetAttachedListener).toHaveBeenCalledTimes(1); - expect(mocks.targetManagerMock.enable).toHaveBeenCalledTimes(1); - - const targetListener = mocks.targetManagerMock.addTargetAttachedListener.mock.calls[0][0]; - expect(sessionMock.addProtocolMessageListener).toHaveBeenCalledTimes(1); - expect(sendCommandMock).toHaveBeenCalledTimes(2); - sendCommandMock - .mockResponse('Network.enable') - .mockResponse('Network.enable') - .mockResponse('Network.enable'); - - targetListener({target: {type: 'page', targetId: 'page-2'}, session: sessionMock}); // new - expect(sessionMock.addProtocolMessageListener).toHaveBeenCalledTimes(2); - expect(sendCommandMock).toHaveBeenCalledTimes(3); - - targetListener({target: {type: 'page', targetId: 'page-3'}, session: sessionMock}); // new - expect(sessionMock.addProtocolMessageListener).toHaveBeenCalledTimes(3); - expect(sendCommandMock).toHaveBeenCalledTimes(4); - - expect(sessionMock.removeProtocolMessageListener).toHaveBeenCalledTimes(0); - await monitor.disable(); - expect(sessionMock.removeProtocolMessageListener).toHaveBeenCalledTimes(3); + + for (const message of devtoolsLog) rootDispatch(message); + const rootStatusLogLength = statusLog.length; + expect(rootStatusLogLength).toBeGreaterThan(0); + + // Add an iframe through a pretend 'sessionattached' event. + const iframe1Session = mockAttachingSession({id: 'iframe1', targetType: 'iframe'}); + await targetManager._onSessionAttached(iframe1Session.asCdpSession()); + + // Dispatch the same count of network requests. + const iframe1Dispatch = createSessionDispatch(iframe1Session); + const iframe1DevtoolsLog = getDevtoolsLog('iframe1'); // Need unique requestIds. + for (const message of iframe1DevtoolsLog) iframe1Dispatch(message); + + expect(statusLog.length).toBe(rootStatusLogLength * 2); + + // Add another iframe through a pretend 'sessionattached' event. + const iframe2Session = mockAttachingSession({id: 'iframe2', targetType: 'iframe'}); + await targetManager._onSessionAttached(iframe2Session.asCdpSession()); + + // Dispatch the same count of network requests. + const iframe2Dispatch = createSessionDispatch(iframe2Session); + const iframe2DevtoolsLog = getDevtoolsLog('iframe2'); // Need unique requestIds. + for (const message of iframe2DevtoolsLog) iframe2Dispatch(message); + + expect(statusLog.length).toBe(rootStatusLogLength * 3); }); it('should have idempotent enable', async () => { + // Two initial calls for TargetManager.enable(). + expect(rootCdpSessionMock.on).toHaveBeenCalledTimes(2); + await monitor.enable(); await monitor.enable(); await monitor.enable(); await monitor.enable(); - expect(sessionMock.on).toHaveBeenCalledTimes(1); - expect(sessionMock.addProtocolMessageListener).toHaveBeenCalledTimes(1); - expect(sendCommandMock).toHaveBeenCalledTimes(2); + // Only one more call. + expect(rootCdpSessionMock.on).toHaveBeenCalledTimes(3); }); }); @@ -174,14 +168,18 @@ describe('NetworkMonitor', () => { }); it('should return the first and last navigation', async () => { - sendCommandMock.mockResponse('Page.getResourceTree', {frameTree: {frame: {id: '1'}}}); + rootCdpSessionMock.send + .mockResponse('Target.setAutoAttach') + .mockResponse('Target.setAutoAttach') + .mockResponse('Target.setAutoAttach') + .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: '1'}}}); await monitor.enable(); const type = 'Navigation'; const frame = /** @type {*} */ ({id: '1', url: 'https://page.example.com'}); - sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame: {...frame, url: 'https://example.com'}, type}}); // eslint-disable-line max-len - sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame: {...frame, url: 'https://intermediate.example.com'}, type}}); // eslint-disable-line max-len - sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame, type}}); + rootDispatch({method: 'Page.frameNavigated', params: {frame: {...frame, url: 'https://example.com'}, type}}); // eslint-disable-line max-len + rootDispatch({method: 'Page.frameNavigated', params: {frame: {...frame, url: 'https://intermediate.example.com'}, type}}); // eslint-disable-line max-len + rootDispatch({method: 'Page.frameNavigated', params: {frame, type}}); expect(await monitor.getNavigationUrls()).toEqual({ requestedUrl: 'https://example.com', @@ -190,7 +188,10 @@ describe('NetworkMonitor', () => { }); it('should handle server redirects', async () => { - sendCommandMock.mockResponse('Page.getResourceTree', {frameTree: {frame: {id: '1'}}}); + rootCdpSessionMock.send + .mockResponse('Target.setAutoAttach') + .mockResponse('Target.setAutoAttach') + .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: '1'}}}); await monitor.enable(); // One server redirect followed by a client redirect @@ -200,13 +201,13 @@ describe('NetworkMonitor', () => { {requestId: '2', startTime: 300, url: 'https://page.example.com', priority: 'VeryHigh'}, ]); for (const event of devtoolsLog) { - sessionMock.dispatch(event); + rootDispatch(event); } const type = 'Navigation'; const frame = /** @type {*} */ ({id: '1', url: 'https://page.example.com'}); - sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame: {...frame, url: 'https://intermediate.example.com'}, type}}); // eslint-disable-line max-len - sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame, type}}); + rootDispatch({method: 'Page.frameNavigated', params: {frame: {...frame, url: 'https://intermediate.example.com'}, type}}); // eslint-disable-line max-len + rootDispatch({method: 'Page.frameNavigated', params: {frame, type}}); expect(await monitor.getNavigationUrls()).toEqual({ requestedUrl: 'https://example.com', @@ -215,14 +216,17 @@ describe('NetworkMonitor', () => { }); it('should ignore non-main-frame navigations', async () => { - sendCommandMock.mockResponse('Page.getResourceTree', {frameTree: {frame: {id: '1'}}}); + rootCdpSessionMock.send + .mockResponse('Target.setAutoAttach') + .mockResponse('Target.setAutoAttach') + .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: '1'}}}); await monitor.enable(); const type = 'Navigation'; const frame = /** @type {*} */ ({id: '1', url: 'https://page.example.com'}); - sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame, type}}); + rootDispatch({method: 'Page.frameNavigated', params: {frame, type}}); const iframe = /** @type {*} */ ({id: '2', url: 'https://iframe.example.com'}); - sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame: iframe, type}}); + rootDispatch({method: 'Page.frameNavigated', params: {frame: iframe, type}}); expect(await monitor.getNavigationUrls()).toEqual({ requestedUrl: 'https://page.example.com', @@ -240,14 +244,14 @@ describe('NetworkMonitor', () => { const loadedLog = []; monitor.on('requeststarted', /** @param {*} r */ r => startedLog.push(r)); monitor.on('requestfinished', /** @param {*} r */ r => loadedLog.push(r)); - for (const message of devtoolsLog) sessionMock.dispatch(message); + for (const message of devtoolsLog) rootDispatch(message); expect(startedLog).toHaveLength(4); expect(loadedLog).toHaveLength(4); }); it('should emit the cycle of network status events', async () => { await monitor.enable(); - for (const message of devtoolsLog) sessionMock.dispatch(message); + for (const message of devtoolsLog) rootDispatch(message); expect(statusLog).toEqual([ // First request starts. @@ -299,14 +303,15 @@ describe('NetworkMonitor', () => { it('should capture single high-pri request state in getters', () => { const startMessage = devtoolsLog.find(event => event.method === 'Network.requestWillBeSent'); - sessionMock.dispatch(startMessage); + if (!startMessage) throw new Error('startMessage not found'); + rootDispatch(startMessage); expect(monitor.isIdle()).toBe(false); expect(monitor.is2Idle()).toBe(true); expect(monitor.isCriticalIdle()).toBe(false); }); it('should not consider cross frame requests critical', () => { - for (const message of devtoolsLog) sessionMock.dispatch(message); + for (const message of devtoolsLog) rootDispatch(message); expect(monitor.isIdle()).toBe(true); expect(monitor.is2Idle()).toBe(true); expect(monitor.isCriticalIdle()).toBe(true); @@ -315,7 +320,8 @@ describe('NetworkMonitor', () => { {requestId: '5', url: 'http://3p.example.com', priority: 'VeryHigh', frameId: 'OOPIF'}, ]); const startMessage = crossFrameLog.find(e => e.method === 'Network.requestWillBeSent'); - sessionMock.dispatch(startMessage); + if (!startMessage) throw new Error('startMessage not found'); + rootDispatch(startMessage); expect(monitor.isIdle()).toBe(false); expect(monitor.is2Idle()).toBe(true); @@ -326,7 +332,7 @@ describe('NetworkMonitor', () => { const startMessage = devtoolsLog.find(event => event.method === 'Network.requestWillBeSent'); if (!startMessage || startMessage.method !== 'Network.requestWillBeSent') throw tscErr; startMessage.params.request.initialPriority = 'Low'; - sessionMock.dispatch(startMessage); + rootDispatch(startMessage); expect(monitor.isIdle()).toBe(false); expect(monitor.is2Idle()).toBe(true); expect(monitor.isCriticalIdle()).toBe(true); @@ -334,7 +340,7 @@ describe('NetworkMonitor', () => { it('should capture multiple request state in getters', () => { const messages = devtoolsLog.filter(event => event.method === 'Network.requestWillBeSent'); - for (const message of messages) sessionMock.dispatch(message); + for (const message of messages) rootDispatch(message); expect(monitor.isIdle()).toBe(false); expect(monitor.is2Idle()).toBe(false); expect(monitor.isCriticalIdle()).toBe(false); @@ -345,7 +351,7 @@ describe('NetworkMonitor', () => { for (const message of messages) { if (message.method !== 'Network.requestWillBeSent') throw tscErr; message.params.request.initialPriority = 'Low'; - sessionMock.dispatch(message); + rootDispatch(message); } expect(monitor.isIdle()).toBe(false); From f4d4c8c0d104975063ae503196f153eef5d05bff Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Fri, 17 Jun 2022 15:20:51 -0500 Subject: [PATCH 6/7] driver-test --- .../test/fraggle-rock/gather/driver-test.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/lighthouse-core/test/fraggle-rock/gather/driver-test.js b/lighthouse-core/test/fraggle-rock/gather/driver-test.js index cce3e2fd0d87..2313d8c574a8 100644 --- a/lighthouse-core/test/fraggle-rock/gather/driver-test.js +++ b/lighthouse-core/test/fraggle-rock/gather/driver-test.js @@ -6,6 +6,7 @@ import Driver from '../../../fraggle-rock/gather/driver.js'; import {fnAny} from '../../test-utils.js'; +import {createMockCdpSession} from './mock-driver.js'; /** @type {Array} */ const DELEGATED_FUNCTIONS = [ @@ -19,20 +20,22 @@ const DELEGATED_FUNCTIONS = [ /** @type {LH.Puppeteer.Page} */ let page; -/** @type {LH.Puppeteer.Target} */ -let pageTarget; -/** @type {LH.Puppeteer.CDPSession} */ -let puppeteerSession; /** @type {Driver} */ let driver; beforeEach(() => { + const puppeteerSession = createMockCdpSession(); + puppeteerSession.send + .mockResponse('Page.enable') + .mockResponse('Target.getTargetInfo', {targetInfo: {type: 'page', targetId: 'page'}}) + .mockResponse('Network.enable') + .mockResponse('Target.setAutoAttach') + .mockResponse('Runtime.runIfWaitingForDebugger'); + + const pageTarget = {createCDPSession: () => puppeteerSession}; + // @ts-expect-error - Individual mock functions are applied as necessary. page = {target: () => pageTarget, url: fnAny()}; - // @ts-expect-error - Individual mock functions are applied as necessary. - pageTarget = {createCDPSession: () => puppeteerSession}; - // @ts-expect-error - Individual mock functions are applied as necessary. - puppeteerSession = {on: fnAny(), off: fnAny(), send: fnAny(), emit: fnAny()}; driver = new Driver(page); }); From 39b9d640ceafd4a381ec5139a9cdcbdcf3a50424 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Fri, 17 Jun 2022 20:03:16 -0500 Subject: [PATCH 7/7] full-page-screenshot-test --- .../test/fraggle-rock/gather/mock-driver.js | 43 ++++------------- .../test/gather/driver/navigation-test.js | 9 +--- .../gatherers/full-page-screenshot-test.js | 47 +++++++++---------- 3 files changed, 30 insertions(+), 69 deletions(-) diff --git a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js index ee81ac20db21..d685b34eeb31 100644 --- a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js +++ b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js @@ -132,26 +132,15 @@ function createMockExecutionContext() { }; } -function createMockTargetManager() { +/** @param {ReturnType} session */ +function createMockTargetManager(session) { return { + rootSession: () => session, enable: fnAny(), disable: fnAny(), - addTargetAttachedListener: createMockOnFn(), - removeTargetAttachedListener: fnAny(), - /** @param {LH.Gatherer.FRProtocolSession} session */ - mockEnable(session) { - this.enable.mockImplementation(async () => { - const listeners = this.addTargetAttachedListener.mock.calls.map(call => call[0]); - const targetWithSession = {target: {type: 'page', targetId: 'page'}, session}; - for (const listener of listeners) await listener(targetWithSession); - }); - }, - reset() { - this.enable = fnAny(); - this.disable = fnAny(); - this.addTargetAttachedListener = createMockOnFn(); - this.removeTargetAttachedListener = fnAny(); - }, + on: createMockOnFn(), + off: fnAny(), + /** @return {import('../../../gather/driver/target-manager.js')} */ asTargetManager() { // @ts-expect-error - We'll rely on the tests passing to know this matches. @@ -164,6 +153,7 @@ function createMockDriver() { const page = createMockPage(); const session = createMockSession(); const context = createMockExecutionContext(); + const targetManager = createMockTargetManager(session); return { _page: page, @@ -174,6 +164,7 @@ function createMockDriver() { connect: fnAny(), disconnect: fnAny(), executionContext: context.asExecutionContext(), + targetManager: targetManager.asTargetManager(), /** @return {Driver} */ asDriver() { @@ -236,20 +227,6 @@ function createMockBaseArtifacts() { }; } -function mockTargetManagerModule() { - const targetManagerMock = createMockTargetManager(); - - /** @type {(instance: any) => (...args: any[]) => any} */ - const proxyCtor = instance => function() { - // IMPORTANT! This must be a `function` not an arrow function so it can be invoked as a constructor. - return instance; - }; - - jest.mock('../../../gather/driver/target-manager.js', () => proxyCtor(targetManagerMock)); - - return targetManagerMock; -} - function createMockContext() { return { driver: createMockDriver(), @@ -290,7 +267,6 @@ function mockDriverSubmodules() { const networkMock = { fetchResponseBodyFromCache: fnAny(), }; - const targetManagerMock = mockTargetManagerModule(); function reset() { navigationMock.gotoURL = fnAny().mockResolvedValue({finalUrl: 'https://example.com', warnings: [], timedOut: false}); @@ -302,7 +278,6 @@ function mockDriverSubmodules() { emulationMock.clearThrottling = fnAny(); emulationMock.emulate = fnAny(); networkMock.fetchResponseBodyFromCache = fnAny().mockResolvedValue(''); - targetManagerMock.reset(); } /** @@ -329,14 +304,12 @@ function mockDriverSubmodules() { storageMock, emulationMock, networkMock, - targetManagerMock, reset, }; } export { mockRunnerModule, - mockTargetManagerModule, mockDriverModule, mockDriverSubmodules, createMockDriver, diff --git a/lighthouse-core/test/gather/driver/navigation-test.js b/lighthouse-core/test/gather/driver/navigation-test.js index ad64574b4e2c..7cef2b6187be 100644 --- a/lighthouse-core/test/gather/driver/navigation-test.js +++ b/lighthouse-core/test/gather/driver/navigation-test.js @@ -7,7 +7,7 @@ import 'lighthouse-logger'; // Needed otherwise `log.timeEnd` errors in navigation.js inexplicably. import {jest} from '@jest/globals'; -import {createMockDriver, mockTargetManagerModule} from '../../fraggle-rock/gather/mock-driver.js'; +import {createMockDriver} from '../../fraggle-rock/gather/mock-driver.js'; import { mockCommands, makePromiseInspectable, @@ -29,8 +29,6 @@ beforeAll(async () => { ({gotoURL, getNavigationWarnings} = (await import('../../../gather/driver/navigation.js'))); }); -const targetManagerMock = mockTargetManagerModule(); - jest.useFakeTimers(); describe('.gotoURL', () => { @@ -42,7 +40,6 @@ describe('.gotoURL', () => { beforeEach(() => { mockDriver = createMockDriver(); driver = mockDriver.asDriver(); - targetManagerMock.mockEnable(driver.defaultSession); mockDriver.defaultSession.sendCommand .mockResponse('Page.enable') // network monitor's Page.enable @@ -54,10 +51,6 @@ describe('.gotoURL', () => { .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: 'ABC'}}}); }); - afterEach(() => { - targetManagerMock.reset(); - }); - it('will track redirects through gotoURL load with warning', async () => { mockDriver.defaultSession.on = mockDriver.defaultSession.once = createMockOnceFn(); diff --git a/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js b/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js index db186e5ffa68..0d3b037bf4af 100644 --- a/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js +++ b/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js @@ -6,22 +6,9 @@ import {jest} from '@jest/globals'; -import {createMockContext, mockDriverSubmodules} from '../../fraggle-rock/gather/mock-driver.js'; -// import FullPageScreenshotGatherer from '../../../gather/gatherers/full-page-screenshot.js'; - -// Some imports needs to be done dynamically, so that their dependencies will be mocked. -// See: https://jestjs.io/docs/ecmascript-modules#differences-between-esm-and-commonjs -// https://github.com/facebook/jest/issues/10025 -/** @typedef {import('../../../gather/gatherers/full-page-screenshot.js')} FullPageScreenshotGatherer */ -/** @type {typeof import('../../../gather/gatherers/full-page-screenshot.js')} */ -let FullPageScreenshotGatherer; - -beforeAll(async () => { - FullPageScreenshotGatherer = - (await import('../../../gather/gatherers/full-page-screenshot.js')).default; -}); +import {createMockContext} from '../../fraggle-rock/gather/mock-driver.js'; +import FullPageScreenshotGatherer from '../../../gather/gatherers/full-page-screenshot.js'; -const mocks = mockDriverSubmodules(); // Headless's default value is (1024 * 16), but this varies by device const maxTextureSizeMock = 1024 * 8; @@ -78,7 +65,6 @@ beforeEach(() => { throw new Error(`unexpected fn ${fn.name}`); } }); - mocks.reset(); }); describe('FullPageScreenshot gatherer', () => { @@ -88,6 +74,7 @@ describe('FullPageScreenshot gatherer', () => { screenSize = {width: 412, height: 412}; mockContext.settings = { + ...mockContext.settings, formFactor: 'mobile', screenEmulation: { height: screenSize.height, @@ -96,6 +83,7 @@ describe('FullPageScreenshot gatherer', () => { disabled: false, }, }; + const artifact = await fpsGatherer.getArtifact(mockContext.asContext()); expect(artifact).toEqual({ @@ -114,6 +102,7 @@ describe('FullPageScreenshot gatherer', () => { screenSize = {width: 412, height: 412}; mockContext.settings = { + ...mockContext.settings, formFactor: 'mobile', screenEmulation: { height: screenSize.height, @@ -125,19 +114,18 @@ describe('FullPageScreenshot gatherer', () => { await fpsGatherer.getArtifact(mockContext.asContext()); - const expectedArgs = { - formFactor: 'mobile', - screenEmulation: { + // Lighthouse-controlled emulation.emulate() sets touch emulation. + const emulationInvocations = mockContext.driver.defaultSession.sendCommand + .findAllInvocations('Emulation.setTouchEmulationEnabled'); + expect(emulationInvocations).toHaveLength(1); + + expect(mockContext.driver.defaultSession.sendCommand).toHaveBeenCalledWith( + 'Emulation.setDeviceMetricsOverride', + expect.objectContaining({ height: 412, width: 412, - disabled: false, mobile: true, - }, - }; - expect(mocks.emulationMock.emulate).toHaveBeenCalledTimes(1); - expect(mocks.emulationMock.emulate).toHaveBeenCalledWith( - mockContext.driver.defaultSession, - expectedArgs + }) ); }); @@ -146,6 +134,7 @@ describe('FullPageScreenshot gatherer', () => { contentSize = {width: 500, height: 1500}; screenSize = {width: 500, height: 500, dpr: 2}; mockContext.settings = { + ...mockContext.settings, screenEmulation: { height: screenSize.height, width: screenSize.width, @@ -157,6 +146,11 @@ describe('FullPageScreenshot gatherer', () => { await fpsGatherer.getArtifact(mockContext.asContext()); + // If not Lighthouse controlled, no touch emulation. + const emulationInvocations = mockContext.driver.defaultSession.sendCommand + .findAllInvocations('Emulation.setTouchEmulationEnabled'); + expect(emulationInvocations).toHaveLength(0); + // Setting up for screenshot. expect(mockContext.driver.defaultSession.sendCommand).toHaveBeenCalledWith( 'Emulation.setDeviceMetricsOverride', @@ -186,6 +180,7 @@ describe('FullPageScreenshot gatherer', () => { contentSize = {width: 412, height: 100000}; screenSize = {width: 412, height: 412, dpr: 1}; mockContext.settings = { + ...mockContext.settings, formFactor: 'mobile', screenEmulation: { height: screenSize.height,