Bug 1470795 Part 11 - Fix bug in record/replay channel initialization, r=mccr8.
authorBrian Hackett <bhackett1024@gmail.com>
Sun, 22 Jul 2018 12:00:53 +0000
changeset 427745 22d28e5778fd7b1c0e7aaf1752fc773111ab39aa
parent 427744 a4a06f53d689f3dedfca8eaf23f417ec54696927
child 427746 a0ee9e803ff24c59245322da5c2f692d803300e1
push id34314
push usercsabou@mozilla.com
push dateMon, 23 Jul 2018 09:31:12 +0000
treeherdermozilla-central@143984185dce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1470795
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1470795 Part 11 - Fix bug in record/replay channel initialization, r=mccr8.
toolkit/recordreplay/ipc/Channel.cpp
toolkit/recordreplay/ipc/Channel.h
toolkit/recordreplay/ipc/ChildIPC.cpp
toolkit/recordreplay/ipc/ChildProcess.cpp
--- a/toolkit/recordreplay/ipc/Channel.cpp
+++ b/toolkit/recordreplay/ipc/Channel.cpp
@@ -54,17 +54,17 @@ OpenChannel(base::ProcessId aMiddlemanPi
 
 } // namespace parent
 
 struct HelloMessage
 {
   int32_t mMagic;
 };
 
-Channel::Channel(size_t aId, const MessageHandler& aHandler)
+Channel::Channel(size_t aId, bool aMiddlemanRecording, const MessageHandler& aHandler)
   : mId(aId)
   , mHandler(aHandler)
   , mInitialized(false)
   , mConnectionFd(0)
   , mFd(0)
   , mMessageBytes(0)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
@@ -81,17 +81,17 @@ Channel::Channel(size_t aId, const Messa
     int rv = HANDLE_EINTR(connect(mFd, (sockaddr*) &addr, SUN_LEN(&addr)));
     MOZ_RELEASE_ASSERT(rv >= 0);
 
     DirectDeleteFile(addr.sun_path);
   } else {
     MOZ_RELEASE_ASSERT(IsMiddleman());
 
     ipc::FileDescriptor connection;
-    if (mId == RecordingId) {
+    if (aMiddlemanRecording) {
       // When starting the recording child process we have not done enough
       // initialization to ask for a channel from the parent, but have also not
       // started the sandbox so we can do it ourselves.
       parent::OpenChannel(base::GetCurrentProcId(), mId, &connection);
     } else {
       dom::ContentChild::GetSingleton()->SendOpenRecordReplayChannel(mId, &connection);
       MOZ_RELEASE_ASSERT(connection.IsValid());
     }
--- a/toolkit/recordreplay/ipc/Channel.h
+++ b/toolkit/recordreplay/ipc/Channel.h
@@ -396,20 +396,16 @@ typedef EmptyMessage<MessageType::Always
 
 class Channel
 {
 public:
   // Note: the handler is responsible for freeing its input message. It will be
   // called on the channel's message thread.
   typedef std::function<void(Message*)> MessageHandler;
 
-  // The recording channel is opened at startup and is initialized differently
-  // from other channels.
-  static const size_t RecordingId = 0;
-
 private:
   // ID for this channel, unique for the middleman.
   size_t mId;
 
   // Callback to invoke off thread on incoming messages.
   MessageHandler mHandler;
 
   // Whether the channel is initialized and ready for outgoing messages.
@@ -438,17 +434,17 @@ private:
   Message* WaitForMessage();
 
   // Main routine for the channel's thread.
   static void ThreadMain(void* aChannel);
 
 public:
   // Initialize this channel, connect to the other side, and spin up a thread
   // to process incoming messages by calling aHandler.
-  Channel(size_t aId, const MessageHandler& aHandler);
+  Channel(size_t aId, bool aMiddlemanRecording, const MessageHandler& aHandler);
 
   size_t GetId() { return mId; }
 
   // Send a message to the other side of the channel. This must be called on
   // the main thread, except for fatal error messages.
   void SendMessage(const Message& aMsg);
 };
 
--- a/toolkit/recordreplay/ipc/ChildIPC.cpp
+++ b/toolkit/recordreplay/ipc/ChildIPC.cpp
@@ -199,17 +199,17 @@ InitRecordingOrReplayingProcess(int* aAr
   MOZ_RELEASE_ASSERT(channelID.isSome());
 
   gMiddlemanPid = middlemanPid.ref();
 
   Maybe<AutoPassThroughThreadEvents> pt;
   pt.emplace();
 
   gMonitor = new Monitor();
-  gChannel = new Channel(channelID.ref(), ChannelMessageHandler);
+  gChannel = new Channel(channelID.ref(), /* aMiddlemanRecording = */ false, ChannelMessageHandler);
 
   pt.reset();
 
   DirectCreatePipe(&gCheckpointWriteFd, &gCheckpointReadFd);
 
   Thread::StartThread(ListenForCheckpointThreadMain, nullptr, false);
 
   pt.emplace();
--- a/toolkit/recordreplay/ipc/ChildProcess.cpp
+++ b/toolkit/recordreplay/ipc/ChildProcess.cpp
@@ -443,23 +443,22 @@ GetArgumentsForChildProcess(base::Proces
   aExtraArgs.push_back(gRecordingFileOption);
   aExtraArgs.push_back(aRecordingFile);
 }
 
 void
 ChildProcessInfo::LaunchSubprocess()
 {
   size_t channelId = gNumChannels++;
-  MOZ_RELEASE_ASSERT((channelId == Channel::RecordingId) == IsRecording());
 
   // Create a new channel every time we launch a new subprocess, without
   // deleting or tearing down the old one's state. This is pretty lame and it
   // would be nice if we could do something better here, especially because
   // with restarts we could create any number of channels over time.
-  mChannel = new Channel(channelId, [=](Message* aMsg) {
+  mChannel = new Channel(channelId, IsRecording(), [=](Message* aMsg) {
       ReceiveChildMessageOnMainThread(channelId, aMsg);
     });
 
   if (IsRecording()) {
     std::vector<std::string> extraArgs;
     GetArgumentsForChildProcess(base::GetCurrentProcId(), channelId,
                                 gRecordingFilename, /* aRecording = */ true, extraArgs);