Implement basic "script.getRealms" command that supports WindowRealmInfo
Categories
(Remote Protocol :: WebDriver BiDi, task, P1)
Tracking
(firefox106 fixed)
Tracking | Status | |
---|---|---|
firefox106 | --- | fixed |
People
(Reporter: whimboo, Assigned: Sasha)
References
(Blocks 1 open bug, )
Details
(Whiteboard: [webdriver:m4] , [wptsync upstream], [webdriver:relnote] )
Attachments
(4 files)
Reporter | ||
Updated•1 year ago
|
Reporter | ||
Comment 1•1 year ago
|
||
If time permits for M4 I would suggest that we also keep this bug at least in the P2 queue for now. Some methods work with realm info or return it so having a proper implementation would be great. Lets discuss in next week's triage meeting.
Reporter | ||
Updated•11 months ago
|
Reporter | ||
Comment 2•10 months ago
|
||
Once bug 1785092 has been fixed this bug should be quite easy to get implemented.
Assignee | ||
Updated•9 months ago
|
Assignee | ||
Updated•9 months ago
|
Assignee | ||
Updated•9 months ago
|
Assignee | ||
Updated•9 months ago
|
Assignee | ||
Comment 3•9 months ago
|
||
Assignee | ||
Comment 4•9 months ago
|
||
Depends on D156049
Assignee | ||
Comment 5•9 months ago
|
||
Depends on D156050
Updated•9 months ago
|
Updated•9 months ago
|
Updated•9 months ago
|
Assignee | ||
Comment 6•9 months ago
•
|
||
While implementing this command, I first went with the approach of forwarding the command to a child process and returning the information of the realms, cached there. For the case when browsing context is not provided, it would mean getting all the browsing contexts and forwarding commands to each one of them.
Then I had a look at bug 1779231. Here we would need to get browsing context for a specified realmId
that means that most likely we would need to have in the parent process a map between at least realmId
and contextId
, but we could of course store also all other required realm information. That could mean that when getRealms
is called, we can just return this map. The only problem is, that we would not have realms stored which were not used for script evaluation. (We also could have already destroyed realms)
The next thought would be that ideally we should listen to script.realmCreated
and script.realmDestroyed
events to build realm info map. Then it can be safely used to return it when getRealms
is called or if evaluate
or callFunction
is called with realmId
and we have to find an associated browsing context.
So it looks like it would be better to block this bug at least on bug 1788657.
Assignee | ||
Updated•9 months ago
|
Assignee | ||
Comment 7•9 months ago
|
||
After the discussion, it was discovered that building the cache might require additional changes to keep it up-to-date, so it was decided to proceed with the approach of always retrieving all the realms from all browsing contexts (if a browsing context id is not provided) and build the cache later in the scope of bug 1788894.
Reporter | ||
Comment 8•9 months ago
|
||
Sasha, I just noticed the following PR from James on GitHub which includes the sandbox name into the WindowRealm
info. Now I wonder if we should wait until it's merged? Otherwise we can also do a follow-up right after this patch series got landed. What's your current plan in handling that?
Assignee | ||
Comment 9•9 months ago
|
||
I would say since this bug blocks the target realm bug, which blocks finishing up the milestone, it would be better to not block it on the spec change and follow up, when the spec actually gets updated.
Assignee | ||
Comment 10•9 months ago
|
||
Depends on D156050
Reporter | ||
Comment 11•9 months ago
|
||
(In reply to Alexandra Borovova [:Sasha] from comment #9)
I would say since this bug blocks the target realm bug, which blocks finishing up the milestone, it would be better to not block it on the spec change and follow up, when the spec actually gets updated.
That sounds good! Thanks.
Reporter | ||
Comment 12•9 months ago
|
||
This bug is required to enable the script module by default. As such moving back into M4.
Updated•9 months ago
|
Comment 13•9 months ago
|
||
Pushed by aborovova@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ffe78a5df18b [bidi] Implement "script.getRealms" command. r=webdriver-reviewers,jdescottes,whimboo https://hg.mozilla.org/integration/autoland/rev/dd1a5096f528 [wdspec] Add support for "script.getRealms" in wdspec tests. r=webdriver-reviewers,jgraham,whimboo https://hg.mozilla.org/integration/autoland/rev/265621d051cb [wdspec] Add a special parameter to bidi commands to return the raw result. r=webdriver-reviewers,jgraham,whimboo https://hg.mozilla.org/integration/autoland/rev/c95d82c6f844 [wdspec] Add tests for "script.getRealms" command. r=webdriver-reviewers,whimboo,jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/35866 for changes under testing/web-platform/tests
Updated•9 months ago
|
Comment 15•9 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ffe78a5df18b
https://hg.mozilla.org/mozilla-central/rev/dd1a5096f528
https://hg.mozilla.org/mozilla-central/rev/265621d051cb
https://hg.mozilla.org/mozilla-central/rev/c95d82c6f844
Reporter | ||
Comment 16•9 months ago
|
||
The patches on this bug implemented the basics for this command by only supporting the type WindowRealmInfo for now.
Upstream PR merged by jgraham
Comment 18•8 months ago
|
||
Adding webdriver:relnote
Added basic support for the getRealms
command of the script
module, currently limited to the WindowRealmInfo type which includes window realms and sandbox realms.
Description
•