1754273 - Emit "browsingContext.contextCreated" events for all open browsing contexts when subscribing to the event

Status: RESOLVED FIXED

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

Description

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

The remote end subscribe steps in the spec require us to emit created events for every open browsing context.

https://w3c.github.io/webdriver-bidi/#ref-for-remote-end-subscribe-steps%E2%91%A1

This can be worked on once the initial implementation has been landed.

Comment 1

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

Moving to P2 because this bug is needed for the implementation on bug 1801284.

Comment 2

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

As discussed on Element with bug 1723102 fixed this is more a 3 points worth bug.

Comment 4

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

Via bug 1882232 we will get a simple test.

Comment 5

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

When we implement that feature we should potentially emit these events before session.subscribe returns. See also: https://github.com/w3c/webdriver-bidi/issues/696

Comment 6

[Liam DeBeasi]

Hey there! I'm interested in working on this bug. I had a couple questions about solving this problem:

1. Do you have any recommendations for how I implement this solution? I could update the subscribe method in session.sys.mjs to grab the browsing contexts and emit the event that way.
2. For tests I'll need to remove the expected fail from test_existing_context. Would you like me to add any other test cases?

Comment 7

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

Hi Liam. First, sorry for the late reply but somehow the needinfo got under the wire. I just noticed it now.

(In reply to Liam DeBeasi from comment #6)

1. Do you have any recommendations for how I implement this solution? I could update the subscribe method in session.sys.mjs to grab the browsing contexts and emit the event that way.

The session.subscribe command handles all kinds of events, so we cannot directly add the new logic there. Instead we should probably call some additional method where the event is actually getting subscribed; this is in the root/browsingContext.sys.mjs module and in BrowsingContextModule::subscribeEvent. But I'm not 100% sure right now if that's really the best place because once we support subscribe priority (see bug 1801284) we have to call those methods in the right order.

Liam, so if you are still interested and want to work on a more complicated bug, you could certainly start on this one. But it's totally fine to switch to some other bug instead. Just let us know. Thanks!

Comment 8

[Liam DeBeasi]

Thanks for the reply! I can take a shot at this ticket. I can add the logic in the file you mentioned as well as add tests. That way, when subscribe priority is implemented we'll have test coverage to ensure this behavior does not accidentally break.

Comment 9

[Liam DeBeasi]

Attachment: Bug 1754273 - Emit browsingContext.contextCreated event for all open contexts when subscribing to the event. r=whimboo

Comment 10

[Liam DeBeasi]

Attachment: Bug 1754273 - [wdspec] Update context_created tests for new browsingContext.contextCreated event emission behavior r=whimboo

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7c415b316abe
https://hg.mozilla.org/integration/autoland/rev/89bbbe4d7bab
Emit browsingContext.contextCreated event for all open contexts when subscribing to the event. r=webdriver-reviewers,jdescottes
https://github.com/mozilla-firefox/firefox/commit/38f668d3ea44
https://hg.mozilla.org/integration/autoland/rev/9782681d7671
[wdspec] Update context_created tests for new browsingContext.contextCreated event emission behavior r=webdriver-reviewers,jdescottes

Comment 12

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

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

Comment 13

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/89bbbe4d7bab
https://hg.mozilla.org/mozilla-central/rev/9782681d7671

Comment 14

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

Upstream PR merged by moz-wptsync-bot

Comment 15

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

This was excellent work! Thanks a lot Liam for spending your valuable time on this particular BiDi event.

Comment 16

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

Hi Liam. I wanted to check back with you if you would be interested on some other bug to work on. Let us know if you have the time, or find one bug yourself. Thanks!

Comment 17

[Liam DeBeasi]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #16)

Hi Liam. I wanted to check back with you if you would be interested on some other bug to work on. Let us know if you have the time, or find one bug yourself. Thanks!

Hey there! Yes, I am interested in taking on another task. Is there a particular area that would be helpful for me to focus on?

Comment 18

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

Great to hear Liam! I wonder if you would be interested to finish off the remaining work on bug 1855028? We haven't gotten to this yet, but it would be great to finally have this API available given that Selenium folks are waiting on it. What do you think?

Comment 19

[Liam DeBeasi]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #18)

Great to hear Liam! I wonder if you would be interested to finish off the remaining work on bug 1855028? We haven't gotten to this yet, but it would be great to finally have this API available given that Selenium folks are waiting on it. What do you think?

Sounds good to me! I'll take a look and let you know if I have questions.