1733126 - about:processes tests kill processes without calling NoteIntentionalCrash

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect, P2)

Description

[Andrew McCreight [:mccr8]]

Bug 1635494 added some tests of the about:processes feature that kills a child process. Unfortunately, this is done without calling NoteIntentionalCrash() in the child process, so the leak checker produces an error because it can't find a leak log for that process (bug 1560096). For some reason, this leak checker failure doesn't make the job fail, so we get intermittent reports of this when some other test in the same job fails.

I assume that the way that about:processes kills a process happens from the parent process, which will make it trickier to do NoteIntentionalCrash(). Maybe the test can send a message to the child and wait for a reply, after the process is created but before the button is clicked.

Comment 1

[Andrew McCreight [:mccr8]]

It looks like NoteIntentionalCrash() is exposed to JS via ChromeUtils.privateNoteIntentionalCrash(). Keep in mind that this must be called in the content process that is about to be killed, and the parent process needs to wait for a reply from the content process before it does the kill to make sure that the logging has completed.

Comment 2

[Andrew McCreight [:mccr8]]

This appears to only fail with Fission.

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: Bug 1733126 - Add an annotation for the leak checker in about:processes tests.

This test kills a child process when Fission is enabled. Add an annotation
to the leak log for the process so that the leak checker doesn't produce an
error.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Changing severity to NA because of being test only.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/310926b78a76
Add an annotation for the leak checker in about:processes tests. r=kmag

Comment 6

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/310926b78a76

Comment 7

[Andrew McCreight [:mccr8]]

Comment on attachment 9243511 [details]
Bug 1733126 - Add an annotation for the leak checker in about:processes tests.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: test only change to fix intermittent failures
• User impact if declined: none
• Fix Landed on Version: 95
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): test only
• String or UUID changes made by this patch: none

Beta/Release Uplift Approval Request

See above

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9243511 [details]
Bug 1733126 - Add an annotation for the leak checker in about:processes tests.

Approved for 94.0b2.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9d89d049f062

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9243511 [details]
Bug 1733126 - Add an annotation for the leak checker in about:processes tests.

Approved for 91.3esr.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/32c499749b7d