Make SCTPtransport enter "closed" state when DTLStransport does.

Bug: webrtc:11090
Change-Id: I30e0b70387746d6c544ed1818f276569d4258cf4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/159888
Reviewed-by: Emad Omara <emadomara@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29810}

p2p/base/fake_dtls_transport.h

@@ -83,7 +83,10 @@ class FakeDtlsTransport : public DtlsTransportInternal {
     ice_transport_->SetReceiving(receiving);
     set_receiving(receiving);
   }
-  void SetDtlsState(DtlsTransportState state) { dtls_state_ = state; }
+  void SetDtlsState(DtlsTransportState state) {
+    dtls_state_ = state;
+    SignalDtlsState(this, dtls_state_);
+  }
 
   // Simulates the two DTLS transports connecting to each other.
   // If |asymmetric| is true this method only affects this FakeDtlsTransport.
@@ -108,12 +111,11 @@ class FakeDtlsTransport : public DtlsTransportInternal {
       if (!asymmetric) {
         dest->SetDestination(this, true);
       }
-      dtls_state_ = DTLS_TRANSPORT_CONNECTED;
       // If the |dtls_role_| is unset, set it to SSL_CLIENT by default.
       if (!dtls_role_) {
         dtls_role_ = std::move(rtc::SSL_CLIENT);
       }
-      SignalDtlsState(this, dtls_state_);
+      SetDtlsState(DTLS_TRANSPORT_CONNECTED);
       ice_transport_->SetDestination(
           static_cast<FakeIceTransport*>(dest->ice_transport()), asymmetric);
     } else {

pc/data_channel.cc

@@ -359,9 +359,10 @@ void DataChannel::OnTransportChannelCreated() {
   }
 }
 
-void DataChannel::OnTransportChannelDestroyed() {
-  // The SctpTransport is going away (for example, because the SCTP m= section
-  // was rejected), so we need to close abruptly.
+void DataChannel::OnTransportChannelClosed() {
+  // The SctpTransport is unusable (for example, because the SCTP m= section
+  // was rejected, or because the DTLS transport closed), so we need to close
+  // abruptly.
   CloseAbruptly();
 }
 

pc/data_channel.h

@@ -185,10 +185,10 @@ class DataChannel : public DataChannelInterface, public sigslot::has_slots<> {
   // Called when the transport channel is created.
   // Only needs to be called for SCTP data channels.
   void OnTransportChannelCreated();
-  // Called when the transport channel is destroyed.
+  // Called when the transport channel is unusable.
   // This method makes sure the DataChannel is disconnected and changes state
   // to kClosed.
-  void OnTransportChannelDestroyed();
+  void OnTransportChannelClosed();
 
   /*******************************************
    * The following methods are for RTP only. *

pc/data_channel_unittest.cc

@@ -610,10 +610,10 @@ TEST_F(SctpDataChannelTest, TransportDestroyedWhileDataBuffered) {
   provider_->set_send_blocked(true);
   EXPECT_TRUE(webrtc_data_channel_->Send(packet));
 
-  // Tell the data channel that its tranpsort is being destroyed.
+  // Tell the data channel that its transport is being destroyed.
   // It should then stop using the transport (allowing us to delete it) and
   // transition to the "closed" state.
-  webrtc_data_channel_->OnTransportChannelDestroyed();
+  webrtc_data_channel_->OnTransportChannelClosed();
   provider_.reset(nullptr);
   EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kClosed,
                  webrtc_data_channel_->state(), kDefaultTimeout);

pc/peer_connection.cc

@@ -1106,7 +1106,7 @@ void PeerConnection::DestroyAllChannels() {
       DestroyTransceiverChannel(transceiver);
     }
   }
-  DestroyDataChannel();
+  DestroyDataChannelTransport();
 }
 
 bool PeerConnection::Initialize(
@@ -3597,7 +3597,7 @@ RTCError PeerConnection::UpdateDataChannel(
   }
   if (content.rejected) {
     RTC_LOG(LS_INFO) << "Rejected data channel, mid=" << content.mid();
-    DestroyDataChannel();
+    DestroyDataChannelTransport();
   } else {
     if (!rtp_data_channel_ && !data_channel_transport_) {
       RTC_LOG(LS_INFO) << "Creating data channel, mid=" << content.mid();
@@ -5907,19 +5907,19 @@ void PeerConnection::OnSctpDataChannelClosed(DataChannel* channel) {
   }
 }
 
-void PeerConnection::OnDataChannelDestroyed() {
+void PeerConnection::OnTransportChannelClosed() {
   // Use a temporary copy of the RTP/SCTP DataChannel list because the
   // DataChannel may callback to us and try to modify the list.
   std::map<std::string, rtc::scoped_refptr<DataChannel>> temp_rtp_dcs;
   temp_rtp_dcs.swap(rtp_data_channels_);
   for (const auto& kv : temp_rtp_dcs) {
-    kv.second->OnTransportChannelDestroyed();
+    kv.second->OnTransportChannelClosed();
   }
 
   std::vector<rtc::scoped_refptr<DataChannel>> temp_sctp_dcs;
   temp_sctp_dcs.swap(sctp_data_channels_);
   for (const auto& channel : temp_sctp_dcs) {
-    channel->OnTransportChannelDestroyed();
+    channel->OnTransportChannelClosed();
   }
 }
 
@@ -6978,7 +6978,7 @@ void PeerConnection::RemoveUnusedChannels(const SessionDescription* desc) {
 
   const cricket::ContentInfo* data_info = cricket::GetFirstDataContent(desc);
   if (!data_info || data_info->rejected) {
-    DestroyDataChannel();
+    DestroyDataChannelTransport();
   }
 }
 
@@ -7709,9 +7709,9 @@ void PeerConnection::DestroyTransceiverChannel(
   }
 }
 
-void PeerConnection::DestroyDataChannel() {
+void PeerConnection::DestroyDataChannelTransport() {
   if (rtp_data_channel_) {
-    OnDataChannelDestroyed();
+    OnTransportChannelClosed();
     DestroyChannelInterface(rtp_data_channel_);
     rtp_data_channel_ = nullptr;
   }
@@ -7723,7 +7723,7 @@ void PeerConnection::DestroyDataChannel() {
   // rtc::Bind will cause "Pure virtual function called" error to appear.
 
   if (sctp_mid_) {
-    OnDataChannelDestroyed();
+    OnTransportChannelClosed();
     network_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
       RTC_DCHECK_RUN_ON(network_thread());
       TeardownDataChannelTransport_n();

pc/peer_connection.h

@@ -854,7 +854,8 @@ class PeerConnection : public PeerConnectionInternal,
   void OnSctpDataChannelClosed(DataChannel* channel)
       RTC_RUN_ON(signaling_thread());
 
-  void OnDataChannelDestroyed() RTC_RUN_ON(signaling_thread());
+  // Called when the transport for the data channels is closed or destroyed.
+  void OnTransportChannelClosed() RTC_RUN_ON(signaling_thread());
   // Called when a valid data channel OPEN message is received.
   void OnDataChannelOpenMessage(const std::string& label,
                                 const InternalDataChannelInit& config)
@@ -1169,14 +1170,19 @@ class PeerConnection : public PeerConnectionInternal,
   const std::string GetTransportName(const std::string& content_name)
       RTC_RUN_ON(signaling_thread());
 
+  // Functions for dealing with transports.
+  // Note that cricket code uses the term "channel" for what other code
+  // refers to as "transport".
+
   // Destroys and clears the BaseChannel associated with the given transceiver,
   // if such channel is set.
   void DestroyTransceiverChannel(
       rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
           transceiver);
 
-  // Destroys the RTP data channel and/or the SCTP data channel and clears it.
-  void DestroyDataChannel() RTC_RUN_ON(signaling_thread());
+  // Destroys the RTP data channel transport and/or the SCTP data channel
+  // transport and clears it.
+  void DestroyDataChannelTransport() RTC_RUN_ON(signaling_thread());
 
   // Destroys the given ChannelInterface.
   // The channel cannot be accessed after this method is called.

pc/sctp_transport.cc

@@ -86,6 +86,8 @@ void SctpTransport::SetDtlsTransport(
     if (internal_sctp_transport_) {
       if (transport) {
         internal_sctp_transport_->SetDtlsTransport(transport->internal());
+        transport->internal()->SignalDtlsState.connect(
+            this, &SctpTransport::OnDtlsStateChange);
         if (info_.state() == SctpTransportState::kNew) {
           next_state = SctpTransportState::kConnecting;
         }
@@ -162,4 +164,15 @@ void SctpTransport::OnAssociationChangeCommunicationUp() {
   UpdateInformation(SctpTransportState::kConnected);
 }
 
+void SctpTransport::OnDtlsStateChange(cricket::DtlsTransportInternal* transport,
+                                      cricket::DtlsTransportState state) {
+  RTC_DCHECK_RUN_ON(owner_thread_);
+  RTC_CHECK(transport == dtls_transport_->internal());
+  if (state == cricket::DTLS_TRANSPORT_CLOSED ||
+      state == cricket::DTLS_TRANSPORT_FAILED) {
+    UpdateInformation(SctpTransportState::kClosed);
+    // TODO(http://bugs.webrtc.org/11090): Close all the data channels
+  }
+}
+
 }  // namespace webrtc

pc/sctp_transport.h

@@ -65,6 +65,8 @@ class SctpTransport : public SctpTransportInterface,
   void OnAssociationChangeCommunicationUp();
   void OnInternalClosingProcedureStartedRemotely(int sid);
   void OnInternalClosingProcedureComplete(int sid);
+  void OnDtlsStateChange(cricket::DtlsTransportInternal* transport,
+                         cricket::DtlsTransportState state);
 
   // Note - owner_thread never changes, but can't be const if we do
   // Invoke() on it.

pc/sctp_transport_unittest.cc

@@ -195,4 +195,17 @@ TEST_F(SctpTransportTest, MaxChannelsSignalled) {
             *(observer_.LastReceivedInformation().MaxChannels()));
 }
 
+TEST_F(SctpTransportTest, CloseWhenTransportCloses) {
+  CreateTransport();
+  transport()->RegisterObserver(observer());
+  AddDtlsTransport();
+  CompleteSctpHandshake();
+  ASSERT_EQ_WAIT(SctpTransportState::kConnected, observer_.State(),
+                 kDefaultTimeout);
+  static_cast<cricket::FakeDtlsTransport*>(dtls_transport_->internal())
+      ->SetDtlsState(cricket::DTLS_TRANSPORT_CLOSED);
+  ASSERT_EQ_WAIT(SctpTransportState::kClosed, observer_.State(),
+                 kDefaultTimeout);
+}
+
 }  // namespace webrtc