1746216 - Implement "session.status" command

Status: RESOLVED FIXED

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

Comment 1

[Julian Descottes [:jdescottes]]

I imagine that similarly to session.new, this should not be implemented in the root/session.jsm module, because it's a static command which is not associated to any session? So it should be added around https://searchfox.org/mozilla-central/rev/dc323d0d9a3b722ca8ff0d1b2b752e5bd00dab9b/remote/webdriver-bidi/WebDriverBiDiConnection.jsm#166

Comment 2

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

Correct. But I also wonder if this could be static method on the session module so that we do not pollute the code in WebDriverBiDiConnection. We might also want to move session.new if that makes sense.

Comment 3

[Julian Descottes [:jdescottes]]

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

Correct. But I also wonder if this could be static method on the session module so that we do not pollute the code in WebDriverBiDiConnection. We might also want to move session.new if that makes sense.

I think it would make sense to implement it in WebDriverBiDi.jsm. This singleton is already responsible for keeping track of already created sessions, which is important to implement session.status.

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1746216 - [bidi] Implement "session.status" command

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1746216 - [wdspec] Add wdspec tests for session.status

Depends on D133941

Comment 6

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

As I see in https://searchfox.org/mozilla-central/source/testing/geckodriver/src/marionette.rs#193-209 we do not implement the status command for WebDriver HTTP in Marionette itself, which might actually still be a good idea so that marionette clients could benefit from. Then we could also share this implementation.

James, what do you think?

Comment 7

[James Graham [:jgraham]]

That only really makes sense if we're using --connect-existing with geckodriver, since otherwise there wouldn't be a Firefox instance to query. And even then it only makes a difference if you imagine that there are multiple geckodriver instances all trying to connect to the same firefox, so you want to query if one of the other instances already started a session.

Basically it seems like a really niche use case which is only worthwhile supporting if it's totally trivial to do so.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/712468713d83
[bidi] Implement "session.status" command r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/c34862fba9e0
[wdspec] Add wdspec tests for session.status r=webdriver-reviewers,whimboo

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/32114 for changes under testing/web-platform/tests

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/712468713d83
https://hg.mozilla.org/mozilla-central/rev/c34862fba9e0

Comment 11

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

(In reply to James Graham [:jgraham] from comment #7)

That only really makes sense if we're using --connect-existing with geckodriver, since otherwise there wouldn't be a Firefox instance to query.

My question was actually more around marionette client, and I know that we wanna keep the current behavior of geckodriver. But there is actually one thing which isn't clear to me. How should geckodriver behave when we are using --connect-existing and at least one BiDi session is already active. Given that only a single session is allowed for the classic webdriver, how could geckodriver connect and create a session? I haven't fully read your other PR for the classic session refactoring, but we would need a way to allow geckodriver to create a new session - maybe HTTP handles its own list of sessions? If yes should there also be the possibility to hook into an existing session if the session id is known? If not BiDi clients would have to re-connect once the HTTP session has been created.

Comment 12

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

This bug has been moved to milestone 2 because it was blocking another M2 bug to get fixed.

Comment 13

[James Graham [:jgraham]]

In reality the status command is not that useful most of the scenarios we care about; generally the person running tests via marionette already knows whether the browser has an existing test connection. This command only really makes sense for WebDriver-as-a-service where you don't have any existing insight into the status of the remote end.

So in general I don't think it's worth putting much effort in here. I don't have a use case for marionette clients to use the status command, and we haven't heard other demand from internal users, and using marionette directly is largely unsupported anyway, so I don't think we should prioritise changing anything there.

Even with the refactoring we are limited to a single HTTP session at a time (although there could be an existing BiDi-only session). Since marionette is only involved in HTTP sessions I think the existing setup is fine; for --connect-existing, right now, if there's already a session with the same geckodriver instance we'd return a status indicating that a session can't be created. If there's a session with another geckodriver instance, we'd return a status that creating a session might be possible, but then when it comes to actually trying to create the session should fail due to the existing session. That mismatch between the status output and the actual effect of trying to create a session is explicitly allowed by the spec.

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot