Skip to content

Commit

Permalink
Bug 1397456 - Always use static name for ipc messages r=billm
Browse files Browse the repository at this point in the history
Never store names in Message. One can get string names from
Message::name() or use IPC::StringFromIPCMessageType() when only
message id is available.

MozReview-Commit-ID: 15ksx6SE90c
  • Loading branch information
kanru committed Sep 14, 2017
1 parent c50302a commit 18c8579
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 39 deletions.
2 changes: 1 addition & 1 deletion ipc/chromium/src/base/pickle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ void Pickle::EndRead(PickleIterator& iter, uint32_t ipcMsgType) const {
uint32_t latencyMs = round((mozilla::TimeStamp::Now() - iter.start_).ToMilliseconds());
if (latencyMs >= kMinTelemetryIPCReadLatencyMs) {
mozilla::Telemetry::Accumulate(mozilla::Telemetry::IPC_READ_MAIN_THREAD_LATENCY_MS,
nsDependentCString(mozilla::ipc::StringFromIPCMessageType(ipcMsgType)),
nsDependentCString(IPC::StringFromIPCMessageType(ipcMsgType)),
latencyMs);
}
}
Expand Down
15 changes: 2 additions & 13 deletions ipc/chromium/src/chrome/common/ipc_message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,12 @@ Message::Message()
&_header->source_event_type);
}
#endif
InitLoggingVariables();
}

Message::Message(int32_t routing_id,
msgid_t type,
uint32_t segment_capacity,
HeaderFlags flags,
const char* const aName,
bool recordWriteLatency)
: Pickle(MSG_HEADER_SZ, segment_capacity) {
MOZ_COUNT_CTOR(IPC::Message);
Expand Down Expand Up @@ -86,7 +84,6 @@ Message::Message(int32_t routing_id,
if (recordWriteLatency) {
create_time_ = mozilla::TimeStamp::Now();
}
InitLoggingVariables(aName);
}

#ifndef MOZ_TASK_TRACER
Expand All @@ -101,12 +98,10 @@ Message::Message(const char* data, int data_len)
: Pickle(MSG_HEADER_SZ_DATA, data, data_len)
{
MOZ_COUNT_CTOR(IPC::Message);
InitLoggingVariables();
}

Message::Message(Message&& other) : Pickle(mozilla::Move(other)) {
MOZ_COUNT_CTOR(IPC::Message);
InitLoggingVariables(other.name_);
#if defined(OS_POSIX)
file_descriptor_set_ = other.file_descriptor_set_.forget();
#endif
Expand All @@ -115,10 +110,9 @@ Message::Message(Message&& other) : Pickle(mozilla::Move(other)) {
/*static*/ Message*
Message::IPDLMessage(int32_t routing_id,
msgid_t type,
HeaderFlags flags,
const char* const name)
HeaderFlags flags)
{
return new Message(routing_id, type, 0, flags, name, true);
return new Message(routing_id, type, 0, flags, true);
}

/*static*/ Message*
Expand All @@ -143,13 +137,8 @@ Message::ForInterruptDispatchError()
return m;
}

void Message::InitLoggingVariables(const char* const aName) {
name_ = aName;
}

Message& Message::operator=(Message&& other) {
*static_cast<Pickle*>(this) = mozilla::Move(other);
InitLoggingVariables(other.name_);
#if defined(OS_POSIX)
file_descriptor_set_.swap(other.file_descriptor_set_);
#endif
Expand Down
17 changes: 5 additions & 12 deletions ipc/chromium/src/chrome/common/ipc_message.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ namespace IPC {

//------------------------------------------------------------------------------

// Generated by IPDL compiler
const char* StringFromIPCMessageType(uint32_t aMessageType);

class Channel;
class Message;
struct LogData;
Expand Down Expand Up @@ -194,7 +197,6 @@ class Message : public Pickle {
msgid_t type,
uint32_t segmentCapacity = 0, // 0 for the default capacity.
HeaderFlags flags = HeaderFlags(),
const char* const name="???",
bool recordWriteLatency=false);

Message(const char* data, int data_len);
Expand All @@ -210,8 +212,7 @@ class Message : public Pickle {
// code.
static Message* IPDLMessage(int32_t routing_id,
msgid_t type,
HeaderFlags flags,
const char* const name);
HeaderFlags flags);

// One-off constructors for special error-handling messages.
static Message* ForSyncDispatchError(NestedLevel level);
Expand Down Expand Up @@ -298,11 +299,7 @@ class Message : public Pickle {
}

const char* name() const {
return name_;
}

void set_name(const char* const aName) {
name_ = aName;
return StringFromIPCMessageType(type());
}

const mozilla::TimeStamp& create_time() const {
Expand Down Expand Up @@ -462,8 +459,6 @@ class Message : public Pickle {
}
#endif

void InitLoggingVariables(const char* const name="???");

#if defined(OS_POSIX)
// The set of file descriptors associated with this message.
RefPtr<FileDescriptorSet> file_descriptor_set_;
Expand All @@ -480,8 +475,6 @@ class Message : public Pickle {
}
#endif

const char* name_;

mozilla::TimeStamp create_time_;

};
Expand Down
4 changes: 2 additions & 2 deletions ipc/glue/MessageChannel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1898,7 +1898,7 @@ MessageChannel::RunMessage(MessageTask& aTask)
NS_IMPL_ISUPPORTS_INHERITED(MessageChannel::MessageTask, CancelableRunnable, nsIRunnablePriority)

MessageChannel::MessageTask::MessageTask(MessageChannel* aChannel, Message&& aMessage)
: CancelableRunnable(StringFromIPCMessageType(aMessage.type()))
: CancelableRunnable(aMessage.name())
, mChannel(aChannel)
, mMessage(Move(aMessage))
, mScheduled(false)
Expand Down Expand Up @@ -2505,7 +2505,7 @@ MessageChannel::MaybeHandleError(Result code, const Message& aMsg, const char* c
}

char reason[512];
const char* msgname = StringFromIPCMessageType(aMsg.type());
const char* msgname = aMsg.name();
if (msgname[0] == '?') {
SprintfLiteral(reason,"(msgtype=0x%X) %s", aMsg.type(), errorMsg);
} else {
Expand Down
2 changes: 0 additions & 2 deletions ipc/glue/ProtocolUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -732,8 +732,6 @@ void
TableToArray(const nsTHashtable<nsPtrHashKey<void>>& aTable,
nsTArray<void*>& aArray);

const char* StringFromIPCMessageType(uint32_t aMessageType);

} // namespace ipc

template<typename Protocol>
Expand Down
6 changes: 2 additions & 4 deletions ipc/ipdl/ipdl.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,7 @@ def normalizedFilename(f):
} // anonymous namespace
namespace mozilla {
namespace ipc {
namespace IPC {
const char* StringFromIPCMessageType(uint32_t aMessageType)
{
Expand Down Expand Up @@ -277,8 +276,7 @@ def normalizedFilename(f):
}
}
} // namespace ipc
} // namespace mozilla
} // namespace IPC
"""

ipdl.writeifmodified(ipcmsgstart.getvalue(), ipcmessagestartpath)
Expand Down
6 changes: 1 addition & 5 deletions ipc/ipdl/ipdl/lower.py
Original file line number Diff line number Diff line change
Expand Up @@ -1705,12 +1705,10 @@ def _generateMessageConstructor(md, segmentSize, protocol, forReply=False):
if forReply:
clsname = md.replyCtorFunc()
msgid = md.replyId()
prettyName = md.prettyReplyName(protocol.name+'::')
replyEnum = 'REPLY'
else:
clsname = md.msgCtorFunc()
msgid = md.msgId()
prettyName = md.prettyMsgName(protocol.name+'::')
replyEnum = 'NOT_REPLY'

nested = md.decl.type.nested
Expand Down Expand Up @@ -1782,16 +1780,14 @@ def messageEnum(valname):
ExprVar(msgid),
ExprLiteral.Int(int(segmentSize)),
flags,
ExprLiteral.String(prettyName),
# Pass `true` to recordWriteLatency to collect telemetry
ExprLiteral.TRUE ])))
else:
func.addstmt(
StmtReturn(ExprCall(ExprVar('IPC::Message::IPDLMessage'),
args=[ routingId,
ExprVar(msgid),
flags,
ExprLiteral.String(prettyName) ])))
flags ])))

return func

Expand Down

0 comments on commit 18c8579

Please sign in to comment.