1766240 - Implement basic "script.getRealms" command that supports WindowRealmInfo

Status: RESOLVED FIXED

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

Comment 1

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

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.

Comment 2

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

Once bug 1785092 has been fixed this bug should be quite easy to get implemented.

Comment 3

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1766240 - [bidi] Implement "script.getRealms" command.

Comment 4

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1766240 - [wdspec] Add support for "script.getRealms" in wdspec tests.

Depends on D156049

Comment 5

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1766240 - [wdspec] Add tests for "script.getRealms" command.

Depends on D156050

Comment 6

[Alexandra Borovova [:Sasha]]

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.

Comment 7

[Alexandra Borovova [:Sasha]]

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.

Comment 8

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

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?

Comment 9

[Alexandra Borovova [:Sasha]]

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.

Comment 10

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1766240 - [wdspec] Add a special parameter to bidi commands to return the raw result.

Depends on D156050

Comment 11

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

(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.

Comment 12

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

This bug is required to enable the script module by default. As such moving back into M4.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

Comment 14

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

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

Comment 15

[Cristina Cozmuta (:CrissCozmuta)]

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

Comment 16

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

The patches on this bug implemented the basics for this command by only supporting the type WindowRealmInfo for now.

Comment 17

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

Upstream PR merged by jgraham

Comment 18

[Julian Descottes [:jdescottes]]

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.