1860955 - Serialize entire browsing context tree for "browsingContext.contextDestroyed" events

Status: RESOLVED FIXED

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

Description

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

As discussed on https://github.com/w3c/webdriver-bidi/pull/578 and which will soon be merged we should send the full browsing context tree with the currently closed browsing context as parent within the payload of the browsingContext.contextDestroyed event.

Comment 1

[Liam DeBeasi]

Hey there,

I had a few questions on the test changes:

1. My understanding is I need to change the children parameter passed to assert_browsing_context to reflect the number of children that should exist on the context now that we pass the entire tree. Is this correct?
2. test_iframe_destroy_parent is the only test in context_destroyed.py that has any children in its tree (from what I can tell). Is this test sufficient in ensuring that children are passed, or would you like me to add a non-iframe test too?
3. Why would test_delete_nested_iframes still pass with children=None? I would have expected I would need to update that to children=0.

Thanks!

Comment 2

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

(In reply to Liam DeBeasi from comment #1)

1. My understanding is I need to change the children parameter passed to assert_browsing_context to reflect the number of children that should exist on the context now that we pass the entire tree. Is this correct?

This argument only checks the number of direct children attached to a given browsing context but not more. That means if we want to assert each and every browsing context you will have to call this method multiple times. A good example you can find in browsing_context/create/user_context.py.

2. test_iframe_destroy_parent is the only test in context_destroyed.py that has any children in its tree (from what I can tell). Is this test sufficient in ensuring that children are passed, or would you like me to add a non-iframe test too?

The test test_new_context is handling a page without any iframes. For frames we would need tests like:

• Page with eg. direct iframes and page gets closed
• Page with nested iframes and page gets closed
• Page with nested iframes and top-level iframe gets removed
• Page with nested iframes and deepest iframe gets removed

Some of those tests already exist and would need an update with this change in behavior.

3. Why would test_delete_nested_iframes still pass with children=None? I would have expected I would need to update that to children=0.

It should not pass. Did you check the event payload what it contains? Is the nested iframe not listed after making the required change to the browsing context module in our WebDriver BiDi implementation?

Comment 3

[Liam DeBeasi]

Attachment: Bug 1860955 - [webdriver-bidi] Serialize entire browsing context tree for "contextDestroyed" events r=whimboo

Comment 4

[Liam DeBeasi]

Attachment: Bug 1860955 - [wdspec] Updating tests for serializing the full context tree for "browsingContext.contextDestroyed" events.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa304da9c184
[webdriver-bidi] Serialize entire browsing context tree for "contextDestroyed" events r=whimboo,webdriver-reviewers
https://hg.mozilla.org/integration/autoland/rev/ceff112065fd
[wdspec] Updating tests for serializing the full context tree for "browsingContext.contextDestroyed" events. r=whimboo,webdriver-reviewers

Comment 6

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

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

Comment 7

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/fa304da9c184
https://hg.mozilla.org/mozilla-central/rev/ceff112065fd

Comment 8

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

Liam, thanks a lot for your work on this bug! If you want feel free to find some other bug yourself or let me know and I can tell a good one.

Comment 9

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

Upstream PR merged by moz-wptsync-bot

Comment 10

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

Upstream PR merged by moz-wptsync-bot

Comment 11

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

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

Liam, thanks a lot for your work on this bug! If you want feel free to find some other bug yourself or let me know and I can tell a good one.

Setting a needinfo for you just in case it might have gone through unnoticed. But take your time in case you are busy at the moment. Thanks!

Comment 12

[Liam DeBeasi]

Hey there! Thanks for adding the needs info. I’ll take a look at the open Webdriver BIDI issues and grab something to work on.