1910243 - Improve test coverage for ChannelWrapper + GC

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, task, P2)

Description

[Rob Wu [:robwu]]

There have been crashes caused by the use of ChannelWrapper when the underlying channel has been closed / garbage-collected.
In the past, bug 1470746, more recently bug 1910114.

While these bugs have been fixed, they did not include automated test coverage.
I'm going to attach a patch with improved test coverage, as well as fixups to issues discovered from the added test coverage.

Comment 1

[Rob Wu [:robwu]]

Attachment: Bug 1910243 - Add test coverage for ChannelWrapper + fix minor issues

Comment 2

[Erich Gubler [:ErichDonGubler] (he/him)]

I'm actually running into a case that the fixups in this patch would seem to handle. I quite consistently crash on pages like https://imdb.com. Attaching the crash stack I'm experiencing.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

(In reply to Erich Gubler [:ErichDonGubler] from comment #2)

I'm actually running into a case that the fixups in this patch would seem to handle. I quite consistently crash on pages like https://imdb.com. Attaching the crash stack I'm experiencing.

Those were the crashes tracked in bug 1910114 and duplicates.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ea9ec869d89
Add test coverage for ChannelWrapper + fix minor issues r=rpl

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/2ea9ec869d89

Comment 6

[Rob Wu [:robwu]]

Attachment: Bug 1910243 - Add test coverage for ChannelWrapper + fix minor issues

Original Revision: https://phabricator.services.mozilla.com/D217899

Comment 7

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: Potentially susceptible to crashes due to lack of test coverage
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: No
• Risk associated with taking this patch: None
• Explanation of risk level: Test-only + fixups to fix issues uncovered by tests
• String changes made/needed: None
• Is Android affected?: no

Comment 8

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/885e9ef8760d