Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove channel support from mapper #2955

Open
2 tasks
OrKoN opened this issue Jan 7, 2025 · 8 comments
Open
2 tasks

Remove channel support from mapper #2955

OrKoN opened this issue Jan 7, 2025 · 8 comments

Comments

@OrKoN
Copy link
Collaborator

OrKoN commented Jan 7, 2025

Channels are used by ChromeDriver to route ChromeDriver's own messages via the mapper.

It is very likely that the same functionality could be supported by implementing a separate global binding for ChromeDriver to access the same CDP connection, thus, removing the complexity of supporting channels in the mapper.

  • explore and support a separate binding in ChromeDriver for communication between ChromeDriver and chromium-bidi.
  • remove channel support from the mapper.

Example of complexity: #2954 changes to the spec w.r.t to the event subscription require maintaining channels as well which makes further spec alignment more difficult.

@OrKoN OrKoN added the BiDi+ label Jan 7, 2025
@OrKoN
Copy link
Collaborator Author

OrKoN commented Jan 7, 2025

cc @sadym-chromium

@OrKoN OrKoN added the P3 label Jan 7, 2025
@sadym-chromium
Copy link
Collaborator

The channels are still useful for supporting multiple web connections to the same session. In this case, the command's response should be sent only to the websocket the command came from, while events should be sent to all the connections.

@OrKoN
Copy link
Collaborator Author

OrKoN commented Jan 8, 2025

The channels are still useful for supporting multiple web connections to the same session. In this case, the command's response should be sent only to the websocket the command came from, while events should be sent to all the connections.

we do not support that scenario, do we?

@sadym-chromium
Copy link
Collaborator

sadym-chromium commented Jan 8, 2025

Not yet, but we don't have any technical limitations to support it. It does not require supporting channels in the events though.

@OrKoN
Copy link
Collaborator Author

OrKoN commented Jan 8, 2025

Not yet, but we don't have any technical limitations to support it. It does not require supporting channels in the events though.

I do not think it requires channels for commands too. One just needs to map a command id to a websocket connection when deciding which websocket should receive the command.

@OrKoN
Copy link
Collaborator Author

OrKoN commented Jan 8, 2025

Not yet, but we don't have any technical limitations to support it. It does not require supporting channels in the events though.

I do not think it requires channels for commands too. One just needs to map a command id to a websocket connection when deciding which websocket should receive the command.

i.e., completely outside of the mapper as mapper does not handle the websockets.

@sadym-chromium
Copy link
Collaborator

I do not think it requires channels for commands too. One just needs to map a command id to a websocket connection when deciding which websocket should receive the command.

In order to be able to map the command response to the specific websocket, we need to mark the command with some unique attribute, we cannot rely on command ID as such an attribute according to the BiDi spec:

From the point of view of the remote end this identifier is opaque and cannot be used internally to identify the command.
Note: This is because the command id is entirely controlled by the local end and isn’t necessarily unique over the course of a session. For example a local end which ignores all responses could use the same command id for each command.

As a work-around, we can re-map command IDs to unique internal ones. But I don't think it would be any better then using an internal attribute goog:channel.

@OrKoN
Copy link
Collaborator Author

OrKoN commented Jan 8, 2025

As a work-around, we can re-map command IDs to unique internal ones. But I don't think it would be any better then using an internal attribute goog:channel.

I think it is better because it does not require synchonized changes in two systems (ChromeDriver and mapper) and remove redundant logic related from channels from the mapper implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants