Skip to content

Commit

Permalink
core(target-manager): only listen to LH sessions (#14385)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine committed Sep 30, 2022
1 parent 9d5c067 commit bf8df46
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 44 deletions.
4 changes: 2 additions & 2 deletions clients/lightrider/lightrider-entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import {Buffer} from 'buffer';

import log from 'lighthouse-logger';
import {Browser} from 'puppeteer-core/lib/esm/puppeteer/common/Browser.js';
import {CDPBrowser} from 'puppeteer-core/lib/esm/puppeteer/common/Browser.js';
import {Connection as PptrConnection} from 'puppeteer-core/lib/esm/puppeteer/common/Connection.js';

import lighthouse, {legacyNavigation} from '../../core/index.js';
Expand Down Expand Up @@ -48,7 +48,7 @@ async function getPageFromConnection(connection) {
connection.channel_.root_.transport_
);

const browser = await Browser._create(
const browser = await CDPBrowser._create(
'chrome',
pptrConnection,
[] /* contextIds */,
Expand Down
10 changes: 2 additions & 8 deletions core/gather/driver/target-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ class TargetManager extends ProtocolEventEmitter {
// @ts-expect-error - pptr currently typed only for single arg emits.
const protocolListener = trueProtocolListener;
cdpSession.on('*', protocolListener);
cdpSession.on('sessionattached', this._onSessionAttached);

const targetWithSession = {
target: target.targetInfo,
Expand Down Expand Up @@ -176,12 +177,6 @@ class TargetManager extends ProtocolEventEmitter {

this._rootCdpSession.on('Page.frameNavigated', this._onFrameNavigated);

const rootConnection = this._rootCdpSession.connection();
if (!rootConnection) {
throw new Error('Connection has been closed.');
}
rootConnection.on('sessionattached', this._onSessionAttached);

await this._rootCdpSession.send('Page.enable');

// Start with the already attached root session.
Expand All @@ -193,11 +188,10 @@ class TargetManager extends ProtocolEventEmitter {
*/
async disable() {
this._rootCdpSession.off('Page.frameNavigated', this._onFrameNavigated);
// No need to remove listener if connection is already closed.
this._rootCdpSession.connection()?.off('sessionattached', this._onSessionAttached);

for (const {cdpSession, protocolListener} of this._targetIdToTargets.values()) {
cdpSession.off('*', protocolListener);
cdpSession.off('sessionattached', this._onSessionAttached);
}

this._enabled = false;
Expand Down
13 changes: 5 additions & 8 deletions core/test/gather/driver/target-manager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import {EventEmitter} from 'events';

import {CDPSession} from 'puppeteer-core/lib/cjs/puppeteer/common/Connection.js';
import {CDPSessionImpl} from 'puppeteer-core/lib/cjs/puppeteer/common/Connection.js';

import {TargetManager} from '../../../gather/driver/target-manager.js';
import {createMockCdpSession} from '../mock-driver.js';
Expand Down Expand Up @@ -77,8 +77,8 @@ describe('TargetManager', () => {
.mockResponse('Runtime.runIfWaitingForDebugger');
await targetManager.enable();

expect(sessionMock.connection().on).toHaveBeenCalled();
const sessionListener = sessionMock.connection().on.mock.calls[0][1];
expect(sessionMock.on).toHaveBeenCalled();
const sessionListener = sessionMock.on.mock.calls[3][1];

// Original, attach.
expect(sendMock.findAllInvocations('Target.getTargetInfo')).toHaveLength(1);
Expand Down Expand Up @@ -230,7 +230,6 @@ describe('TargetManager', () => {
await targetManager.disable();

expect(sessionMock.off).toHaveBeenCalled();
expect(sessionMock.connection().off).toHaveBeenCalled();
});
});

Expand All @@ -248,7 +247,7 @@ describe('TargetManager', () => {
const mockCdpConnection = new MockCdpConnection();
/** @type {LH.Puppeteer.CDPSession} */
// @ts-expect-error - close enough to the real thing.
const cdpSession = new CDPSession(mockCdpConnection, '', sessionId);
const cdpSession = new CDPSessionImpl(mockCdpConnection, '', sessionId);
return cdpSession;
}

Expand Down Expand Up @@ -277,9 +276,7 @@ describe('TargetManager', () => {
.mockResponse('Target.setAutoAttach')
.mockResponse('Runtime.runIfWaitingForDebugger');

const rootConnection = rootSession.connection();
if (!rootConnection) throw new Error('no connection');
rootConnection.emit('sessionattached', iframeSession);
rootSession.emit('sessionattached', iframeSession);

// Wait for iframe session to be attached.
await new Promise(resolve => setTimeout(resolve, 0));
Expand Down
4 changes: 2 additions & 2 deletions core/test/gather/session-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import {EventEmitter} from 'events';

import {CDPSession} from 'puppeteer/lib/cjs/puppeteer/common/Connection.js';
import {CDPSessionImpl} from 'puppeteer-core/lib/cjs/puppeteer/common/Connection.js';

import {ProtocolSession} from '../../gather/session.js';
import {
Expand All @@ -29,7 +29,7 @@ describe('ProtocolSession', () => {

beforeEach(() => {
// @ts-expect-error - Individual mock functions are applied as necessary.
puppeteerSession = new CDPSession({_rawSend: fnAny(), send: fnAny()}, '', 'root');
puppeteerSession = new CDPSessionImpl({_rawSend: fnAny(), send: fnAny()}, '', 'root');
session = new ProtocolSession(puppeteerSession);
});

Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
"pako": "^2.0.3",
"preact": "^10.7.2",
"pretty-json-stringify": "^0.0.2",
"puppeteer": "^16.1.0",
"puppeteer": "^18.0.5",
"resolve": "^1.20.0",
"rollup": "^2.52.7",
"rollup-plugin-node-resolve": "^5.2.0",
Expand Down Expand Up @@ -202,7 +202,7 @@
"open": "^8.4.0",
"parse-cache-control": "1.0.1",
"ps-list": "^8.0.0",
"puppeteer-core": "^16.1.0",
"puppeteer-core": "^18.0.5",
"robots-parser": "^3.0.0",
"semver": "^5.3.0",
"speedline-core": "^1.4.3",
Expand Down
3 changes: 2 additions & 1 deletion viewer/test/viewer-test-pptr.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ describe('Lighthouse Viewer', () => {
});

it('should support swapping locales', async () => {
function queryLocaleState() {
async function queryLocaleState() {
await viewerPage.waitForSelector('.lh-locale-selector');
return viewerPage.$$eval('.lh-locale-selector', (elems) => {
const selectEl = elems[0];
const optionEls = [...selectEl.querySelectorAll('option')];
Expand Down
33 changes: 12 additions & 21 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2923,7 +2923,7 @@ delayed-stream@~1.0.0:
resolved "https://registry.yarnpkg.com/delayed-stream/-/delayed-stream-1.0.0.tgz#df3ae199acadfb7d440aaae0b29e2272b24ec619"
integrity sha1-3zrhmayt+31ECqrgsp4icrJOxhk=

devtools-protocol@0.0.1019158, devtools-protocol@0.0.1034970:
devtools-protocol@0.0.1034970, devtools-protocol@0.0.1036444:
version "0.0.1034970"
resolved "https://registry.yarnpkg.com/devtools-protocol/-/devtools-protocol-0.0.1034970.tgz#edbbdfee461def49bff61fa8780138ce4a1e105f"
integrity sha512-kC7Wo+7z+Bo202DVB7qVqccreL+RpcGk/6eCrpM1qj2azag6UCMg05GL3ty2adg8CXWFpUGdMeyFJfIN8lQtgw==
Expand Down Expand Up @@ -3643,7 +3643,7 @@ find-up@^2.0.0, find-up@^2.1.0:
dependencies:
locate-path "^2.0.0"

find-up@^4.0.0, find-up@^4.1.0:
find-up@^4.1.0:
version "4.1.0"
resolved "https://registry.yarnpkg.com/find-up/-/find-up-4.1.0.tgz#97afe7d6cdc0bc5928584b7c8d7b16e8a9aa5d19"
integrity sha512-PpOwAdQ/YlXQ2vj8a3h8IipDuYRi3wceVQQGYWxNINccq40Anw7BlsEXCMbt1Zt+OLA6Fq9suIpIWD0OsnISlw==
Expand Down Expand Up @@ -5856,13 +5856,6 @@ pirates@^4.0.4:
resolved "https://registry.yarnpkg.com/pirates/-/pirates-4.0.5.tgz#feec352ea5c3268fb23a37c702ab1699f35a5f3b"
integrity sha512-8V9+HQPupnaXMA23c5hvl69zXvTwTzyAYasnkb0Tts4XvO4CliqONMOnvlq26rkhLC3nWDFBJf73LU1e1VZLaQ==

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==
dependencies:
find-up "^4.0.0"

pkg-dir@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-2.0.0.tgz#f6d5d1109e19d63edf428e0bd57e12777615334b"
Expand Down Expand Up @@ -5986,35 +5979,33 @@ punycode@^2.1.0, punycode@^2.1.1:
resolved "https://registry.yarnpkg.com/punycode/-/punycode-2.1.1.tgz#b58b010ac40c22c5657616c8d2c2c02c7bf479ec"
integrity sha512-XRsRjdf+j5ml+y/6GKHPZbrF/8p2Yga0JPtdqTIY2Xe5ohJPD9saDJJLPvp9+NSBprVvevdXZybnj2cv8OEd0A==

puppeteer-core@^16.1.0:
version "16.1.0"
resolved "https://registry.yarnpkg.com/puppeteer-core/-/puppeteer-core-16.1.0.tgz#0485312363e6e1d65889d4b31de677bd36f872e4"
integrity sha512-Eu9FCqdWU2PU/RY53sa+JTsbFiQg5fJyaHX5DP0WZ4+lVLVdMfR9dwPimRkSl9NEcArm7lZMpiDlVCYelE90ZA==
puppeteer-core@^18.0.5:
version "18.0.5"
resolved "https://registry.yarnpkg.com/puppeteer-core/-/puppeteer-core-18.0.5.tgz#c37e0f52e92f1c91bf56cebc5a718f00698a5566"
integrity sha512-JM83vUAmWdzZfUEDTGlkSdGFq0/TiVbFjQrb6BJVeLJAWVYlkO4YvyFAaTWdCM5Kz+ovg4g7xOSAU4FRh4Cv/w==
dependencies:
cross-fetch "3.1.5"
debug "4.3.4"
devtools-protocol "0.0.1019158"
devtools-protocol "0.0.1036444"
extract-zip "2.0.1"
https-proxy-agent "5.0.1"
pkg-dir "4.2.0"
progress "2.0.3"
proxy-from-env "1.1.0"
rimraf "3.0.2"
tar-fs "2.1.1"
unbzip2-stream "1.4.3"
ws "8.8.1"

puppeteer@^16.1.0:
version "16.1.0"
resolved "https://registry.yarnpkg.com/puppeteer/-/puppeteer-16.1.0.tgz#06a32dc347c94642601017fbf83e1d37379b9651"
integrity sha512-lhykJLbH2bbBaP3NfYI2Vj0T4ctrdfVdEVf8glZITPnLfqrJ0nfUzAYuIz5YcA79k5lmFKANIhEXex+jQChU3g==
puppeteer@^18.0.5:
version "18.0.5"
resolved "https://registry.yarnpkg.com/puppeteer/-/puppeteer-18.0.5.tgz#873223b17b92345182c5b5e8cfbd6f3117f1547d"
integrity sha512-s4erjxU0VtKojPvF+KvLKG6OHUPw7gO2YV1dtOsoryyCbhrs444fXb4QZqGWuTv3V/rgSCUzeixxu34g0ZkSMA==
dependencies:
cross-fetch "3.1.5"
debug "4.3.4"
devtools-protocol "0.0.1019158"
devtools-protocol "0.0.1036444"
extract-zip "2.0.1"
https-proxy-agent "5.0.1"
pkg-dir "4.2.0"
progress "2.0.3"
proxy-from-env "1.1.0"
rimraf "3.0.2"
Expand Down

0 comments on commit bf8df46

Please sign in to comment.