1713442 - MessageHandler: Support events

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Description

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

The CommandsHandler architecture should not only be able to route commands from the Parent Process to a specific context (and return the response).

It should also be able to bubble up events from specific contexts back to the parent process.

See https://phabricator.services.mozilla.com/D115700 for a prototype implementation.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

And not to forget we might want to make event handling protocol agnostic so that we might be able to also let CDP use these modules. As such any internal routing / forwarding might be based on generic event data, and only when returning specific BiDi or CDP events over the WebSocket connection, these data needs to be adapted to the used protocol.

Comment 2

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #1)

And not to forget we might want to make event handling protocol agnostic so that we might be able to also let CDP use these modules. As such any internal routing / forwarding might be based on generic event data, and only when returning specific BiDi or CDP events over the WebSocket connection, these data needs to be adapted to the used protocol.

I agree! The Modules implemented with the CommandsHandler architecture should not be linked to any protocol. We will most likely map them 1:1 to BiDi commands in the beginning. But whoever creates the topmost CommandsHandler in the parent process should be able to also perform some light transformation (command name, property names etc...).

The CommandsHandler architecture really needs to be considered as a solution to route & broadcast commands, to bubble events, but nothing protocol specific. (and nothing in the prototype is protocol-specific unless I forgot something)

For instance if we think about the Script module. BiDi will have a Script Module, which will expose an "execute" or "evaluate" command. DevTools might have a "Runtime::evaluateExpression" command (commands in DevTools are a client side layer which is able to call the server). Both might support different arguments, have slightly different expectations. The underlying module implemented using the CommandsHandler architecture should support both.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Some other topic that came up while discussing the commands handler architecture in the referenced PoC:

For WebDriver BiDi and CDP, which both allow events to be received by a connected WebSocket client, we have to subscribe to certain observers and register event listeners. While the latter are per context/object, observer notifications are global. As such I would be interested to know if there is a proposed way of handling these when BiDi / CDP has to listen to them for eg. different sessions.

Olli, would it be ok to have registrations for the same observer notification done multiple times, or should we register for each one only once and then filter incoming notifications on our own? Maybe registering multiple times is more performant given that all this code is implemented in XPCOM? Thanks!

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Not sure what kind of notification this is about. But registering multiple times should be fine. (Just remember to remove also then multiple times)

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Olli. That sounds good and we will make use of it that way.

Comment 6

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Attachment: Bug 1713442 - [remote] Support events in windowglobal MessageHandlers

Depends on D120586

Comment 7

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Attachment: Bug 1713442 - [remote] TEMPORARY: Add mochitests for message-handler events

Depends on D120640

Comment 8

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Attachment: Bug 1713442 - [remote] TEMPORARY: Add Session.subscribe and Log.entryAdded

Depends on D120641

Not supposed to be part of Bug 1713442, but this illustrates how MessageHandlers can be used to implement Session.subscribe + Log.entryAdded

Comment 9

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Attachment: Bug 1713442 - [remote] Add wdspec test for session subscribe

Depends on D120655

Comment 10

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Attachment: Bug 1713442 - [remote] log.entryAdded wdspec test attempt

Depends on D121118

Comment 11

[Julian Descottes [:jdescottes] (PTO 20 oct -> 25 oct)]

Attachment: Bug 1713442 - [remote] Rename all messagehandler test modules to follow the correct naming pattern

Depends on D120640

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/78b98f879e14
[remote] Support events in windowglobal MessageHandlers r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/c172c929fcca
[remote] Rename all messagehandler test modules to follow the correct naming pattern r=webdriver-reviewers,whimboo

Comment 14

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/78b98f879e14
https://hg.mozilla.org/mozilla-central/rev/c172c929fcca