1967785 - NetworkDecodedBodySizeMap can throw if called after destroy

Status: RESOLVED FIXED

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

Description

[Julian Descottes [:jdescottes]]

In NetworkDecodedBodySizeMap.destroy, we nullify all the internal maps. But if for any reason we still call the NetworkDecodedBodySizeMap, we will then throw.

We should guard those methods to avoid JS errors.

Note that the NetworkDecodedBodySizeMap is destroyed from network module's destroy() method at https://searchfox.org/mozilla-central/rev/dbef1a2f75798fb0136b7428d959c8feb09ad5d1/remote/webdriver-bidi/modules/root/network.sys.mjs#357.
We already remove all event listeners in the network module itself, but that doesn't apply to the current NetworkEventRecord instances, which are still listening to their own error/stop events https://searchfox.org/mozilla-central/rev/dbef1a2f75798fb0136b7428d959c8feb09ad5d1/remote/shared/listeners/NetworkEventRecord.sys.mjs#71-72

At this point it means the session is already being destroyed, so the main side effect of those errors is log pollution.

Comment 1

[Julian Descottes [:jdescottes]]

Note that an easy way to address this would be to simply reset the two maps to new Map in the destroy, instead of nullifying them.

Comment 2

[Anthony Mclamb]

Hey Julian Descottes, I have a fix for this but I'm having trouble figuring out how to manually test my changes. Do you have any tips?

Comment 3

[Julian Descottes [:jdescottes]]

Hi Anthony!

(In reply to Anthony Mclamb from comment #2)

Hey Julian Descottes, I have a fix for this but I'm having trouble figuring out how to manually test my changes. Do you have any tips?

We have a test for this class at https://searchfox.org/mozilla-central/source/remote/shared/test/xpcshell/test_NetworkDecodedBodySizeMap.js#1
You can run it using

./mach test remote/shared/test/xpcshell/test_NetworkDecodedBodySizeMap.js

As you can see in this test the last step is to call destroy(), you can try to call all of the APIs after that getDecodedBodySize, setDecodedBodySize and setAuthenticationAttemptMapping and at least check that they don't throw.

Comment 4

[Julian Descottes [:jdescottes]]

Also taking this, too much log pollution on real websites.

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1967785 - [bidi] Avoid log pollution from NetworkDecodedBodySizeMap

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/678e86bb258c
https://hg.mozilla.org/integration/autoland/rev/635eae74e96a
[bidi] Avoid log pollution from NetworkDecodedBodySizeMap r=webdriver-reviewers,whimboo

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/635eae74e96a