Implement "browser.createUserContext" command
Categories
(Remote Protocol :: WebDriver BiDi, enhancement, P2)
Tracking
(firefox124 fixed)
| Tracking | Status | |
|---|---|---|
| firefox124 | --- | fixed |
People
(Reporter: whimboo, Assigned: jdescottes)
References
(Blocks 1 open bug, )
Details
(Whiteboard: [webdriver:m10][wptsync upstream][webdriver:relnote])
Attachments
(4 files)
This bug will cover the implementation of the browser.createUserContext command once the BiDi spec includes this API.
|
Reporter | |
Updated•5 months ago
|
|
|
Reporter | |
Updated•5 months ago
|
|
Assignee | |
Updated•4 months ago
|
|
|
Assignee | |
Comment 1•4 months ago
|
||
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.
|
|
Assignee | |
Comment 2•4 months ago
|
||
|
|
Assignee | |
Comment 3•4 months ago
|
||
Depends on D198946
|
|
Assignee | |
Comment 4•4 months ago
|
||
Depends on D198968
|
|
Assignee | |
Comment 5•4 months ago
|
||
Depends on D198969
Pushed by jdescottes@mozilla.com: 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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/44166 for changes under testing/web-platform/tests
|
||
Comment 8•4 months ago
|
||
| bugherder | ||
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
Upstream PR merged by moz-wptsync-bot
|
||
Comment 10•3 months ago
|
||
:jdescottes is there anything here you would like to mention in the 124 release notes?
|
|
Assignee | |
Comment 11•3 months ago
|
||
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.
|
|
Reporter | |
Updated•2 months ago
|
Description
•