1886889 - Audit unused message manager message listeners

Status: RESOLVED INCOMPLETE

(Firefox :: Security, task)

Description

[Andrew McCreight [:mccr8]]

One interesting thing about bug 1886852 is that it involves message manager listeners that are completely dead code. We could log when listeners are added and messages are received, in the parent process, do that across a full try push, and hopefully get a small-ish list of potential things to audit. (This could also happen for JS actor messages, but those are much newer and thus less likely to have cruft.)

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Bug 1886889 - Log when parent adds message listeners or gets messages.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: log analyzer script

Here's a basic analyzer which will produce a list of listeners that never got anything, and messages where nobody was registered. The latter turned up child-process-shutdown which ContentParent produces but nothing seems to listen to, for whatever that is worth.

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: message manager listeners that don't get messages

Comment 4

[Andrew McCreight [:mccr8]]

The TLDR is that the non-sessions store ones all have at least SOME test coverage, except for Push:Clear and Push:ReportError.

Comment 5

[Andrew McCreight [:mccr8]]

I filed bug 1887014 about the two Push messages not having coverage.

It might be worth checking whether the various unused messages are actually used in scenarios you'd expect them, eg the JSON viewer save.

Comment 6

[:Gijs (he/him)]

(In reply to Andrew McCreight [:mccr8] from comment #5)

It might be worth checking whether the various unused messages are actually used in scenarios you'd expect them, eg the JSON viewer save.

bug 1872307 is maybe relevant here. Also potentially bug 1562051 comment 4.

Comment 7

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

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

For more information, please visit BugBot documentation.

Comment 8

[Andrew McCreight [:mccr8]]

There's nothing obviously bad left. This probably isn't going anywhere further at this point.