Replace SignalClosed sigslot with absl::AnyInvocable

This restricts the interface such that only a single onclose handler
can be set and that only one OnClose() notification will be fired.

That behavior is the same as how the previous sigslot was being
used, but the difference is that, in addition to removing sigslot,
this pattern is now more explicitly checked in the design.

Bug: webrtc:11943
Change-Id: I469c8cab3d62544988c8145b326af60b06b76d8e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343340
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41920}

media/sctp/dcsctp_transport.cc

@@ -617,7 +617,13 @@ void DcSctpTransport::ConnectTransportSignals() {
                 const rtc::ReceivedPacket& packet) {
         OnTransportReadPacket(transport, packet);
       });
-  transport_->SignalClosed.connect(this, &DcSctpTransport::OnTransportClosed);
+  transport_->SetOnCloseCallback([this]() {
+    RTC_DCHECK_RUN_ON(network_thread_);
+    RTC_DLOG(LS_VERBOSE) << debug_name_ << "->OnTransportClosed().";
+    if (data_channel_sink_) {
+      data_channel_sink_->OnTransportClosed({});
+    }
+  });
 }
 
 void DcSctpTransport::DisconnectTransportSignals() {
@@ -627,7 +633,7 @@ void DcSctpTransport::DisconnectTransportSignals() {
   }
   transport_->SignalWritableState.disconnect(this);
   transport_->DeregisterReceivedPacketCallback(this);
-  transport_->SignalClosed.disconnect(this);
+  transport_->SetOnCloseCallback(nullptr);
 }
 
 void DcSctpTransport::OnTransportWritableState(
@@ -656,15 +662,6 @@ void DcSctpTransport::OnTransportReadPacket(
   }
 }
 
-void DcSctpTransport::OnTransportClosed(
-    rtc::PacketTransportInternal* transport) {
-  RTC_DCHECK_RUN_ON(network_thread_);
-  RTC_DLOG(LS_VERBOSE) << debug_name_ << "->OnTransportClosed().";
-  if (data_channel_sink_) {
-    data_channel_sink_->OnTransportClosed({});
-  }
-}
-
 void DcSctpTransport::MaybeConnectSocket() {
   if (transport_ && transport_->writable() && socket_ &&
       socket_->state() == dcsctp::SocketState::kClosed) {

media/sctp/dcsctp_transport.h

@@ -98,8 +98,6 @@ class DcSctpTransport : public cricket::SctpTransportInternal,
   void OnTransportWritableState(rtc::PacketTransportInternal* transport);
   void OnTransportReadPacket(rtc::PacketTransportInternal* transport,
                              const rtc::ReceivedPacket& packet);
-  void OnTransportClosed(rtc::PacketTransportInternal* transport);
-
   void MaybeConnectSocket();
 
   rtc::Thread* network_thread_;

p2p/base/dtls_transport.cc

@@ -733,7 +733,7 @@ void DtlsTransport::OnDtlsEvent(rtc::StreamInterface* dtls, int sig, int err) {
         RTC_LOG(LS_INFO) << ToString() << ": DTLS transport closed by remote";
         set_writable(false);
         set_dtls_state(webrtc::DtlsTransportState::kClosed);
-        SignalClosed(this);
+        NotifyOnClose();
       } else if (ret == rtc::SR_ERROR) {
         // Remote peer shut down the association with an error.
         RTC_LOG(LS_INFO)
@@ -742,7 +742,7 @@ void DtlsTransport::OnDtlsEvent(rtc::StreamInterface* dtls, int sig, int err) {
             << read_error;
         set_writable(false);
         set_dtls_state(webrtc::DtlsTransportState::kFailed);
-        SignalClosed(this);
+        NotifyOnClose();
       }
     } while (ret == rtc::SR_SUCCESS);
   }

p2p/base/fake_packet_transport.h

@@ -99,6 +99,7 @@ class FakePacketTransport : public PacketTransportInternal {
     SignalNetworkRouteChanged(network_route);
   }
 
+  using PacketTransportInternal::NotifyOnClose;
   using PacketTransportInternal::NotifyPacketReceived;
 
  private:

p2p/base/packet_transport_internal.cc

@@ -40,10 +40,25 @@ void PacketTransportInternal::DeregisterReceivedPacketCallback(void* id) {
   received_packet_callback_list_.RemoveReceivers(id);
 }
 
+void PacketTransportInternal::SetOnCloseCallback(
+    absl::AnyInvocable<void() &&> callback) {
+  RTC_DCHECK_RUN_ON(&network_checker_);
+  RTC_DCHECK(!on_close_ || !callback);
+  on_close_ = std::move(callback);
+}
+
 void PacketTransportInternal::NotifyPacketReceived(
     const rtc::ReceivedPacket& packet) {
   RTC_DCHECK_RUN_ON(&network_checker_);
   received_packet_callback_list_.Send(this, packet);
 }
 
+void PacketTransportInternal::NotifyOnClose() {
+  RTC_DCHECK_RUN_ON(&network_checker_);
+  if (on_close_) {
+    std::move(on_close_)();
+    on_close_ = nullptr;
+  }
+}
+
 }  // namespace rtc

p2p/base/packet_transport_internal.h

@@ -98,19 +98,21 @@ class RTC_EXPORT PacketTransportInternal : public sigslot::has_slots<> {
   sigslot::signal1<absl::optional<rtc::NetworkRoute>> SignalNetworkRouteChanged;
 
   // Signalled when the transport is closed.
-  sigslot::signal1<PacketTransportInternal*> SignalClosed;
+  void SetOnCloseCallback(absl::AnyInvocable<void() &&> callback);
 
  protected:
   PacketTransportInternal();
   ~PacketTransportInternal() override;
 
   void NotifyPacketReceived(const rtc::ReceivedPacket& packet);
+  void NotifyOnClose();
 
   webrtc::SequenceChecker network_checker_{webrtc::SequenceChecker::kDetached};
 
  private:
   webrtc::CallbackList<PacketTransportInternal*, const rtc::ReceivedPacket&>
       received_packet_callback_list_ RTC_GUARDED_BY(&network_checker_);
+  absl::AnyInvocable<void() &&> on_close_;
 };
 
 }  // namespace rtc

p2p/base/packet_transport_internal_unittest.cc

@@ -41,4 +41,15 @@ TEST(PacketTransportInternal,
   packet_transport.DeregisterReceivedPacketCallback(&receiver);
 }
 
+TEST(PacketTransportInternal, NotifiesOnceOnClose) {
+  rtc::FakePacketTransport packet_transport("test");
+  int call_count = 0;
+  packet_transport.SetOnCloseCallback([&]() { ++call_count; });
+  ASSERT_EQ(call_count, 0);
+  packet_transport.NotifyOnClose();
+  EXPECT_EQ(call_count, 1);
+  packet_transport.NotifyOnClose();
+  EXPECT_EQ(call_count, 1);  // Call count should not have increased.
+}
+
 }  // namespace