diff --git a/api/test/fake_media_transport.h b/api/test/fake_media_transport.h index 1ef7c1f3e4..730d4973eb 100644 --- a/api/test/fake_media_transport.h +++ b/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. diff --git a/pc/jseptransport.cc b/pc/jseptransport.cc index 368b32f6f7..88d20a45b1 100644 --- a/pc/jseptransport.cc +++ b/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 diff --git a/pc/jseptransport.h b/pc/jseptransport.h index 8883218742..952f2ccb8f 100644 --- a/pc/jseptransport.h +++ b/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 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); }; diff --git a/pc/jseptransportcontroller.cc b/pc/jseptransportcontroller.cc index 19b502592d..1613e1e791 100644 --- a/pc/jseptransportcontroller.cc +++ b/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) { diff --git a/pc/jseptransportcontroller_unittest.cc b/pc/jseptransportcontroller_unittest.cc index 08b1f9bd13..1ab5d29c24 100644 --- a/pc/jseptransportcontroller_unittest.cc +++ b/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( + transport_controller_->GetDtlsTransport(kAudioMid1)); + auto fake_video_dtls = static_cast( + 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( + 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( + 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( + transport_controller_->GetDtlsTransport(kAudioMid1)); + auto fake_video_dtls = static_cast( + 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( + transport_controller_->GetMediaTransport(kAudioMid1)); + + media_transport->SetState(webrtc::MediaTransportState::kWritable); + + media_transport = static_cast( + 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();