Backed out 2 changesets (bug 1807049, bug 1828389) for mozilla::ThreadEventTarget::Dispatch xpcshell related crashes. CLOSED TREE

Backed out changeset bbd8b8bbf677 (bug 1828389)
Backed out changeset 03bd5f26f9af (bug 1807049)

dom/ipc/ContentParent.cpp

@@ -1889,18 +1889,17 @@ bool ContentParent::ShutDownProcess(ShutDownMethod aMethod) {
     qms->AbortOperationsForProcess(mChildID);
   }
 
-  if (aMethod == CLOSE_CHANNEL || aMethod == CLOSE_CHANNEL_WITH_ERROR) {
+  // If Close() fails with an error, we'll end up back in this function, but
+  // with aMethod = CLOSE_CHANNEL_WITH_ERROR.
+
+  if (aMethod == CLOSE_CHANNEL) {
     if (!mCalledClose) {
       MaybeLogBlockShutdownDiagnostics(
           this, "ShutDownProcess: Closing channel.", __FILE__, __LINE__);
-      // Close()/CloseWithError() can only be called once: They kick off the
-      // destruction sequence.
+      // Close() can only be called once: It kicks off the destruction
+      // sequence.
       mCalledClose = true;
-      if (aMethod == CLOSE_CHANNEL_WITH_ERROR) {
-        CloseWithError();
-      } else {
-        Close();
-      }
+      Close();
     }
     result = true;
   }
@@ -2085,11 +2084,10 @@ void ContentParent::ProcessingError(Result aCode, const char* aReason) {
   if (MsgDropped == aCode) {
     return;
   }
-  // Other errors are big deals.
 #ifndef FUZZING
+  // Other errors are big deals.
   KillHard(aReason);
 #endif
-  ShutDownProcess(CLOSE_CHANNEL_WITH_ERROR);
 }
 
 void ContentParent::ActorDestroy(ActorDestroyReason why) {
@@ -4547,7 +4545,6 @@ void ContentParent::KillHard(const char* aReason) {
   ProcessHandle otherProcessHandle;
   if (!base::OpenProcessHandle(OtherPid(), &otherProcessHandle)) {
     NS_ERROR("Failed to open child process when attempting kill.");
-    ShutDownProcess(CLOSE_CHANNEL_WITH_ERROR);
     return;
   }
 
@@ -4568,10 +4565,6 @@ void ContentParent::KillHard(const char* aReason) {
     mSubprocess->SetAlreadyDead();
   }
 
-  // After we've killed the remote process, also ensure we close the IPC channel
-  // with an error to immediately stop all IPC communication on this channel.
-  ShutDownProcess(CLOSE_CHANNEL_WITH_ERROR);
-
   // EnsureProcessTerminated has responsibilty for closing otherProcessHandle.
   XRE_GetIOMessageLoop()->PostTask(
       NewRunnableFunction("EnsureProcessTerminatedRunnable",

dom/media/ipc/RemoteDecoderManagerChild.cpp

@@ -859,7 +859,7 @@ void RemoteDecoderManagerChild::DeallocateSurfaceDescriptor(
       })));
 }
 
-void RemoteDecoderManagerChild::HandleFatalError(const char* aMsg) {
+void RemoteDecoderManagerChild::HandleFatalError(const char* aMsg) const {
   dom::ContentChild::FatalErrorIfNotUsingGPUProcess(aMsg, OtherPid());
 }
 

dom/media/ipc/RemoteDecoderManagerChild.h

@@ -107,7 +107,7 @@ class RemoteDecoderManagerChild final
       RemoteDecodeIn aLocation);
 
  protected:
-  void HandleFatalError(const char* aMsg) override;
+  void HandleFatalError(const char* aMsg) const override;
 
   PRemoteDecoderChild* AllocPRemoteDecoderChild(
       const RemoteDecoderInfoIPDL& aRemoteDecoderInfo,

gfx/ipc/VsyncBridgeChild.cpp

@@ -119,7 +119,7 @@ void VsyncBridgeChild::ProcessingError(Result aCode, const char* aReason) {
                      "Processing error in VsyncBridgeChild");
 }
 
-void VsyncBridgeChild::HandleFatalError(const char* aMsg) {
+void VsyncBridgeChild::HandleFatalError(const char* aMsg) const {
   dom::ContentChild::FatalErrorIfNotUsingGPUProcess(aMsg, OtherPid());
 }
 

gfx/ipc/VsyncBridgeChild.h

@@ -31,7 +31,7 @@ class VsyncBridgeChild final : public PVsyncBridgeChild {
 
   void NotifyVsync(const VsyncEvent& aVsync, const layers::LayersId& aLayersId);
 
-  void HandleFatalError(const char* aMsg) override;
+  void HandleFatalError(const char* aMsg) const override;
 
  private:
   VsyncBridgeChild(RefPtr<VsyncIOThreadHolder>, const uint64_t& aProcessToken);

gfx/layers/ipc/CompositorManagerChild.cpp

@@ -211,7 +211,7 @@ void CompositorManagerChild::ActorDestroy(ActorDestroyReason aReason) {
   }
 }
 
-void CompositorManagerChild::HandleFatalError(const char* aMsg) {
+void CompositorManagerChild::HandleFatalError(const char* aMsg) const {
   dom::ContentChild::FatalErrorIfNotUsingGPUProcess(aMsg, OtherPid());
 }
 

gfx/layers/ipc/CompositorManagerChild.h

@@ -83,7 +83,7 @@ class CompositorManagerChild : public PCompositorManagerChild {
 
   void ActorDestroy(ActorDestroyReason aReason) override;
 
-  void HandleFatalError(const char* aMsg) override;
+  void HandleFatalError(const char* aMsg) const override;
 
   void ProcessingError(Result aCode, const char* aReason) override;
 

gfx/layers/ipc/ImageBridgeChild.cpp

@@ -920,7 +920,7 @@ bool ImageBridgeChild::CanSend() const {
   return mCanSend;
 }
 
-void ImageBridgeChild::HandleFatalError(const char* aMsg) {
+void ImageBridgeChild::HandleFatalError(const char* aMsg) const {
   dom::ContentChild::FatalErrorIfNotUsingGPUProcess(aMsg, OtherPid());
 }
 

gfx/layers/ipc/ImageBridgeChild.h

@@ -312,7 +312,7 @@ class ImageBridgeChild final : public PImageBridgeChild,
 
   bool InForwarderThread() override { return InImageBridgeChildThread(); }
 
-  void HandleFatalError(const char* aMsg) override;
+  void HandleFatalError(const char* aMsg) const override;
 
   wr::MaybeExternalImageId GetNextExternalImageId() override;
 

gfx/layers/ipc/UiCompositorControllerChild.cpp

@@ -210,7 +210,7 @@ void UiCompositorControllerChild::ProcessingError(Result aCode,
   }
 }
 
-void UiCompositorControllerChild::HandleFatalError(const char* aMsg) {
+void UiCompositorControllerChild::HandleFatalError(const char* aMsg) const {
   dom::ContentChild::FatalErrorIfNotUsingGPUProcess(aMsg, OtherPid());
 }
 

gfx/layers/ipc/UiCompositorControllerChild.h

@@ -77,7 +77,7 @@ class UiCompositorControllerChild final
  protected:
   void ActorDestroy(ActorDestroyReason aWhy) override;
   void ProcessingError(Result aCode, const char* aReason) override;
-  void HandleFatalError(const char* aMsg) override;
+  void HandleFatalError(const char* aMsg) const override;
   mozilla::ipc::IPCResult RecvToolbarAnimatorMessageFromCompositor(
       const int32_t& aMessage);
   mozilla::ipc::IPCResult RecvRootFrameMetrics(const ScreenPoint& aScrollOffset,

gfx/layers/ipc/VideoBridgeChild.cpp

@@ -166,7 +166,7 @@ bool VideoBridgeChild::IsSameProcess() const {
   return OtherPid() == base::GetCurrentProcId();
 }
 
-void VideoBridgeChild::HandleFatalError(const char* aMsg) {
+void VideoBridgeChild::HandleFatalError(const char* aMsg) const {
   dom::ContentChild::FatalErrorIfNotUsingGPUProcess(aMsg, OtherPid());
 }
 

gfx/layers/ipc/VideoBridgeChild.h

@@ -63,7 +63,7 @@ class VideoBridgeChild final : public PVideoBridgeChild,
   static void Open(Endpoint<PVideoBridgeChild>&& aEndpoint);
 
  protected:
-  void HandleFatalError(const char* aMsg) override;
+  void HandleFatalError(const char* aMsg) const override;
   bool DispatchAllocShmemInternal(size_t aSize, mozilla::ipc::Shmem* aShmem,
                                   bool aUnsafe);
   void ProxyAllocShmemNow(SynchronousTask* aTask, size_t aSize,

gfx/vr/ipc/VRManagerChild.cpp

@@ -589,7 +589,7 @@ void VRManagerChild::StopActivity() {
   Unused << SendStopActivity();
 }
 
-void VRManagerChild::HandleFatalError(const char* aMsg) {
+void VRManagerChild::HandleFatalError(const char* aMsg) const {
   dom::ContentChild::FatalErrorIfNotUsingGPUProcess(aMsg, OtherPid());
 }
 

gfx/vr/ipc/VRManagerChild.h

@@ -102,7 +102,7 @@ class VRManagerChild : public PVRManagerChild {
   void FireDOMVRDisplayPresentChangeEvent(uint32_t aDisplayID);
   void FireDOMVRDisplayConnectEventsForLoad(VRManagerEventObserver* aObserver);
 
-  void HandleFatalError(const char* aMsg) override;
+  void HandleFatalError(const char* aMsg) const override;
   void ActorDestroy(ActorDestroyReason aReason) override;
 
   void RunPuppet(const nsTArray<uint64_t>& aBuffer, dom::Promise* aPromise,

ipc/glue/MessageChannel.cpp

@@ -482,6 +482,11 @@ MessageChannel::~MessageChannel() {
             "MessageChannel destroyed without being closed "
             "(mChannelState == ChannelConnected).");
         break;
+      case ChannelTimeout:
+        MOZ_CRASH(
+            "MessageChannel destroyed without being closed "
+            "(mChannelState == ChannelTimeout).");
+        break;
       case ChannelClosing:
         MOZ_CRASH(
             "MessageChannel destroyed without being closed "
@@ -583,11 +588,6 @@ bool MessageChannel::Connected() const {
   return ChannelConnected == mChannelState;
 }
 
-bool MessageChannel::ConnectedOrClosing() const {
-  mMonitor->AssertCurrentThreadOwns();
-  return ChannelConnected == mChannelState || ChannelClosing == mChannelState;
-}
-
 bool MessageChannel::CanSend() const {
   if (!mMonitor) {
     return false;
@@ -600,7 +600,6 @@ void MessageChannel::Clear() {
   AssertWorkerThread();
   mMonitor->AssertCurrentThreadOwns();
   MOZ_DIAGNOSTIC_ASSERT(IsClosedLocked(), "MessageChannel cleared too early?");
-  MOZ_ASSERT(ChannelClosed == mChannelState || ChannelError == mChannelState);
 
   // Don't clear mWorkerThread; we use it in AssertWorkerThread().
   //
@@ -681,7 +680,6 @@ bool MessageChannel::Open(ScopedPort aPort, Side aSide,
     mWorkerThread = eventTarget;
     mShutdownTask = shutdownTask;
     mLink = MakeUnique<PortLink>(this, std::move(aPort));
-    mChannelState = ChannelConnected;
     mSide = aSide;
   }
 
@@ -937,9 +935,8 @@ bool MessageChannel::MaybeInterceptSpecialIOMessage(const Message& aMsg) {
 
   if (MSG_ROUTING_NONE == aMsg.routing_id()) {
     if (GOODBYE_MESSAGE_TYPE == aMsg.type()) {
-      // We've received a GOODBYE message, close the connection and mark
-      // ourselves as "Closing".
-      mLink->Close();
+      // :TODO: Sort out Close() on this side racing with Close() on the
+      // other side
       mChannelState = ChannelClosing;
       if (LoggingEnabled()) {
         printf(
@@ -949,14 +946,6 @@ bool MessageChannel::MaybeInterceptSpecialIOMessage(const Message& aMsg) {
             static_cast<uint32_t>(base::GetCurrentProcId()),
             (mSide == ChildSide) ? "child" : "parent");
       }
-
-      // Notify the worker thread that the connection has been closed, as we
-      // will not receive an `OnChannelErrorFromLink` after calling
-      // `mLink->Close()`.
-      if (AwaitingSyncReply()) {
-        NotifyWorkerThread();
-      }
-      PostErrorNotifyTask();
       return true;
     } else if (CANCEL_MESSAGE_TYPE == aMsg.type()) {
       IPC_LOG("Cancel from message");
@@ -1028,7 +1017,6 @@ bool MessageChannel::ShouldDeferMessage(const Message& aMsg) {
 
 void MessageChannel::OnMessageReceivedFromLink(UniquePtr<Message> aMsg) {
   mMonitor->AssertCurrentThreadOwns();
-  MOZ_ASSERT(mChannelState == ChannelConnected);
 
   if (MaybeInterceptSpecialIOMessage(*aMsg)) {
     return;
@@ -1442,7 +1430,7 @@ bool MessageChannel::Send(UniquePtr<Message> aMsg, UniquePtr<Message>* aReply) {
 bool MessageChannel::HasPendingEvents() {
   AssertWorkerThread();
   mMonitor->AssertCurrentThreadOwns();
-  return ConnectedOrClosing() && !mPending.isEmpty();
+  return Connected() && !mPending.isEmpty();
 }
 
 bool MessageChannel::ProcessPendingRequest(ActorLifecycleProxy* aProxy,
@@ -1457,7 +1445,7 @@ bool MessageChannel::ProcessPendingRequest(ActorLifecycleProxy* aProxy,
   msgid_t msgType = aUrgent->type();
 
   DispatchMessage(aProxy, std::move(aUrgent));
-  if (!ConnectedOrClosing()) {
+  if (!Connected()) {
     ReportConnectionError("ProcessPendingRequest", msgType);
     return false;
   }
@@ -1503,7 +1491,7 @@ void MessageChannel::RunMessage(ActorLifecycleProxy* aProxy,
 
   UniquePtr<Message>& msg = aTask.Msg();
 
-  if (!ConnectedOrClosing()) {
+  if (!Connected()) {
     ReportConnectionError("RunMessage", msg->type());
     return;
   }
@@ -1915,8 +1903,12 @@ void MessageChannel::ReportConnectionError(const char* aFunctionName,
     case ChannelClosed:
       errorMsg = "Closed channel: cannot send/recv";
       break;
+    case ChannelTimeout:
+      errorMsg = "Channel timeout: cannot send/recv";
+      break;
     case ChannelClosing:
-      errorMsg = "Channel closing: too late to send, messages will be lost";
+      errorMsg =
+          "Channel closing: too late to send/recv, messages will be lost";
       break;
     case ChannelError:
       errorMsg = "Channel error: cannot send/recv";
@@ -1999,7 +1991,6 @@ bool MessageChannel::MaybeHandleError(Result code, const Message& aMsg,
 
 void MessageChannel::OnChannelErrorFromLink() {
   mMonitor->AssertCurrentThreadOwns();
-  MOZ_ASSERT(mChannelState == ChannelConnected);
 
   IPC_LOG("OnChannelErrorFromLink");
 
@@ -2007,20 +1998,22 @@ void MessageChannel::OnChannelErrorFromLink() {
     NotifyWorkerThread();
   }
 
-  if (mAbortOnError) {
-    // mAbortOnError is set by main actors (e.g., ContentChild) to ensure
-    // that the process terminates even if normal shutdown is prevented.
-    // A MOZ_CRASH() here is not helpful because crash reporting relies
-    // on the parent process which we know is dead or otherwise unusable.
-    //
-    // Additionally, the parent process can (and often is) killed on Android
-    // when apps are backgrounded. We don't need to report a crash for
-    // normal behavior in that case.
-    printf_stderr("Exiting due to channel error.\n");
-    ProcessChild::QuickExit();
+  if (ChannelClosing != mChannelState) {
+    if (mAbortOnError) {
+      // mAbortOnError is set by main actors (e.g., ContentChild) to ensure
+      // that the process terminates even if normal shutdown is prevented.
+      // A MOZ_CRASH() here is not helpful because crash reporting relies
+      // on the parent process which we know is dead or otherwise unusable.
+      //
+      // Additionally, the parent process can (and often is) killed on Android
+      // when apps are backgrounded. We don't need to report a crash for
+      // normal behavior in that case.
+      printf_stderr("Exiting due to channel error.\n");
+      ProcessChild::QuickExit();
+    }
+    mChannelState = ChannelError;
+    mMonitor->Notify();
   }
-  mChannelState = ChannelError;
-  mMonitor->Notify();
 
   PostErrorNotifyTask();
 }
@@ -2029,9 +2022,9 @@ void MessageChannel::NotifyMaybeChannelError(ReleasableMonitorAutoLock& aLock) {
   AssertWorkerThread();
   mMonitor->AssertCurrentThreadOwns();
   aLock.AssertCurrentThreadOwns();
-  MOZ_ASSERT(mChannelState != ChannelConnected);
 
-  if (ChannelClosing == mChannelState || ChannelClosed == mChannelState) {
+  // TODO sort out Close() on this side racing with Close() on the other side
+  if (ChannelClosing == mChannelState) {
     // the channel closed, but we received a "Goodbye" message warning us
     // about it. no worries
     mChannelState = ChannelClosed;
@@ -2039,10 +2032,11 @@ void MessageChannel::NotifyMaybeChannelError(ReleasableMonitorAutoLock& aLock) {
     return;
   }
 
-  MOZ_ASSERT(ChannelError == mChannelState);
-
   Clear();
 
+  // Oops, error!  Let the listener know about it.
+  mChannelState = ChannelError;
+
   // IPDL assumes these notifications do not fire twice, so we do not let
   // that happen.
   if (mNotifiedChannelDone) {
@@ -2108,31 +2102,38 @@ class GoodbyeMessage : public IPC::Message {
   }
 };
 
+void MessageChannel::SynchronouslyClose() {
+  AssertWorkerThread();
+  mMonitor->AssertCurrentThreadOwns();
+  mLink->SendClose();
+
+  MOZ_RELEASE_ASSERT(!mIsSameThreadChannel || ChannelClosed == mChannelState,
+                     "same-thread channel failed to synchronously close?");
+
+  while (ChannelClosed != mChannelState) mMonitor->Wait();
+}
+
 void MessageChannel::CloseWithError() {
   AssertWorkerThread();
 
-  // This lock guard may be reset by `NotifyMaybeChannelError` before invoking
-  // listener callbacks which may destroy this `MessageChannel`.
-  ReleasableMonitorAutoLock lock(*mMonitor);
-
-  switch (mChannelState) {
-    case ChannelError:
-      // Already errored, ensure we notify if we haven't yet.
-      NotifyMaybeChannelError(lock);
-      return;
-    case ChannelClosed:
-      // Already closed, we can't do anything.
-      return;
-    default:
-      // Either connected or closing, immediately convert to an error, and
-      // notify.
-      MOZ_ASSERT(mChannelState == ChannelConnected ||
-                 mChannelState == ChannelClosing);
-      mLink->Close();
-      mChannelState = ChannelError;
-      NotifyMaybeChannelError(lock);
-      return;
+  MonitorAutoLock lock(*mMonitor);
+  if (ChannelConnected != mChannelState) {
+    return;
   }
+  SynchronouslyClose();
+  mChannelState = ChannelError;
+  PostErrorNotifyTask();
+}
+
+void MessageChannel::CloseWithTimeout() {
+  AssertWorkerThread();
+
+  MonitorAutoLock lock(*mMonitor);
+  if (ChannelConnected != mChannelState) {
+    return;
+  }
+  SynchronouslyClose();
+  mChannelState = ChannelTimeout;
 }
 
 void MessageChannel::NotifyImpendingShutdown() {
@@ -2154,6 +2155,7 @@ void MessageChannel::Close() {
 
   switch (mChannelState) {
     case ChannelError:
+    case ChannelTimeout:
       // See bug 538586: if the listener gets deleted while the
       // IO thread's NotifyChannelError event is still enqueued
       // and subsequently deletes us, then the error event will
@@ -2172,8 +2174,7 @@ void MessageChannel::Close() {
       if (ChannelConnected == mChannelState) {
         SendMessageToLink(MakeUnique<GoodbyeMessage>());
       }
-      mLink->Close();
-      mChannelState = ChannelClosed;
+      SynchronouslyClose();
       NotifyChannelClosed(lock);
       return;
   }

ipc/glue/MessageChannel.h

@@ -100,6 +100,7 @@ using RejectCallback = std::function<void(ResponseRejectReason)>;
 enum ChannelState {
   ChannelClosed,
   ChannelConnected,
+  ChannelTimeout,
   ChannelClosing,
   ChannelError
 };
@@ -197,10 +198,12 @@ class MessageChannel : HasResultCodes {
   // Close the underlying transport channel.
   void Close() MOZ_EXCLUDES(*mMonitor);
 
-  // Close the underlying transport channel, treating the closure as a
-  // connection error.
+  // Force the channel to behave as if a channel error occurred. Valid
+  // for process links only, not thread links.
   void CloseWithError() MOZ_EXCLUDES(*mMonitor);
 
+  void CloseWithTimeout() MOZ_EXCLUDES(*mMonitor);
+
   void SetAbortOnError(bool abort) MOZ_EXCLUDES(*mMonitor) {
     MonitorAutoLock lock(*mMonitor);
     mAbortOnError = abort;
@@ -444,16 +447,8 @@ class MessageChannel : HasResultCodes {
     return mDispatchingAsyncMessageNestedLevel;
   }
 
-  // Check if there is still a live connection to our peer. This may change to
-  // `false` at any time due to the connection to our peer being closed or
-  // dropped (e.g. due to a crash).
   bool Connected() const MOZ_REQUIRES(*mMonitor);
 
-  // Check if there is either still a live connection to our peer, or we have
-  // received a `Goodbye` from our peer, and are actively shutting down our
-  // connection with our peer.
-  bool ConnectedOrClosing() const MOZ_REQUIRES(*mMonitor);
-
  private:
   // Executed on the IO thread.
   void NotifyWorkerThread() MOZ_REQUIRES(*mMonitor);
@@ -463,6 +458,9 @@ class MessageChannel : HasResultCodes {
   bool MaybeInterceptSpecialIOMessage(const Message& aMsg)
       MOZ_REQUIRES(*mMonitor);
 
+  // Tell the IO thread to close the channel and wait for it to ACK.
+  void SynchronouslyClose() MOZ_REQUIRES(*mMonitor);
+
   // Returns true if ShouldDeferMessage(aMsg) is guaranteed to return true.
   // Otherwise, the result of ShouldDeferMessage(aMsg) may be true or false,
   // depending on context.

ipc/glue/MessageLink.cpp

@@ -65,6 +65,8 @@ PortLink::PortLink(MessageChannel* aChan, ScopedPort aPort)
   mObserver = new PortObserverThunk(mChan->mMonitor, this);
   mNode->SetPortObserver(mPort, mObserver);
 
+  mChan->mChannelState = ChannelConnected;
+
   // Dispatch an event to the IO loop to trigger an initial
   // `OnPortStatusChanged` to deliver any pending messages. This needs to be run
   // asynchronously from a different thread (or in the case of a same-thread
@@ -130,9 +132,13 @@ void PortLink::SendMessage(UniquePtr<Message> aMessage) {
   }
 }
 
-void PortLink::Close() {
+void PortLink::SendClose() {
   mChan->mMonitor->AssertCurrentThreadOwns();
 
+  // Our channel has been closed, mark it as such.
+  mChan->mChannelState = ChannelClosed;
+  mChan->mMonitor->Notify();
+
   if (!mObserver) {
     // We're already being closed.
     return;

ipc/glue/MessageLink.h

@@ -53,11 +53,7 @@ class MessageLink {
   // n.b.: These methods all require that the channel monitor is
   // held when they are invoked.
   virtual void SendMessage(mozilla::UniquePtr<Message> msg) = 0;
-
-  // Synchronously close the connection, such that no further notifications will
-  // be delivered to the MessageChannel instance. Must be called with the
-  // channel monitor held.
-  virtual void Close() = 0;
+  virtual void SendClose() = 0;
 
   virtual bool IsClosed() const = 0;
 
@@ -80,7 +76,7 @@ class PortLink final : public MessageLink {
   virtual ~PortLink();
 
   void SendMessage(UniquePtr<Message> aMessage) override;
-  void Close() override;
+  void SendClose() override;
 
   bool IsClosed() const override;
 

ipc/glue/ProtocolUtils.cpp

@@ -389,20 +389,17 @@ Maybe<IProtocol*> IProtocol::ReadActor(IPC::MessageReader* aReader,
   return Some(listener);
 }
 
-void IProtocol::FatalError(const char* const aErrorMsg) {
+void IProtocol::FatalError(const char* const aErrorMsg) const {
   HandleFatalError(aErrorMsg);
 }
 
-void IProtocol::HandleFatalError(const char* aErrorMsg) {
+void IProtocol::HandleFatalError(const char* aErrorMsg) const {
   if (IProtocol* manager = Manager()) {
     manager->HandleFatalError(aErrorMsg);
     return;
   }
 
   mozilla::ipc::FatalError(aErrorMsg, mSide == ParentSide);
-  if (CanSend()) {
-    GetIPCChannel()->CloseWithError();
-  }
 }
 
 bool IProtocol::AllocShmem(size_t aSize, Shmem* aOutMem) {
@@ -633,8 +630,6 @@ void IToplevelProtocol::NotifyImpendingShutdown() {
 
 void IToplevelProtocol::Close() { GetIPCChannel()->Close(); }
 
-void IToplevelProtocol::CloseWithError() { GetIPCChannel()->CloseWithError(); }
-
 void IToplevelProtocol::SetReplyTimeoutMs(int32_t aTimeoutMs) {
   GetIPCChannel()->SetReplyTimeoutMs(aTimeoutMs);
 }

ipc/glue/ProtocolUtils.h

@@ -242,8 +242,8 @@ class IProtocol : public HasResultCodes {
   bool AllocUnsafeShmem(size_t aSize, Shmem* aOutMem);
   bool DeallocShmem(Shmem& aMem);
 
-  void FatalError(const char* const aErrorMsg);
-  virtual void HandleFatalError(const char* aErrorMsg);
+  void FatalError(const char* const aErrorMsg) const;
+  virtual void HandleFatalError(const char* aErrorMsg) const;
 
  protected:
   virtual ~IProtocol();
@@ -433,7 +433,6 @@ class IToplevelProtocol : public IProtocol {
   void NotifyImpendingShutdown();
 
   void Close();
-  void CloseWithError();
 
   void SetReplyTimeoutMs(int32_t aTimeoutMs);
 

ipc/ipdl/test/cxx/TestActorPunning.cpp

@@ -23,7 +23,7 @@ mozilla::ipc::IPCResult TestActorPunningParent::RecvPun(
 // By default, fatal errors kill the parent process, but this makes it
 // hard to test, so instead we use the previous behavior and kill the
 // child process.
-void TestActorPunningParent::HandleFatalError(const char* aErrorMsg) {
+void TestActorPunningParent::HandleFatalError(const char* aErrorMsg) const {
   if (!!strcmp(aErrorMsg, "Error deserializing 'PTestActorPunningSubParent'")) {
     fail("wrong fatal error");
   }

ipc/ipdl/test/cxx/TestActorPunning.h

@@ -38,7 +38,7 @@ class TestActorPunningParent : public PTestActorPunningParent {
     QuitParent();
   }
 
-  virtual void HandleFatalError(const char* aErrorMsg) override;
+  virtual void HandleFatalError(const char* aErrorMsg) const override;
 };
 
 class TestActorPunningPunnedParent : public PTestActorPunningPunnedParent {

ipc/ipdl/test/cxx/TestBadActor.cpp

@@ -19,7 +19,7 @@ void TestBadActorParent::Main() {
 // By default, fatal errors kill the parent process, but this makes it
 // hard to test, so instead we use the previous behavior and kill the
 // child process.
-void TestBadActorParent::HandleFatalError(const char* aErrorMsg) {
+void TestBadActorParent::HandleFatalError(const char* aErrorMsg) const {
   if (!!strcmp(aErrorMsg, "incoming message racing with actor deletion")) {
     fail("wrong fatal error");
   }

ipc/ipdl/test/cxx/TestBadActor.h

@@ -31,7 +31,7 @@ class TestBadActorParent : public PTestBadActorParent {
     QuitParent();
   }
 
-  virtual void HandleFatalError(const char* aErrorMsg) override;
+  virtual void HandleFatalError(const char* aErrorMsg) const override;
 
   PTestBadActorSubParent* AllocPTestBadActorSubParent();