Closed Bug 1693004 Opened 7 months ago Closed 4 months ago

Add support for "webSocketUrl” capability to geckodriver

Categories

(Testing :: geckodriver, task, P2)

Default
task
Points:
2

Tracking

(firefox91 fixed)

RESOLVED FIXED
91 Branch
Tracking Status
firefox91 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

(Whiteboard: [bidi-m1-mvp])

Attachments

(1 file)

Bug 1692984 will implement support for the webSocketUrl capability in Marionette. For geckodriver we need a pass-through, and input validation for a boolean and being true in case it has been passed.

Note that right now we also have moz:debuggerAddress that enables the CDP specific code path by specifying the --remote-debugging-port command line argument. That needs to stay untouched given that webSocketUrl can change per new session capabilities, and should not require a restart of Firefox.

As such the CDP specific API's will always be available when BiDi is enabled.

Maybe we can ignore the --remote-debugging-port and don't have to specify it when only moz:debuggerAddress is given, and always enable both protocols.

Blocks: 1693828
No longer blocks: 1691396
Points: --- → 2
Priority: -- → P2
Blocks: 1708633

Something that I noticed just now is that for older releases of Firefox that will not support the webSocketUrl capability, Marionette would just pass through the capability and as such return it as true. As such when we add support for this capability to geckodriver we have to make sure to filter it out if the return value isn't a string type. The Bidi spec isn't clear here what should be returned, and maybe a pass-through should also be avoided?

Also I have trouble to find the right spot to inject the handling when webSocketUrl is false. In such a case it needs to be converted to null (Option::None). I assume it needs to be done as early as possible. So maybe at the end of SpecNewSessionParameters::validate()?

James, mind giving an advice to the above two topics? Thanks!

Flags: needinfo?(james)

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

Also I have trouble to find the right spot to inject the handling when webSocketUrl is false. In such a case it needs to be converted to null (Option::None). I assume it needs to be done as early as possible. So maybe at the end of SpecNewSessionParameters::validate()?

I think that the suggested location works fine. And it has to be done after the for loop given that capabilities is immutable while iterating through all the items. As such consider this topic as resolved.

See Also: → 1713775

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

Something that I noticed just now is that for older releases of Firefox that will not support the webSocketUrl capability, Marionette would just pass through the capability and as such return it as true. As such when we add support for this capability to geckodriver we have to make sure to filter it out if the return value isn't a string type. The Bidi spec isn't clear here what should be returned, and maybe a pass-through should also be avoided?

As discussed with James earlier today we always want to know as early as possible if a required capability is supported or not, and as such might even not have to start Firefox. But in case of webSocketUrl it's more difficult because:

  1. Older Firefox releases don't support BiDi
  2. Upcoming beta releases won't have Bidi enabled by default as long as BiDi isn't stable enough

Especially because of 2) there is no way to detect BiDi support before launching Firefox. As such it would be good to start simple and rely on the webSocketUrl capability as passed back. If it's true, which happens for older releases, geckodriver would have to immediately delete the new session again and return an error.

For Firefox 90 we want to get bug 1713775 fixed so that Marionette doesn't pass-through unknown capabilities. As such already Marionette would raise a session not created error, and geckodriver won't have to destroy the session.

Ultimately Marionette would have to do the merge of alwaysMatch and firstMatch capabilities, and do the validation checks. But that we want to defer until session.new() gets implemented in bug 1713784.

The "webSocketUrl" capability offers an opt-in
mechanism for WebDriver HTTP implementations to
make use of WebDriver BiDi, the bi-directional
protocol based on a WebSocket connection.

If the used version of Firefox has support for
WebDriver BiDi enabled, and the capability is
set to "true", it will be returned as part of
the "New Session" capabilities and contains
the host and port of the WebSocket server.

Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Attachment #9224898 - Attachment description: WIP: Bug 1693004 - [geckodriver] Add support for the "webSocketUrl" capability. → Bug 1693004 - [geckodriver] Add support for the "webSocketUrl" capability.
Flags: needinfo?(james)
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c3b8e4927e2
[geckodriver] Add support for the "webSocketUrl" capability. r=webdriver-reviewers,jgraham
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch
Blocks: 1686110
You need to log in before you can comment on or make changes to this bug.