IPC message name is confusing and a little broken

RESOLVED FIXED in Firefox 58

Status

()

Core
IPC
P3
normal
RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: billm, Assigned: kanru)

Tracking

unspecified
mozilla58
Points:
---

Firefox Tracking Flags

(firefox58 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Right now, we have IPC::Message::name(), and it's supposed to return the name of the message. Except that it only works sometimes. I don't think we ever assign anything to it for messages that we receive. For those, you have to use StringFromIPCMessageType.

As a consequence, I think some of our telemetry code is broken.
https://hg.mozilla.org/releases/mozilla-beta/annotate/dcb3e24852c4/ipc/glue/MessageChannel.cpp#l2062
I don't think we'll have a message name here. It will just be "???" (the default value for the aName parameter here:
http://searchfox.org/mozilla-central/rev/4d8e389498a08668cce9ebf6232cc96be178c3e4/ipc/chromium/src/chrome/common/ipc_message.h#385

Can you take a look at fixing this, Kan-Ru? Personally I think we should just get rid of IPC::Message::name() and always use StringFromIPCMessageType. Or maybe we could change name() to return the result of StringFromIPCMessageType.
Flags: needinfo?(kchen)
(Assignee)

Updated

11 months ago
Assignee: nobody → kchen
Flags: needinfo?(kchen)

Comment 2

10 months ago
If we want to use StringFromIPCMessageType, I should really try to get bug 1383911 in shape so StringFromIPCMessageType becomes more efficient.

(In reply to Bill McCloskey [PTO, back 18 Sep] (:billm) from comment #0)
> Right now, we have IPC::Message::name(), and it's supposed to return the
> name of the message. Except that it only works sometimes. I don't think we
> ever assign anything to it for messages that we receive. For those, you have
> to use StringFromIPCMessageType.

We definitely used to jam a name into messages we received...I remember trying to do clever things with Message::name() and then discovering that we casted away constness to set the name, which led me to abandoning plans to do anything.
Comment hidden (mozreview-request)

Updated

10 months ago
Priority: -- → P3
(Reporter)

Comment 4

10 months ago
mozreview-review
Comment on attachment 8910591 [details]
Bug 1397456 - Always use static name for ipc messages

https://reviewboard.mozilla.org/r/182030/#review188096

Thanks. This is perfect.
Attachment #8910591 - Flags: review?(wmccloskey) → review+

Comment 5

10 months ago
Pushed by kchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/87ed4a4213d8
Always use static name for ipc messages r=billm

Comment 6

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/87ed4a4213d8
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.