1870848 - Implement "browser.createUserContext" command

Status: RESOLVED FIXED

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

Description

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

This bug will cover the implementation of the browser.createUserContext command once the BiDi spec includes this API.

Comment 1

[Julian Descottes [:jdescottes]]

I've started to implement to overall support for user contexts (ie this bug + remove user contexts, as well as supporting them when creating tabs, returning the tree etc...). I think it makes sense to look at the overall problem before fixing individual bugs otherwise we will end up having to change the approach bug after bug.

problem overview (based on the current PR)

Requirements

At the moment, consumers of user contexts will be the bidi browser module (to create and remove user contexts), and the browsingContext module (to populate browsingContextInfo).

Potentially, the existing CDP Target module could also reuse this code?

The id of the default user context should be "default" when exposed by WebDriver BiDi.

For all other user contexts, the ids might be UUIDs, might be session-specific, but the current spec PR is not 100% clear about that. It would be nice to avoid exposing platform ids for Firefox in any case. Internally we use a counter for user context ids, so they will look predictable and clients might rely on this if we don't translate them to opaque ids (eg UUIDs).

Option A - Opaque ids but not session specific: shared singleton

One option is to create a UserContextManager which would be responsible for translating between opaque id and internal user context id. If we don't have to make the ids session specific, then this can simply be a singleton living under shared/webdriver. It is webdriver-specific because the "default" id for the default user context is from the BiDi specs.

Option B - Session specific ids: helper owned by RootMessageHandler

If we have to make ids session specific, it becomes a bit more tricky.

We need to instantiate one UserContextManager per session. Such session-specific helpers shared by several modules are usually exposed on the MessageHandler (here: RootMessageHandler). But the logic about the "default" user context is webdriver specific, while the MessageHandler is not webdriver specific. We could make the API a bit more complex, eg UserContextManager could expose something such as "isDefaultUserContext/getDefaultUserContext" and then individual modules would be responsible for exchanging "default" with the UUID of the default user context. Overall this feels confusing, and if we already have a map between opaque ids and platform user context ids, it would be nice if it could also handle the "default" one.

Option C - Session specific ids with "default" id: helper owned by a BiDi module

If we want to keep the "default" id handled by the helper, we could let the browser module own this helper, and expose it via an internal command which would then be used by the browsingContext module. But since commands are async by default, it will add a bunch of async code. Maybe we should introduce an easier way for modules to call each other when we know they are owned by the same MessageHandler?

Option D - Session specific ids with "default" id: helper owned by a webdriver session

Maybe the easiest option in order to satisfy session specific ids + not having to translate the "default" id in all modules is to just have the UserContextManager owner by the shared/webdriver/Session? We can expose a getter to retrieve the UserContextManager instance on the Session class, and then modules can just use getWebDriverSessionById to get the Session object.

Overall this is another use case where we miss an easy way to share session-specific & webdriver-specific logic between modules. I think options C or D are both interesting. Option C allows to leverage the existing architecture. Option D allows for interop between bidi and classic.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1870848 - [remote] Introduce shared webdriver helper UserContextManager

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1870848 - [bidi] Implement browser.createUserContext command

Depends on D198946

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1870848 - [wdspec] Add create_user_context command to bidi browser module

Depends on D198968

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1870848 - [wdspec] Add test for browser.createUserContext command

Depends on D198969

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c3048e68b16b
[remote] Introduce shared webdriver helper UserContextManager r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/6fd9889ead6c
[bidi] Implement browser.createUserContext command r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/83099a33268b
[wdspec] Add create_user_context command to bidi browser module r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/60fe798e08d4
[wdspec] Add test for browser.createUserContext command r=webdriver-reviewers,whimboo

Comment 7

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

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

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/c3048e68b16b
https://hg.mozilla.org/mozilla-central/rev/6fd9889ead6c
https://hg.mozilla.org/mozilla-central/rev/83099a33268b
https://hg.mozilla.org/mozilla-central/rev/60fe798e08d4

Comment 9

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

Upstream PR merged by moz-wptsync-bot

Comment 10

[Dianna Smith [:diannaS]]

:jdescottes is there anything here you would like to mention in the 124 release notes?

Comment 11

[Julian Descottes [:jdescottes]]

Hi! We usually only write release notes for webdriver updates in the Developer release notes (https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/124#webdriver_conformance_webdriver_bidi_marionette) , and we will do that a bit later in the cycle.

I think it's not worth mentioning in the main release notes for Firefox.