diff --git a/lighthouse-core/fraggle-rock/gather/session.js b/lighthouse-core/fraggle-rock/gather/session.js index d7f0116113c6..afe204050bcc 100644 --- a/lighthouse-core/fraggle-rock/gather/session.js +++ b/lighthouse-core/fraggle-rock/gather/session.js @@ -5,28 +5,47 @@ */ 'use strict'; +const EventEmitter = require('events').EventEmitter; const LHError = require('../../lib/lh-error.js'); // Controls how long to wait for a response after sending a DevTools protocol command. const DEFAULT_PROTOCOL_TIMEOUT = 30000; +/** @typedef {new () => LH.Protocol.StrictEventEmitter} CrdpEventMessageEmitter */ +const CrdpEventEmitter = /** @type {CrdpEventMessageEmitter} */ (EventEmitter); + /** @implements {LH.Gatherer.FRProtocolSession} */ -class ProtocolSession { +class ProtocolSession extends CrdpEventEmitter { /** * @param {LH.Puppeteer.CDPSession} cdpSession */ constructor(cdpSession) { + super(); + this._cdpSession = cdpSession; /** @type {LH.Crdp.Target.TargetInfo|undefined} */ this._targetInfo = undefined; /** @type {number|undefined} */ this._nextProtocolTimeout = undefined; + + this._handleProtocolEvent = this._handleProtocolEvent.bind(this); + this._cdpSession.on('*', this._handleProtocolEvent); } id() { return this._cdpSession.id(); } + /** + * Re-emit protocol events from the underlying CDPSession. + * @template {keyof LH.CrdpEvents} E + * @param {E} method + * @param {LH.CrdpEvents[E]} params + */ + _handleProtocolEvent(method, ...params) { + this.emit(method, ...params); + } + /** @param {LH.Crdp.Target.TargetInfo} targetInfo */ setTargetInfo(targetInfo) { this._targetInfo = targetInfo; @@ -53,36 +72,6 @@ class ProtocolSession { this._nextProtocolTimeout = ms; } - /** - * Bind listeners for protocol events. - * @template {keyof LH.CrdpEvents} E - * @param {E} eventName - * @param {(...args: LH.CrdpEvents[E]) => void} callback - */ - on(eventName, callback) { - this._cdpSession.on(eventName, /** @type {*} */ (callback)); - } - - /** - * Bind listeners for protocol events. - * @template {keyof LH.CrdpEvents} E - * @param {E} eventName - * @param {(...args: LH.CrdpEvents[E]) => void} callback - */ - once(eventName, callback) { - this._cdpSession.once(eventName, /** @type {*} */ (callback)); - } - - /** - * Bind listeners for protocol events. - * @template {keyof LH.CrdpEvents} E - * @param {E} eventName - * @param {(...args: LH.CrdpEvents[E]) => void} callback - */ - off(eventName, callback) { - this._cdpSession.off(eventName, /** @type {*} */ (callback)); - } - /** * @template {keyof LH.CrdpCommands} C * @param {C} method @@ -116,7 +105,7 @@ class ProtocolSession { * @return {Promise} */ async dispose() { - this._cdpSession.removeAllListeners(); + this._cdpSession.off('*', this._handleProtocolEvent); await this._cdpSession.detach(); } } diff --git a/lighthouse-core/gather/driver/network-monitor.js b/lighthouse-core/gather/driver/network-monitor.js index 9274217de17a..65b3a3275b9e 100644 --- a/lighthouse-core/gather/driver/network-monitor.js +++ b/lighthouse-core/gather/driver/network-monitor.js @@ -63,6 +63,8 @@ class NetworkMonitor { this.removeListener = emitter.removeListener.bind(emitter); /** @type {typeof emitter['removeAllListeners']} */ this.removeAllListeners = emitter.removeAllListeners.bind(emitter); + /** @type {typeof emitter['listenerCount']} */ + this.listenerCount = emitter.listenerCount.bind(emitter); } /** diff --git a/lighthouse-core/test/fraggle-rock/gather/session-test.js b/lighthouse-core/test/fraggle-rock/gather/session-test.js index 2772e7d7bf75..6543dcee688b 100644 --- a/lighthouse-core/test/fraggle-rock/gather/session-test.js +++ b/lighthouse-core/test/fraggle-rock/gather/session-test.js @@ -4,6 +4,8 @@ * 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 {EventEmitter} from 'events'; + import {jest} from '@jest/globals'; import {CDPSession} from 'puppeteer/lib/cjs/puppeteer/common/Connection.js'; @@ -27,35 +29,70 @@ describe('ProtocolSession', () => { beforeEach(() => { // @ts-expect-error - Individual mock functions are applied as necessary. - puppeteerSession = new CDPSession({_rawSend: fnAny()}); + puppeteerSession = new CDPSession({_rawSend: fnAny(), send: fnAny()}, '', 'root'); session = new ProtocolSession(puppeteerSession); }); - /** @type {Array<'on'|'off'|'once'>} */ - const delegateMethods = ['on', 'once', 'off']; - for (const method of delegateMethods) { - describe(`.${method}`, () => { - it('delegates to puppeteer', async () => { - const puppeteerFn = puppeteerSession[method] = fnAny(); - const callback = () => undefined; + describe('responds to events from the underlying CDPSession', () => { + it('once', async () => { + const callback = fnAny(); - session[method]('Page.frameNavigated', callback); - expect(puppeteerFn).toHaveBeenCalledWith('Page.frameNavigated', callback); - }); + session.once('Page.frameNavigated', callback); + puppeteerSession.emit('Page.frameNavigated', {id: 1}); + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith({id: 1}); + + puppeteerSession.emit('Page.frameNavigated', {id: 2}); + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('on', async () => { + const callback = fnAny(); + + session.on('Page.frameNavigated', callback); + puppeteerSession.emit('Page.frameNavigated', {id: 1}); + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith({id: 1}); + + puppeteerSession.emit('Page.frameNavigated', {id: 2}); + expect(callback).toHaveBeenCalledTimes(2); + expect(callback).toHaveBeenCalledWith({id: 2}); }); - } + + it('off', async () => { + const callback = fnAny(); + + session.on('Page.frameNavigated', callback); + puppeteerSession.emit('Page.frameNavigated', {id: 1}); + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith({id: 1}); + + session.off('Page.frameNavigated', callback); + puppeteerSession.emit('Page.frameNavigated', {id: 2}); + expect(callback).toHaveBeenCalledTimes(1); + }); + }); describe('.dispose', () => { it('should detach from the session', async () => { const detach = fnAny(); - const removeAllListeners = fnAny(); + class MockCdpSession extends EventEmitter { + constructor() { + super(); + + this.detach = detach; + } + } + // @ts-expect-error - we want to use a more limited test. - puppeteerSession = {detach, emit: fnAny(), removeAllListeners}; + puppeteerSession = new MockCdpSession(); session = new ProtocolSession(puppeteerSession); + expect(puppeteerSession.listenerCount('*')).toBe(1); + await session.dispose(); expect(detach).toHaveBeenCalled(); - expect(removeAllListeners).toHaveBeenCalled(); + expect(puppeteerSession.listenerCount('*')).toBe(0); }); }); diff --git a/lighthouse-core/test/gather/driver/network-monitor-test.js b/lighthouse-core/test/gather/driver/network-monitor-test.js index 4c74d0d8ea0a..00df325acbc4 100644 --- a/lighthouse-core/test/gather/driver/network-monitor-test.js +++ b/lighthouse-core/test/gather/driver/network-monitor-test.js @@ -151,14 +151,13 @@ describe('NetworkMonitor', () => { }); it('should have idempotent enable', async () => { - // Two initial calls for TargetManager.enable(). - expect(rootCdpSessionMock.on).toHaveBeenCalledTimes(2); + const initialListenerCount = targetManager.listenerCount('protocolevent'); await monitor.enable(); await monitor.enable(); await monitor.enable(); await monitor.enable(); - // Only one more call. - expect(rootCdpSessionMock.on).toHaveBeenCalledTimes(3); + // Only one more listener added for `monitor.enable()`. + expect(targetManager.listenerCount('protocolevent')).toBe(initialListenerCount + 1); }); }); diff --git a/lighthouse-core/test/gather/driver/target-manager-test.js b/lighthouse-core/test/gather/driver/target-manager-test.js index f450dbedef50..845298ae1547 100644 --- a/lighthouse-core/test/gather/driver/target-manager-test.js +++ b/lighthouse-core/test/gather/driver/target-manager-test.js @@ -118,13 +118,19 @@ describe('TargetManager', () => { }); it('should listen to target before resuming', async () => { + let targetListeningAsserted = false; + + // Intercept listener for all protocol events and ensure target is still paused. 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); + const getTargetInfoCount = sendMock.findAllInvocations('Target.getTargetInfo').length; + const setAutoAttachCount = sendMock.findAllInvocations('Target.setAutoAttach').length; + const resumeCount = sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger').length; + + // There may be many listeners for all protocol events, so just ensure this one occurred. + if (eventName === '*' && + getTargetInfoCount === 1 && setAutoAttachCount === 0 && resumeCount === 0) { + targetListeningAsserted = true; } })); @@ -132,8 +138,15 @@ describe('TargetManager', () => { .mockResponse('Target.getTargetInfo', {targetInfo}) .mockResponse('Network.enable') .mockResponse('Target.setAutoAttach'); + + expect(sendMock.findAllInvocations('Target.getTargetInfo')).toHaveLength(0); + expect(sendMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(0); + expect(sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger')).toHaveLength(0); + await targetManager.enable(); + expect(targetListeningAsserted).toBe(true); + expect(sendMock.findAllInvocations('Target.getTargetInfo')).toHaveLength(1); expect(sendMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(1); expect(sendMock.findAllInvocations('Runtime.runIfWaitingForDebugger')).toHaveLength(1); diff --git a/types/protocol.d.ts b/types/protocol.d.ts index a27e92b14b5a..67c88bf987a3 100644 --- a/types/protocol.d.ts +++ b/types/protocol.d.ts @@ -67,6 +67,8 @@ declare module Protocol { once(event: E, listener: (...args: TEventRecord[E]) => void): void; emit(event: E, ...request: TEventRecord[E]): void; + + listenerCount(event: E): number; } }