Merging upstream version 127.0.

Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>

9 files changed, 276 insertions, 219 deletions

diff --git a/ipc/glue/BackgroundImpl.cpp b/ipc/glue/BackgroundImpl.cpp
index bba09c261a..bf040a18a4 100644
--- a/ipc/glue/BackgroundImpl.cpp
+++ b/ipc/glue/BackgroundImpl.cpp
@@ -561,7 +561,12 @@ class ChildImpl final : public BackgroundChildImpl {
 #endif
   }
 
-  NS_INLINE_DECL_REFCOUNTING(ChildImpl, override)
+  // This type is threadsafe refcounted as actors managed by it may be destroyed
+  // after the thread it is bound to dies, and hold a reference to this object.
+  //
+  // It is _not_ safe to use this type or any methods on it from off of the
+  // thread it was created for.
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ChildImpl, override)
 
  private:
   // Forwarded from BackgroundChild.
diff --git a/ipc/glue/Endpoint.cpp b/ipc/glue/Endpoint.cpp
index 3391f8b359..728c37fa3f 100644
--- a/ipc/glue/Endpoint.cpp
+++ b/ipc/glue/Endpoint.cpp
@@ -57,7 +57,7 @@ UntypedManagedEndpoint::~UntypedManagedEndpoint() {
 }
 
 bool UntypedManagedEndpoint::BindCommon(IProtocol* aActor,
-                                        IProtocol* aManager) {
+                                        IRefCountedProtocol* aManager) {
   MOZ_ASSERT(aManager);
   if (!mInner) {
     NS_WARNING("Cannot bind to invalid endpoint");
@@ -88,7 +88,7 @@ bool UntypedManagedEndpoint::BindCommon(IProtocol* aActor,
   mInner.reset();
 
   // Our typed caller will insert the actor into the managed container.
-  aActor->SetManagerAndRegister(aManager, id);
+  MOZ_ALWAYS_TRUE(aActor->SetManagerAndRegister(aManager, id));
 
   aManager->GetIPCChannel()->Send(
       MakeUnique<IPC::Message>(id, MANAGED_ENDPOINT_BOUND_MESSAGE_TYPE));
diff --git a/ipc/glue/Endpoint.h b/ipc/glue/Endpoint.h
index d7eea94dfc..e4d32a7119 100644
--- a/ipc/glue/Endpoint.h
+++ b/ipc/glue/Endpoint.h
@@ -214,7 +214,7 @@ class UntypedManagedEndpoint {
 
   ~UntypedManagedEndpoint() noexcept;
 
-  bool BindCommon(IProtocol* aActor, IProtocol* aManager);
+  bool BindCommon(IProtocol* aActor, IRefCountedProtocol* aManager);
 
  private:
   friend struct IPDLParamTraits<UntypedManagedEndpoint>;
@@ -267,7 +267,8 @@ class ManagedEndpoint : public UntypedManagedEndpoint {
   ManagedEndpoint(const PrivateIPDLInterface&, IProtocol* aActor)
       : UntypedManagedEndpoint(aActor) {}
 
-  bool Bind(const PrivateIPDLInterface&, PFooSide* aActor, IProtocol* aManager,
+  bool Bind(const PrivateIPDLInterface&, PFooSide* aActor,
+            IRefCountedProtocol* aManager,
             ManagedContainer<PFooSide>& aContainer) {
     if (!BindCommon(aActor, aManager)) {
       return false;
diff --git a/ipc/glue/IPCMessageUtilsSpecializations.h b/ipc/glue/IPCMessageUtilsSpecializations.h
index e9742c5c74..ec0620d057 100644
--- a/ipc/glue/IPCMessageUtilsSpecializations.h
+++ b/ipc/glue/IPCMessageUtilsSpecializations.h
@@ -775,6 +775,7 @@ struct ParamTraits<mozilla::net::LinkHeader> {
     WriteParam(aWriter, aParam.mSizes);
     WriteParam(aWriter, aParam.mType);
     WriteParam(aWriter, aParam.mMedia);
+    WriteParam(aWriter, aParam.mAnchor);
     WriteParam(aWriter, aParam.mCrossOrigin);
     WriteParam(aWriter, aParam.mReferrerPolicy);
     WriteParam(aWriter, aParam.mAs);
@@ -812,6 +813,9 @@ struct ParamTraits<mozilla::net::LinkHeader> {
     if (!ReadParam(aReader, &aResult->mMedia)) {
       return false;
     }
+    if (!ReadParam(aReader, &aResult->mAnchor)) {
+      return false;
+    }
     if (!ReadParam(aReader, &aResult->mCrossOrigin)) {
       return false;
     }
diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp
index 16eb897175..544ce59b8e 100644
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -1604,9 +1604,13 @@ nsresult MessageChannel::MessageTask::Run() {
     return NS_OK;
   }
 
+  Channel()->AssertWorkerThread();
+  mMonitor->AssertSameMonitor(*Channel()->mMonitor);
+
 #ifdef FUZZING_SNAPSHOT
   if (!mIsFuzzMsg) {
-    if (fuzzing::Nyx::instance().started()) {
+    if (fuzzing::Nyx::instance().started() && XRE_IsParentProcess() &&
+        Channel()->IsCrossProcess()) {
       // Once we started fuzzing, prevent non-fuzzing tasks from being
       // run and potentially blocking worker threads.
       //
@@ -1622,8 +1626,6 @@ nsresult MessageChannel::MessageTask::Run() {
   }
 #endif
 
-  Channel()->AssertWorkerThread();
-  mMonitor->AssertSameMonitor(*Channel()->mMonitor);
   proxy = Channel()->Listener()->GetLifecycleProxy();
   Channel()->RunMessage(proxy, *this);
 
diff --git a/ipc/glue/NodeController.cpp b/ipc/glue/NodeController.cpp
index 7781771660..325eff1834 100644
--- a/ipc/glue/NodeController.cpp
+++ b/ipc/glue/NodeController.cpp
@@ -307,67 +307,78 @@ void NodeController::DropPeer(NodeName aNodeName) {
   mNode->LostConnectionToNode(aNodeName);
 }
 
-void NodeController::ForwardEvent(const NodeName& aNode,
-                                  UniquePtr<Event> aEvent) {
-  if (aNode == mName) {
-    (void)mNode->AcceptEvent(mName, std::move(aEvent));
-  } else {
-    // On Windows and macOS, messages holding HANDLEs or mach ports must be
-    // relayed via the broker process so it can transfer ownership.
-    bool needsRelay = false;
+void NodeController::ContactRemotePeer(const NodeName& aNode,
+                                       UniquePtr<Event> aEvent) {
+  // On Windows and macOS, messages holding HANDLEs or mach ports must be
+  // relayed via the broker process so it can transfer ownership.
+  bool needsRelay = false;
 #if defined(XP_WIN) || defined(XP_DARWIN)
-    if (!IsBroker() && aNode != kBrokerNodeName &&
-        aEvent->type() == Event::kUserMessage) {
-      auto* userEvent = static_cast<UserMessageEvent*>(aEvent.get());
-      needsRelay =
-          userEvent->HasMessage() &&
-          userEvent->GetMessage<IPC::Message>()->num_relayed_attachments() > 0;
-    }
+  if (aEvent && !IsBroker() && aNode != kBrokerNodeName &&
+      aEvent->type() == Event::kUserMessage) {
+    auto* userEvent = static_cast<UserMessageEvent*>(aEvent.get());
+    needsRelay =
+        userEvent->HasMessage() &&
+        userEvent->GetMessage<IPC::Message>()->num_relayed_attachments() > 0;
+  }
 #endif
 
-    UniquePtr<IPC::Message> message =
+  UniquePtr<IPC::Message> message;
+  if (aEvent) {
+    message =
         SerializeEventMessage(std::move(aEvent), needsRelay ? &aNode : nullptr);
     MOZ_ASSERT(message->is_relay() == needsRelay,
                "Message relay status set incorrectly");
+  }
 
-    RefPtr<NodeChannel> peer;
-    RefPtr<NodeChannel> broker;
-    bool needsIntroduction = false;
-    {
-      auto state = mState.Lock();
+  RefPtr<NodeChannel> peer;
+  RefPtr<NodeChannel> broker;
+  bool needsIntroduction = false;
+  bool needsBroker = needsRelay;
+  {
+    auto state = mState.Lock();
 
-      // Check if we know this peer. If we don't, we'll need to request an
-      // introduction.
-      peer = state->mPeers.Get(aNode);
-      if (!peer || needsRelay) {
-        if (IsBroker()) {
-          NODECONTROLLER_WARNING("Ignoring message '%s' to unknown peer %s",
-                                 message->name(), ToString(aNode).c_str());
-          return;
-        }
-
-        broker = state->mPeers.Get(kBrokerNodeName);
-        if (!broker) {
-          NODECONTROLLER_WARNING(
-              "Ignoring message '%s' to peer %s due to a missing broker",
-              message->name(), ToString(aNode).c_str());
-          return;
-        }
-
-        if (!needsRelay) {
-          auto& queue =
-              state->mPendingMessages.LookupOrInsertWith(aNode, [&]() {
-                needsIntroduction = true;
-                return Queue<UniquePtr<IPC::Message>, 64>{};
-              });
-          queue.Push(std::move(message));
-        }
+    // Check if we know this peer. If we don't, we'll need to request an
+    // introduction.
+    peer = state->mPeers.Get(aNode);
+    if (!peer) {
+      // We don't know the peer, check if we've already requested an
+      // introduction, or if we need to request a new one.
+      auto& queue = state->mPendingMessages.LookupOrInsertWith(aNode, [&]() {
+        needsIntroduction = true;
+        needsBroker = true;
+        return Queue<UniquePtr<IPC::Message>, 64>{};
+      });
+      // If we aren't relaying, queue up the message to be sent.
+      if (message && !needsRelay) {
+        queue.Push(std::move(message));
       }
     }
 
-    MOZ_ASSERT(!needsIntroduction || !needsRelay,
-               "Only one of the two should ever be set");
+    if (needsBroker && !IsBroker()) {
+      broker = state->mPeers.Get(kBrokerNodeName);
+    }
+  }
 
+  if (needsBroker && !broker) {
+    NODECONTROLLER_WARNING(
+        "Dropping message '%s'; no connection to unknown peer %s",
+        message ? message->name() : "<null>", ToString(aNode).c_str());
+    if (needsIntroduction) {
+      // We have no broker and will never be able to be introduced to this node.
+      // Queue a task to clean up any ports connected to it.
+      XRE_GetIOMessageLoop()->PostTask(NewRunnableMethod<NodeName>(
+          "NodeController::DropPeer", this, &NodeController::DropPeer, aNode));
+    }
+    return;
+  }
+
+  if (needsIntroduction) {
+    NODECONTROLLER_LOG(LogLevel::Info, "Requesting introduction to peer %s",
+                       ToString(aNode).c_str());
+    broker->RequestIntroduction(aNode);
+  }
+
+  if (message) {
     if (needsRelay) {
       NODECONTROLLER_LOG(LogLevel::Info,
                          "Relaying message '%s' for peer %s due to %" PRIu32
@@ -376,15 +387,22 @@ void NodeController::ForwardEvent(const NodeName& aNode,
                          message->num_relayed_attachments());
       MOZ_ASSERT(message->num_relayed_attachments() > 0 && broker);
       broker->SendEventMessage(std::move(message));
-    } else if (needsIntroduction) {
-      MOZ_ASSERT(broker);
-      broker->RequestIntroduction(aNode);
     } else if (peer) {
       peer->SendEventMessage(std::move(message));
     }
   }
 }
 
+void NodeController::ForwardEvent(const NodeName& aNode,
+                                  UniquePtr<Event> aEvent) {
+  MOZ_ASSERT(aEvent, "cannot forward null event");
+  if (aNode == mName) {
+    (void)mNode->AcceptEvent(mName, std::move(aEvent));
+  } else {
+    ContactRemotePeer(aNode, std::move(aEvent));
+  }
+}
+
 void NodeController::BroadcastEvent(UniquePtr<Event> aEvent) {
   UniquePtr<IPC::Message> message =
       SerializeEventMessage(std::move(aEvent), nullptr, BROADCAST_MESSAGE_TYPE);
@@ -415,6 +433,11 @@ void NodeController::PortStatusChanged(const PortRef& aPortRef) {
   }
 }
 
+void NodeController::ObserveRemoteNode(const NodeName& aNode) {
+  MOZ_ASSERT(aNode != mName);
+  ContactRemotePeer(aNode, nullptr);
+}
+
 void NodeController::OnEventMessage(const NodeName& aFromNode,
                                     UniquePtr<IPC::Message> aMessage) {
   AssertIOThread();
@@ -715,8 +738,6 @@ void NodeController::OnAcceptInvite(const NodeName& aFromNode,
   Invite invite;
   {
     auto state = mState.Lock();
-    MOZ_ASSERT(state->mPendingMessages.IsEmpty(),
-               "Shouldn't have pending messages in broker");
 
     // Try to remove the source node from our invites list and insert it into
     // our peers map under the new name.
@@ -840,6 +861,9 @@ void NodeController::CleanUp() {
       lostConnections.AppendElement(chan.GetKey());
       channelsToClose.AppendElement(chan.GetData());
     }
+    for (const auto& pending : state->mPendingMessages.Keys()) {
+      lostConnections.AppendElement(pending);
+    }
     for (const auto& invite : state->mInvites.Values()) {
       channelsToClose.AppendElement(invite.mChannel);
       portsToClose.AppendElement(invite.mToMerge);
diff --git a/ipc/glue/NodeController.h b/ipc/glue/NodeController.h
index 5356b85084..c1c2d33750 100644
--- a/ipc/glue/NodeController.h
+++ b/ipc/glue/NodeController.h
@@ -121,6 +121,11 @@ class NodeController final : public mojo::core::ports::NodeDelegate,
   // Stop communicating with this peer. Must be called on the IO thread.
   void DropPeer(NodeName aNodeName);
 
+  // Ensure that there is a direct connection to a remote node, requesting an
+  // introduction if there is not.
+  // If provided, will optionally send an event to the remote node.
+  void ContactRemotePeer(const NodeName& aNode, UniquePtr<Event> aEvent);
+
   // Message Handlers
   void OnEventMessage(const NodeName& aFromNode,
                       UniquePtr<IPC::Message> aMessage) override;
@@ -138,6 +143,7 @@ class NodeController final : public mojo::core::ports::NodeDelegate,
   void ForwardEvent(const NodeName& aNode, UniquePtr<Event> aEvent) override;
   void BroadcastEvent(UniquePtr<Event> aEvent) override;
   void PortStatusChanged(const PortRef& aPortRef) override;
+  void ObserveRemoteNode(const NodeName& aNode) override;
 
   const NodeName mName;
   const UniquePtr<Node> mNode;
diff --git a/ipc/glue/ProtocolUtils.cpp b/ipc/glue/ProtocolUtils.cpp
index b29221d476..42bab98320 100644
--- a/ipc/glue/ProtocolUtils.cpp
+++ b/ipc/glue/ProtocolUtils.cpp
@@ -251,12 +251,6 @@ ActorLifecycleProxy::ActorLifecycleProxy(IProtocol* aActor) : mActor(aActor) {
   MOZ_ASSERT(mActor->CanSend(),
              "Cannot create LifecycleProxy for non-connected actor!");
 
-  // Take a reference to our manager's lifecycle proxy to try to hold it &
-  // ensure it doesn't die before us.
-  if (mActor->mManager) {
-    mManager = mActor->mManager->mLifecycleProxy;
-  }
-
   // Record that we've taken our first reference to our actor.
   mActor->ActorAlloc();
 }
@@ -321,6 +315,7 @@ IProtocol::~IProtocol() {
   // Gecko, simply emit a warning and clear the weak backreference from our
   // LifecycleProxy back to us.
   if (mLifecycleProxy) {
+    MOZ_ASSERT(mLinkStatus != LinkStatus::Inactive);
     NS_WARNING(
         nsPrintfCString("Actor destructor for '%s%s' called before IPC "
                         "lifecycle complete!\n"
@@ -329,34 +324,13 @@ IProtocol::~IProtocol() {
             .get());
 
     mLifecycleProxy->mActor = nullptr;
-
-    // If we are somehow being destroyed while active, make sure that the
-    // existing IPC reference has been freed. If the status of the actor is
-    // `Destroyed`, the reference has already been freed, and we shouldn't free
-    // it a second time.
-    MOZ_ASSERT(mLinkStatus != LinkStatus::Inactive);
-    if (mLinkStatus != LinkStatus::Destroyed) {
-      NS_IF_RELEASE(mLifecycleProxy);
-    }
     mLifecycleProxy = nullptr;
   }
 }
 
 // The following methods either directly forward to the toplevel protocol, or
 // almost directly do.
-int32_t IProtocol::Register(IProtocol* aRouted) {
-  return mToplevel->Register(aRouted);
-}
-int32_t IProtocol::RegisterID(IProtocol* aRouted, int32_t aId) {
-  return mToplevel->RegisterID(aRouted, aId);
-}
 IProtocol* IProtocol::Lookup(int32_t aId) { return mToplevel->Lookup(aId); }
-void IProtocol::Unregister(int32_t aId) {
-  if (aId == mId) {
-    mId = kFreedActorId;
-  }
-  return mToplevel->Unregister(aId);
-}
 
 Shmem::SharedMemory* IProtocol::CreateSharedMemory(size_t aSize, bool aUnsafe,
                                                    int32_t* aId) {
@@ -383,11 +357,6 @@ nsISerialEventTarget* IProtocol::GetActorEventTarget() {
   return GetIPCChannel()->GetWorkerEventTarget();
 }
 
-void IProtocol::SetId(int32_t aId) {
-  MOZ_ASSERT(mId == aId || mLinkStatus == LinkStatus::Inactive);
-  mId = aId;
-}
-
 Maybe<IProtocol*> IProtocol::ReadActor(IPC::MessageReader* aReader,
                                        bool aNullable,
                                        const char* aActorDescription,
@@ -487,26 +456,60 @@ bool IProtocol::DeallocShmem(Shmem& aMem) {
   return ok;
 }
 
-void IProtocol::SetManager(IProtocol* aManager) {
+void IProtocol::SetManager(IRefCountedProtocol* aManager) {
   MOZ_RELEASE_ASSERT(!mManager || mManager == aManager);
   mManager = aManager;
   mToplevel = aManager->mToplevel;
 }
 
-void IProtocol::SetManagerAndRegister(IProtocol* aManager) {
-  // Set the manager prior to registering so registering properly inherits
-  // the manager's event target.
-  SetManager(aManager);
+bool IProtocol::SetManagerAndRegister(IRefCountedProtocol* aManager,
+                                      int32_t aId) {
+  MOZ_RELEASE_ASSERT(mLinkStatus == LinkStatus::Inactive,
+                     "Actor must be inactive to SetManagerAndRegister");
 
-  aManager->Register(this);
-}
+  // Set to `false` if the actor is to be torn down after registration.
+  bool success = true;
 
-void IProtocol::SetManagerAndRegister(IProtocol* aManager, int32_t aId) {
   // Set the manager prior to registering so registering properly inherits
   // the manager's event target.
   SetManager(aManager);
 
-  aManager->RegisterID(this, aId);
+  mId = aId == kNullActorId ? mToplevel->NextId() : aId;
+  while (mToplevel->mActorMap.Contains(mId)) {
+    // The ID already existing is an error case, but we want to proceed with
+    // registration so that we can tear down the actor cleanly - generate a new
+    // ID for that case.
+    NS_WARNING("Actor already exists with the selected ID!");
+    mId = mToplevel->NextId();
+    success = false;
+  }
+
+  RefPtr<ActorLifecycleProxy> proxy = ActorConnected();
+  mToplevel->mActorMap.InsertOrUpdate(mId, proxy);
+  MOZ_ASSERT(proxy->Get() == this);
+
+  // If our manager is already dying, mark ourselves as doomed as well.
+  if (aManager && aManager->mLinkStatus != LinkStatus::Connected) {
+    mLinkStatus = LinkStatus::Doomed;
+    if (aManager->mLinkStatus != LinkStatus::Doomed) {
+      // Our manager is already fully dead, make sure we call
+      // `ActorDisconnected`.
+      success = false;
+    }
+  }
+
+  // If setting the manager failed, call `ActorDisconnected` and return false.
+  if (!success) {
+    ActorDisconnected(FailedConstructor);
+    MOZ_ASSERT(mLinkStatus == LinkStatus::Destroyed);
+    return false;
+  }
+  return true;
+}
+
+void IProtocol::UnlinkManager() {
+  mToplevel = nullptr;
+  mManager = nullptr;
 }
 
 bool IProtocol::ChannelSend(UniquePtr<IPC::Message> aMsg) {
@@ -540,9 +543,9 @@ void IProtocol::WarnMessageDiscarded(IPC::Message* aMsg) {
 }
 #endif
 
-void IProtocol::ActorConnected() {
+already_AddRefed<ActorLifecycleProxy> IProtocol::ActorConnected() {
   if (mLinkStatus != LinkStatus::Inactive) {
-    return;
+    return nullptr;
   }
 
 #ifdef FUZZING_SNAPSHOT
@@ -552,73 +555,81 @@ void IProtocol::ActorConnected() {
   mLinkStatus = LinkStatus::Connected;
 
   MOZ_ASSERT(!mLifecycleProxy, "double-connecting live actor");
-  mLifecycleProxy = new ActorLifecycleProxy(this);
-  NS_ADDREF(mLifecycleProxy);  // Reference freed in DestroySubtree();
+  RefPtr<ActorLifecycleProxy> proxy = new ActorLifecycleProxy(this);
+  mLifecycleProxy = proxy;
+  return proxy.forget();
 }
 
-void IProtocol::DoomSubtree() {
-  MOZ_ASSERT(CanSend(), "dooming non-connected actor");
-  MOZ_ASSERT(mLifecycleProxy, "dooming zombie actor");
-
-  nsTArray<RefPtr<ActorLifecycleProxy>> managed;
-  AllManagedActors(managed);
-  for (ActorLifecycleProxy* proxy : managed) {
-    // Guard against actor being disconnected or destroyed during previous Doom
-    IProtocol* actor = proxy->Get();
-    if (actor && actor->CanSend()) {
-      actor->DoomSubtree();
-    }
+void IProtocol::ActorDisconnected(ActorDestroyReason aWhy) {
+  MOZ_ASSERT(mLifecycleProxy, "destroying zombie actor");
+  // If the actor has already been marked as `Destroyed`, there's nothing to do.
+  if (mLinkStatus != LinkStatus::Connected &&
+      mLinkStatus != LinkStatus::Doomed) {
+    return;
   }
 
-  // ActorDoom is called immediately before changing state, this allows messages
-  // to be sent during ActorDoom immediately before the channel is closed and
-  // sending messages is disabled.
-  ActorDoom();
-  mLinkStatus = LinkStatus::Doomed;
-}
+  // Mark the entire subtree as doomed so that no further messages can be
+  // sent/recieved, and newly created managed actors are immediately marked as
+  // doomed on creation.
+  DoomSubtree();
 
-void IProtocol::DestroySubtree(ActorDestroyReason aWhy) {
-  MOZ_ASSERT(CanRecv(), "destroying non-connected actor");
-  MOZ_ASSERT(mLifecycleProxy, "destroying zombie actor");
+  // Perform the steps to fully destroy an actor after it has been unregistered
+  // from its manager.
+  auto doActorDestroy = [toplevel = mToplevel, ipcChannel = GetIPCChannel()](
+                            IProtocol* actor, ActorDestroyReason why) {
+    MOZ_ASSERT(actor->mLinkStatus == LinkStatus::Doomed,
+               "Actor must be doomed when calling doActorDestroy");
+    MOZ_ASSERT(actor->AllManagedActorsCount() == 0,
+               "All managed actors must have been destroyed first");
+
+    // Mark the actor as Destroyed, ensuring we can't re-enter `ActorDestroy`,
+    // even if an callback spins a nested event loop.
+    actor->mLinkStatus = LinkStatus::Destroyed;
 
 #ifdef FUZZING_SNAPSHOT
-  fuzzing::IPCFuzzController::instance().OnActorDestroyed(this);
+    fuzzing::IPCFuzzController::instance().OnActorDestroyed(actor);
 #endif
 
-  int32_t id = Id();
+    int32_t id = actor->mId;
+    if (IProtocol* manager = actor->Manager()) {
+      actor->mId = kFreedActorId;
+      auto entry = toplevel->mActorMap.Lookup(id);
+      MOZ_DIAGNOSTIC_ASSERT(entry && *entry == actor->GetLifecycleProxy(),
+                            "ID must be present and reference this actor");
+      entry.Remove();
+      manager->RemoveManagee(actor->GetProtocolId(), actor);
+    }
+
+    ipcChannel->RejectPendingResponsesForActor(id);
+    actor->ActorDestroy(why);
+  };
 
-  // If we're a managed actor, unregister from our manager
-  if (Manager()) {
-    Unregister(id);
-  }
+  // Hold all ActorLifecycleProxy instances for managed actors until we return.
+  nsTArray<RefPtr<ActorLifecycleProxy>> proxyHolder;
+  proxyHolder.AppendElement(GetLifecycleProxy());
 
-  // Destroy subtree
+  // Invoke `ActorDestroy` for all managed actors in the subtree. These are
+  // handled one at a time, so that new actors which are potentially registered
+  // during `ActorDestroy` callbacks are not missed.
   ActorDestroyReason subtreeWhy = aWhy;
   if (aWhy == Deletion || aWhy == FailedConstructor) {
     subtreeWhy = AncestorDeletion;
   }
-
-  nsTArray<RefPtr<ActorLifecycleProxy>> managed;
-  AllManagedActors(managed);
-  for (ActorLifecycleProxy* proxy : managed) {
-    // Guard against actor being disconnected or destroyed during previous
-    // Destroy
-    IProtocol* actor = proxy->Get();
-    if (actor && actor->CanRecv()) {
-      actor->DestroySubtree(subtreeWhy);
+  while (IProtocol* actor = PeekManagedActor()) {
+    // If the selected actor manages other actors, destroy those first.
+    while (IProtocol* inner = actor->PeekManagedActor()) {
+      actor = inner;
     }
-  }
 
-  // Ensure that we don't send any messages while we're calling `ActorDestroy`
-  // by setting our state to `Doomed`.
-  mLinkStatus = LinkStatus::Doomed;
+    proxyHolder.AppendElement(actor->GetLifecycleProxy());
+    doActorDestroy(actor, subtreeWhy);
+  }
 
-  // The actor is being destroyed, reject any pending responses, invoke
-  // `ActorDestroy` to destroy it, and then clear our status to
-  // `LinkStatus::Destroyed`.
-  GetIPCChannel()->RejectPendingResponsesForActor(id);
-  ActorDestroy(aWhy);
-  mLinkStatus = LinkStatus::Destroyed;
+  // Destroy ourselves if we were not not otherwise destroyed while destroying
+  // managed actors.
+  if (mLinkStatus == LinkStatus::Doomed) {
+    doActorDestroy(this, aWhy);
+  }
 }
 
 IToplevelProtocol::IToplevelProtocol(const char* aName, ProtocolId aProtoId,
@@ -689,28 +700,11 @@ int32_t IToplevelProtocol::NextId() {
   return (++mLastLocalId << 2) | tag;
 }
 
-int32_t IToplevelProtocol::Register(IProtocol* aRouted) {
-  if (aRouted->Id() != kNullActorId && aRouted->Id() != kFreedActorId) {
-    // If there's already an ID, just return that.
-    return aRouted->Id();
+IProtocol* IToplevelProtocol::Lookup(int32_t aId) {
+  if (auto entry = mActorMap.Lookup(aId)) {
+    return entry.Data()->Get();
   }
-  return RegisterID(aRouted, NextId());
-}
-
-int32_t IToplevelProtocol::RegisterID(IProtocol* aRouted, int32_t aId) {
-  aRouted->SetId(aId);
-  aRouted->ActorConnected();
-  MOZ_ASSERT(!mActorMap.Contains(aId), "Don't insert with an existing ID");
-  mActorMap.InsertOrUpdate(aId, aRouted);
-  return aId;
-}
-
-IProtocol* IToplevelProtocol::Lookup(int32_t aId) { return mActorMap.Get(aId); }
-
-void IToplevelProtocol::Unregister(int32_t aId) {
-  MOZ_ASSERT(mActorMap.Contains(aId),
-             "Attempting to remove an ID not in the actor map");
-  mActorMap.Remove(aId);
+  return nullptr;
 }
 
 Shmem::SharedMemory* IToplevelProtocol::CreateSharedMemory(size_t aSize,
diff --git a/ipc/glue/ProtocolUtils.h b/ipc/glue/ProtocolUtils.h
index 2800a41c91..995ef2f256 100644
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -124,12 +124,16 @@ enum class LinkStatus : uint8_t {
   // A live link is connected to the other side of this actor.
   Connected,
 
-  // The link has begun being destroyed. Messages may still be received, but
-  // cannot be sent. (exception: sync replies may be sent while Doomed).
+  // The link has begun being destroyed. Messages may no longer be sent. The
+  // ActorDestroy method is queued to be called, but has not been invoked yet,
+  // as managed actors still need to be destroyed first.
+  //
+  // NOTE: While no new IPC can be received at this point, `CanRecv` will still
+  // be true until `LinkStatus::Destroyed`.
   Doomed,
 
-  // The link has been destroyed, and messages will no longer be sent or
-  // received.
+  // The actor has been destroyed, and ActorDestroy has been called, however an
+  // ActorLifecycleProxy still holds a reference to the actor.
   Destroyed,
 };
 
@@ -171,12 +175,8 @@ class IProtocol : public HasResultCodes {
   IToplevelProtocol* ToplevelProtocol() { return mToplevel; }
   const IToplevelProtocol* ToplevelProtocol() const { return mToplevel; }
 
-  // The following methods either directly forward to the toplevel protocol, or
-  // almost directly do.
-  int32_t Register(IProtocol* aRouted);
-  int32_t RegisterID(IProtocol* aRouted, int32_t aId);
+  // Lookup() is forwarded directly to the toplevel protocol.
   IProtocol* Lookup(int32_t aId);
-  void Unregister(int32_t aId);
 
   Shmem::SharedMemory* CreateSharedMemory(size_t aSize, bool aUnsafe,
                                           int32_t* aId);
@@ -197,13 +197,16 @@ class IProtocol : public HasResultCodes {
   const char* GetProtocolName() const { return ProtocolIdToName(mProtocolId); }
 
   int32_t Id() const { return mId; }
-  IProtocol* Manager() const { return mManager; }
+  IRefCountedProtocol* Manager() const { return mManager; }
 
   ActorLifecycleProxy* GetLifecycleProxy() { return mLifecycleProxy; }
   WeakActorLifecycleProxy* GetWeakLifecycleProxy();
 
   Side GetSide() const { return mSide; }
   bool CanSend() const { return mLinkStatus == LinkStatus::Connected; }
+
+  // Returns `true` for an active actor until the actor's `ActorDestroy` method
+  // has been called.
   bool CanRecv() const {
     return mLinkStatus == LinkStatus::Connected ||
            mLinkStatus == LinkStatus::Doomed;
@@ -235,17 +238,20 @@ class IProtocol : public HasResultCodes {
   friend class IPDLResolverInner;
   friend class UntypedManagedEndpoint;
 
-  void SetId(int32_t aId);
+  // We have separate functions because the accessibility code and BrowserParent
+  // manually calls SetManager.
+  void SetManager(IRefCountedProtocol* aManager);
 
-  // We have separate functions because the accessibility code manually
-  // calls SetManager.
-  void SetManager(IProtocol* aManager);
+  // Clear `mManager` and `mToplevel` to nullptr. Only intended to be called
+  // within the unlink implementation of cycle collected IPDL actors with cycle
+  // collected managers.
+  void UnlinkManager();
 
   // Sets the manager for the protocol and registers the protocol with
   // its manager, setting up channels for the protocol as well.  Not
   // for use outside of IPDL.
-  void SetManagerAndRegister(IProtocol* aManager);
-  void SetManagerAndRegister(IProtocol* aManager, int32_t aId);
+  bool SetManagerAndRegister(IRefCountedProtocol* aManager,
+                             int32_t aId = kNullActorId);
 
   // Helpers for calling `Send` on our underlying IPC channel.
   bool ChannelSend(UniquePtr<IPC::Message> aMsg);
@@ -265,28 +271,31 @@ class IProtocol : public HasResultCodes {
     }
   }
 
-  // Collect all actors managed by this object in an array. To make this safer
-  // to iterate, `ActorLifecycleProxy` references are returned rather than raw
-  // actor pointers.
-  virtual void AllManagedActors(
-      nsTArray<RefPtr<ActorLifecycleProxy>>& aActors) const = 0;
-
   virtual uint32_t AllManagedActorsCount() const = 0;
 
   // Internal method called when the actor becomes connected.
-  void ActorConnected();
+  already_AddRefed<ActorLifecycleProxy> ActorConnected();
+
+  // Internal method called when actor becomes disconnected.
+  void ActorDisconnected(ActorDestroyReason aWhy);
+
+  // Called by DoomSubtree on each managed actor to mark it as Doomed and
+  // prevent further IPC.
+  void SetDoomed() {
+    MOZ_ASSERT(mLinkStatus == LinkStatus::Connected ||
+                   mLinkStatus == LinkStatus::Doomed,
+               "Invalid link status for SetDoomed");
+    mLinkStatus = LinkStatus::Doomed;
+  }
+  virtual void DoomSubtree() = 0;
 
-  // Called immediately before setting the actor state to doomed, and triggering
-  // async actor destruction. Messages may be sent from this callback, but no
-  // later.
-  // FIXME(nika): This is currently unused!
-  virtual void ActorDoom() {}
-  void DoomSubtree();
+  // Internal function returning an arbitrary directly managed actor. Used to
+  // identify managed actors to destroy when tearing down an actor tree.
+  virtual IProtocol* PeekManagedActor() = 0;
 
   // Called when the actor has been destroyed due to an error, a __delete__
   // message, or a __doom__ reply.
   virtual void ActorDestroy(ActorDestroyReason aWhy) {}
-  void DestroySubtree(ActorDestroyReason aWhy);
 
   // Called when IPC has acquired its first reference to the actor. This method
   // may take references which will later be freed by `ActorDealloc`.
@@ -313,7 +322,7 @@ class IProtocol : public HasResultCodes {
   Side mSide;
   LinkStatus mLinkStatus;
   ActorLifecycleProxy* mLifecycleProxy;
-  IProtocol* mManager;
+  RefPtr<IRefCountedProtocol> mManager;
   IToplevelProtocol* mToplevel;
 };
 
@@ -414,6 +423,7 @@ class IRefCountedProtocol : public IProtocol {
  * this protocol actor.
  */
 class IToplevelProtocol : public IRefCountedProtocol {
+  friend class IProtocol;
   template <class PFooSide>
   friend class Endpoint;
 
@@ -423,12 +433,8 @@ class IToplevelProtocol : public IRefCountedProtocol {
   ~IToplevelProtocol() = default;
 
  public:
-  // Shadow methods on IProtocol which are implemented directly on toplevel
-  // actors.
-  int32_t Register(IProtocol* aRouted);
-  int32_t RegisterID(IProtocol* aRouted, int32_t aId);
+  // Shadows the method on IProtocol, which will forward to the top.
   IProtocol* Lookup(int32_t aId);
-  void Unregister(int32_t aId);
 
   Shmem::SharedMemory* CreateSharedMemory(size_t aSize, bool aUnsafe,
                                           int32_t* aId);
@@ -441,8 +447,6 @@ class IToplevelProtocol : public IRefCountedProtocol {
 
   void SetOtherProcessId(base::ProcessId aOtherPid);
 
-  virtual void OnChannelClose() = 0;
-  virtual void OnChannelError() = 0;
   virtual void ProcessingError(Result aError, const char* aMsgName) {}
 
   bool Open(ScopedPort aPort, const nsID& aMessageChannelId,
@@ -508,7 +512,26 @@ class IToplevelProtocol : public IRefCountedProtocol {
 
   virtual void OnChannelReceivedMessage(const Message& aMsg) {}
 
-  void OnIPCChannelOpened() { ActorConnected(); }
+  // MessageChannel lifecycle callbacks.
+  void OnIPCChannelOpened() {
+    // Leak the returned ActorLifecycleProxy reference. It will be destroyed in
+    // `OnChannelClose` or `OnChannelError`.
+    Unused << ActorConnected();
+  }
+  void OnChannelClose() {
+    // Re-acquire the ActorLifecycleProxy reference acquired in
+    // OnIPCChannelOpened.
+    RefPtr<ActorLifecycleProxy> proxy = dont_AddRef(GetLifecycleProxy());
+    ActorDisconnected(NormalShutdown);
+    DeallocShmems();
+  }
+  void OnChannelError() {
+    // Re-acquire the ActorLifecycleProxy reference acquired in
+    // OnIPCChannelOpened.
+    RefPtr<ActorLifecycleProxy> proxy = dont_AddRef(GetLifecycleProxy());
+    ActorDisconnected(AbnormalShutdown);
+    DeallocShmems();
+  }
 
   base::ProcessId OtherPidMaybeInvalid() const { return mOtherPid; }
 
@@ -523,7 +546,7 @@ class IToplevelProtocol : public IRefCountedProtocol {
   // NOTE NOTE NOTE
   // Used to be on mState
   int32_t mLastLocalId;
-  IDMap<IProtocol*> mActorMap;
+  IDMap<RefPtr<ActorLifecycleProxy>> mActorMap;
   IDMap<RefPtr<Shmem::SharedMemory>> mShmemMap;
 
   MessageChannel mChannel;
@@ -652,10 +675,6 @@ class ActorLifecycleProxy {
 
   IProtocol* MOZ_NON_OWNING_REF mActor;
 
-  // Hold a reference to the actor's manager's ActorLifecycleProxy to help
-  // prevent it from dying while we're still alive!
-  RefPtr<ActorLifecycleProxy> mManager;
-
   // When requested, the current self-referencing weak reference for this
   // ActorLifecycleProxy.
   RefPtr<WeakActorLifecycleProxy> mWeakProxy;
@@ -753,6 +772,8 @@ class ManagedContainer {
     }
   }
 
+  Protocol* Peek() { return mArray.IsEmpty() ? nullptr : mArray.LastElement(); }
+
   void Clear() { mArray.Clear(); }
 
  private: