Add support for custom browser to client messages

[jgraham]

This introduces a script.Channel type which deserializes to a function that when called sends a script.message event with its first argument.

It also changes preload scripts to work as functions rather than just being evaluated directly. This allows arguments to be passed in particuarly for the use of channels so that preload scripts can install event listeners that message back to the client.

---

Preview | Diff

[jgraham]

There are a couple of open issues around the proposed change to make preload scripts work like callFunction rather than evaluate. This is required if we want to pass in a Channel as an argument when defining the preload scripts, but it comes with some tradeoffs:

• How to report an error when deserializing the arguments. We have to do this at the point at which the script is executed, so it can't be a response to the command. If the deserialization fails it's not really a javascript error that we can report through report an exception because it's purely about invalid data in the WebDriver protocol layer.
• Does it make sense to allow all argument types? In particular primitives and channels probably make sense, but remote references are going to be at best fragile.
• Is it OK to have a function for which you can't get the return value? I don't think we have a use case for the preload script function returning a value, but it still seems somewhat strange that it should be dropped entirely.

So I think there are a few options:

1. We could add an event which happens after a preload script, indicating either the result of the function, or an error. That would make it unambiguous that something went wrong with a specific preload script, but I don't know of other use cases for the event.
2. We could restrict preload scripts to getting primitives and channels as the arguments. In that case deserialization wouldn't depend on external factors (e.g. whether some node or window is accessible), so we could just return an "invalid argument" error to the addPreloadScript command.
3. We could model failure to deserialize an argument as some kind of JS-level error, even though it's not really.
4. We could revert to using evaluation for preload scripts, and allow channels (only) to be injected as some kind of script-scoped variable.

I think I prefer Option 2, but it does restrict the use cases somewhat. For example you can't open a page, then install a preload script, with a SharedReference to a node on the first page as an argument, and then window.open a bunch of other browsing contexts which run the reload script and access the node on the original page. But since it would only work in cases where the Documents were mutually accessible anyway, there's probably a workaround.

[whimboo]

@jgraham there is one other question that I have. In regards of the usage for Selenium I think that they also wanted to have the possibility to install such a script that then can be re-used to not have to send the specific atoms for each and every eg. element displayed check. How would this actually work with the callFunction like method? There has to be a way to trigger the script with the SharedReference as argument.

[jgraham]

Something like:

webdriver.installPreloadScript(
  () => {
    window.isDisplayed = element => {
      // Implementation
    }
  },
  sandbox="selenium-internal")

and then later

webdriver.callFunction(elem => isDisplayed(elem), arguments=[element], context=contextId, sandbox="selenium-internal")

[whimboo]

I see. So the channel would actually only be required for the pre-load script itself and in those cases when it is needed. Otherwise it can be left-out. Thanks.

[css-meeting-bot]

The Browser Testing and Tools Working Group just discussed Pre-load script - support for custom browser to client messages.

The full IRC log of that discussion

<jgraham> Topic: Pre-load script - support for custom browser to client messages
<jgraham> Github: https://github.com//pull/361
<orkon> jgraham (IRC): in December, we discussed how to implement sending custom messages from the browser to the client. We should implement by having a special type that serialised to a function that is when called sends a message. We have not considered how it interacts with preload scripts. But preload scripts don't accept arguments. We then thought we can convert scripts to functions. But there are some problems (see the issue).
<orkon> jgraham (IRC): when you have arguments, you can pass any argument and provide a handle to an object but it does not exist in the preload script, so it fails every time the preload script runs.
<orkon> jgraham (IRC): currently they are JS errors but actually are preload errors
<orkon> jgraham (IRC): we have a couple of options 1) don't care about return values 2) throw a JS exception for arguments 3) or we can only have arguments that can be serialzed statically, no shared references
<orkon> jgraham (IRC): there could be cases where shared references are needed but they are fragile
<orkon> jgraham (IRC): there could be also events for the results of the preload scripts
<jgraham> q?
<JimEvans> q+
<jgraham> ack JimEvans
<sadym> q+
<orkon> Jim Evans: I like the idea of being able to know the result of the preload result if we change them to be functions. I like an idea for the event containing a result or an error. But the clients don't need to subscribe to the event, if they don't need to.
<orkon> Jim Evans: how does the remote end needs to do to raise the event?
<orkon> jgraham (IRC): it deserialises to a function and the function emits the event
<jgraham> ack sadym
<orkon> jgraham (IRC): it doest the impl magic to make the event emitte
<orkon> s/emitte/emitted/
<jgraham> q+
<orkon> sadym (IRC): arguments and events as the result sound reasonable. We can consider rolling back to the option with allowing only channels because the use case when we want to reuse the sharedId seems artificial. But it sounds like a slippery road to provide this functionality.
<jgraham> ack jgraham
<orkon> jgraham (IRC): I think I agree with that. Realistically, passing primitive values does not make much sense. Passing sharedId only works in limited circumstances if you know that all preload scrips are in the same origin but I don't have a use case.
<orkon> jgraham (IRC): my concern with the event is: obviously running a preload script can cause an exception and we emit it as a js error and you need to listen for log events to find that out. On other hand, if we had a specific event, then you kind of have to subscribe to it. E.g., if you had a bug where a preload script does not work, you would not know.
<orkon> jgraham (IRC): perhaps we could parse the script early but not runtime exceptions
<orkon> jgraham (IRC): I think none of the tradeoffs are great actually
<jgraham> q?
<sadym> q+
<orkon> jgraham (IRC): I think I am going to try implementing changes in the PR to be the option: it is still a function, can accept only channels as args and all exceptions are js exceptions. If it is not an option, mention this now or comment on the issue.
<jgraham> ack sadym
<orkon> sadym (IRC): what other option do we have if we rollback from callFunction to evaluate where there was a global handle to raise the event.
<orkon> jgraham (IRC): I believe that does not solve any problems that channels don't solve
<orkon> jgraham (IRC): no reason to prefer globals over channels
<JimEvans> q+
<jgraham> ack JimEvans
<orkon> Jim Evans: looked at the print PR, nothing controversial.
<jgraham> RRSAgent: make minutes
<RRSAgent> I have made the request to generate https://www.w3.org/2023/02/08-webdriver-minutes.html jgraham
<jgraham> Zakim, bye
<Zakim> leaving. As of this point the attendees have been JimEvans, orkon, brwalder, whimboo, jgraham, patrickangle_, Sasha, jdescottes, sadym, simons
<jgraham> RRSAgent: bye
<RRSAgent> I see 1 open action item saved in https://www.w3.org/2023/02/08-webdriver-actions.rdf :
<RRSAgent> ACTION: whimboo to file a BiDi issue covering this scenario [1]
<RRSAgent> recorded in https://www.w3.org/2023/02/08-webdriver-irc#T17-33-21
<JimEvans> jgraham: I'm evidently quite dense, because I'm still not quite getting how the message for a preload script would get raised. Is it as simple as the function definition being something like `(channel) => channel = document.title;` for example? And that would send the title back in the script.message event?

[jgraham]

I updated this as follows:

• Only accept script.Channel as an argument to preload scripts
• Error reporting all happens with JS exceptions

One quality of life improvement we could make would be to return an error to the command if the script can't be evaluated as a function body. The question is where that evaluation happens e.g. if I pass in window.foo = 1 as the preload script, we don't want foo=1 to be set on some existing global, before noticing that the script is not in fact a function.

[sadym-chromium]

Can we pick other naming, as channel is already used in out implementation-specific extension for other purpose?

[jgraham]

The precedent is the use of the word "Channel" for the same concept basically everywhere else on the platform e.g. MessageChannel, BroadcastChannel, etc.

[jgraham]

To help with reviewing, I've split the preload script changes out into #381 and made this PR just about the addition of the message channel. This PR is based on that one, so it needs to merge first.

[jimevans]

@sadym-chromium Are you happy with the most recent changes here? I’d really like to get this merged, if there is consensus that it’s ready.