1847563 - `browsingContext.domContentLoaded` can have context set to null

Status: RESOLVED FIXED

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

Description

[Nik]

Attachment: firefox-bidi-domContentLoaded.txt

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/115.0.0.0 Safari/537.36

Steps to reproduce:

Run the following Puppeteer test in evaluation.spec.ts
should work with unicode chars
should throw when evaluation triggers reload
should await promise

Attached Puppeteer debugging logs + Firefox Traces.

Actual results:

Individually all pass as expected, but when run one after the other an event browsingContext.domContentLoaded with context:null sneaks in.
This is not per spec so our implementation fails
Most likely do the code in should throw when evaluation triggers reload as it triggers a navigation and we close the page soon after.

Expected results:

The event should not be fired if its not related to any browsingContext.

Comment 1

[Nik]

Also happens for this pair of Puppeteer tests in page.spec.ts:
should respect timeout
should respect default navigation timeout

Comment 2

[Julian Descottes [:jdescottes]]

Thanks! I can easily reproduce by running commands inspired from the should throw when evaluation triggers reload test:

• browsingContext.create
• browsingContext.navigate to "about:blank"
• script.callFunction with location.reload
• browsingContext.close

And I agree that if we receive events for a browsing context which no longer exists, we should just discard them.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:whimboo, could you have a look please?

For more information, please visit BugBot documentation.

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1847563 - [remote] Allow MessageHandler modules to cancel intercepted events

Depends on D187898

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1847563 - [remote] Add isValidCanonicalBrowsingContext to check against discarded contexts

Depends on D188101

If the browsing context is discarded, it will not have any embedderElement attached, and we cannot process it as expected.
Add the helper and use it for the existing spots checking for valid browsing contexts.

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1847563 - [bidi] Cancel intercepted bidi events for invalid contexts

Depends on D188102

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bccc4a0a8662
[remote] Allow MessageHandler modules to cancel intercepted events r=webdriver-reviewers,Sasha
https://hg.mozilla.org/integration/autoland/rev/75072208d649
[remote] Add isValidCanonicalBrowsingContext to check against discarded contexts r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/6706e17fe8c3
[bidi] Cancel intercepted bidi events for invalid contexts r=webdriver-reviewers,whimboo

Comment 8

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/bccc4a0a8662
https://hg.mozilla.org/mozilla-central/rev/75072208d649
https://hg.mozilla.org/mozilla-central/rev/6706e17fe8c3