1920053 - Several navigation methods leak registered observers through the ProgressListener

Status: RESOLVED FIXED

(Remote Protocol :: Agent, defect, P2)

Description

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

I noticed that issue when investigating bug 1891706. Whenever we open a new tab the method waitForInitialNavigationCompleted is called to await the initial about:blank page to be loaded.

Because this method uses the ProgressListener class which itself makes use of the PromptListener class several observer notifications are getting registered when the PromptListener is started to listen.

In the waitForInitialNavigationCompleted method we for sure wait for the navigation to be done but we never call listener.destroy(). As such the progress listener will be removed when it gets out of scope but all the registered observers remain active.

To fix that we have to explicitly call listener.destroy() before leaving the scope of the method.

Comment 1

[Julian Descottes [:jdescottes]]

As mentioned on Matrix, we also never call destroy on the ProgressListener instances created at:

• https://searchfox.org/mozilla-central/rev/43450868e9f3e26da88a02837e5a43c93728ea95/remote/webdriver-bidi/modules/root/browsingContext.sys.mjs#1565-1626
• https://searchfox.org/mozilla-central/rev/43450868e9f3e26da88a02837e5a43c93728ea95/remote/marionette/navigate.sys.mjs#222-236

Comment 2

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

Indeed. We should fix those all. Thanks!

Comment 3

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

Attachment: Bug 1920053 - [remote] Make sure to call ProgressListener.destroy() to not leak registered observers and events.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7feb4c39dd8c
[remote] Make sure to call ProgressListener.destroy() to not leak registered observers and events. r=webdriver-reviewers,jdescottes

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/7feb4c39dd8c