Make context/navigable id per session. #565
Conversation
Also make it clear that context ids actually relate to navigables, not browsing contexts. This part is a temporary workaround while the rest of the spec continues to pass in browsing contexts rather than navigables.
Co-authored-by: Thiago Perrotta <[email protected]>
|
Is there an issue/discussion for this PR? I wonder if it's worth making context IDs to be session-specific: I see the value of being able to get a context id in one session and start a different session for a different task (e.g., for tracing or network monitoring) on that specific context id. |
|
My assumption has been that as far as possible sessions should be isolated from each other. If you have multiple tools sharing data, they can probably also share a session id and just operate entirely within the same session. This also matches what we already do for node ids. |
|
I think it was brought up by someone at TPAC that there could be cases where you would want to use a dedicated tool (for example, a network capturing tool) that would create a session to do a specific task in the running instance. At the same it might not be possible to use an existing session because the event subscriptions might not be compatible between tools: e.g., the tool or the main program might not handle the events in the same way. |
|
The Browser Testing and Tools Working Group just discussed The full IRC log of that discussion<AutomatedTester> Topic: Make context/navigable id per session<AutomatedTester> github: https://github.com//pull/565 <orkon> ScribeNick: orkon <jgraham_> (I will note that the question later in the agenda of "Should we make setFiles part of performActions instead?" is exactly why I don't like that we have these one-off commands that enable sequencing multiple things) <jrandolf> jgraham_: +1 <orkon> whimboo: describes the issue about if ids are shared between the sessions, there are pros and cons for this approach and we can start a discussion if ids should be the same <jgraham_> q+ <orkon> q? <jgraham_> ack next <orkon> ack next <orkon> jgraham_ (IRC): I wrote the PR to make context ids isolated so that different sessions don't know about each and if you want to share the data between two clients you can use the same session <orkon> q+ <jgraham_> ack next <jgraham_> ScribeNick: jgraham_ <orkon> we lost meeting? <jrandolf> Uh <whimboo> please just rejoin <AutomatedTester> sorry... <AutomatedTester> I have restarted it <shs96c> :) <jgraham_> orkon: There could be cases where it's interesting to not have isolation for context id e.g. debugging so you can tell if objects are the same or not. it might be nice to have it as an option, but I don't know if it's possible. <jgraham_> ScribeNick: orkon <jgraham_> q? <whimboo> q+ <jgraham_> ack whimboo <orkon> whimboo: if we can use the same session if the client needs the same id. If we do not need it, then a new session can be created <jgraham_> q+ <orkon> ack next <orkon> jgraham_ (IRC): is it easier to debug if the browser state is a global shared state, not known about other sessions and perhaps we should have revises the decision about the node ids? <orkon> jgraham_ (IRC): on the other hand, I worry if there will be a security risk here. But perhaps a session should not be a security boundary. <orkon> q+ <jgraham_> ScribeNick: jgraham_ <jgraham_> ack orkon <jgraham_> orkon: Isolating node ids seems OK, but I'm not sure what the difference is between contexts and elements. Maybe it's because events are bound to context ids. I think there would still be ways to identify the same tab / frame. No strong opinion. <jgraham_> ScribeNick: orkon <orkon> q? |
| string uniquely identifying that browsing context within a specific [=bidi | ||
| session=]. | ||
|
|
||
| A [=BiDi session=] has a <dfn>context id map</dfn> which is a weak map between |
There was a problem hiding this comment.
We actually need the same for WebDriver classic. Otherwise the ids are not interchangeable between both protocols. And I think that both protocols should use the same underlying weak map. So probably this is more a WebDriver classic related change?
| [=error=] with [=error code=] [=no such frame=] | ||
| 1. Let |context id map| be |session|'s [=context id map=]. | ||
|
|
||
| 1. For each |navigable| → |id| of |context id map|: |
There was a problem hiding this comment.
Note that WeakMaps do not support enumeration and as such there is currently no way to actually retrieve the navigable corresponding to the context id at the moment.
| The <dfn export for=modules>browsingContext</dfn> module contains commands and | ||
| events relating to browsing contexts. | ||
| events relating to [=navigables=]. | ||
|
|
||
| Note: For historic reasons this module is called <code>browsingContext</code> | ||
| rather than <code>navigable</code>, and the protocol uses the term | ||
| <code>context</code> to refer to navigables, particularly as a field in command | ||
| and response parameters. | ||
|
|
||
| Issue: Update the spec to use [=navigable=] consistently and remove | ||
| uses of [=/browsing context=]. |
There was a problem hiding this comment.
Should we land this part? I can extract it into a PR
Also make it clear that context ids actually relate to navigables, not browsing contexts. This part is a temporary workaround while the rest of the spec continues to pass in browsing contexts rather than navigables.
Preview | Diff