1730470 - [meta] browsingContext.contextCreated event

Status: NEW

(Remote Protocol :: WebDriver BiDi, enhancement)

Description

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

This event is needed for clients to handle open and newly created browsing contexts.

Comment 1

[Liam DeBeasi]

Hey Henrik! I hope you are doing well. I was looking to help get the remaining contextCreated tickets wrapped and was planning to start with https://bugzilla.mozilla.org/show_bug.cgi?id=1920952.

It sounds like the work here is to utilize browser.getClientWindows to send information about the window in the new clientWindow property on the contextCreated event and add some tests. Is there anything else that would be helpful for me to know before I get started? Thanks!

Comment 2

[Alexandra Borovova [:Sasha]]

Hi Liam, sorry for the delay, Henrik is not around at the moment, but hopefully I can help you :)

Feel free to pick up the work on https://bugzilla.mozilla.org/show_bug.cgi?id=1920952.
A bit more context here: we have a shared helper which is used for multiple browsingContext commands and events (including browsingContext.contextCreated): https://searchfox.org/mozilla-central/source/remote/webdriver-bidi/modules/root/browsingContext.sys.mjs#1713-1750. So you would need to add the clientWindow field there in the return contextInfo object. You're right that you can utilize the approach which is used in browser.getClientWindows method, more specifically, you can use lazy.windowManager.getIdForWindow method to retrieve the required id (see here). Let me know if something is unclear, or you have more questions.

Comment 3

[Liam DeBeasi]

Thank you! I was able to get a fix in place. One follow up question:

I'm currently getting the window to pass to getIdForWindow using the following code:

const targetTab = lazy.TabManager.getTabForBrowsingContext(context);
const targetWindow = lazy.TabManager.getWindowForTab(targetTab);

Is there a simpler way to grab the window associated with the context?

Comment 4

[Alexandra Borovova [:Sasha]]

(In reply to Liam DeBeasi from comment #3)

Thank you! I was able to get a fix in place. One follow up question:

I'm currently getting the window to pass to getIdForWindow using the following code:

const targetTab = lazy.TabManager.getTabForBrowsingContext(context);
const targetWindow = lazy.TabManager.getWindowForTab(targetTab);

Is there a simpler way to grab the window associated with the context?

No, I don't think that we have a simpler way to do it. We do the same in the other places.

Comment 5

[Liam DeBeasi]

Thanks! I have most of the tests updated, though I have one failure that I am currently confused by. In test_new_context, I am updating the test to assert that the clientWindow in the contextCreated event matches the actual clientWindow on the top-level context:

@pytest.mark.parametrize("type_hint", ["tab", "window"])
async def test_new_context(bidi_session, wait_for_event, wait_for_future_safe, subscribe_events, type_hint):
    await subscribe_events([CONTEXT_CREATED_EVENT])

    on_entry = wait_for_event(CONTEXT_CREATED_EVENT)
    top_level_context = await bidi_session.browsing_context.create(type_hint=type_hint)
    context_info = await wait_for_future_safe(on_entry)
    
+   contexts = await bidi_session.browsing_context.get_tree(root=top_level_context["context"])

    assert_browsing_context(
        context_info,
        top_level_context["context"],
        children=None,
        url="about:blank",
        parent=None,
        user_context="default",
+       client_window=contexts[0]["clientWindow"]
    )

This works fine for the tab parameter but currently fails for the window parameter. The reason for this failure is that getTabForBrowsingContext returns undefined. As a result, I am unable to look up the window based on the browsing context.

Here is what my code looks like inside of getBrowsingContextInfo:

const contextInfo = {
  // ...existing code
};

const targetTab = lazy.TabManager.getTabForBrowsingContext(context);
if (targetTab) {
  const targetWindow = lazy.TabManager.getWindowForTab(targetTab);
  contextInfo.clientWindow =
    lazy.windowManager.getIdForWindow(targetWindow);
}

However, when I later call bidi_session.browsing_context.get_tree in the test, I can see that getTabForBrowsingContext is returning the tab when called again, and therefore I can look up the window. Is there a race condition here, or am I missing something about that code?

Comment 6

[Alexandra Borovova [:Sasha]]

(In reply to Liam DeBeasi from comment #5)

However, when I later call bidi_session.browsing_context.get_tree in the test, I can see that getTabForBrowsingContext is returning the tab when called again, and therefore I can look up the window. Is there a race condition here, or am I missing something about that code?

Yes, I've checked, and it looks like at the moment we send "browsingContext.contextCreated" event for the window when a tab (or some of the other properties in this chain) is not attached to the browsing context yet. I could not really find a quick solution here, so I would suggest that we leave it to a follow-up. I will file it when your patches are ready, and then we can add the comment. So for now you can keep that if statement to not throw an error and allow the test to fail at the assertion, and update the metadata for this test case to mark it as expected to fail. For this you will have to add to this file: https://searchfox.org/mozilla-central/source/testing/web-platform/meta/webdriver/tests/bidi/browsing_context/context_created/context_created.py.ini the following entry:

  [test_new_context[window\]]
    expected: FAIL

We will also add a link to the bug when it's filled. Let me know if anything is unclear.