Skip to content

Commit e77da6b

Browse files
szuendDevtools-frontend LUCI CQ
authored andcommitted
[protocol] Nuke ParallelConnection/proxyConnection
LH and recorder player both use the new CDPConnection interface so we can safely remove the parallel connection now. Bug: 453469270 Change-Id: I5d42635f64e9c51af6a990f774a61bdc50119b7a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7106262 Commit-Queue: Simon Zünd <[email protected]> Reviewed-by: Alex Rudenko <[email protected]>
1 parent 2e6530e commit e77da6b

File tree

5 files changed

+7
-315
lines changed

5 files changed

+7
-315
lines changed

front_end/core/protocol_client/InspectorBackend.ts

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,6 @@ export class SessionRouter implements CDPConnection {
201201
readonly #pendingLongPollingMessageIds = new Set<number>();
202202
readonly #sessions = new Map<string, {
203203
target: TargetBase,
204-
proxyConnection: ConnectionTransport|undefined|null,
205204
}>();
206205
#pendingScripts: Array<() => void> = [];
207206
readonly #callbacks = new Map<number, CallbackWithDebugInfo>();
@@ -232,19 +231,8 @@ export class SessionRouter implements CDPConnection {
232231
this.#observers.delete(observer);
233232
}
234233

235-
registerSession(target: TargetBase, sessionId: string, proxyConnection?: ConnectionTransport|null): void {
236-
// Only the Audits panel uses proxy connections. If it is ever possible to have multiple active at the
237-
// same time, it should be tested thoroughly.
238-
if (proxyConnection) {
239-
for (const session of this.#sessions.values()) {
240-
if (session.proxyConnection) {
241-
console.error('Multiple simultaneous proxy connections are currently unsupported');
242-
break;
243-
}
244-
}
245-
}
246-
247-
this.#sessions.set(sessionId, {target, proxyConnection});
234+
registerSession(target: TargetBase, sessionId: string): void {
235+
this.#sessions.set(sessionId, {target});
248236
}
249237

250238
unregisterSession(sessionId: string): void {
@@ -333,29 +321,9 @@ export class SessionRouter implements CDPConnection {
333321

334322
const messageObject = ((typeof message === 'string') ? JSON.parse(message) : message) as CDPReceivableMessage;
335323

336-
// Send all messages to proxy connections.
337-
for (const session of this.#sessions.values()) {
338-
if (!session.proxyConnection) {
339-
continue;
340-
}
341-
342-
if (!session.proxyConnection.onMessage) {
343-
InspectorBackend.reportProtocolError(
344-
'Protocol Error: the session has a proxyConnection with no _onMessage', messageObject);
345-
continue;
346-
}
347-
348-
session.proxyConnection.onMessage(messageObject);
349-
}
350-
351324
const sessionId = messageObject.sessionId || '';
352325
const session = this.#sessions.get(sessionId);
353326

354-
// If this message is directly for the target controlled by the proxy connection, don't handle it.
355-
if (session?.proxyConnection) {
356-
return;
357-
}
358-
359327
if (session?.target.getNeedsNodeJSPatching()) {
360328
NodeURL.patch(messageObject);
361329
}

front_end/core/sdk/ChildTargetManager.ts

Lines changed: 5 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ import type * as ProtocolProxyApi from '../../generated/protocol-proxy-api.js';
77
import type * as Protocol from '../../generated/protocol.js';
88
import * as Common from '../common/common.js';
99
import * as Host from '../host/host.js';
10-
import type * as ProtocolClient from '../protocol_client/protocol_client.js';
1110

12-
import {ParallelConnection} from './Connections.js';
1311
import {PrimaryPageChangeType, ResourceTreeModel} from './ResourceTreeModel.js';
1412
import {SDKModel} from './SDKModel.js';
1513
import {SecurityOriginManager} from './SecurityOriginManager.js';
@@ -36,7 +34,6 @@ export class ChildTargetManager extends SDKModel<EventTypes> implements Protocol
3634
readonly #targetInfos = new Map<Protocol.Target.TargetID, Protocol.Target.TargetInfo>();
3735
readonly #childTargetsBySessionId = new Map<Protocol.Target.SessionID, Target>();
3836
readonly #childTargetsById = new Map<Protocol.Target.TargetID|'main', Target>();
39-
readonly #parallelConnections = new Map<string, ProtocolClient.ConnectionTransport.ConnectionTransport>();
4037
#parentTargetId: Protocol.Target.TargetID|null = null;
4138

4239
constructor(parentTarget: Target) {
@@ -244,51 +241,18 @@ export class ChildTargetManager extends SDKModel<EventTypes> implements Protocol
244241
}
245242

246243
detachedFromTarget({sessionId}: Protocol.Target.DetachedFromTargetEvent): void {
247-
if (this.#parallelConnections.has(sessionId)) {
248-
this.#parallelConnections.delete(sessionId);
249-
} else {
250-
const target = this.#childTargetsBySessionId.get(sessionId);
251-
if (target) {
252-
target.dispose('target terminated');
253-
this.#childTargetsBySessionId.delete(sessionId);
254-
this.#childTargetsById.delete(target.id());
255-
}
244+
const target = this.#childTargetsBySessionId.get(sessionId);
245+
if (target) {
246+
target.dispose('target terminated');
247+
this.#childTargetsBySessionId.delete(sessionId);
248+
this.#childTargetsById.delete(target.id());
256249
}
257250
}
258251

259252
receivedMessageFromTarget({}: Protocol.Target.ReceivedMessageFromTargetEvent): void {
260253
// We use flatten protocol.
261254
}
262255

263-
async createParallelConnection(onMessage: (arg0: Object|string) => void):
264-
Promise<{connection: ProtocolClient.ConnectionTransport.ConnectionTransport, sessionId: string}> {
265-
// The main Target id is actually just `main`, instead of the real targetId.
266-
// Get the real id (requires an async operation) so that it can be used synchronously later.
267-
const targetId = await this.getParentTargetId();
268-
const {connection, sessionId} =
269-
await this.createParallelConnectionAndSessionForTarget(this.#parentTarget, targetId);
270-
connection.setOnMessage(onMessage);
271-
this.#parallelConnections.set(sessionId, connection);
272-
return {connection, sessionId};
273-
}
274-
275-
private async createParallelConnectionAndSessionForTarget(target: Target, targetId: Protocol.Target.TargetID):
276-
Promise<{
277-
connection: ProtocolClient.ConnectionTransport.ConnectionTransport,
278-
sessionId: string,
279-
}> {
280-
const targetAgent = target.targetAgent();
281-
const targetRouter = (target.router() as ProtocolClient.InspectorBackend.SessionRouter);
282-
const sessionId = (await targetAgent.invoke_attachToTarget({targetId, flatten: true})).sessionId;
283-
const connection = new ParallelConnection(targetRouter.connection(), sessionId);
284-
targetRouter.registerSession(target, sessionId, connection);
285-
connection.setOnDisconnect(() => {
286-
targetRouter.unregisterSession(sessionId);
287-
void targetAgent.invoke_detachFromTarget({sessionId});
288-
});
289-
return {connection, sessionId};
290-
}
291-
292256
targetInfos(): Protocol.Target.TargetInfo[] {
293257
return Array.from(this.#targetInfos.values());
294258
}

front_end/core/sdk/Connections.ts

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -213,55 +213,6 @@ export class StubConnection implements ProtocolClient.ConnectionTransport.Connec
213213
}
214214
}
215215

216-
export interface ParallelConnectionInterface extends ProtocolClient.ConnectionTransport.ConnectionTransport {
217-
getSessionId: () => string;
218-
getOnDisconnect: () => ((arg0: string) => void) | null;
219-
}
220-
221-
export class ParallelConnection implements ParallelConnectionInterface {
222-
readonly #connection: ProtocolClient.ConnectionTransport.ConnectionTransport;
223-
#sessionId: string;
224-
onMessage: ((arg0: Object) => void)|null = null;
225-
#onDisconnect: ((arg0: string) => void)|null = null;
226-
constructor(connection: ProtocolClient.ConnectionTransport.ConnectionTransport, sessionId: string) {
227-
this.#connection = connection;
228-
this.#sessionId = sessionId;
229-
}
230-
231-
setOnMessage(onMessage: (arg0: Object) => void): void {
232-
this.onMessage = onMessage;
233-
}
234-
235-
setOnDisconnect(onDisconnect: (arg0: string) => void): void {
236-
this.#onDisconnect = onDisconnect;
237-
}
238-
239-
getOnDisconnect(): ((arg0: string) => void)|null {
240-
return this.#onDisconnect;
241-
}
242-
243-
sendRawMessage(message: string): void {
244-
const messageObject = JSON.parse(message);
245-
// If the message isn't for a specific session, it must be for the root session.
246-
if (!messageObject.sessionId) {
247-
messageObject.sessionId = this.#sessionId;
248-
}
249-
this.#connection.sendRawMessage(JSON.stringify(messageObject));
250-
}
251-
252-
getSessionId(): string {
253-
return this.#sessionId;
254-
}
255-
256-
async disconnect(): Promise<void> {
257-
if (this.#onDisconnect) {
258-
this.#onDisconnect.call(null, 'force disconnect');
259-
}
260-
this.#onDisconnect = null;
261-
this.onMessage = null;
262-
}
263-
}
264-
265216
export async function initMainConnection(
266217
createRootTarget: () => Promise<void>,
267218
onConnectionLost: (message: Platform.UIString.LocalizedString) => void): Promise<void> {

front_end/services/puppeteer/PuppeteerConnection.ts

Lines changed: 0 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -3,65 +3,10 @@
33
// found in the LICENSE file.
44

55
import type * as ProtocolClient from '../../core/protocol_client/protocol_client.js';
6-
import type * as SDK from '../../core/sdk/sdk.js';
76
import type {ProtocolMapping} from '../../generated/protocol-mapping.js';
87
import type * as Protocol from '../../generated/protocol.js';
98
import * as puppeteer from '../../third_party/puppeteer/puppeteer.js';
109

11-
class Transport implements puppeteer.ConnectionTransport {
12-
#connection: SDK.Connections.ParallelConnectionInterface;
13-
14-
constructor(connection: SDK.Connections.ParallelConnectionInterface) {
15-
this.#connection = connection;
16-
}
17-
18-
send(data: string): void {
19-
this.#connection.sendRawMessage(data);
20-
}
21-
22-
close(): void {
23-
void this.#connection.disconnect();
24-
}
25-
26-
set onmessage(cb: (message: string) => void) {
27-
this.#connection.setOnMessage((message: unknown) => {
28-
const data = (message) as {id: number, method: string, params: unknown, sessionId?: string};
29-
if (!data.sessionId) {
30-
return;
31-
}
32-
33-
return cb(JSON.stringify({
34-
...data,
35-
// Puppeteer is expecting to use the default session, but we give it a non-default session in #connection.
36-
// Replace that sessionId with undefined so Puppeteer treats it as default.
37-
sessionId: data.sessionId === this.#connection.getSessionId() ? undefined : data.sessionId,
38-
}));
39-
});
40-
}
41-
42-
set onclose(cb: () => void) {
43-
const prev = this.#connection.getOnDisconnect();
44-
this.#connection.setOnDisconnect(reason => {
45-
if (prev) {
46-
prev(reason);
47-
}
48-
if (cb) {
49-
cb();
50-
}
51-
});
52-
}
53-
}
54-
55-
class PuppeteerConnection extends puppeteer.Connection {
56-
override async onMessage(message: string): Promise<void> {
57-
const msgObj = JSON.parse(message) as {id: number, method: string, params: unknown, sessionId?: string};
58-
if (msgObj.sessionId && !this._sessions.has(msgObj.sessionId)) {
59-
return;
60-
}
61-
void super.onMessage(message);
62-
}
63-
}
64-
6510
/**
6611
* This class serves as a puppeteer.Connection while sending/receiving CDP messages
6712
* over DevTools' own SessionRouter.
@@ -124,49 +69,6 @@ class PuppeteerConnectionAdapter extends puppeteer.Connection implements
12469
}
12570

12671
export class PuppeteerConnectionHelper {
127-
/** @deprecated Use {@link connectPuppeteerToConnectionViaTab} instead. */
128-
static async connectPuppeteerToConnectionViaTabLegacy(options: {
129-
connection: SDK.Connections.ParallelConnectionInterface,
130-
rootTargetId: string,
131-
isPageTargetCallback: (targetInfo: Protocol.Target.TargetInfo) => boolean,
132-
}): Promise<{
133-
page: puppeteer.Page | null,
134-
browser: puppeteer.Browser,
135-
puppeteerConnection: puppeteer.Connection,
136-
}> {
137-
const {connection, rootTargetId, isPageTargetCallback} = options;
138-
// Pass an empty message handler because it will be overwritten by puppeteer anyways.
139-
const transport = new Transport(connection);
140-
141-
// url is an empty string in this case parallel to:
142-
// https://github.com/puppeteer/puppeteer/blob/f63a123ecef86693e6457b07437a96f108f3e3c5/src/common/BrowserConnector.ts#L72
143-
const puppeteerConnection = new PuppeteerConnection('', transport);
144-
145-
const browserPromise = puppeteer.Browser._create(
146-
puppeteerConnection,
147-
[] /* contextIds */,
148-
false /* ignoreHTTPSErrors */,
149-
undefined /* defaultViewport */,
150-
undefined /* DownloadBehavior */,
151-
undefined /* process */,
152-
undefined /* closeCallback */,
153-
undefined /* targetFilterCallback */,
154-
target => isPageTargetCallback((target as puppeteer.Target)._getTargetInfo()),
155-
false /* waitForInitiallyDiscoveredTargets */,
156-
);
157-
158-
const [, browser] = await Promise.all([
159-
puppeteerConnection._createSession({targetId: rootTargetId}, /* emulateAutoAttach= */ true),
160-
browserPromise,
161-
]);
162-
163-
await browser.waitForTarget(t => t.type() === 'page');
164-
165-
const pages = await browser.pages();
166-
167-
return {page: pages[0] as puppeteer.Page, browser, puppeteerConnection};
168-
}
169-
17072
static async connectPuppeteerToConnectionViaTab(options: {
17173
connection: ProtocolClient.CDPConnection.CDPConnection,
17274
targetId: Protocol.Target.TargetID,

0 commit comments

Comments
 (0)