[protocol] Extract DevToolsCDPConnection class from SessionRouter [devtools/devtools-frontend : main]
Simon Zünd (Gerrit)
Simon Zünd added 1 comment![]( "Open in Gerrit")
onEvent<T extends Event>(event: CDPEvent<T>): void {
const sessionId = event.sessionId || '';
const session = this.#sessions.get(sessionId);
session?.target.dispatch(event as unknown as EventMessage);
}Technically we could get rid of `SessionRouter` alltogether in a follow-up:
The target base class subscribes to `onEvent` and then only dispatches where the session ID matches, i.e.
```
onEvent<T extends Event>(event: CDPEvent<T>): void {
if (event.sessionId === this.sessionId) this.dispatch(event)
}
```
The downside of that is that instead of getting exactly one session and dispatching on that, we call N `onEvent` observers, where N is the number of active sessions.
Not sure if that is a concern, but CDP is rather talkative. Wdyt?
Related details
- Alex Rudenko
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Alex Rudenko added 2 comments![]( "Open in Gerrit")
window.setTimeout(() => {can we use globalThis or nothing at all?
onEvent<T extends Event>(event: CDPEvent<T>): void {
const sessionId = event.sessionId || '';
const session = this.#sessions.get(sessionId);
session?.target.dispatch(event as unknown as EventMessage);
}Technically we could get rid of `SessionRouter` alltogether in a follow-up:
The target base class subscribes to `onEvent` and then only dispatches where the session ID matches, i.e.
```
onEvent<T extends Event>(event: CDPEvent<T>): void {
if (event.sessionId === this.sessionId) this.dispatch(event)
}```
The downside of that is that instead of getting exactly one session and dispatching on that, we call N `onEvent` observers, where N is the number of active sessions.
Not sure if that is a concern, but CDP is rather talkative. Wdyt?
- Simon Zünd
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd added 2 comments![]( "Open in Gerrit")
can we use globalThis or nothing at all?
Done
onEvent<T extends Event>(event: CDPEvent<T>): void {
const sessionId = event.sessionId || '';
const session = this.#sessions.get(sessionId);
session?.target.dispatch(event as unknown as EventMessage);
}Alex RudenkoTechnically we could get rid of `SessionRouter` alltogether in a follow-up:
The target base class subscribes to `onEvent` and then only dispatches where the session ID matches, i.e.
```
onEvent<T extends Event>(event: CDPEvent<T>): void {
if (event.sessionId === this.sessionId) this.dispatch(event)
}```
The downside of that is that instead of getting exactly one session and dispatching on that, we call N `onEvent` observers, where N is the number of active sessions.
Not sure if that is a concern, but CDP is rather talkative. Wdyt?
I'd say let's keep SessionRouter
- Alex Rudenko
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Alex Rudenko voted Code-Review+1![]( "Open in Gerrit")
- Simon Zünd
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd voted Commit-Queue+2![]( "Open in Gerrit")
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Devtools-frontend LUCI CQ (Gerrit)
Devtools-frontend LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[protocol] Extract DevToolsCDPConnection class from SessionRouter
This CL is mostly a mechanical change that splits the existing
"SessionRouter" into a "connection part" and a "session management"
part.
R=alexr...@chromium.org
- M config/gni/devtools_grd_files.gni
- M front_end/core/protocol_client/BUILD.gn
- A front_end/core/protocol_client/DevToolsCDPConnection.ts
- M front_end/core/protocol_client/InspectorBackend.ts
- M front_end/core/protocol_client/protocol_client.ts
- M front_end/entrypoints/lighthouse_worker/LighthouseWorkerService.ts
- M front_end/panels/lighthouse/LighthouseProtocolService.ts
- M front_end/panels/recorder/models/RecordingPlayer.ts
- M test/e2e_non_hosted/puppeteer/puppeteer_test.ts
Code-Review: +1 by Alex Rudenko
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |