1927829 - Create a helper to assert if a browsing context is a top-level browsing context

Status: RESOLVED FIXED

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

Description

[Alexandra Borovova [:Sasha]]

We have quite a few places in WebDriver BiDi modules where we have to assert if a browsing context is a top-level browsing context, but we use slightly different error messages in case it's not top-level, so would be great to create a shared assert helper and use it everywhere where it's required to have the same error message.

Comment 1

[Spencer]

I would like to work on this task if it's still available for assignment.

If so, would you be able to describe a specific location of code where the top-level browsing context is asserted so I can know where to start from?

Comment 2

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

That's great to hear! I can help out here while Sasha is not around.

The assertion that we want should get added to the following module (you can check existing assertions in that file to get an impressions how it has to look like):

https://searchfox.org/mozilla-central/source/remote/shared/webdriver/Assert.sys.mjs

Checks for top-level browsing contexts that we want to replace with calling the assertion helper look like this:

https://searchfox.org/mozilla-central/rev/8c9c85c74e366c11ffacbb5a2e457b33b0acc9cd/remote/webdriver-bidi/modules/root/browsingContext.sys.mjs#249-253

Here some examples and you can filter out those that do not throw an error:
https://searchfox.org/mozilla-central/search?q=context.parent&path=remote%2Fwebdriver-bidi%2Fmodules&case=true&regexp=false

Don't hesitate to ask if you have further questions, or join us in our #webdriver Matrix channel in case you want to directly chat with us. Thanks!

Comment 3

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

Spencer, do you have any other questions in how to get started? Please let us know. Thanks!

Comment 4

[Spencer]

Attachment: Bug 1927829 - [WebDriverBiDi] Created topLevel assert to determine if browsing context is top-level. r=Sasha

-Created the topLevel assert method in Assert.sys.mjs that checks if a browsing context is top-level.
-Replaced existing top-level checks with the topLevel method, consolidating the error message responses.

Comment 5

[Spencer]

Apologies for the delay! I had a quite busy past couple of weeks, but now I can turn my attention to this for a while.

I submitted a differential with the necessary changes, but prior to that I sent in a try submission and on https://treeherder.mozilla.org/push-health/push?revision=3b671b46bbedbc078774ec557be1a5645748e3c6&repo=try&testGroup=pr&selectedTest=toolkitmozappsextensionstestbrowserbrowserhtmldetailviewjs&selectedTaskId=487405936&selectedJobName=toolkit%2Fmozapps%2Fextensions%2Ftest%2Fbrowser%2Fbrowser_html_detail_view.js+test-linux1804-64-qr%2Fopt-mochitest-browser-chrome-swr-a11y-checks-4 I had several test failures. They on the surface don't directly reference my changes in their points of failure but since the commit prior to mine did not have any standout issues on its try submission, my assert.topLevel method I made to replace the relevant browsing context checks must not be working as intended.

Do you have any thoughts about my changes that stand out, or do you have any resources I can look at in order to understand how to navigate using Treeherder's troubleshooting tools? I'm having trouble figuring out how to approach resolving these test issues.

Comment 6

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

Sasha already replied on the submitted Phabricator revision and if something is unclear you can always ask her for further details. But yes, those failures are not related to your changes.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/917968df04f2
[WebDriverBiDi] Created topLevel assert to determine if browsing context is top-level. r=Sasha,webdriver-reviewers

Comment 8

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/917968df04f2