1988723 - Remove support for "contexts" argument in "session.unsubscribe"

Status: RESOLVED FIXED

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

Description

[Alexandra Borovova [:Sasha]]

Support for "contexts" argument was removed from "session.unsubscribe" command in this PR. We should also remove it from our implementation.

Comment 1

[Noble Chinonso]

Thanks! I’ll take a look and start digging into it.

Comment 2

[Noble Chinonso]

Thanks! I’ll take a look and start digging into it.

Comment 3

[Noble Chinonso]

Hi, could you point me to the file that needs changes? I've looked around but couldn't locate where the contexts argument is handled in the session.unsubscribe implementation.

Comment 4

[Alexandra Borovova [:Sasha]]

Hi! Sure, I guess maybe it's not clear how to locate the WebDriver BiDi commands code. So the approach here is that the first part of the command identifies the module name, in this case session, and the second part is an actual command name, so here is unsubscribe. Modules live in https://searchfox.org/firefox-main/source/remote/webdriver-bidi/modules/. There we have root, windowglobal, and windowglobal-in-root folders. This defines where a module is loaded (parent or content process). But for the commands, the entry point is always the parent process, so you have to look at the root folder. There you can see the session module: https://searchfox.org/firefox-main/source/remote/webdriver-bidi/modules/root/session.sys.mjs. So now you can search for the unsubsribe command in this file: https://searchfox.org/firefox-main/source/remote/webdriver-bidi/modules/root/session.sys.mjs#226. And as you can see in its parameters, there is the context field: https://searchfox.org/firefox-main/source/remote/webdriver-bidi/modules/root/session.sys.mjs#227. I hope it's clearer now :)

If you have more questions, feel free to join our matrix channel: https://matrix.to/#/#webdriver:mozilla.org and just drop the question there. Then we can help you quicker :)

Comment 5

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

Hi Noble, did the information from Sasha help you to better understand the task on this bug? Please let us know if you need more details. Thanks!

Comment 6

[Alexandra Borovova [:Sasha]]

Hi Noble, do you have any questions/problems with implementing the required changes? Or do you just not have time to work on it at the moment?

Comment 7

[BugBot [:suhaib / :marco]]

Redirect a needinfo that is pending on an inactive user to the triage owner.
:whimboo, since the bug has recent activity, could you please find another way to get the information or close the bug as INCOMPLETE if it is not actionable?

For more information, please visit BugBot documentation.

Comment 8

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

Resetting assignee for now given that Noble seems to be away.

Comment 9

[Sameem [:sameembaba]]

Hey there! Can I work on this?

Comment 10

[Alexandra Borovova [:Sasha]]

Hi Sameem!
Sure, go ahead. Please let me know if you have any questions.
I'll assign you to the bug.

Comment 11

[Sameem [:sameembaba]]

Which tests should i run to make sure my code is correct after removing the context support?

Comment 12

[Alexandra Borovova [:Sasha]]

(In reply to Sameem [:sameembaba] from comment #11)

Which tests should i run to make sure my code is correct after removing the context support?

I think the best would be to run all tests in /testing/web-platform/meta/webdriver/tests/bidi just to make sure we don't try to unsubscribe from any individual contexts in some test.

Comment 13

[Sameem [:sameembaba]]

Attachment: Bug 1988723 - Remove support for 'contexts' argument in 'session.unsubscribe'. r=sasha

Comment 14

[Sameem [:sameembaba]]

I did all tests on my machine and they are passing perfectly fine. I have pushed the patch now.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/83f766378cc6
https://hg.mozilla.org/integration/autoland/rev/26e832aa20ff
Remove support for 'contexts' argument in 'session.unsubscribe'. r=Sasha

Comment 16

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/26e832aa20ff

Comment 17

[Alexandra Borovova [:Sasha]]

Thanks again, Sameem, for your contribution!
If you're interested in doing more, I can suggest, for example, this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1943359.
It's another improvement for the session module that you worked on here, so you're already familiar with the code, but it might be a little bit more interesting. Let me know if you would like to work on it and if you have any questions.

Comment 18

[Sameem [:sameembaba]]

Yeah, I would love to work on it!

Comment 19

[Alexandra Borovova [:Sasha]]

Great! I've assigned the bug to you