From 6af035a643d54f27cc8d052f48c07b5941e00d65 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 26 Aug 2021 19:08:47 -0500 Subject: [PATCH 01/11] core(fr): collect OOPIF network data --- .../test-definitions/oopif/oopif-config.js | 9 ++ .../oopif/oopif-iframe-audit.js | 9 ++ lighthouse-core/fraggle-rock/gather/driver.js | 3 + .../fraggle-rock/gather/session.js | 36 ++++- lighthouse-core/gather/devtools-log.js | 12 +- lighthouse-core/gather/driver.js | 15 +++ .../gather/driver/network-monitor.js | 56 +++++++- .../gather/driver/target-manager.js | 125 +++++++++++++++++ .../gather/gatherers/devtools-log.js | 30 ++--- lighthouse-core/lib/network-recorder.js | 8 ++ .../test/fraggle-rock/gather/mock-driver.js | 3 + .../test/fraggle-rock/gather/session-test.js | 57 ++++++++ .../test/gather/devtools-log-test.js | 10 ++ .../gather/driver/network-monitor-test.js | 12 +- package.json | 4 +- types/gatherer.d.ts | 3 + yarn.lock | 127 +++++++++--------- 17 files changed, 421 insertions(+), 98 deletions(-) create mode 100644 lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js create mode 100644 lighthouse-core/gather/driver/target-manager.js diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js index 18945ecad024..b85a794b4a0a 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js @@ -11,6 +11,15 @@ */ module.exports = { extends: 'lighthouse:default', + categories: { + performance: { + title: 'Performance', + auditRefs: [{id: 'iframe-elements', weight: 0}], + }, + }, + audits: [ + {path: `${__dirname}/oopif-iframe-audit.js`}, + ], settings: { // This test runs in CI and hits the outside network of a live site. // Be a little more forgiving on how long it takes all network requests of several nested iframes diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js new file mode 100644 index 000000000000..dfd996d808b6 --- /dev/null +++ b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js @@ -0,0 +1,9 @@ +module.exports = { + meta: { + id: 'iframe-elements', + title: 'IFrame Elements', + description: 'Audit to force the inclusion of IFrameElements artifact', + requiredArtifacts: ['IFrameElements'], + }, + audit: () => ({score: 1}), +}; diff --git a/lighthouse-core/fraggle-rock/gather/driver.js b/lighthouse-core/fraggle-rock/gather/driver.js index 57259685fc05..bf4845610e38 100644 --- a/lighthouse-core/fraggle-rock/gather/driver.js +++ b/lighthouse-core/fraggle-rock/gather/driver.js @@ -16,6 +16,7 @@ const throwNotConnectedFn = () => { /** @type {LH.Gatherer.FRProtocolSession} */ const defaultSession = { + setTargetInfo: throwNotConnectedFn, hasNextProtocolTimeout: throwNotConnectedFn, getNextProtocolTimeout: throwNotConnectedFn, setNextProtocolTimeout: throwNotConnectedFn, @@ -24,6 +25,8 @@ const defaultSession = { off: throwNotConnectedFn, addProtocolMessageListener: throwNotConnectedFn, removeProtocolMessageListener: throwNotConnectedFn, + addSessionAttachedListener: throwNotConnectedFn, + removeSessionAttachedListener: throwNotConnectedFn, sendCommand: throwNotConnectedFn, }; diff --git a/lighthouse-core/fraggle-rock/gather/session.js b/lighthouse-core/fraggle-rock/gather/session.js index 4168ccd96888..1e307bfa0326 100644 --- a/lighthouse-core/fraggle-rock/gather/session.js +++ b/lighthouse-core/fraggle-rock/gather/session.js @@ -18,8 +18,12 @@ class ProtocolSession { */ constructor(session) { this._session = session; + /** @type {LH.Crdp.Target.TargetInfo|undefined} */ + this._targetInfo = undefined; /** @type {number|undefined} */ this._nextProtocolTimeout = undefined; + /** @type {WeakMap} */ + this._callbackMap = new WeakMap(); // FIXME: Monkeypatch puppeteer to be able to listen to *all* protocol events. // This patched method will now emit a copy of every event on `*`. @@ -27,13 +31,22 @@ class ProtocolSession { // @ts-expect-error - Test for the monkeypatch. if (originalEmit[SessionEmitMonkeypatch]) return; session.emit = (method, ...args) => { - originalEmit.call(session, '*', {method, params: args[0]}); + // OOPIF sessions need to emit their sessionId so downstream processors can recognize + // the appropriate target to use. + const sessionId = this._targetInfo && this._targetInfo.type === 'iframe' ? + this._targetInfo.targetId : undefined; + originalEmit.call(session, '*', {method, params: args[0], sessionId}); return originalEmit.call(session, method, ...args); }; // @ts-expect-error - It's monkeypatching 🤷‍♂️. session.emit[SessionEmitMonkeypatch] = true; } + /** @param {LH.Crdp.Target.TargetInfo} targetInfo */ + setTargetInfo(targetInfo) { + this._targetInfo = targetInfo; + } + /** * @return {boolean} */ @@ -75,6 +88,27 @@ class ProtocolSession { this._session.once(eventName, /** @type {*} */ (callback)); } + /** + * Bind to the puppeteer `sessionattached` listener and return an LH ProtocolSession. + * @param {(session: ProtocolSession) => void} callback + */ + addSessionAttachedListener(callback) { + /** @param {import('puppeteer').CDPSession} session */ + const listener = session => callback(new ProtocolSession(session)); + this._callbackMap.set(callback, listener); + this._session.connection().on('sessionattached', listener); + } + + /** + * Unbind to the puppeteer `sessionattached` listener. + * @param {(session: ProtocolSession) => void} callback + */ + removeSessionAttachedListener(callback) { + const listener = this._callbackMap.get(callback); + if (!listener) return; + this._session.connection().off('sessionattached', listener); + } + /** * Bind to our custom event that fires for *any* protocol event. * @param {(payload: LH.Protocol.RawEventMessage) => void} callback diff --git a/lighthouse-core/gather/devtools-log.js b/lighthouse-core/gather/devtools-log.js index e6afcbc2d795..69916729d2bb 100644 --- a/lighthouse-core/gather/devtools-log.js +++ b/lighthouse-core/gather/devtools-log.js @@ -46,9 +46,15 @@ class DevtoolsLog { * @param {LH.Protocol.RawEventMessage} message */ record(message) { - if (this._isRecording && (!this._filter || this._filter.test(message.method))) { - this._messages.push(message); - } + // We're not recording, skip the rest of the checks. + if (!this._isRecording) return; + // The event was likely an internal puppeteer method that uses Symbols. + if (typeof message.method !== 'string') return; + // The event didn't pass our filter, do not record it. + if (this._filter && !this._filter.test(message.method)) return; + + // We passed all the checks, record the message. + this._messages.push(message); } } diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index e12ecf37a412..d747b033611d 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -198,6 +198,21 @@ class Driver { this._connection.off('protocolevent', callback); } + /** @param {LH.Crdp.Target.TargetInfo} targetInfo */ + setTargetInfo(targetInfo) { // eslint-disable-line no-unused-vars + throw new Error(`OOPIF handling in legacy driver is implicit`); + } + + /** @param {(session: LH.Gatherer.FRProtocolSession) => void} callback */ + addSessionAttachedListener(callback) { // eslint-disable-line no-unused-vars + throw new Error(`OOPIF handling in legacy driver is implicit`); + } + + /** @param {(session: LH.Gatherer.FRProtocolSession) => void} callback */ + removeSessionAttachedListener(callback) { // eslint-disable-line no-unused-vars + throw new Error(`OOPIF handling in legacy driver is implicit`); + } + /** * Debounce enabling or disabling domains to prevent driver users from * stomping on each other. Maintains an internal count of the times a domain diff --git a/lighthouse-core/gather/driver/network-monitor.js b/lighthouse-core/gather/driver/network-monitor.js index 8ba4d1db17c1..7ce513bfb889 100644 --- a/lighthouse-core/gather/driver/network-monitor.js +++ b/lighthouse-core/gather/driver/network-monitor.js @@ -12,9 +12,11 @@ const log = require('lighthouse-logger'); const {EventEmitter} = require('events'); +const MessageLog = require('../devtools-log.js'); 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_ */ @@ -23,6 +25,8 @@ const URL = require('../../lib/url-shim.js'); class NetworkMonitor extends EventEmitter { /** @type {NetworkRecorder|undefined} */ _networkRecorder = undefined; + /** @type {TargetManager|undefined} */ + _targetManager = undefined; /** @type {Array} */ _frameNavigations = []; @@ -31,11 +35,19 @@ class NetworkMonitor extends EventEmitter { super(); this._session = session; + this._messageLog = new MessageLog(/^(Page|Network)\./); + + this._onTargetAttached = this._onTargetAttached.bind(this); + + /** @type {Map} */ + this._sessions = new Map(); + /** @param {LH.Crdp.Page.FrameNavigatedEvent} event */ this._onFrameNavigated = event => this._frameNavigations.push(event.frame); /** @param {LH.Protocol.RawEventMessage} event */ this._onProtocolMessage = event => { + this._messageLog.record(event); if (!this._networkRecorder) return; this._networkRecorder.dispatch(event); }; @@ -49,6 +61,19 @@ class NetworkMonitor extends EventEmitter { this.off = (event, listener) => super.off(event, listener); } + /** + * @param {{target: {targetId: string}, session: LH.Gatherer.FRProtocolSession}} session + */ + async _onTargetAttached({session, target}) { + const targetId = target.targetId; + if (this._sessions.has(targetId)) return; + + this._sessions.set(targetId, session); + session.addProtocolMessageListener(this._onProtocolMessage); + + await session.sendCommand('Network.enable'); + } + /** * @return {Promise} */ @@ -56,7 +81,9 @@ class NetworkMonitor extends EventEmitter { if (this._networkRecorder) return; this._frameNavigations = []; + this._sessions = new Map(); this._networkRecorder = new NetworkRecorder(); + this._targetManager = new TargetManager(this._session); /** * Reemit the same network recorder events. @@ -71,22 +98,38 @@ class NetworkMonitor extends EventEmitter { this._networkRecorder.on('requeststarted', reEmit('requeststarted')); this._networkRecorder.on('requestloaded', reEmit('requestloaded')); + this._messageLog.reset(); + this._messageLog.beginRecording(); + this._session.on('Page.frameNavigated', this._onFrameNavigated); - this._session.addProtocolMessageListener(this._onProtocolMessage); + this._targetManager.addTargetAttachedListener(this._onTargetAttached); - await this._session.sendCommand('Page.enable'); - await this._session.sendCommand('Network.enable'); + await this._targetManager.enable(); } /** * @return {Promise} */ async disable() { + if (!this._targetManager) return; + + this._messageLog.reset(); + this._messageLog.endRecording(); + this._session.off('Page.frameNavigated', this._onFrameNavigated); this._session.removeProtocolMessageListener(this._onProtocolMessage); + this._targetManager.removeTargetAttachedListener(this._onTargetAttached); + + for (const session of this._sessions.values()) { + session.removeProtocolMessageListener(this._onProtocolMessage); + } + + await this._targetManager.disable(); this._frameNavigations = []; this._networkRecorder = undefined; + this._targetManager = undefined; + this._sessions = new Map(); } /** @return {Promise} */ @@ -103,6 +146,13 @@ class NetworkMonitor extends EventEmitter { return finalNavigation && finalNavigation.url; } + /** + * @return {LH.DevtoolsLog} + */ + getMessageLog() { + return this._messageLog.messages; + } + /** * @return {Array} */ diff --git a/lighthouse-core/gather/driver/target-manager.js b/lighthouse-core/gather/driver/target-manager.js new file mode 100644 index 000000000000..bab5073e4894 --- /dev/null +++ b/lighthouse-core/gather/driver/target-manager.js @@ -0,0 +1,125 @@ +/** + * @license Copyright 2021 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * 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. + */ +'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 log = require('lighthouse-logger'); + +/** @typedef {{target: LH.Crdp.Target.TargetInfo, session: LH.Gatherer.FRProtocolSession}} TargetWithSession */ + +class TargetManager { + /** @param {LH.Gatherer.FRProtocolSession} session */ + constructor(session) { + this._enabled = false; + this._session = session; + /** @type {Array<(targetWithSession: TargetWithSession) => Promise|void>} */ + this._listeners = []; + + this._onSessionAttached = this._onSessionAttached.bind(this); + + /** @type {Map} */ + this._targets = new Map(); + + /** @param {LH.Crdp.Page.FrameNavigatedEvent} event */ + this._onFrameNavigated = async event => { + // Child frames are handled in `_onSessionAttached`. + if (event.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._session.sendCommand('Target.setAutoAttach', { + autoAttach: true, + flatten: true, + waitForDebuggerOnStart: true, + }); + }; + } + + /** + * @param {LH.Gatherer.FRProtocolSession} session + */ + async _onSessionAttached(session) { + try { + const target = await session.sendCommand('Target.getTargetInfo').catch(() => null); + const targetType = target && target.targetInfo.type; + const hasValidTargetType = targetType === 'page' || targetType === 'iframe'; + if (!target || !hasValidTargetType) return; + + const targetId = target.targetInfo.targetId; + session.setTargetInfo(target.targetInfo); + if (this._targets.has(targetId)) return; + + const targetName = target.targetInfo.url || target.targetInfo.targetId; + log.verbose('target-manager', `target ${targetName} attached`); + + const targetWithSession = {target: target.targetInfo, session}; + this._targets.set(targetId, targetWithSession); + for (const listener of this._listeners) await listener(targetWithSession); + + await session.sendCommand('Target.setAutoAttach', { + autoAttach: true, + flatten: true, + waitForDebuggerOnStart: true, + }); + } catch (err) { + // Sometimes targets can be closed before we even have a chance to listen to their network activity. + if (/Target closed/.test(err.stack)) return; + + 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(() => {}); + } + } + + /** + * @return {Promise} + */ + async enable() { + if (this._enabled) return; + + this._enabled = true; + this._targets = new Map(); + + this._session.on('Page.frameNavigated', this._onFrameNavigated); + this._session.addSessionAttachedListener(this._onSessionAttached); + + await this._session.sendCommand('Page.enable'); + await this._onSessionAttached(this._session); + } + + /** + * @return {Promise} + */ + async disable() { + this._session.off('Page.frameNavigated', this._onFrameNavigated); + this._session.removeSessionAttachedListener(this._onSessionAttached); + + this._enabled = false; + this._targets = 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 699e85c3fc81..84c000e5fc5e 100644 --- a/lighthouse-core/gather/gatherers/devtools-log.js +++ b/lighthouse-core/gather/gatherers/devtools-log.js @@ -11,7 +11,7 @@ * This protocol log can be used to recreate the network records using lib/network-recorder.js. */ -const MessageLog = require('../devtools-log.js'); +const NetworkMonitor = require('../driver/network-monitor.js'); const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js'); class DevtoolsLog extends FRGatherer { @@ -26,38 +26,32 @@ class DevtoolsLog extends FRGatherer { constructor() { super(); - this._messageLog = new MessageLog(/^(Page|Network)\./); + /** @type {NetworkMonitor|undefined} */ + this._networkMonitor = undefined; - /** @param {LH.Protocol.RawEventMessage} e */ - this._onProtocolMessage = e => this._messageLog.record(e); + /** @type {LH.DevtoolsLog} */ + this._devtoolsLog = []; } /** * @param {LH.Gatherer.FRTransitionalContext} passContext */ async startSensitiveInstrumentation({driver}) { - this._messageLog.reset(); - this._messageLog.beginRecording(); - driver.defaultSession.addProtocolMessageListener(this._onProtocolMessage); - - // TODO(FR-COMPAT): use a dedicated session for these - await driver.defaultSession.sendCommand('Page.enable'); - await driver.defaultSession.sendCommand('Network.enable'); + this._networkMonitor = new NetworkMonitor(driver.defaultSession); + this._networkMonitor.enable(); } - /** - * @param {LH.Gatherer.FRTransitionalContext} passContext - */ - async stopSensitiveInstrumentation({driver}) { - this._messageLog.endRecording(); - driver.defaultSession.removeProtocolMessageListener(this._onProtocolMessage); + async stopSensitiveInstrumentation() { + if (!this._networkMonitor) return; + this._devtoolsLog = this._networkMonitor.getMessageLog(); + this._networkMonitor.disable(); } /** * @return {Promise} */ async getArtifact() { - return this._messageLog.messages; + return this._devtoolsLog; } } diff --git a/lighthouse-core/lib/network-recorder.js b/lighthouse-core/lib/network-recorder.js index 3149e9519aa3..9fb1fe56c5ff 100644 --- a/lighthouse-core/lib/network-recorder.js +++ b/lighthouse-core/lib/network-recorder.js @@ -5,6 +5,7 @@ */ 'use strict'; +const log = require('lighthouse-logger'); const NetworkRequest = require('./network-request.js'); const EventEmitter = require('events').EventEmitter; @@ -88,6 +89,7 @@ class NetworkRecorder extends EventEmitter { request.onRequestWillBeSent(data); request.sessionId = event.sessionId; this.onRequestStarted(request); + log.verbose('network', `request will be sent to ${request.url}`); return; } @@ -108,6 +110,7 @@ class NetworkRecorder extends EventEmitter { redirectedRequest.onRequestWillBeSent(modifiedData); originalRequest.onRedirectResponse(data); + log.verbose('network', `${originalRequest.url} redirected to ${redirectedRequest.url}`); originalRequest.redirectDestination = redirectedRequest; redirectedRequest.redirectSource = originalRequest; @@ -124,6 +127,7 @@ class NetworkRecorder extends EventEmitter { const data = event.params; const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId); if (!request) return; + log.verbose('network', `${request.url} served from cache`); request.onRequestServedFromCache(); } @@ -134,6 +138,7 @@ class NetworkRecorder extends EventEmitter { const data = event.params; const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId); if (!request) return; + log.verbose('network', `${request.url} response received`); request.onResponseReceived(data); } @@ -144,6 +149,7 @@ class NetworkRecorder extends EventEmitter { const data = event.params; const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId); if (!request) return; + log.verbose('network', `${request.url} data received`); request.onDataReceived(data); } @@ -154,6 +160,7 @@ class NetworkRecorder extends EventEmitter { const data = event.params; const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId); if (!request) return; + log.verbose('network', `${request.url} loading finished`); request.onLoadingFinished(data); this.onRequestFinished(request); } @@ -165,6 +172,7 @@ class NetworkRecorder extends EventEmitter { const data = event.params; const request = this._findRealRequestAndSetSession(data.requestId, event.sessionId); if (!request) return; + log.verbose('network', `${request.url} loading failed`); request.onLoadingFailed(data); this.onRequestFinished(request); } diff --git a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js index 333f94535f70..27b7d34b52e2 100644 --- a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js +++ b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js @@ -23,6 +23,7 @@ const {defaultSettings} = require('../../../config/constants.js'); function createMockSession() { return { + setTargetInfo: jest.fn(), sendCommand: createMockSendCommandFn({useSessionId: false}), setNextProtocolTimeout: jest.fn(), once: createMockOnceFn(), @@ -30,6 +31,8 @@ function createMockSession() { off: jest.fn(), addProtocolMessageListener: createMockOnFn(), removeProtocolMessageListener: jest.fn(), + addSessionAttachedListener: createMockOnFn(), + removeSessionAttachedListener: jest.fn(), /** @return {LH.Gatherer.FRProtocolSession} */ asSession() { diff --git a/lighthouse-core/test/fraggle-rock/gather/session-test.js b/lighthouse-core/test/fraggle-rock/gather/session-test.js index ed3e198dd42a..a881677cfceb 100644 --- a/lighthouse-core/test/fraggle-rock/gather/session-test.js +++ b/lighthouse-core/test/fraggle-rock/gather/session-test.js @@ -68,6 +68,25 @@ describe('ProtocolSession', () => { expect(regularListener).toHaveBeenCalledTimes(1); expect(allListener).toHaveBeenCalledTimes(2); }); + + it('should include sessionId for iframes', () => { + // @ts-expect-error - we want to use a more limited test of a real event emitter. + puppeteerSession = new EventEmitter(); + session = new ProtocolSession(puppeteerSession); + + const listener = jest.fn(); + const targetInfo = {title: '', url: '', attached: true, canAccessOpener: false}; + + puppeteerSession.on('*', 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'>} */ @@ -127,6 +146,44 @@ describe('ProtocolSession', () => { }); }); + describe('.addSessionAttachedListener', () => { + it('should listen for new sessions', () => { + const mockOn = jest.fn(); + // @ts-expect-error - we want to use a more limited, controllable test + puppeteerSession = {connection: () => ({on: mockOn}), emit: jest.fn()}; + session = new ProtocolSession(puppeteerSession); + + // Make sure we listen for the event. + const listener = jest.fn(); + session.addSessionAttachedListener(listener); + expect(mockOn).toHaveBeenCalledWith('sessionattached', expect.any(Function)); + + // Make sure we wrap the return in a ProtocolSession. + mockOn.mock.calls[0][1]({emit: jest.fn()}); + expect(listener).toHaveBeenCalledWith(expect.any(ProtocolSession)); + }); + }); + + describe('.removeSessionAttachedListener', () => { + it('should stop listening for new sessions', () => { + const mockOn = jest.fn(); + const mockOff = jest.fn(); + // @ts-expect-error - we want to use a more limited, controllable test + puppeteerSession = {connection: () => ({on: mockOn, off: mockOff}), emit: jest.fn()}; + session = new ProtocolSession(puppeteerSession); + + // Make sure we listen for the event. + const userListener = jest.fn(); + session.addSessionAttachedListener(userListener); + expect(mockOn).toHaveBeenCalledWith('sessionattached', expect.any(Function)); + + // Make sure we unlisten the mapped function, not just the user's listener. + const installedListener = mockOn.mock.calls[0][1]; + session.removeSessionAttachedListener(userListener); + expect(mockOff).toHaveBeenCalledWith('sessionattached', installedListener); + }); + }); + describe('.sendCommand', () => { it('delegates to puppeteer', async () => { const send = puppeteerSession.send = jest.fn().mockResolvedValue(123); diff --git a/lighthouse-core/test/gather/devtools-log-test.js b/lighthouse-core/test/gather/devtools-log-test.js index 345bf2828e45..1f1728ddb3e5 100644 --- a/lighthouse-core/test/gather/devtools-log-test.js +++ b/lighthouse-core/test/gather/devtools-log-test.js @@ -42,6 +42,16 @@ describe('DevtoolsLog', () => { assert.equal(messageLog.messages[0].method, pageMsg.method); }); + it('ignores messages with Symbols', () => { + messageLog.beginRecording(); + messageLog.record(pageMsg); // will record + messageLog.record(networkMsg); // will record + messageLog.record({method: Symbol('Network.requestWillBeSent')}); // won't record + messageLog.endRecording(); + assert.equal(messageLog.messages.length, 2); + assert.equal(messageLog.messages[0].method, pageMsg.method); + }); + it('records everything when no filter provided', () => { messageLog = new DevtoolsLog(); messageLog.beginRecording(); diff --git a/lighthouse-core/test/gather/driver/network-monitor-test.js b/lighthouse-core/test/gather/driver/network-monitor-test.js index 373b856a663c..0955cea1f20f 100644 --- a/lighthouse-core/test/gather/driver/network-monitor-test.js +++ b/lighthouse-core/test/gather/driver/network-monitor-test.js @@ -123,10 +123,11 @@ describe('NetworkMonitor', () => { sendCommandMock.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: '1'}}}); - sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame: {...frame, url: '2'}}}); - sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame}}); + sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame: {...frame, url: '1'}, type}}); // eslint-disable-line max-len + sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame: {...frame, url: '2'}, type}}); // eslint-disable-line max-len + sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame, type}}); expect(await monitor.getFinalNavigationUrl()).toEqual('https://page.example.com'); }); @@ -135,10 +136,11 @@ describe('NetworkMonitor', () => { sendCommandMock.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}}); + sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame, type}}); const iframe = /** @type {*} */ ({id: '2', url: 'https://iframe.example.com'}); - sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame: iframe}}); + sessionMock.dispatch({method: 'Page.frameNavigated', params: {frame: iframe, type}}); expect(await monitor.getFinalNavigationUrl()).toEqual('https://page.example.com'); }); diff --git a/package.json b/package.json index 262146e79894..e1aa6686b2de 100644 --- a/package.json +++ b/package.json @@ -137,7 +137,7 @@ "cpy": "^7.0.1", "cross-env": "^7.0.2", "csv-validator": "^0.0.3", - "devtools-protocol": "0.0.863986", + "devtools-protocol": "0.0.901419", "eslint": "^7.23.0", "eslint-config-google": "^0.9.1", "eslint-plugin-local-rules": "0.1.0", @@ -159,7 +159,7 @@ "pako": "^2.0.3", "preact": "^10.5.14", "pretty-json-stringify": "^0.0.2", - "puppeteer": "^9.1.1", + "puppeteer": "^10.2.0", "rollup": "^2.50.6", "rollup-plugin-commonjs": "^10.1.0", "rollup-plugin-terser": "^7.0.2", diff --git a/types/gatherer.d.ts b/types/gatherer.d.ts index 3b46b4693f1c..329f721af7d8 100644 --- a/types/gatherer.d.ts +++ b/types/gatherer.d.ts @@ -22,6 +22,7 @@ import {Trace, DevtoolsLog} from './artifacts'; declare module Gatherer { /** The Lighthouse wrapper around a raw CDP session. */ interface FRProtocolSession { + setTargetInfo(targetInfo: LH.Crdp.Target.TargetInfo): void; hasNextProtocolTimeout(): boolean; getNextProtocolTimeout(): number; setNextProtocolTimeout(ms: number): void; @@ -29,6 +30,8 @@ declare module Gatherer { once(event: TEvent, callback: (...args: LH.CrdpEvents[TEvent]) => void): void; addProtocolMessageListener(callback: (payload: Protocol.RawEventMessage) => void): void removeProtocolMessageListener(callback: (payload: Protocol.RawEventMessage) => void): void + addSessionAttachedListener(callback: (session: FRProtocolSession) => void): void + removeSessionAttachedListener(callback: (session: FRProtocolSession) => void): void off(event: TEvent, callback: (...args: LH.CrdpEvents[TEvent]) => void): void; sendCommand(method: TMethod, ...params: LH.CrdpCommands[TMethod]['paramsType']): Promise; } diff --git a/yarn.lock b/yarn.lock index ae890878d9a3..563009cd817d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2936,7 +2936,7 @@ dateformat@^1.0.11, dateformat@^1.0.12: get-stdin "^4.0.1" meow "^3.3.0" -debug@4, debug@^4.0.1, debug@^4.1.0, debug@^4.1.1: +debug@4, debug@4.3.1, debug@^4.0.1, debug@^4.1.0, debug@^4.1.1: version "4.3.1" resolved "https://registry.yarnpkg.com/debug/-/debug-4.3.1.tgz#f0d229c505e0c6d8c49ac553d1b13dc183f6b2ee" integrity sha512-doEwdvm4PCeK4K3RQN2ZC2BYUBaxwLARCqZmMjtF8a51J2Rb0xpVloFRnCODwqjpwnAoao4pelN8l3RJdv3gRQ== @@ -3068,15 +3068,10 @@ detective@^5.2.0: defined "^1.0.0" minimist "^1.1.1" -devtools-protocol@0.0.863986: - version "0.0.863986" - resolved "https://registry.yarnpkg.com/devtools-protocol/-/devtools-protocol-0.0.863986.tgz#a9f1b13daeb1dba671580e59f3a8aa9cb0c921c8" - integrity sha512-WMf5KuRLsLwJMp9JdawSvoEpxZPqyyNeOZ3YR8QF8lE9IVHbbpdWeuXV22SJxPUemFeznvVlwSBeQz91nL+41A== - -devtools-protocol@0.0.869402: - version "0.0.869402" - resolved "https://registry.yarnpkg.com/devtools-protocol/-/devtools-protocol-0.0.869402.tgz#03ade701761742e43ae4de5dc188bcd80f156d8d" - integrity sha512-VvlVYY+VDJe639yHs5PHISzdWTLL3Aw8rO4cvUtwvoxFd6FHbE4OpHHcde52M6096uYYazAmd4l0o5VuFRO2WA== +devtools-protocol@0.0.901419: + version "0.0.901419" + resolved "https://registry.yarnpkg.com/devtools-protocol/-/devtools-protocol-0.0.901419.tgz#79b5459c48fe7e1c5563c02bd72f8fec3e0cebcd" + integrity sha512-4INMPwNm9XRpBukhNbF7OB6fNTTCaI8pzy/fXg0xQzAy5h3zL1P8xT3QazgKqBrb/hAYwIBizqDBZ7GtJE74QQ== diff-sequences@^24.9.0: version "24.9.0" @@ -3640,7 +3635,7 @@ extglob@^2.0.4: snapdragon "^0.8.1" to-regex "^3.0.1" -extract-zip@^2.0.0: +extract-zip@2.0.1: version "2.0.1" resolved "https://registry.yarnpkg.com/extract-zip/-/extract-zip-2.0.1.tgz#663dca56fe46df890d5f131ef4a06d22bb8ba13a" integrity sha512-GDhU9ntwuKyGXdZBUgTIe+vXnWj0fppUEtMDL0+idd5Sta8TGpHssn/eusA9mrPr9qNDym6SxAYZjNvCn/9RBg== @@ -4362,7 +4357,7 @@ https-browserify@^1.0.0: resolved "https://registry.yarnpkg.com/https-browserify/-/https-browserify-1.0.0.tgz#ec06c10e0a34c0f2faf199f7fd7fc78fffd03c73" integrity sha1-7AbBDgo0wPL68Zn3/X/Hj//QPHM= -https-proxy-agent@^5.0.0: +https-proxy-agent@5.0.0, https-proxy-agent@^5.0.0: version "5.0.0" resolved "https://registry.yarnpkg.com/https-proxy-agent/-/https-proxy-agent-5.0.0.tgz#e2a90542abb68a762e0a0850f6c9edadfd8506b2" integrity sha512-EkYm5BcKUGiduxzSt3Eppko+PiNWNEpa4ySk9vTC6wDsQJW9rHSa+UhGNJoRYp7bz6Ht1eaRIa6QaJqO5rCFbA== @@ -6077,7 +6072,7 @@ mkdirp@1.x: resolved "https://registry.yarnpkg.com/mkdirp/-/mkdirp-1.0.4.tgz#3eb5ed62622756d79a5f0e2a221dfebad75c2f7e" integrity sha512-vVqVZQyf3WLx2Shd0qJ9xuvqgAyKPLAiqITEtqW0oIUjzo3PePDd6fW9iFz30ef7Ysp/oiWqbhszeGWW2T6Gzw== -mkdirp@~0.5.1: +mkdirp@^0.5.1, mkdirp@~0.5.1: version "0.5.5" resolved "https://registry.yarnpkg.com/mkdirp/-/mkdirp-0.5.5.tgz#d91cefd62d1436ca0f41620e251288d420099def" integrity sha512-NKmAlESf6jMGym1++R0Ra7wvhV+wFW63FaSOFPwRahvea0gMUcGUhVeAg/0BC0wiv9ih5NYPB1Wn1UEI1/L+xQ== @@ -6171,6 +6166,11 @@ next-tick@^1.0.0: resolved "https://registry.yarnpkg.com/next-tick/-/next-tick-1.0.0.tgz#ca86d1fe8828169b0120208e3dc8424b9db8342c" integrity sha1-yobR/ogoFpsBICCOPchCS524NCw= +node-fetch@2.6.1, node-fetch@^2.6.1: + version "2.6.1" + resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052" + integrity sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw== + node-fetch@3.0.0-beta.9: version "3.0.0-beta.9" resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-3.0.0-beta.9.tgz#0a7554cfb824380dd6812864389923c783c80d9b" @@ -6179,11 +6179,6 @@ node-fetch@3.0.0-beta.9: data-uri-to-buffer "^3.0.1" fetch-blob "^2.1.1" -node-fetch@^2.6.1: - version "2.6.1" - resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052" - integrity sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw== - node-int64@^0.4.0: version "0.4.0" resolved "https://registry.yarnpkg.com/node-int64/-/node-int64-0.4.0.tgz#87a9065cdb355d3182d8f94ce11188b825c68a3b" @@ -6679,7 +6674,7 @@ pirates@^4.0.1: dependencies: node-modules-regexp "^1.0.0" -pkg-dir@^4.1.0, pkg-dir@^4.2.0: +pkg-dir@4.2.0, pkg-dir@^4.1.0, pkg-dir@^4.2.0: version "4.2.0" resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-4.2.0.tgz#f099133df7ede422e81d1d8448270eeb3e4261f3" integrity sha512-HRDzbaKjC+AOWVXxAU/x54COGeIv9eb+6CkDSQoNTt4XyWoIJvuPsXizxu/Fr23EiekbtZwmh1IcIG/l/a10GQ== @@ -6766,7 +6761,12 @@ process@~0.11.0: resolved "https://registry.yarnpkg.com/process/-/process-0.11.10.tgz#7332300e840161bda3e69a1d1d91a7d4bc16f182" integrity sha1-czIwDoQBYb2j5podHZGn1LwW8YI= -progress@^2.0.0, progress@^2.0.1: +progress@2.0.1: + version "2.0.1" + resolved "https://registry.yarnpkg.com/progress/-/progress-2.0.1.tgz#c9242169342b1c29d275889c95734621b1952e31" + integrity sha512-OE+a6vzqazc+K6LxJrX5UPyKFvGnL5CYmq2jFGNIBWHpc4QyE49/YOumcrpQFJpfejmvRtbJzgO1zPmMCqlbBg== + +progress@^2.0.0: version "2.0.3" resolved "https://registry.yarnpkg.com/progress/-/progress-2.0.3.tgz#7e8cf8d8f5b8f239c1bc68beb4eb78567d572ef8" integrity sha512-7PiHtLll5LdnKIMw100I+8xJXR5gW2QwWYkT6iJva0bXitZKa/XMrSbdmg3r2Xnaidz9Qumd0VPaMrZlF9V9sA== @@ -6779,7 +6779,7 @@ prompts@^2.0.1: kleur "^3.0.2" sisteransi "^1.0.0" -proxy-from-env@^1.1.0: +proxy-from-env@1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/proxy-from-env/-/proxy-from-env-1.1.0.tgz#e102f16ca355424865755d2c9e8ea4f24d58c3e2" integrity sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg== @@ -6836,23 +6836,23 @@ pupa@^2.0.1: dependencies: escape-goat "^2.0.0" -puppeteer@^9.1.1: - version "9.1.1" - resolved "https://registry.yarnpkg.com/puppeteer/-/puppeteer-9.1.1.tgz#f74b7facf86887efd6c6b9fabb7baae6fdce012c" - integrity sha512-W+nOulP2tYd/ZG99WuZC/I5ljjQQ7EUw/jQGcIb9eu8mDlZxNY2SgcJXTLG9h5gRvqA3uJOe4hZXYsd3EqioMw== - dependencies: - debug "^4.1.0" - devtools-protocol "0.0.869402" - extract-zip "^2.0.0" - https-proxy-agent "^5.0.0" - node-fetch "^2.6.1" - pkg-dir "^4.2.0" - progress "^2.0.1" - proxy-from-env "^1.1.0" - rimraf "^3.0.2" - tar-fs "^2.0.0" - unbzip2-stream "^1.3.3" - ws "^7.2.3" +puppeteer@^10.2.0: + version "10.2.0" + resolved "https://registry.yarnpkg.com/puppeteer/-/puppeteer-10.2.0.tgz#7d8d7fda91e19a7cfd56986e0275448e6351849e" + integrity sha512-OR2CCHRashF+f30+LBOtAjK6sNtz2HEyTr5FqAvhf8lR/qB3uBRoIZOwQKgwoyZnMBsxX7ZdazlyBgGjpnkiMw== + dependencies: + debug "4.3.1" + devtools-protocol "0.0.901419" + extract-zip "2.0.1" + https-proxy-agent "5.0.0" + node-fetch "2.6.1" + pkg-dir "4.2.0" + progress "2.0.1" + proxy-from-env "1.1.0" + rimraf "3.0.2" + tar-fs "2.0.0" + unbzip2-stream "1.3.3" + ws "7.4.6" q@^1.4.1: version "1.5.1" @@ -7161,6 +7161,13 @@ reusify@^1.0.4: resolved "https://registry.yarnpkg.com/reusify/-/reusify-1.0.4.tgz#90da382b1e126efc02146e90845a88db12925d76" integrity sha512-U9nH88a3fc/ekCF1l0/UP1IosiuIjyTh7hBvXVMHYgVcfGvt897Xguj2UOLDeI5BG2m7/uwyaLVT6fbtCwTyzw== +rimraf@3.0.2, rimraf@^3.0.0, rimraf@^3.0.2: + version "3.0.2" + resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-3.0.2.tgz#f1a5402ba6220ad52cc1282bac1ae3aa49fd061a" + integrity sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA== + dependencies: + glob "^7.1.3" + rimraf@^2.6.2: version "2.6.3" resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.6.3.tgz#b2d104fe0d8fb27cf9e0a1cda8262dd3833c6cab" @@ -7168,13 +7175,6 @@ rimraf@^2.6.2: dependencies: glob "^7.1.3" -rimraf@^3.0.0, rimraf@^3.0.2: - version "3.0.2" - resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-3.0.2.tgz#f1a5402ba6220ad52cc1282bac1ae3aa49fd061a" - integrity sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA== - dependencies: - glob "^7.1.3" - ripemd160@^2.0.0, ripemd160@^2.0.1: version "2.0.2" resolved "https://registry.yarnpkg.com/ripemd160/-/ripemd160-2.0.2.tgz#a1c1a6f624751577ba5d07914cbc92850585890c" @@ -7862,15 +7862,15 @@ tabulator-tables@^4.9.3: resolved "https://registry.yarnpkg.com/tabulator-tables/-/tabulator-tables-4.9.3.tgz#89ea8f9bffc11ba9a789369b5165ac82da26f4f0" integrity sha512-iwwQqAEGGxlgrBpcmJJvMJrfjGLcCXOB3AOb/DGkXqBy1YKoYA36hIl7qXGp6Jo8dSkzFAlDT6pKLZgyhs9OnQ== -tar-fs@^2.0.0: - version "2.1.1" - resolved "https://registry.yarnpkg.com/tar-fs/-/tar-fs-2.1.1.tgz#489a15ab85f1f0befabb370b7de4f9eb5cbe8784" - integrity sha512-V0r2Y9scmbDRLCNex/+hYzvp/zyYjvFbHPNgVTKfQvVrb6guiE/fxP+XblDNR011utopbkex2nM4dHNV6GDsng== +tar-fs@2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/tar-fs/-/tar-fs-2.0.0.tgz#677700fc0c8b337a78bee3623fdc235f21d7afad" + integrity sha512-vaY0obB6Om/fso8a8vakQBzwholQ7v5+uy+tF3Ozvxv1KNezmVQAiWtcNmMHFSFPqL3dJA8ha6gdtFbfX9mcxA== dependencies: chownr "^1.1.1" - mkdirp-classic "^0.5.2" + mkdirp "^0.5.1" pump "^3.0.0" - tar-stream "^2.1.4" + tar-stream "^2.0.0" tar-stream@^1.5.0: version "1.6.2" @@ -7885,7 +7885,7 @@ tar-stream@^1.5.0: to-buffer "^1.1.1" xtend "^4.0.0" -tar-stream@^2.1.4: +tar-stream@^2.0.0: version "2.2.0" resolved "https://registry.yarnpkg.com/tar-stream/-/tar-stream-2.2.0.tgz#acad84c284136b060dc3faa64474aa9aebd77287" integrity sha512-ujeqbceABgwMZxEJnk2HDY2DlnUZ+9oEcb1KzTVfYHio0UE6dG71n60d8D2I4qNvleWrrXpmjpt7vZeF1LnMZQ== @@ -8240,10 +8240,10 @@ unbox-primitive@^1.0.0: has-symbols "^1.0.2" which-boxed-primitive "^1.0.2" -unbzip2-stream@^1.3.3: - version "1.4.3" - resolved "https://registry.yarnpkg.com/unbzip2-stream/-/unbzip2-stream-1.4.3.tgz#b0da04c4371311df771cdc215e87f2130991ace7" - integrity sha512-mlExGW4w71ebDJviH16lQLtZS32VKqsSfk80GCfUlwT/4/hNRFsoscrF/c++9xinkMzECL1uL9DDwXqFWkruPg== +unbzip2-stream@1.3.3: + version "1.3.3" + resolved "https://registry.yarnpkg.com/unbzip2-stream/-/unbzip2-stream-1.3.3.tgz#d156d205e670d8d8c393e1c02ebd506422873f6a" + integrity sha512-fUlAF7U9Ah1Q6EieQ4x4zLNejrRvDWUYmxXUpN3uziFYCHapjWFaCAnreY9bGgxzaMCFAPPpYNng57CypwJVhg== dependencies: buffer "^5.2.1" through "^2.3.8" @@ -8588,6 +8588,11 @@ write-file-atomic@^3.0.0: signal-exit "^3.0.2" typedarray-to-buffer "^3.1.5" +ws@7.4.6, ws@^7.0.0, ws@^7.4.5: + version "7.4.6" + resolved "https://registry.yarnpkg.com/ws/-/ws-7.4.6.tgz#5654ca8ecdeee47c33a9a4bf6d28e2be2980377c" + integrity sha512-YmhHDO4MzaDLB+M9ym/mDA5z0naX8j7SIlT8f8z+I0VtzsRbekxEutHSme7NPS2qE8StCYQNUnfWdXta/Yu85A== + ws@^6.1.0: version "6.2.1" resolved "https://registry.yarnpkg.com/ws/-/ws-6.2.1.tgz#442fdf0a47ed64f59b6a5d8ff130f4748ed524fb" @@ -8595,16 +8600,6 @@ ws@^6.1.0: dependencies: async-limiter "~1.0.0" -ws@^7.0.0, ws@^7.4.5: - version "7.4.6" - resolved "https://registry.yarnpkg.com/ws/-/ws-7.4.6.tgz#5654ca8ecdeee47c33a9a4bf6d28e2be2980377c" - integrity sha512-YmhHDO4MzaDLB+M9ym/mDA5z0naX8j7SIlT8f8z+I0VtzsRbekxEutHSme7NPS2qE8StCYQNUnfWdXta/Yu85A== - -ws@^7.2.3: - version "7.4.4" - resolved "https://registry.yarnpkg.com/ws/-/ws-7.4.4.tgz#383bc9742cb202292c9077ceab6f6047b17f2d59" - integrity sha512-Qm8k8ojNQIMx7S+Zp8u/uHOx7Qazv3Yv4q68MiWWWOJhiwG5W3x7iqmRtJo8xxrciZUY4vRxUTJCKuRnF28ZZw== - xdg-basedir@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/xdg-basedir/-/xdg-basedir-4.0.0.tgz#4bc8d9984403696225ef83a1573cbbcb4e79db13" From 762f18b0dc5fa4f70278fcde9db0b46b264114c2 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Fri, 27 Aug 2021 16:47:28 -0500 Subject: [PATCH 02/11] tests --- .../gather/driver/network-monitor.js | 4 +- .../gather/driver/target-manager.js | 4 +- .../test/fraggle-rock/gather/mock-driver.js | 33 ++++ .../gather/driver/network-monitor-test.js | 47 +++++ .../test/gather/driver/target-manager-test.js | 182 ++++++++++++++++++ lighthouse-core/test/gather/mock-commands.js | 9 + 6 files changed, 275 insertions(+), 4 deletions(-) create mode 100644 lighthouse-core/test/gather/driver/target-manager-test.js diff --git a/lighthouse-core/gather/driver/network-monitor.js b/lighthouse-core/gather/driver/network-monitor.js index 7ce513bfb889..5622530048b2 100644 --- a/lighthouse-core/gather/driver/network-monitor.js +++ b/lighthouse-core/gather/driver/network-monitor.js @@ -78,7 +78,7 @@ class NetworkMonitor extends EventEmitter { * @return {Promise} */ async enable() { - if (this._networkRecorder) return; + if (this._targetManager) return; this._frameNavigations = []; this._sessions = new Map(); @@ -104,6 +104,7 @@ class NetworkMonitor extends EventEmitter { this._session.on('Page.frameNavigated', this._onFrameNavigated); this._targetManager.addTargetAttachedListener(this._onTargetAttached); + await this._session.sendCommand('Page.enable'); await this._targetManager.enable(); } @@ -117,7 +118,6 @@ class NetworkMonitor extends EventEmitter { this._messageLog.endRecording(); this._session.off('Page.frameNavigated', this._onFrameNavigated); - this._session.removeProtocolMessageListener(this._onProtocolMessage); this._targetManager.removeTargetAttachedListener(this._onTargetAttached); for (const session of this._sessions.values()) { diff --git a/lighthouse-core/gather/driver/target-manager.js b/lighthouse-core/gather/driver/target-manager.js index bab5073e4894..44fa1e4803f5 100644 --- a/lighthouse-core/gather/driver/target-manager.js +++ b/lighthouse-core/gather/driver/target-manager.js @@ -49,7 +49,7 @@ class TargetManager { async _onSessionAttached(session) { try { const target = await session.sendCommand('Target.getTargetInfo').catch(() => null); - const targetType = target && target.targetInfo.type; + const targetType = target && target.targetInfo && target.targetInfo.type; const hasValidTargetType = targetType === 'page' || targetType === 'iframe'; if (!target || !hasValidTargetType) return; @@ -71,7 +71,7 @@ class TargetManager { }); } catch (err) { // Sometimes targets can be closed before we even have a chance to listen to their network activity. - if (/Target closed/.test(err.stack)) return; + if (/Target closed/.test(err.message)) return; throw err; } finally { diff --git a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js index 27b7d34b52e2..cbabd09e58ee 100644 --- a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js +++ b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js @@ -91,6 +91,26 @@ function createMockExecutionContext() { }; } +function createMockTargetManager() { + return { + enable: jest.fn(), + disable: jest.fn(), + addTargetAttachedListener: createMockOnFn(), + removeTargetAttachedListener: jest.fn(), + reset() { + this.enable = jest.fn(); + this.disable = jest.fn(); + this.addTargetAttachedListener = createMockOnFn(); + this.removeTargetAttachedListener = jest.fn(); + }, + /** @return {import('../../../gather/driver/target-manager.js')} */ + asTargetManager() { + // @ts-expect-error - We'll rely on the tests passing to know this matches. + return this; + }, + }; +} + function createMockDriver() { const page = createMockPage(); const session = createMockSession(); @@ -168,6 +188,7 @@ function mockDriverSubmodules() { const networkMock = { fetchResponseBodyFromCache: jest.fn(), }; + const targetManagerMock = createMockTargetManager(); function reset() { navigationMock.gotoURL = jest.fn().mockResolvedValue({finalUrl: 'https://example.com', warnings: [], timedOut: false}); @@ -177,6 +198,7 @@ function mockDriverSubmodules() { emulationMock.clearThrottling = jest.fn(); emulationMock.emulate = jest.fn(); networkMock.fetchResponseBodyFromCache = jest.fn().mockResolvedValue(''); + targetManagerMock.reset(); } /** @@ -187,18 +209,29 @@ function mockDriverSubmodules() { const get = (target, name) => { return (...args) => target[name](...args); }; + + /** @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/navigation.js', () => new Proxy(navigationMock, {get})); jest.mock('../../../gather/driver/prepare.js', () => new Proxy(prepareMock, {get})); jest.mock('../../../gather/driver/storage.js', () => new Proxy(storageMock, {get})); jest.mock('../../../gather/driver/network.js', () => new Proxy(networkMock, {get})); + jest.mock('../../../gather/driver/target-manager.js', () => proxyCtor(targetManagerMock)); jest.mock('../../../lib/emulation.js', () => new Proxy(emulationMock, {get})); + reset(); + return { navigationMock, prepareMock, storageMock, emulationMock, networkMock, + targetManagerMock, reset, }; } diff --git a/lighthouse-core/test/gather/driver/network-monitor-test.js b/lighthouse-core/test/gather/driver/network-monitor-test.js index 0955cea1f20f..5f6dd228490c 100644 --- a/lighthouse-core/test/gather/driver/network-monitor-test.js +++ b/lighthouse-core/test/gather/driver/network-monitor-test.js @@ -5,6 +5,9 @@ */ 'use strict'; +const {mockDriverSubmodules} = require('../../fraggle-rock/gather/mock-driver.js'); +const mocks = mockDriverSubmodules(); + const NetworkMonitor = require('../../../gather/driver/network-monitor.js'); const NetworkRequest = require('../../../lib/network-request.js'); const networkRecordsToDevtoolsLog = require('../../network-records-to-devtools-log.js'); @@ -66,6 +69,13 @@ describe('NetworkMonitor', () => { monitor.on('network-2-idle', () => statusLog.push('network-2-idle')); 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'}, @@ -78,6 +88,10 @@ describe('NetworkMonitor', () => { devtoolsLog = [...startEvents, ...restEvents]; }); + afterEach(() => { + mocks.targetManagerMock.reset(); + }); + describe('.enable() / .disable()', () => { it('should not record anything when disabled', async () => { for (const message of devtoolsLog) sessionMock.dispatch(message); @@ -89,6 +103,7 @@ describe('NetworkMonitor', () => { for (const message of devtoolsLog) sessionMock.dispatch(message); expect(sessionMock.on).toHaveBeenCalled(); expect(sessionMock.addProtocolMessageListener).toHaveBeenCalled(); + expect(mocks.targetManagerMock.enable).toHaveBeenCalled(); expect(statusLog.length).toBeGreaterThan(0); }); @@ -101,9 +116,41 @@ describe('NetworkMonitor', () => { expect(sessionMock.off).toHaveBeenCalled(); expect(sessionMock.addProtocolMessageListener).toHaveBeenCalled(); expect(sessionMock.removeProtocolMessageListener).toHaveBeenCalled(); + expect(mocks.targetManagerMock.enable).toHaveBeenCalled(); + expect(mocks.targetManagerMock.disable).toHaveBeenCalled(); 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'}, session: sessionMock}); // duplicate + expect(sessionMock.addProtocolMessageListener).toHaveBeenCalledTimes(1); + expect(sendCommandMock).toHaveBeenCalledTimes(2); + + 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); + }); + it('should have idempotent enable', async () => { await monitor.enable(); await monitor.enable(); diff --git a/lighthouse-core/test/gather/driver/target-manager-test.js b/lighthouse-core/test/gather/driver/target-manager-test.js new file mode 100644 index 000000000000..42eed21ef37c --- /dev/null +++ b/lighthouse-core/test/gather/driver/target-manager-test.js @@ -0,0 +1,182 @@ +/** + * @license Copyright 2021 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * 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. + */ +'use strict'; + +const TargetManager = require('../../../gather/driver/target-manager.js'); +const {createMockSession} = require('../../fraggle-rock/gather/mock-driver.js'); +const { + makePromiseInspectable, + createDecomposedPromise, + flushAllTimersAndMicrotasks, +} = require('../../test-utils.js'); + +/* eslint-env jest */ + +jest.useFakeTimers(); + +/** + * + * @param {{type?: string, targetId?: string}} [overrides] + * @return {LH.Crdp.Target.TargetInfo} + */ +function createTargetInfo(overrides) { + return { + type: 'page', + targetId: 'page', + title: '', + url: '', + attached: true, + canAccessOpener: false, + ...overrides, + }; +} + +describe('TargetManager', () => { + let sessionMock = createMockSession(); + let sendCommandMock = sessionMock.sendCommand; + let targetManager = new TargetManager(sessionMock.asSession()); + let targetInfo = createTargetInfo(); + + beforeEach(() => { + sessionMock = createMockSession(); + sessionMock.sendCommand + .mockResponse('Page.enable') + .mockResponse('Runtime.runIfWaitingForDebugger'); + sendCommandMock = sessionMock.sendCommand; + targetManager = new TargetManager(sessionMock.asSession()); + targetInfo = createTargetInfo(); + }); + + describe('.enable()', () => { + it('should autoattach to root session', async () => { + sessionMock.sendCommand + .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Target.setAutoAttach'); + await targetManager.enable(); + + const invocations = sendCommandMock.findAllInvocations('Target.setAutoAttach'); + expect(invocations).toHaveLength(1); + + expect(sessionMock.setTargetInfo).toHaveBeenCalledWith(targetInfo); + }); + + it('should autoattach to further unique sessions', async () => { + sessionMock.sendCommand + .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('Target.setAutoAttach') + .mockResponse('Target.setAutoAttach') + .mockResponse('Target.setAutoAttach') + + .mockResponse('Runtime.runIfWaitingForDebugger') + .mockResponse('Runtime.runIfWaitingForDebugger') + .mockResponse('Runtime.runIfWaitingForDebugger') + .mockResponse('Runtime.runIfWaitingForDebugger'); + await targetManager.enable(); + + expect(sessionMock.addSessionAttachedListener).toHaveBeenCalled(); + const sessionListener = sessionMock.addSessionAttachedListener.mock.calls[0][0]; + + await sessionListener(sessionMock); + expect(sendCommandMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(1); + + await sessionListener(sessionMock); + expect(sendCommandMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(2); + + await sessionListener(sessionMock); + expect(sendCommandMock.findAllInvocations('Target.setAutoAttach')).toHaveLength(3); + }); + + it('should ignore non-frame targets', async () => { + targetInfo.type = 'worker'; + sessionMock.sendCommand + .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Target.setAutoAttach'); + await targetManager.enable(); + + const invocations = sendCommandMock.findAllInvocations('Target.setAutoAttach'); + expect(invocations).toHaveLength(0); + }); + + it('should fire listeners before target attached', async () => { + + }); + + it('should handle target closed gracefully', async () => { + sessionMock.sendCommand.mockResponse('Target.getTargetInfo', {targetInfo}); + const targetClosedError = new Error('Target closed'); + targetManager.addTargetAttachedListener(jest.fn().mockRejectedValue(targetClosedError)); + await targetManager.enable(); + }); + + it('should throw other listener errors', async () => { + sessionMock.sendCommand.mockResponse('Target.getTargetInfo', {targetInfo}); + const targetClosedError = new Error('Fatal error'); + targetManager.addTargetAttachedListener(jest.fn().mockRejectedValue(targetClosedError)); + await expect(targetManager.enable()).rejects.toMatchObject({message: 'Fatal error'}); + }); + + it('should resume the target when finished', async () => { + sessionMock.sendCommand.mockResponse('Target.getTargetInfo', {}); + await targetManager.enable(); + + const invocations = sendCommandMock.findAllInvocations('Runtime.runIfWaitingForDebugger'); + expect(invocations).toHaveLength(1); + }); + + it('should autoattach on main frame navigation', async () => { + sessionMock.sendCommand + .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Target.setAutoAttach') + .mockResponse('Target.setAutoAttach'); + await targetManager.enable(); + + const onFrameNavigation = sessionMock.on.getListeners('Page.frameNavigated')[0]; + onFrameNavigation({frame: {}}); // note the lack of a `parentId` + + const invocations = sendCommandMock.findAllInvocations('Target.setAutoAttach'); + expect(invocations).toHaveLength(2); + }); + + it('should not autoattach on subframe navigation', async () => { + sessionMock.sendCommand + .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Target.setAutoAttach') + .mockResponse('Target.setAutoAttach'); + await targetManager.enable(); + + const onFrameNavigation = sessionMock.on.getListeners('Page.frameNavigated')[0]; + onFrameNavigation({frame: {parentId: 'root'}}); + + const invocations = sendCommandMock.findAllInvocations('Target.setAutoAttach'); + expect(invocations).toHaveLength(1); + }); + + it('should be idempotent', async () => { + sessionMock.sendCommand + .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Target.setAutoAttach'); + await targetManager.enable(); + await targetManager.enable(); + await targetManager.enable(); + + const invocations = sendCommandMock.findAllInvocations('Target.setAutoAttach'); + expect(invocations).toHaveLength(1); + }); + }); + + describe('.disable()', () => { + it('should uninstall listeners', async () => { + await targetManager.disable(); + + expect(sessionMock.off).toHaveBeenCalled(); + expect(sessionMock.removeSessionAttachedListener).toHaveBeenCalled(); + }); + }); +}); diff --git a/lighthouse-core/test/gather/mock-commands.js b/lighthouse-core/test/gather/mock-commands.js index 63d588e086f4..72649cf5ac80 100644 --- a/lighthouse-core/test/gather/mock-commands.js +++ b/lighthouse-core/test/gather/mock-commands.js @@ -107,6 +107,15 @@ function createMockSendCommandFn(options) { call => call[0] === command && (!useSessionId || call[1] === sessionId) )[useSessionId ? 2 : 1]; }, + /** + * @param {keyof LH.CrdpCommands} command + * @param {string=} sessionId + */ + findAllInvocations(command, sessionId) { + return mockFn.mock.calls.filter( + call => call[0] === command && (!useSessionId || call[1] === sessionId) + ).map(invocation => useSessionId ? invocation[2] : invocation[1]); + }, }); return mockFn; From 1d464b3cc0f2421083d6de31cb9a987a1b2615d5 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Mon, 30 Aug 2021 10:41:20 -0500 Subject: [PATCH 03/11] fix tests --- lighthouse-core/gather/driver.js | 6 ++-- .../test/fraggle-rock/gather/mock-driver.js | 32 ++++++++++++++----- .../test/gather/driver/navigation-test.js | 9 +++++- lighthouse-core/test/gather/mock-commands.js | 2 +- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index d747b033611d..f49bb4a5bf83 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -200,17 +200,17 @@ class Driver { /** @param {LH.Crdp.Target.TargetInfo} targetInfo */ setTargetInfo(targetInfo) { // eslint-disable-line no-unused-vars - throw new Error(`OOPIF handling in legacy driver is implicit`); + // OOPIF handling in legacy driver is implicit. } /** @param {(session: LH.Gatherer.FRProtocolSession) => void} callback */ addSessionAttachedListener(callback) { // eslint-disable-line no-unused-vars - throw new Error(`OOPIF handling in legacy driver is implicit`); + // OOPIF handling in legacy driver is implicit. } /** @param {(session: LH.Gatherer.FRProtocolSession) => void} callback */ removeSessionAttachedListener(callback) { // eslint-disable-line no-unused-vars - throw new Error(`OOPIF handling in legacy driver is implicit`); + // OOPIF handling in legacy driver is implicit. } /** diff --git a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js index cbabd09e58ee..38366d9e61b6 100644 --- a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js +++ b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js @@ -97,6 +97,14 @@ function createMockTargetManager() { disable: jest.fn(), addTargetAttachedListener: createMockOnFn(), removeTargetAttachedListener: jest.fn(), + /** @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 = jest.fn(); this.disable = jest.fn(); @@ -151,6 +159,20 @@ function mockDriverModule(driverProvider) { }; } +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(), @@ -188,7 +210,7 @@ function mockDriverSubmodules() { const networkMock = { fetchResponseBodyFromCache: jest.fn(), }; - const targetManagerMock = createMockTargetManager(); + const targetManagerMock = mockTargetManagerModule(); function reset() { navigationMock.gotoURL = jest.fn().mockResolvedValue({finalUrl: 'https://example.com', warnings: [], timedOut: false}); @@ -210,17 +232,10 @@ function mockDriverSubmodules() { return (...args) => target[name](...args); }; - /** @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/navigation.js', () => new Proxy(navigationMock, {get})); jest.mock('../../../gather/driver/prepare.js', () => new Proxy(prepareMock, {get})); jest.mock('../../../gather/driver/storage.js', () => new Proxy(storageMock, {get})); jest.mock('../../../gather/driver/network.js', () => new Proxy(networkMock, {get})); - jest.mock('../../../gather/driver/target-manager.js', () => proxyCtor(targetManagerMock)); jest.mock('../../../lib/emulation.js', () => new Proxy(emulationMock, {get})); reset(); @@ -238,6 +253,7 @@ function mockDriverSubmodules() { module.exports = { 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 359d57e35c60..0fbc0458a86d 100644 --- a/lighthouse-core/test/gather/driver/navigation-test.js +++ b/lighthouse-core/test/gather/driver/navigation-test.js @@ -5,8 +5,10 @@ */ 'use strict'; +const {createMockDriver, mockTargetManagerModule} = require('../../fraggle-rock/gather/mock-driver.js'); // eslint-disable-line max-len +const targetManagerMock = mockTargetManagerModule(); + const {gotoURL, getNavigationWarnings} = require('../../../gather/driver/navigation.js'); -const {createMockDriver} = require('../../fraggle-rock/gather/mock-driver.js'); const { createMockOnceFn, makePromiseInspectable, @@ -26,6 +28,7 @@ describe('.gotoURL', () => { beforeEach(() => { mockDriver = createMockDriver(); driver = mockDriver.asDriver(); + targetManagerMock.mockEnable(driver.defaultSession); mockDriver.defaultSession.sendCommand .mockResponse('Page.enable') // network monitor's Page.enable @@ -37,6 +40,10 @@ 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/mock-commands.js b/lighthouse-core/test/gather/mock-commands.js index 72649cf5ac80..ca5240b14aed 100644 --- a/lighthouse-core/test/gather/mock-commands.js +++ b/lighthouse-core/test/gather/mock-commands.js @@ -63,7 +63,7 @@ function createMockSendCommandFn(options) { const indexOfResponse = mockResponses .findIndex(entry => entry.command === command && entry.sessionId === sessionId); - if (indexOfResponse === -1) throw new Error(`${command} unimplemented`); + if (indexOfResponse === -1) return Promise.reject(new Error(`${command} unimplemented`)); const {response, delay} = mockResponses[indexOfResponse]; mockResponses.splice(indexOfResponse, 1); const returnValue = typeof response === 'function' ? response(...args) : response; From 8ee75135545397d97ebaa01c04b78e5131ea5d25 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Mon, 30 Aug 2021 10:57:15 -0500 Subject: [PATCH 04/11] lint --- .../test-definitions/oopif/oopif-iframe-audit.js | 7 +++++++ lighthouse-core/test/gather/driver/target-manager-test.js | 5 ----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js index dfd996d808b6..18a87854998f 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js @@ -1,3 +1,10 @@ +/** + * @license Copyright 2021 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * 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. + */ +'use strict'; + module.exports = { meta: { id: 'iframe-elements', diff --git a/lighthouse-core/test/gather/driver/target-manager-test.js b/lighthouse-core/test/gather/driver/target-manager-test.js index 42eed21ef37c..57f62462373c 100644 --- a/lighthouse-core/test/gather/driver/target-manager-test.js +++ b/lighthouse-core/test/gather/driver/target-manager-test.js @@ -7,11 +7,6 @@ const TargetManager = require('../../../gather/driver/target-manager.js'); const {createMockSession} = require('../../fraggle-rock/gather/mock-driver.js'); -const { - makePromiseInspectable, - createDecomposedPromise, - flushAllTimersAndMicrotasks, -} = require('../../test-utils.js'); /* eslint-env jest */ From c17902675991a4181e4627b9b2642bbf8ec4d895 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Mon, 30 Aug 2021 16:46:19 -0500 Subject: [PATCH 05/11] fix oopif bundle soke --- .../smokehouse/lighthouse-runners/bundle.js | 27 +++++++++++++++++++ .../test/smokehouse/report-assert.js | 15 ++++++++--- .../test-definitions/oopif/oopif-config.js | 1 + .../oopif/oopif-expectations.js | 1 + 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/lighthouse-cli/test/smokehouse/lighthouse-runners/bundle.js b/lighthouse-cli/test/smokehouse/lighthouse-runners/bundle.js index e79a5b0424ec..20d64cfdf204 100644 --- a/lighthouse-cli/test/smokehouse/lighthouse-runners/bundle.js +++ b/lighthouse-cli/test/smokehouse/lighthouse-runners/bundle.js @@ -30,6 +30,32 @@ global.require = originalRequire; // @ts-expect-error - not worth giving test global an actual type. const lighthouse = global.runBundledLighthouse; +/** + * Clean the config of any additional audits that might not be able to be referenced. + * A new audit is detected by finding any audit entry with a path and no options. + * + * @param {LH.Config.Json=} configJson + */ +function cleanConfig(configJson) { + if (!configJson) return; + + if (configJson.audits) { + configJson.audits = configJson.audits.filter(audit => { + // Just a string, is adding a new audit that won't be in the bundle, prune it. + if (typeof audit === 'string') return false; + // If it's the audit implementation directly, it's a new audit, prune it. (though not currently possible) + if (typeof audit !== 'object') return false; + // Keep the audit only if it is setting options, which is the signal that it's an existing core audit. + return Boolean(audit.options); + }); + } + + if (configJson.categories) { + // Categories are only defined for introduction of new categories. + delete configJson.categories; + } +} + /** * Launch Chrome and do a full Lighthouse run via the Lighthouse CLI. * @param {string} url @@ -42,6 +68,7 @@ async function runLighthouse(url, configJson, testRunnerOptions = {}) { const launchedChrome = await ChromeLauncher.launch(); const port = launchedChrome.port; const connection = new ChromeProtocol(port); + cleanConfig(configJson); // Run Lighthouse. const logLevel = testRunnerOptions.isDebug ? 'info' : undefined; diff --git a/lighthouse-cli/test/smokehouse/report-assert.js b/lighthouse-cli/test/smokehouse/report-assert.js index d46cf09a4e66..5ae15453fef7 100644 --- a/lighthouse-cli/test/smokehouse/report-assert.js +++ b/lighthouse-cli/test/smokehouse/report-assert.js @@ -157,9 +157,11 @@ function makeComparison(name, actualResult, expectedResult) { * @param {LocalConsole} localConsole * @param {LH.Result} lhr * @param {Smokehouse.ExpectedRunnerResult} expected + * @param {{isBundled?: boolean}=} reportOptions */ -function pruneExpectations(localConsole, lhr, expected) { +function pruneExpectations(localConsole, lhr, expected, reportOptions) { const isFraggleRock = lhr.configSettings.channel === 'fraggle-rock-cli'; + const isBundled = reportOptions && reportOptions.isBundled; /** * Lazily compute the Chrome version because some reports are explicitly asserting error conditions. @@ -211,6 +213,12 @@ function pruneExpectations(localConsole, lhr, expected) { `Actual channel: ${lhr.configSettings.channel}`, ].join(' ')); delete obj[key]; + } else if (value._skipInBundled && !isBundled) { + localConsole.log([ + `[${key}] marked as skip in bundled and runner is bundled, pruning expectation:`, + JSON.stringify(value, null, 2), + ].join(' ')); + delete obj[key]; } else { pruneRecursively(value); } @@ -218,6 +226,7 @@ function pruneExpectations(localConsole, lhr, expected) { delete obj._legacyOnly; delete obj._fraggleRockOnly; + delete obj._skipInBundled; delete obj._minChromiumMilestone; delete obj._maxChromiumMilestone; } @@ -358,13 +367,13 @@ function reportAssertion(localConsole, assertion) { * summary. Returns count of passed and failed tests. * @param {{lhr: LH.Result, artifacts: LH.Artifacts, networkRequests?: string[]}} actual * @param {Smokehouse.ExpectedRunnerResult} expected - * @param {{isDebug?: boolean}=} reportOptions + * @param {{isDebug?: boolean, isBundled?: boolean}=} reportOptions * @return {{passed: number, failed: number, log: string}} */ function report(actual, expected, reportOptions = {}) { const localConsole = new LocalConsole(); - expected = pruneExpectations(localConsole, actual.lhr, expected); + expected = pruneExpectations(localConsole, actual.lhr, expected, reportOptions); const comparisons = collateResults(localConsole, actual, expected); let correctCount = 0; diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js index b85a794b4a0a..8aeb188cbe8c 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js @@ -18,6 +18,7 @@ module.exports = { }, }, audits: [ + // Include an audit that *forces* the IFrameElements artifact to be used for our test. {path: `${__dirname}/oopif-iframe-audit.js`}, ], settings: { diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-expectations.js index a31e1fe25d88..28928a14d456 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-expectations.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-expectations.js @@ -28,6 +28,7 @@ module.exports = { }, }, artifacts: { + _skipInBundled: true, IFrameElements: [ { id: 'oopif', From bbb12bdec4e9cfeb238f2a4e808b6f95872cc10a Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 2 Sep 2021 20:57:04 -0500 Subject: [PATCH 06/11] feedback --- .../test-definitions/oopif/oopif-config.js | 4 +++- lighthouse-core/gather/driver/network-monitor.js | 1 - lighthouse-core/gather/driver/target-manager.js | 12 ++++++------ .../test/gather/driver/target-manager-test.js | 10 +++++++++- lighthouse-core/test/gather/mock-commands.js | 7 +++---- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js index 8aeb188cbe8c..d429274f561f 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js @@ -5,6 +5,8 @@ */ 'use strict'; +const {LH_ROOT} = require('../../../../../root.js'); + /** * @type {LH.Config.Json} * Config file for running the OOPIF tests @@ -19,7 +21,7 @@ module.exports = { }, audits: [ // Include an audit that *forces* the IFrameElements artifact to be used for our test. - {path: `${__dirname}/oopif-iframe-audit.js`}, + {path: `${LH_ROOT}/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js`}, // eslint-disable-line max-len ], settings: { // This test runs in CI and hits the outside network of a live site. diff --git a/lighthouse-core/gather/driver/network-monitor.js b/lighthouse-core/gather/driver/network-monitor.js index 5622530048b2..34699fb9be0e 100644 --- a/lighthouse-core/gather/driver/network-monitor.js +++ b/lighthouse-core/gather/driver/network-monitor.js @@ -66,7 +66,6 @@ class NetworkMonitor extends EventEmitter { */ async _onTargetAttached({session, target}) { const targetId = target.targetId; - if (this._sessions.has(targetId)) return; this._sessions.set(targetId, session); session.addProtocolMessageListener(this._onProtocolMessage); diff --git a/lighthouse-core/gather/driver/target-manager.js b/lighthouse-core/gather/driver/target-manager.js index 44fa1e4803f5..54297b3d1e77 100644 --- a/lighthouse-core/gather/driver/target-manager.js +++ b/lighthouse-core/gather/driver/target-manager.js @@ -44,8 +44,8 @@ class TargetManager { } /** - * @param {LH.Gatherer.FRProtocolSession} session - */ + * @param {LH.Gatherer.FRProtocolSession} session + */ async _onSessionAttached(session) { try { const target = await session.sendCommand('Target.getTargetInfo').catch(() => null); @@ -81,8 +81,8 @@ class TargetManager { } /** - * @return {Promise} - */ + * @return {Promise} + */ async enable() { if (this._enabled) return; @@ -97,8 +97,8 @@ class TargetManager { } /** - * @return {Promise} - */ + * @return {Promise} + */ async disable() { this._session.off('Page.frameNavigated', this._onFrameNavigated); this._session.removeSessionAttachedListener(this._onSessionAttached); diff --git a/lighthouse-core/test/gather/driver/target-manager-test.js b/lighthouse-core/test/gather/driver/target-manager-test.js index 57f62462373c..5daba98740c5 100644 --- a/lighthouse-core/test/gather/driver/target-manager-test.js +++ b/lighthouse-core/test/gather/driver/target-manager-test.js @@ -100,7 +100,15 @@ describe('TargetManager', () => { }); it('should fire listeners before target attached', async () => { - + sessionMock.sendCommand + .mockResponse('Target.getTargetInfo', {targetInfo}) + .mockResponse('Target.setAutoAttach'); + targetManager.addTargetAttachedListener(jest.fn().mockImplementation(() => { + const setAutoAttachCalls = sessionMock.sendCommand.mock.calls + .filter(call => call[0] === 'Target.setAutoAttach'); + expect(setAutoAttachCalls).toHaveLength(0); + })); + await targetManager.enable(); }); it('should handle target closed gracefully', async () => { diff --git a/lighthouse-core/test/gather/mock-commands.js b/lighthouse-core/test/gather/mock-commands.js index ca5240b14aed..2beec4a71cc3 100644 --- a/lighthouse-core/test/gather/mock-commands.js +++ b/lighthouse-core/test/gather/mock-commands.js @@ -54,7 +54,7 @@ function createMockSendCommandFn(options) { * @param {string|undefined=} sessionId * @param {LH.CrdpCommands[C]['paramsType']} args */ - (command, sessionId, ...args) => { + async (command, sessionId, ...args) => { if (!useSessionId) { // @ts-expect-error - If sessionId isn't used, it *is* args. args = [sessionId, ...args]; @@ -63,13 +63,12 @@ function createMockSendCommandFn(options) { const indexOfResponse = mockResponses .findIndex(entry => entry.command === command && entry.sessionId === sessionId); - if (indexOfResponse === -1) return Promise.reject(new Error(`${command} unimplemented`)); + if (indexOfResponse === -1) throw new Error(`${command} unimplemented`); const {response, delay} = mockResponses[indexOfResponse]; mockResponses.splice(indexOfResponse, 1); const returnValue = typeof response === 'function' ? response(...args) : response; if (delay) return new Promise(resolve => setTimeout(() => resolve(returnValue), delay)); - // @ts-expect-error: Some covariant type stuff doesn't work here. idk, I'm not a type scientist. - return Promise.resolve(returnValue); + return returnValue; }); const mockFn = Object.assign(mockFnImpl, { From 15d96ff680358ce6820751e6f1e7682bf83baa0e Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 2 Sep 2021 20:57:28 -0500 Subject: [PATCH 07/11] Update lighthouse-core/fraggle-rock/gather/session.js Co-authored-by: Brendan Kenny --- lighthouse-core/fraggle-rock/gather/session.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/fraggle-rock/gather/session.js b/lighthouse-core/fraggle-rock/gather/session.js index 1e307bfa0326..38eef234e394 100644 --- a/lighthouse-core/fraggle-rock/gather/session.js +++ b/lighthouse-core/fraggle-rock/gather/session.js @@ -32,7 +32,7 @@ class ProtocolSession { if (originalEmit[SessionEmitMonkeypatch]) return; session.emit = (method, ...args) => { // OOPIF sessions need to emit their sessionId so downstream processors can recognize - // the appropriate target to use. + // the target the event came from. const sessionId = this._targetInfo && this._targetInfo.type === 'iframe' ? this._targetInfo.targetId : undefined; originalEmit.call(session, '*', {method, params: args[0], sessionId}); From 5110856692bf364300c30f0d5836448a13615566 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Tue, 7 Sep 2021 10:50:34 -0500 Subject: [PATCH 08/11] include oopif iframe audit in bundle --- .../smokehouse/lighthouse-runners/bundle.js | 52 +++++-------------- .../test-definitions/oopif/oopif-config.js | 4 +- .../oopif/oopif-expectations.js | 1 - .../audits}/oopif-iframe-audit.js | 13 +++++ 4 files changed, 28 insertions(+), 42 deletions(-) rename {lighthouse-cli/test/smokehouse/test-definitions/oopif => lighthouse-core/audits}/oopif-iframe-audit.js (50%) diff --git a/lighthouse-cli/test/smokehouse/lighthouse-runners/bundle.js b/lighthouse-cli/test/smokehouse/lighthouse-runners/bundle.js index 20d64cfdf204..de9547c93180 100644 --- a/lighthouse-cli/test/smokehouse/lighthouse-runners/bundle.js +++ b/lighthouse-cli/test/smokehouse/lighthouse-runners/bundle.js @@ -30,32 +30,6 @@ global.require = originalRequire; // @ts-expect-error - not worth giving test global an actual type. const lighthouse = global.runBundledLighthouse; -/** - * Clean the config of any additional audits that might not be able to be referenced. - * A new audit is detected by finding any audit entry with a path and no options. - * - * @param {LH.Config.Json=} configJson - */ -function cleanConfig(configJson) { - if (!configJson) return; - - if (configJson.audits) { - configJson.audits = configJson.audits.filter(audit => { - // Just a string, is adding a new audit that won't be in the bundle, prune it. - if (typeof audit === 'string') return false; - // If it's the audit implementation directly, it's a new audit, prune it. (though not currently possible) - if (typeof audit !== 'object') return false; - // Keep the audit only if it is setting options, which is the signal that it's an existing core audit. - return Boolean(audit.options); - }); - } - - if (configJson.categories) { - // Categories are only defined for introduction of new categories. - delete configJson.categories; - } -} - /** * Launch Chrome and do a full Lighthouse run via the Lighthouse CLI. * @param {string} url @@ -68,20 +42,22 @@ async function runLighthouse(url, configJson, testRunnerOptions = {}) { const launchedChrome = await ChromeLauncher.launch(); const port = launchedChrome.port; const connection = new ChromeProtocol(port); - cleanConfig(configJson); - // Run Lighthouse. - const logLevel = testRunnerOptions.isDebug ? 'info' : undefined; - const runnerResult = await lighthouse(url, {port, logLevel}, configJson, connection); - if (!runnerResult) throw new Error('No runnerResult'); + try { + // Run Lighthouse. + const logLevel = testRunnerOptions.isDebug ? 'info' : undefined; + const runnerResult = await lighthouse(url, {port, logLevel}, configJson, connection); + if (!runnerResult) throw new Error('No runnerResult'); - // Clean up and return results. - await launchedChrome.kill(); - return { - lhr: runnerResult.lhr, - artifacts: runnerResult.artifacts, - log: '', // TODO: if want to run in parallel, need to capture lighthouse-logger output. - }; + return { + lhr: runnerResult.lhr, + artifacts: runnerResult.artifacts, + log: '', // TODO: if want to run in parallel, need to capture lighthouse-logger output. + }; + } finally { + // Clean up and return results. + await launchedChrome.kill(); + } } module.exports = { diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js index d429274f561f..4ff4aea982d6 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js @@ -5,8 +5,6 @@ */ 'use strict'; -const {LH_ROOT} = require('../../../../../root.js'); - /** * @type {LH.Config.Json} * Config file for running the OOPIF tests @@ -21,7 +19,7 @@ module.exports = { }, audits: [ // Include an audit that *forces* the IFrameElements artifact to be used for our test. - {path: `${LH_ROOT}/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js`}, // eslint-disable-line max-len + {path: 'oopif-iframe-audit'}, ], settings: { // This test runs in CI and hits the outside network of a live site. diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-expectations.js index 28928a14d456..a31e1fe25d88 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-expectations.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-expectations.js @@ -28,7 +28,6 @@ module.exports = { }, }, artifacts: { - _skipInBundled: true, IFrameElements: [ { id: 'oopif', diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js b/lighthouse-core/audits/oopif-iframe-audit.js similarity index 50% rename from lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js rename to lighthouse-core/audits/oopif-iframe-audit.js index 18a87854998f..33ca1f3872e3 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-iframe-audit.js +++ b/lighthouse-core/audits/oopif-iframe-audit.js @@ -5,6 +5,19 @@ */ 'use strict'; +/** + * @fileoverview This is a fake audit used exclusively in smoke tests to force inclusion of IFrameElements artifact. + * It is included here for complex reasons in the way the bundled smoketests work. + * + * The smokehouse configs are evaluated first in the node CLI side (which requires an absolute path using LH_ROOT). + * The smokehouse configs are then *re-evaluated* in the bundled context for execution by Lighthouse (which *cannot* use an absolute path using LH_ROOT). + * + * This mismatch in environment demands that the audit path in the config must be context-aware, + * yet the require-graph for the config is included before even the CLI knows that it will be using + * a bundled runner. Rather than force a massive smoketest architecture change, we include a harmless, + * test-only audit in our core list instead. + */ + module.exports = { meta: { id: 'iframe-elements', From 1f3c1879ef381f3f82973dce47164d1ba57e7678 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 8 Sep 2021 10:20:57 -0500 Subject: [PATCH 09/11] feedback --- lighthouse-core/audits/oopif-iframe-audit.js | 2 +- .../gather/driver/network-monitor.js | 47 +++++++++---------- .../gather/gatherers/devtools-log.js | 16 +++++-- .../gather/driver/network-monitor-test.js | 4 -- types/protocol.d.ts | 2 + 5 files changed, 36 insertions(+), 35 deletions(-) diff --git a/lighthouse-core/audits/oopif-iframe-audit.js b/lighthouse-core/audits/oopif-iframe-audit.js index 33ca1f3872e3..efdc31433bf2 100644 --- a/lighthouse-core/audits/oopif-iframe-audit.js +++ b/lighthouse-core/audits/oopif-iframe-audit.js @@ -20,7 +20,7 @@ module.exports = { meta: { - id: 'iframe-elements', + id: 'oopif-iframe-audit', title: 'IFrame Elements', description: 'Audit to force the inclusion of IFrameElements artifact', requiredArtifacts: ['IFrameElements'], diff --git a/lighthouse-core/gather/driver/network-monitor.js b/lighthouse-core/gather/driver/network-monitor.js index 34699fb9be0e..778f8a179300 100644 --- a/lighthouse-core/gather/driver/network-monitor.js +++ b/lighthouse-core/gather/driver/network-monitor.js @@ -12,7 +12,6 @@ const log = require('lighthouse-logger'); const {EventEmitter} = require('events'); -const MessageLog = require('../devtools-log.js'); const NetworkRecorder = require('../../lib/network-recorder.js'); const NetworkRequest = require('../../lib/network-request.js'); const URL = require('../../lib/url-shim.js'); @@ -21,8 +20,11 @@ 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 {LH.Protocol.StrictEventEmitter} NetworkMonitorEmitter */ -class NetworkMonitor extends EventEmitter { +/** @implements {NetworkMonitorEmitter} */ +class NetworkMonitor { /** @type {NetworkRecorder|undefined} */ _networkRecorder = undefined; /** @type {TargetManager|undefined} */ @@ -32,11 +34,8 @@ class NetworkMonitor extends EventEmitter { /** @param {LH.Gatherer.FRProtocolSession} session */ constructor(session) { - super(); this._session = session; - this._messageLog = new MessageLog(/^(Page|Network)\./); - this._onTargetAttached = this._onTargetAttached.bind(this); /** @type {Map} */ @@ -47,18 +46,27 @@ class NetworkMonitor extends EventEmitter { /** @param {LH.Protocol.RawEventMessage} event */ this._onProtocolMessage = event => { - this._messageLog.record(event); + this.emit('protocolmessage', event); if (!this._networkRecorder) return; this._networkRecorder.dispatch(event); }; - // Redefine the event emitter types with a narrower type signature. - /** @param {NetworkMonitorEvent} event @param {*} listener */ - this.on = (event, listener) => super.on(event, listener); - /** @param {NetworkMonitorEvent} event @param {*} listener */ - this.once = (event, listener) => super.once(event, listener); - /** @param {NetworkMonitorEvent} event @param {*} listener */ - this.off = (event, listener) => super.off(event, listener); + // Attach the event emitter types to this class. + const emitter = /** @type {NetworkMonitorEmitter} */ (new EventEmitter()); + /** @type {typeof emitter['emit']} */ + this.emit = emitter.emit.bind(emitter); + /** @type {typeof emitter['on']} */ + this.on = emitter.on.bind(emitter); + /** @type {typeof emitter['once']} */ + this.once = emitter.once.bind(emitter); + /** @type {typeof emitter['off']} */ + this.off = emitter.off.bind(emitter); + /** @type {typeof emitter['addListener']} */ + this.addListener = emitter.addListener.bind(emitter); + /** @type {typeof emitter['removeListener']} */ + this.removeListener = emitter.removeListener.bind(emitter); + /** @type {typeof emitter['removeAllListeners']} */ + this.removeAllListeners = emitter.removeAllListeners.bind(emitter); } /** @@ -97,9 +105,6 @@ class NetworkMonitor extends EventEmitter { this._networkRecorder.on('requeststarted', reEmit('requeststarted')); this._networkRecorder.on('requestloaded', reEmit('requestloaded')); - this._messageLog.reset(); - this._messageLog.beginRecording(); - this._session.on('Page.frameNavigated', this._onFrameNavigated); this._targetManager.addTargetAttachedListener(this._onTargetAttached); @@ -113,9 +118,6 @@ class NetworkMonitor extends EventEmitter { async disable() { if (!this._targetManager) return; - this._messageLog.reset(); - this._messageLog.endRecording(); - this._session.off('Page.frameNavigated', this._onFrameNavigated); this._targetManager.removeTargetAttachedListener(this._onTargetAttached); @@ -145,13 +147,6 @@ class NetworkMonitor extends EventEmitter { return finalNavigation && finalNavigation.url; } - /** - * @return {LH.DevtoolsLog} - */ - getMessageLog() { - return this._messageLog.messages; - } - /** * @return {Array} */ diff --git a/lighthouse-core/gather/gatherers/devtools-log.js b/lighthouse-core/gather/gatherers/devtools-log.js index 84c000e5fc5e..3038aba09aa5 100644 --- a/lighthouse-core/gather/gatherers/devtools-log.js +++ b/lighthouse-core/gather/gatherers/devtools-log.js @@ -12,6 +12,7 @@ */ const NetworkMonitor = require('../driver/network-monitor.js'); +const MessageLog = require('../devtools-log.js'); const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js'); class DevtoolsLog extends FRGatherer { @@ -29,29 +30,36 @@ class DevtoolsLog extends FRGatherer { /** @type {NetworkMonitor|undefined} */ this._networkMonitor = undefined; - /** @type {LH.DevtoolsLog} */ - this._devtoolsLog = []; + this._messageLog = new MessageLog(/^(Page|Network)\./); + + /** @param {LH.Protocol.RawEventMessage} e */ + this._onProtocolMessage = e => this._messageLog.record(e); } /** * @param {LH.Gatherer.FRTransitionalContext} passContext */ async startSensitiveInstrumentation({driver}) { + this._messageLog.reset(); + this._messageLog.beginRecording(); + this._networkMonitor = new NetworkMonitor(driver.defaultSession); + this._networkMonitor.on('protocolmessage', this._onProtocolMessage); this._networkMonitor.enable(); } async stopSensitiveInstrumentation() { if (!this._networkMonitor) return; - this._devtoolsLog = this._networkMonitor.getMessageLog(); + this._messageLog.endRecording(); this._networkMonitor.disable(); + this._networkMonitor.off('protocolmessage', this._onProtocolMessage); } /** * @return {Promise} */ async getArtifact() { - return this._devtoolsLog; + return this._messageLog.messages; } } diff --git a/lighthouse-core/test/gather/driver/network-monitor-test.js b/lighthouse-core/test/gather/driver/network-monitor-test.js index 5f6dd228490c..536d5d91a88a 100644 --- a/lighthouse-core/test/gather/driver/network-monitor-test.js +++ b/lighthouse-core/test/gather/driver/network-monitor-test.js @@ -134,10 +134,6 @@ describe('NetworkMonitor', () => { .mockResponse('Network.enable') .mockResponse('Network.enable'); - targetListener({target: {type: 'page', targetId: 'page'}, session: sessionMock}); // duplicate - expect(sessionMock.addProtocolMessageListener).toHaveBeenCalledTimes(1); - expect(sendCommandMock).toHaveBeenCalledTimes(2); - targetListener({target: {type: 'page', targetId: 'page-2'}, session: sessionMock}); // new expect(sessionMock.addProtocolMessageListener).toHaveBeenCalledTimes(2); expect(sendCommandMock).toHaveBeenCalledTimes(3); diff --git a/types/protocol.d.ts b/types/protocol.d.ts index 6912245d76d7..2ca8612c3985 100644 --- a/types/protocol.d.ts +++ b/types/protocol.d.ts @@ -39,6 +39,8 @@ declare module Protocol { type StrictEventEmitter> = { on(event: E, listener: (...args: TEventRecord[E]) => void): void; + off(event: E, listener: Function): void; + addListener(event: E, listener: (...args: TEventRecord[E]) => void): void; removeListener(event: E, listener: Function): void; From b5d3d4adb585384a9479a16e7e266ad6675b6d9e Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 8 Sep 2021 11:04:39 -0500 Subject: [PATCH 10/11] rename --- .../test/smokehouse/test-definitions/oopif/oopif-config.js | 2 +- .../{oopif-iframe-audit.js => oopif-iframe-test-audit.js} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename lighthouse-core/audits/{oopif-iframe-audit.js => oopif-iframe-test-audit.js} (97%) diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js index 4ff4aea982d6..e30fed8576a5 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js @@ -19,7 +19,7 @@ module.exports = { }, audits: [ // Include an audit that *forces* the IFrameElements artifact to be used for our test. - {path: 'oopif-iframe-audit'}, + {path: 'oopif-iframe-test-audit'}, ], settings: { // This test runs in CI and hits the outside network of a live site. diff --git a/lighthouse-core/audits/oopif-iframe-audit.js b/lighthouse-core/audits/oopif-iframe-test-audit.js similarity index 97% rename from lighthouse-core/audits/oopif-iframe-audit.js rename to lighthouse-core/audits/oopif-iframe-test-audit.js index efdc31433bf2..4d09aeace738 100644 --- a/lighthouse-core/audits/oopif-iframe-audit.js +++ b/lighthouse-core/audits/oopif-iframe-test-audit.js @@ -20,7 +20,7 @@ module.exports = { meta: { - id: 'oopif-iframe-audit', + id: 'oopif-iframe-test-audit', title: 'IFrame Elements', description: 'Audit to force the inclusion of IFrameElements artifact', requiredArtifacts: ['IFrameElements'], From f03370a83241ef655839b3804d0d0838c9966d1a Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 8 Sep 2021 11:25:16 -0500 Subject: [PATCH 11/11] fix smoke --- .github/workflows/smoke.yml | 2 +- .../test/smokehouse/test-definitions/oopif/oopif-config.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index 1e1a8c7908c9..044888e1fb8d 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -106,7 +106,7 @@ jobs: - run: sudo apt-get install xvfb - name: yarn smoke --fraggle-rock - run: xvfb-run --auto-servernum yarn smoke --debug --fraggle-rock -j=1 --retries=2 --invert-match pwa offline oopif perf-diagnostics-third-party + run: xvfb-run --auto-servernum yarn smoke --debug --fraggle-rock -j=1 --retries=2 --invert-match pwa offline # Fail if any changes were written to source files. - run: git diff --exit-code diff --git a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js index e30fed8576a5..bc7ef0a3e367 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/oopif/oopif-config.js @@ -14,7 +14,7 @@ module.exports = { categories: { performance: { title: 'Performance', - auditRefs: [{id: 'iframe-elements', weight: 0}], + auditRefs: [{id: 'oopif-iframe-test-audit', weight: 0}], }, }, audits: [