Bug 1397456 - Always use static name for ipc messages r=billm draft
authorKan-Ru Chen <kanru@kanru.info>
Thu, 14 Sep 2017 16:08:57 +0800
changeset 668134 67023414def750b01a5d48005ac2c0ac897d7920
parent 668025 47f7b6c64265bc7bdd22eef7ab71abc97cf3f8bf
child 732596 7a771d52f8610155cd83ffae241cd84e34901342
push id80934
push userbmo:kchen@mozilla.com
push dateThu, 21 Sep 2017 04:32:00 +0000
reviewersbillm
bugs1397456
milestone57.0a1
Bug 1397456 - Always use static name for ipc messages r=billm 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
ipc/chromium/src/base/pickle.cc
ipc/chromium/src/chrome/common/ipc_message.cc
ipc/chromium/src/chrome/common/ipc_message.h
ipc/glue/MessageChannel.cpp
ipc/glue/ProtocolUtils.h
ipc/ipdl/ipdl.py
ipc/ipdl/ipdl/lower.py
--- a/ipc/chromium/src/base/pickle.cc
+++ b/ipc/chromium/src/base/pickle.cc
@@ -471,17 +471,17 @@ bool Pickle::WriteSentinel(uint32_t sent
 
 void Pickle::EndRead(PickleIterator& iter, uint32_t ipcMsgType) const {
   DCHECK(iter.iter_.Done());
 
   if (NS_IsMainThread() && ipcMsgType != 0) {
     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);
     }
   }
 }
 
 void Pickle::BeginWrite(uint32_t length, uint32_t alignment) {
   DCHECK(alignment % 4 == 0) << "Must be at least 32-bit aligned!";
 
--- a/ipc/chromium/src/chrome/common/ipc_message.cc
+++ b/ipc/chromium/src/chrome/common/ipc_message.cc
@@ -46,24 +46,22 @@ Message::Message()
   if (UseTaskTracerHeader()) {
     header()->flags.SetTaskTracer();
     HeaderTaskTracer* _header = static_cast<HeaderTaskTracer*>(header());
     GetCurTraceInfo(&_header->source_event_id,
                     &_header->parent_task_id,
                     &_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);
   header()->routing = routing_id;
   header()->type = type;
   header()->flags = flags;
 #if defined(OS_POSIX)
   header()->num_fds = 0;
@@ -81,49 +79,45 @@ Message::Message(int32_t routing_id,
     GetCurTraceInfo(&_header->source_event_id,
                     &_header->parent_task_id,
                     &_header->source_event_type);
   }
 #endif
   if (recordWriteLatency) {
     create_time_ = mozilla::TimeStamp::Now();
   }
-  InitLoggingVariables(aName);
 }
 
 #ifndef MOZ_TASK_TRACER
 #define MSG_HEADER_SZ_DATA sizeof(Header)
 #else
 #define MSG_HEADER_SZ_DATA                                            \
   (reinterpret_cast<const Header*>(data)->flags.IsTaskTracer() ? \
    sizeof(HeaderTaskTracer) : sizeof(Header))
 #endif
 
 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
 }
 
 /*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*
 Message::ForSyncDispatchError(NestedLevel level)
 {
   auto* m = new Message(0, 0, 0, HeaderFlags(level));
   auto& flags = m->header()->flags;
   flags.SetSync();
@@ -138,23 +132,18 @@ Message::ForInterruptDispatchError()
   auto* m = new Message();
   auto& flags = m->header()->flags;
   flags.SetInterrupt();
   flags.SetReply();
   flags.SetReplyError();
   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
   return *this;
 }
 
 
 #if defined(OS_POSIX)
--- a/ipc/chromium/src/chrome/common/ipc_message.h
+++ b/ipc/chromium/src/chrome/common/ipc_message.h
@@ -26,16 +26,19 @@ struct FileDescriptor;
 }
 
 class FileDescriptorSet;
 
 namespace IPC {
 
 //------------------------------------------------------------------------------
 
+// Generated by IPDL compiler
+const char* StringFromIPCMessageType(uint32_t aMessageType);
+
 class Channel;
 class Message;
 struct LogData;
 
 class Message : public Pickle {
  public:
   typedef uint32_t msgid_t;
 
@@ -189,34 +192,32 @@ class Message : public Pickle {
   // destination WebView ID.
   //
   // NOTE: `recordWriteLatency` is only passed by IPDL generated message code,
   // and is used to trigger the IPC_WRITE_LATENCY_MS telemetry.
   Message(int32_t routing_id,
           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);
 
   Message(const Message& other) = delete;
   Message(Message&& other);
   Message& operator=(const Message& other) = delete;
   Message& operator=(Message&& other);
 
   // Helper method for the common case (default segmentCapacity, recording
   // the write latency of messages) of IPDL message creation.  This helps
   // move the malloc and some of the parameter setting out of autogenerated
   // 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);
   static Message* ForInterruptDispatchError();
 
   NestedLevel nested_level() const {
     return header()->flags.Level();
   }
@@ -293,21 +294,17 @@ class Message : public Pickle {
     return header()->seqno;
   }
 
   void set_seqno(int32_t aSeqno) {
     header()->seqno = aSeqno;
   }
 
   const char* name() const {
-    return name_;
-  }
-
-  void set_name(const char* const aName) {
-    name_ = aName;
+    return StringFromIPCMessageType(type());
   }
 
   const mozilla::TimeStamp& create_time() const {
     return create_time_;
   }
 
 #if defined(OS_POSIX)
   uint32_t num_fds() const;
@@ -457,36 +454,32 @@ class Message : public Pickle {
   Header* header() {
     return headerT<Header>();
   }
   const Header* header() const {
     return headerT<Header>();
   }
 #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_;
 
   // Ensure that a FileDescriptorSet is allocated
   void EnsureFileDescriptorSet();
 
   FileDescriptorSet* file_descriptor_set() {
     EnsureFileDescriptorSet();
     return file_descriptor_set_.get();
   }
   const FileDescriptorSet* file_descriptor_set() const {
     return file_descriptor_set_.get();
   }
 #endif
 
-  const char* name_;
-
   mozilla::TimeStamp create_time_;
 
 };
 
 class MessageInfo {
 public:
     typedef uint32_t msgid_t;
 
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -1889,17 +1889,17 @@ MessageChannel::RunMessage(MessageTask& 
     }
 
     DispatchMessage(Move(msg));
 }
 
 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)
 {
 }
 
 nsresult
 MessageChannel::MessageTask::Run()
@@ -2494,17 +2494,17 @@ MessageChannel::MaybeHandleError(Result 
         break;
 
     default:
         MOZ_CRASH("unknown Result code");
         return false;
     }
 
     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 {
         SprintfLiteral(reason,"%s %s", msgname, errorMsg);
     }
 
     PrintErrorMessage(mSide, channelName, reason);
 
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -724,18 +724,16 @@ CreateEndpoints(const PrivateIPDLInterfa
 
   return NS_OK;
 }
 
 void
 TableToArray(const nsTHashtable<nsPtrHashKey<void>>& aTable,
              nsTArray<void*>& aArray);
 
-const char* StringFromIPCMessageType(uint32_t aMessageType);
-
 } // namespace ipc
 
 template<typename Protocol>
 class ManagedContainer : public nsTHashtable<nsPtrHashKey<Protocol>>
 {
   typedef nsTHashtable<nsPtrHashKey<Protocol>> BaseClass;
 
 public:
--- a/ipc/ipdl/ipdl.py
+++ b/ipc/ipdl/ipdl.py
@@ -240,18 +240,17 @@ for protocol in sorted(allmessages.keys(
         elif not msg.endswith('End'):
             print >>ipc_msgtype_name,  "  %s__%s," % (protocol, msg)
 
 print >>ipc_msgtype_name, """
 };
 
 } // anonymous namespace
 
-namespace mozilla {
-namespace ipc {
+namespace IPC {
 
 const char* StringFromIPCMessageType(uint32_t aMessageType)
 {
   switch (aMessageType) {
 """
 
 for protocol in sorted(allmessages.keys()):
     for (msg, num) in allmessages[protocol].idnums:
@@ -272,14 +271,13 @@ print >>ipc_msgtype_name, """
     return "GOODBYE_MESSAGE";
   case CANCEL_MESSAGE_TYPE:
     return "CANCEL_MESSAGE";
   default:
     return "<unknown IPC msg name>";
   }
 }
 
-} // namespace ipc
-} // namespace mozilla
+} // namespace IPC
 """
 
 ipdl.writeifmodified(ipcmsgstart.getvalue(), ipcmessagestartpath)
 ipdl.writeifmodified(ipc_msgtype_name.getvalue(), ipc_msgtype_name_path)
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -1700,22 +1700,20 @@ class _GenerateProtocolCode(ipdl.ast.Vis
         return transitionfunc
 
 ##--------------------------------------------------
 
 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
     prio = md.decl.type.prio
     compress = md.decl.type.compress
 
     routingId = ExprVar('routingId')
 
@@ -1777,26 +1775,24 @@ def _generateMessageConstructor(md, segm
     segmentSize = int(segmentSize)
     if segmentSize:
         func.addstmt(
             StmtReturn(ExprNew(Type('IPC::Message'),
                                args=[ routingId,
                                       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
 
 ##--------------------------------------------------
 
 class _ComputeTypeDeps(TypeVisitor):
     '''Pass that gathers the C++ types that a particular IPDL type
 (recursively) depends on.  There are two kinds of dependencies: (i)