Closed Bug 1397456 Opened 7 years ago Closed 7 years ago

IPC message name is confusing and a little broken

Categories

(Core :: IPC, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: billm, Assigned: kanru)

Details

Attachments

(1 file)

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: nobody → kchen
Flags: needinfo?(kchen)
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.
Priority: -- → P3
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+
Pushed by kchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/87ed4a4213d8
Always use static name for ipc messages r=billm
https://hg.mozilla.org/mozilla-central/rev/87ed4a4213d8
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: