Plug-in media transport state listener

IceConnected state (transport state) now includes the state of the
MediaTransport.

This is a first change of two. Second change will add state change
signals to the PeerConnectionInterface informing separately about
ice+media transport vs ice+dtls.

Bug: webrtc:9719
Change-Id: I5731530073e8f26dfc8b188778d268b815da7052
Reviewed-on: https://webrtc-review.googlesource.com/c/108901
Commit-Queue: Peter Slatala <psla@webrtc.org>
Reviewed-by: Bjorn Mellem <mellem@webrtc.org>
Reviewed-by: Anton Sukhanov <sukhanov@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25473}

api/test/fake_media_transport.h

@@ -57,9 +57,6 @@ class FakeMediaTransport : public MediaTransportInterface {
   void SetTargetTransferRateObserver(
       webrtc::TargetTransferRateObserver* observer) override {}
 
-  void SetMediaTransportStateCallback(
-      MediaTransportStateCallback* callback) override {}
-
   RTCError SendData(int channel_id,
                     const SendDataParams& params,
                     const rtc::CopyOnWriteBuffer& buffer) override {
@@ -70,8 +67,20 @@ class FakeMediaTransport : public MediaTransportInterface {
 
   void SetDataSink(DataChannelSink* sink) override {}
 
+  void SetMediaTransportStateCallback(
+      MediaTransportStateCallback* callback) override {
+    state_callback_ = callback;
+  }
+
+  void SetState(webrtc::MediaTransportState state) {
+    if (state_callback_) {
+      state_callback_->OnStateChanged(state);
+    }
+  }
+
  private:
   const MediaTransportSettings settings_;
+  MediaTransportStateCallback* state_callback_;
 };
 
 // Fake media transport factory creates fake media transport.

pc/jseptransport.cc

@@ -115,9 +115,17 @@ JsepTransport::JsepTransport(
     RTC_DCHECK(!sdes_transport);
     dtls_srtp_transport_ = std::move(dtls_srtp_transport);
   }
+
+  if (media_transport_) {
+    media_transport_->SetMediaTransportStateCallback(this);
+  }
 }
 
-JsepTransport::~JsepTransport() {}
+JsepTransport::~JsepTransport() {
+  if (media_transport_) {
+    media_transport_->SetMediaTransportStateCallback(nullptr);
+  }
+}
 
 webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription(
     const JsepTransportDescription& jsep_description,
@@ -636,4 +644,12 @@ bool JsepTransport::GetTransportStats(DtlsTransportInternal* dtls_transport,
   return true;
 }
 
+void JsepTransport::OnStateChanged(webrtc::MediaTransportState state) {
+  // TODO(bugs.webrtc.org/9719) This method currently fires on the network
+  // thread, but media transport does not make such guarantees. We need to make
+  // sure this callback is guaranteed to be executed on the network thread.
+  media_transport_state_ = state;
+  SignalMediaTransportStateChanged();
+}
+
 }  // namespace cricket

pc/jseptransport.h

@@ -71,7 +71,8 @@ struct JsepTransportDescription {
 //
 // On Threading: JsepTransport performs work solely on the network thread, and
 // so its methods should only be called on the network thread.
-class JsepTransport : public sigslot::has_slots<> {
+class JsepTransport : public sigslot::has_slots<>,
+                      public webrtc::MediaTransportStateCallback {
  public:
   // |mid| is just used for log statements in order to identify the Transport.
   // Note that |local_certificate| is allowed to be null since a remote
@@ -171,11 +172,19 @@ class JsepTransport : public sigslot::has_slots<> {
     return media_transport_.get();
   }
 
+  // Returns the latest media transport state.
+  webrtc::MediaTransportState media_transport_state() const {
+    return media_transport_state_;
+  }
+
   // This is signaled when RTCP-mux becomes active and
   // |rtcp_dtls_transport_| is destroyed. The JsepTransportController will
   // handle the signal and update the aggregate transport states.
   sigslot::signal<> SignalRtcpMuxActive;
 
+  // This is signaled for changes in |media_transport_| state.
+  sigslot::signal<> SignalMediaTransportStateChanged;
+
   // TODO(deadbeef): The methods below are only public for testing. Should make
   // them utility functions or objects so they can be tested independently from
   // this class.
@@ -231,6 +240,9 @@ class JsepTransport : public sigslot::has_slots<> {
   bool GetTransportStats(DtlsTransportInternal* dtls_transport,
                          TransportStats* stats);
 
+  // Invoked whenever the state of the media transport changes.
+  void OnStateChanged(webrtc::MediaTransportState state) override;
+
   const std::string mid_;
   // needs-ice-restart bit as described in JSEP.
   bool needs_ice_restart_ = false;
@@ -257,6 +269,11 @@ class JsepTransport : public sigslot::has_slots<> {
   // Optional media transport (experimental).
   std::unique_ptr<webrtc::MediaTransportInterface> media_transport_;
 
+  // If |media_transport_| is provided, this variable represents the state of
+  // media transport.
+  webrtc::MediaTransportState media_transport_state_ =
+      webrtc::MediaTransportState::kPending;
+
   RTC_DISALLOW_COPY_AND_ASSIGN(JsepTransport);
 };
 

pc/jseptransportcontroller.cc

@@ -1041,6 +1041,8 @@ RTCError JsepTransportController::MaybeCreateJsepTransport(
           std::move(media_transport));
   jsep_transport->SignalRtcpMuxActive.connect(
       this, &JsepTransportController::UpdateAggregateStates_n);
+  jsep_transport->SignalMediaTransportStateChanged.connect(
+      this, &JsepTransportController::UpdateAggregateStates_n);
   SetTransportForMid(content_info.name, jsep_transport.get());
 
   jsep_transports_by_name_[content_info.name] = std::move(jsep_transport);
@@ -1234,6 +1236,10 @@ void JsepTransportController::UpdateAggregateStates_n() {
       PeerConnectionInterface::PeerConnectionState::kNew;
   cricket::IceGatheringState new_gathering_state = cricket::kIceGatheringNew;
   bool any_failed = false;
+
+  // TODO(http://bugs.webrtc.org/9719) If(when) media_transport disables
+  // dtls_transports entirely, the below line will have to be changed to account
+  // for the fact that dtls transports might be absent.
   bool all_connected = !dtls_transports.empty();
   bool all_completed = !dtls_transports.empty();
   bool any_gathering = false;
@@ -1262,6 +1268,31 @@ void JsepTransportController::UpdateAggregateStates_n() {
     dtls_state_counts[dtls->dtls_state()]++;
     ice_state_counts[dtls->ice_transport()->GetIceTransportState()]++;
   }
+
+  for (auto it = jsep_transports_by_name_.begin();
+       it != jsep_transports_by_name_.end(); ++it) {
+    auto jsep_transport = it->second.get();
+    if (!jsep_transport->media_transport()) {
+      continue;
+    }
+
+    // There is no 'kIceConnectionDisconnected', so we only need to handle
+    // connected and completed.
+    // We treat kClosed as failed, because if it happens before shutting down
+    // media transports it means that there was a failure.
+    // MediaTransportInterface allows to flip back and forth between kWritable
+    // and kPending, but there does not exist an implementation that does that,
+    // and the contract of jsep transport controller doesn't quite expect that.
+    // When this happens, we would go from connected to connecting state, but
+    // this may change in future.
+    any_failed |= jsep_transport->media_transport_state() ==
+                  webrtc::MediaTransportState::kClosed;
+    all_completed &= jsep_transport->media_transport_state() ==
+                     webrtc::MediaTransportState::kWritable;
+    all_connected &= jsep_transport->media_transport_state() ==
+                     webrtc::MediaTransportState::kWritable;
+  }
+
   if (any_failed) {
     new_connection_state = cricket::kIceConnectionFailed;
   } else if (all_completed) {

pc/jseptransportcontroller_unittest.cc

@@ -730,7 +730,8 @@ TEST_F(JsepTransportControllerTest, SignalConnectionStateFailed) {
   EXPECT_EQ(1, combined_connection_state_signal_count_);
 }
 
-TEST_F(JsepTransportControllerTest, SignalConnectionStateConnected) {
+TEST_F(JsepTransportControllerTest,
+       SignalConnectionStateConnectedNoMediaTransport) {
   CreateJsepTransportController(JsepTransportController::Config());
   auto description = CreateSessionDescriptionWithoutBundle();
   EXPECT_TRUE(transport_controller_
@@ -778,6 +779,93 @@ TEST_F(JsepTransportControllerTest, SignalConnectionStateConnected) {
   EXPECT_EQ(2, combined_connection_state_signal_count_);
 }
 
+TEST_F(JsepTransportControllerTest,
+       SignalConnectionStateConnectedWithMediaTransport) {
+  FakeMediaTransportFactory fake_media_transport_factory;
+  JsepTransportController::Config config;
+  config.media_transport_factory = &fake_media_transport_factory;
+  CreateJsepTransportController(config);
+  auto description = CreateSessionDescriptionWithoutBundle();
+  AddCryptoSettings(description.get());
+  EXPECT_TRUE(transport_controller_
+                  ->SetLocalDescription(SdpType::kOffer, description.get())
+                  .ok());
+
+  auto fake_audio_dtls = static_cast<FakeDtlsTransport*>(
+      transport_controller_->GetDtlsTransport(kAudioMid1));
+  auto fake_video_dtls = static_cast<FakeDtlsTransport*>(
+      transport_controller_->GetDtlsTransport(kVideoMid1));
+  fake_audio_dtls->SetWritable(true);
+  fake_video_dtls->SetWritable(true);
+  // Decreasing connection count from 2 to 1 triggers connection state event.
+  fake_audio_dtls->fake_ice_transport()->SetConnectionCount(2);
+  fake_audio_dtls->fake_ice_transport()->SetConnectionCount(1);
+  fake_video_dtls->fake_ice_transport()->SetConnectionCount(2);
+  fake_video_dtls->fake_ice_transport()->SetConnectionCount(1);
+  fake_audio_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED);
+  fake_video_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED);
+
+  // Still not connected, because we are waiting for media transport.
+  EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_,
+                 kTimeout);
+
+  FakeMediaTransport* media_transport = static_cast<FakeMediaTransport*>(
+      transport_controller_->GetMediaTransport(kAudioMid1));
+
+  media_transport->SetState(webrtc::MediaTransportState::kWritable);
+  EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_,
+                 kTimeout);
+
+  // Still waiting for the second media transport.
+  media_transport = static_cast<FakeMediaTransport*>(
+      transport_controller_->GetMediaTransport(kVideoMid1));
+  media_transport->SetState(webrtc::MediaTransportState::kWritable);
+
+  EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout);
+}
+
+TEST_F(JsepTransportControllerTest,
+       SignalConnectionStateFailedWhenMediaTransportClosed) {
+  FakeMediaTransportFactory fake_media_transport_factory;
+  JsepTransportController::Config config;
+  config.media_transport_factory = &fake_media_transport_factory;
+  CreateJsepTransportController(config);
+  auto description = CreateSessionDescriptionWithoutBundle();
+  AddCryptoSettings(description.get());
+  EXPECT_TRUE(transport_controller_
+                  ->SetLocalDescription(SdpType::kOffer, description.get())
+                  .ok());
+
+  auto fake_audio_dtls = static_cast<FakeDtlsTransport*>(
+      transport_controller_->GetDtlsTransport(kAudioMid1));
+  auto fake_video_dtls = static_cast<FakeDtlsTransport*>(
+      transport_controller_->GetDtlsTransport(kVideoMid1));
+  fake_audio_dtls->SetWritable(true);
+  fake_video_dtls->SetWritable(true);
+  // Decreasing connection count from 2 to 1 triggers connection state event.
+  fake_audio_dtls->fake_ice_transport()->SetConnectionCount(2);
+  fake_audio_dtls->fake_ice_transport()->SetConnectionCount(1);
+  fake_video_dtls->fake_ice_transport()->SetConnectionCount(2);
+  fake_video_dtls->fake_ice_transport()->SetConnectionCount(1);
+  fake_audio_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED);
+  fake_video_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED);
+
+  FakeMediaTransport* media_transport = static_cast<FakeMediaTransport*>(
+      transport_controller_->GetMediaTransport(kAudioMid1));
+
+  media_transport->SetState(webrtc::MediaTransportState::kWritable);
+
+  media_transport = static_cast<FakeMediaTransport*>(
+      transport_controller_->GetMediaTransport(kVideoMid1));
+
+  media_transport->SetState(webrtc::MediaTransportState::kWritable);
+
+  EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout);
+
+  media_transport->SetState(webrtc::MediaTransportState::kClosed);
+  EXPECT_EQ_WAIT(cricket::kIceConnectionFailed, connection_state_, kTimeout);
+}
+
 TEST_F(JsepTransportControllerTest, SignalConnectionStateComplete) {
   CreateJsepTransportController(JsepTransportController::Config());
   auto description = CreateSessionDescriptionWithoutBundle();