diff --git a/api/peerconnectioninterface.cc b/api/peerconnectioninterface.cc index b4148d7480..f2953b5b0c 100644 --- a/api/peerconnectioninterface.cc +++ b/api/peerconnectioninterface.cc @@ -159,6 +159,11 @@ RTCError PeerConnectionInterface::SetBitrate( return SetBitrate(bitrate); } +PeerConnectionInterface::PeerConnectionState +PeerConnectionInterface::peer_connection_state() { + return PeerConnectionInterface::PeerConnectionState::kNew; +} + bool PeerConnectionInterface::StartRtcEventLog(rtc::PlatformFile file, int64_t max_size_bytes) { return false; diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h index d0f96a02f1..a8063b101e 100644 --- a/api/peerconnectioninterface.h +++ b/api/peerconnectioninterface.h @@ -160,7 +160,7 @@ enum class SdpSemantics { kPlanB, kUnifiedPlan }; class PeerConnectionInterface : public rtc::RefCountInterface { public: - // See https://w3c.github.io/webrtc-pc/#state-definitions + // See https://w3c.github.io/webrtc-pc/#dom-rtcsignalingstate enum SignalingState { kStable, kHaveLocalOffer, @@ -170,12 +170,24 @@ class PeerConnectionInterface : public rtc::RefCountInterface { kClosed, }; + // See https://w3c.github.io/webrtc-pc/#dom-rtcicegatheringstate enum IceGatheringState { kIceGatheringNew, kIceGatheringGathering, kIceGatheringComplete }; + // See https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnectionstate + enum class PeerConnectionState { + kNew, + kConnecting, + kConnected, + kDisconnected, + kFailed, + kClosed, + }; + + // See https://w3c.github.io/webrtc-pc/#dom-rtciceconnectionstate enum IceConnectionState { kIceConnectionNew, kIceConnectionChecking, @@ -978,11 +990,13 @@ class PeerConnectionInterface : public rtc::RefCountInterface { virtual SignalingState signaling_state() = 0; // Returns the aggregate state of all ICE *and* DTLS transports. - // TODO(deadbeef): Implement "PeerConnectionState" according to the standard, - // to aggregate ICE+DTLS state, and change the scope of IceConnectionState to - // be just the ICE layer. See: crbug.com/webrtc/6145 + // TODO(jonasolsson): Replace with standardized_ice_connection_state once it + // is ready, see crbug.com/webrtc/6145 virtual IceConnectionState ice_connection_state() = 0; + // Returns the aggregated state of all ICE and DTLS transports. + virtual PeerConnectionState peer_connection_state(); + virtual IceGatheringState ice_gathering_state() = 0; // Starts RtcEventLog using existing file. Takes ownership of |file| and @@ -1051,6 +1065,10 @@ class PeerConnectionObserver { virtual void OnIceConnectionChange( PeerConnectionInterface::IceConnectionState new_state) = 0; + // Called any time the PeerConnectionState changes. + virtual void OnConnectionChange( + PeerConnectionInterface::PeerConnectionState new_state) {} + // Called any time the IceGatheringState changes. virtual void OnIceGatheringChange( PeerConnectionInterface::IceGatheringState new_state) = 0; diff --git a/p2p/base/fakedtlstransport.h b/p2p/base/fakedtlstransport.h index 085d6e0ca1..f28f7273c6 100644 --- a/p2p/base/fakedtlstransport.h +++ b/p2p/base/fakedtlstransport.h @@ -83,6 +83,7 @@ class FakeDtlsTransport : public DtlsTransportInternal { ice_transport_->SetReceiving(receiving); set_receiving(receiving); } + void SetDtlsState(DtlsTransportState state) { dtls_state_ = state; } // Simulates the two DTLS transports connecting to each other. // If |asymmetric| is true this method only affects this FakeDtlsTransport. diff --git a/p2p/base/fakeicetransport.h b/p2p/base/fakeicetransport.h index 4ba0a2d2d2..646aed45c5 100644 --- a/p2p/base/fakeicetransport.h +++ b/p2p/base/fakeicetransport.h @@ -115,6 +115,15 @@ class FakeIceTransport : public IceTransportInternal { } webrtc::IceTransportState GetIceTransportState() const override { + if (connection_count_ == 0) { + return had_connection_ ? webrtc::IceTransportState::kFailed + : webrtc::IceTransportState::kNew; + } + + if (connection_count_ == 1) { + return webrtc::IceTransportState::kCompleted; + } + return webrtc::IceTransportState::kConnected; } diff --git a/pc/dtlssrtptransport.cc b/pc/dtlssrtptransport.cc index e31b2f5872..2835d34a46 100644 --- a/pc/dtlssrtptransport.cc +++ b/pc/dtlssrtptransport.cc @@ -301,6 +301,8 @@ void DtlsSrtpTransport::OnDtlsState(cricket::DtlsTransportInternal* transport, RTC_DCHECK(transport == rtp_dtls_transport_ || transport == rtcp_dtls_transport_); + SignalDtlsStateChange(); + if (state != cricket::DTLS_TRANSPORT_CONNECTED) { ResetParams(); return; diff --git a/pc/dtlssrtptransport.h b/pc/dtlssrtptransport.h index 498f02e917..cac560e79a 100644 --- a/pc/dtlssrtptransport.h +++ b/pc/dtlssrtptransport.h @@ -42,7 +42,8 @@ class DtlsSrtpTransport : public SrtpTransport { void UpdateRecvEncryptedHeaderExtensionIds( const std::vector& recv_extension_ids); - sigslot::signal2 SignalDtlsSrtpSetupFailure; + sigslot::signal SignalDtlsSrtpSetupFailure; + sigslot::signal<> SignalDtlsStateChange; RTCError SetSrtpSendKey(const cricket::CryptoParams& params) override { return RTCError(RTCErrorType::UNSUPPORTED_OPERATION, diff --git a/pc/jseptransportcontroller.cc b/pc/jseptransportcontroller.cc index 991d769e71..f00c22d965 100644 --- a/pc/jseptransportcontroller.cc +++ b/pc/jseptransportcontroller.cc @@ -488,6 +488,8 @@ JsepTransportController::CreateDtlsSrtpTransport( rtcp_dtls_transport); dtls_srtp_transport->SetActiveResetSrtpParams( config_.active_reset_srtp_params); + dtls_srtp_transport->SignalDtlsStateChange.connect( + this, &JsepTransportController::UpdateAggregateStates_n); return dtls_srtp_transport; } @@ -1158,12 +1160,20 @@ void JsepTransportController::UpdateAggregateStates_n() { auto dtls_transports = GetDtlsTransports(); cricket::IceConnectionState new_connection_state = cricket::kIceConnectionConnecting; + PeerConnectionInterface::IceConnectionState new_ice_connection_state = + PeerConnectionInterface::IceConnectionState::kIceConnectionNew; + PeerConnectionInterface::PeerConnectionState new_combined_state = + PeerConnectionInterface::PeerConnectionState::kNew; cricket::IceGatheringState new_gathering_state = cricket::kIceGatheringNew; bool any_failed = false; bool all_connected = !dtls_transports.empty(); bool all_completed = !dtls_transports.empty(); bool any_gathering = false; bool all_done_gathering = !dtls_transports.empty(); + + std::map ice_state_counts; + std::map dtls_state_counts; + for (const auto& dtls : dtls_transports) { any_failed = any_failed || dtls->ice_transport()->GetState() == cricket::IceTransportState::STATE_FAILED; @@ -1180,6 +1190,9 @@ void JsepTransportController::UpdateAggregateStates_n() { all_done_gathering = all_done_gathering && dtls->ice_transport()->gathering_state() == cricket::kIceGatheringComplete; + + dtls_state_counts[dtls->dtls_state()]++; + ice_state_counts[dtls->ice_transport()->GetIceTransportState()]++; } if (any_failed) { new_connection_state = cricket::kIceConnectionFailed; @@ -1196,6 +1209,127 @@ void JsepTransportController::UpdateAggregateStates_n() { }); } + // Compute the current RTCIceConnectionState as described in + // https://www.w3.org/TR/webrtc/#dom-rtciceconnectionstate. + // The PeerConnection is responsible for handling the "closed" state. + int total_ice_checking = ice_state_counts[IceTransportState::kChecking]; + int total_ice_connected = ice_state_counts[IceTransportState::kConnected]; + int total_ice_completed = ice_state_counts[IceTransportState::kCompleted]; + int total_ice_failed = ice_state_counts[IceTransportState::kFailed]; + int total_ice_disconnected = + ice_state_counts[IceTransportState::kDisconnected]; + int total_ice_closed = ice_state_counts[IceTransportState::kClosed]; + int total_ice_new = ice_state_counts[IceTransportState::kNew]; + int total_ice = dtls_transports.size(); + + if (total_ice_failed > 0) { + // Any of the RTCIceTransports are in the "failed" state. + new_ice_connection_state = PeerConnectionInterface::kIceConnectionFailed; + } else if (total_ice_disconnected > 0) { + // Any of the RTCIceTransports are in the "disconnected" state and none of + // them are in the "failed" state. + new_ice_connection_state = + PeerConnectionInterface::kIceConnectionDisconnected; + } else if (total_ice_checking > 0) { + // Any of the RTCIceTransports are in the "checking" state and none of them + // are in the "disconnected" or "failed" state. + new_ice_connection_state = PeerConnectionInterface::kIceConnectionChecking; + } else if (total_ice_completed + total_ice_closed == total_ice && + total_ice_completed > 0) { + // All RTCIceTransports are in the "completed" or "closed" state and at + // least one of them is in the "completed" state. + new_ice_connection_state = PeerConnectionInterface::kIceConnectionCompleted; + } else if (total_ice_connected + total_ice_completed + total_ice_closed == + total_ice && + total_ice_connected > 0) { + // All RTCIceTransports are in the "connected", "completed" or "closed" + // state and at least one of them is in the "connected" state. + new_ice_connection_state = PeerConnectionInterface::kIceConnectionConnected; + } else if ((total_ice_new > 0 && + total_ice_checking + total_ice_disconnected + total_ice_failed == + 0) || + total_ice == total_ice_closed) { + // Any of the RTCIceTransports are in the "new" state and none of them are + // in the "checking", "disconnected" or "failed" state, or all + // RTCIceTransports are in the "closed" state, or there are no transports. + new_ice_connection_state = PeerConnectionInterface::kIceConnectionNew; + } else { + RTC_NOTREACHED(); + } + + if (standardized_ice_connection_state_ != new_ice_connection_state) { + standardized_ice_connection_state_ = new_ice_connection_state; + invoker_.AsyncInvoke( + RTC_FROM_HERE, signaling_thread_, [this, new_ice_connection_state] { + SignalStandardizedIceConnectionState(new_ice_connection_state); + }); + } + + // Compute the current RTCPeerConnectionState as described in + // https://www.w3.org/TR/webrtc/#dom-rtcpeerconnectionstate. + // The PeerConnection is responsible for handling the "closed" state. + // Note that "connecting" is only a valid state for DTLS transports while + // "checking", "completed" and "disconnected" are only valid for ICE + // transports. + int total_connected = total_ice_connected + + dtls_state_counts[cricket::DTLS_TRANSPORT_CONNECTED]; + int total_dtls_connecting = + dtls_state_counts[cricket::DTLS_TRANSPORT_CONNECTING]; + int total_failed = + total_ice_failed + dtls_state_counts[cricket::DTLS_TRANSPORT_FAILED]; + int total_closed = + total_ice_closed + dtls_state_counts[cricket::DTLS_TRANSPORT_CLOSED]; + int total_new = + total_ice_new + dtls_state_counts[cricket::DTLS_TRANSPORT_NEW]; + int total_transports = total_ice * 2; + + if (total_failed > 0) { + // Any of the RTCIceTransports or RTCDtlsTransports are in a "failed" state. + new_combined_state = PeerConnectionInterface::PeerConnectionState::kFailed; + } else if (total_ice_disconnected > 0 && + total_dtls_connecting + total_ice_checking == 0) { + // Any of the RTCIceTransports or RTCDtlsTransports are in the + // "disconnected" state and none of them are in the "failed" or "connecting" + // or "checking" state. + new_combined_state = + PeerConnectionInterface::PeerConnectionState::kDisconnected; + } else if (total_dtls_connecting + total_ice_checking > 0) { + // Any of the RTCIceTransports or RTCDtlsTransports are in the "connecting" + // or "checking" state and none of them is in the "failed" state. + new_combined_state = + PeerConnectionInterface::PeerConnectionState::kConnecting; + } else if (total_connected + total_ice_completed + total_closed == + total_transports && + total_connected + total_ice_completed > 0) { + // All RTCIceTransports and RTCDtlsTransports are in the "connected", + // "completed" or "closed" state and at least one of them is in the + // "connected" or "completed" state. + new_combined_state = + PeerConnectionInterface::PeerConnectionState::kConnected; + } else if ((total_new > 0 && total_dtls_connecting + total_ice_checking + + total_failed + total_ice_disconnected == + 0) || + total_transports == total_closed) { + // Any of the RTCIceTransports or RTCDtlsTransports are in the "new" state + // and none of the transports are in the "connecting", "checking", "failed" + // or "disconnected" state, or all transports are in the "closed" state, or + // there are no transports. + // + // Note that if none of the other conditions hold this is guaranteed to be + // true. + new_combined_state = PeerConnectionInterface::PeerConnectionState::kNew; + } else { + RTC_NOTREACHED(); + } + + if (combined_connection_state_ != new_combined_state) { + combined_connection_state_ = new_combined_state; + invoker_.AsyncInvoke(RTC_FROM_HERE, signaling_thread_, + [this, new_combined_state] { + SignalConnectionState(new_combined_state); + }); + } + if (all_done_gathering) { new_gathering_state = cricket::kIceGatheringComplete; } else if (any_gathering) { diff --git a/pc/jseptransportcontroller.h b/pc/jseptransportcontroller.h index 5d0f5ceb23..5747990cd6 100644 --- a/pc/jseptransportcontroller.h +++ b/pc/jseptransportcontroller.h @@ -181,6 +181,11 @@ class JsepTransportController : public sigslot::has_slots<> { // Else => connecting sigslot::signal1 SignalIceConnectionState; + sigslot::signal1 + SignalConnectionState; + sigslot::signal1 + SignalStandardizedIceConnectionState; + // If all transports done gathering => complete, // Else if any are gathering => gathering, // Else => new @@ -322,9 +327,16 @@ class JsepTransportController : public sigslot::has_slots<> { // (BaseChannel/SctpTransport) and the JsepTransport underneath. std::map mid_to_transport_; - // Aggregate state for Transports. + // Aggregate states for Transports. + // standardized_ice_connection_state_ is intended to replace + // ice_connection_state, see bugs.webrtc.org/9308 cricket::IceConnectionState ice_connection_state_ = cricket::kIceConnectionConnecting; + PeerConnectionInterface::IceConnectionState + standardized_ice_connection_state_ = + PeerConnectionInterface::kIceConnectionNew; + PeerConnectionInterface::PeerConnectionState combined_connection_state_ = + PeerConnectionInterface::PeerConnectionState::kNew; cricket::IceGatheringState ice_gathering_state_ = cricket::kIceGatheringNew; Config config_; diff --git a/pc/jseptransportcontroller_unittest.cc b/pc/jseptransportcontroller_unittest.cc index d36b8f3ce1..ba9b72d208 100644 --- a/pc/jseptransportcontroller_unittest.cc +++ b/pc/jseptransportcontroller_unittest.cc @@ -84,6 +84,10 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, void ConnectTransportControllerSignals() { transport_controller_->SignalIceConnectionState.connect( this, &JsepTransportControllerTest::OnConnectionState); + transport_controller_->SignalStandardizedIceConnectionState.connect( + this, &JsepTransportControllerTest::OnStandardizedIceConnectionState); + transport_controller_->SignalConnectionState.connect( + this, &JsepTransportControllerTest::OnCombinedConnectionState); transport_controller_->SignalIceGatheringState.connect( this, &JsepTransportControllerTest::OnGatheringState); transport_controller_->SignalIceCandidatesGathered.connect( @@ -243,6 +247,24 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, ++connection_state_signal_count_; } + void OnStandardizedIceConnectionState( + PeerConnectionInterface::IceConnectionState state) { + if (!signaling_thread_->IsCurrent()) { + signaled_on_non_signaling_thread_ = true; + } + ice_connection_state_ = state; + ++ice_connection_state_signal_count_; + } + + void OnCombinedConnectionState( + PeerConnectionInterface::PeerConnectionState state) { + if (!signaling_thread_->IsCurrent()) { + signaled_on_non_signaling_thread_ = true; + } + combined_connection_state_ = state; + ++combined_connection_state_signal_count_; + } + void OnGatheringState(cricket::IceGatheringState state) { if (!signaling_thread_->IsCurrent()) { signaled_on_non_signaling_thread_ = true; @@ -274,12 +296,18 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, // Information received from signals from transport controller. cricket::IceConnectionState connection_state_ = cricket::kIceConnectionConnecting; + PeerConnectionInterface::IceConnectionState ice_connection_state_ = + PeerConnectionInterface::kIceConnectionNew; + PeerConnectionInterface::PeerConnectionState combined_connection_state_ = + PeerConnectionInterface::PeerConnectionState::kNew; bool receiving_ = false; cricket::IceGatheringState gathering_state_ = cricket::kIceGatheringNew; // transport_name => candidates std::map candidates_; // Counts of each signal emitted. int connection_state_signal_count_ = 0; + int ice_connection_state_signal_count_ = 0; + int combined_connection_state_signal_count_ = 0; int receiving_signal_count_ = 0; int gathering_state_signal_count_ = 0; int candidates_signal_count_ = 0; @@ -617,6 +645,12 @@ TEST_F(JsepTransportControllerTest, SignalConnectionStateFailed) { fake_ice->SetConnectionCount(0); EXPECT_EQ_WAIT(cricket::kIceConnectionFailed, connection_state_, kTimeout); EXPECT_EQ(1, connection_state_signal_count_); + EXPECT_EQ(PeerConnectionInterface::kIceConnectionFailed, + ice_connection_state_); + EXPECT_EQ(1, ice_connection_state_signal_count_); + EXPECT_EQ(PeerConnectionInterface::PeerConnectionState::kFailed, + combined_connection_state_); + EXPECT_EQ(1, combined_connection_state_signal_count_); } TEST_F(JsepTransportControllerTest, SignalConnectionStateConnected) { @@ -644,13 +678,27 @@ TEST_F(JsepTransportControllerTest, SignalConnectionStateConnected) { EXPECT_EQ_WAIT(cricket::kIceConnectionFailed, connection_state_, kTimeout); EXPECT_EQ(1, connection_state_signal_count_); + EXPECT_EQ(PeerConnectionInterface::kIceConnectionFailed, + ice_connection_state_); + EXPECT_EQ(1, ice_connection_state_signal_count_); + EXPECT_EQ(PeerConnectionInterface::PeerConnectionState::kFailed, + combined_connection_state_); + EXPECT_EQ(1, combined_connection_state_signal_count_); + fake_audio_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED); + fake_video_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED); // Set the connection count to be 2 and the cricket::FakeIceTransport will set // the transport state to be STATE_CONNECTING. fake_video_dtls->fake_ice_transport()->SetConnectionCount(2); fake_video_dtls->SetWritable(true); EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout); EXPECT_EQ(2, connection_state_signal_count_); + EXPECT_EQ(PeerConnectionInterface::kIceConnectionConnected, + ice_connection_state_); + EXPECT_EQ(2, ice_connection_state_signal_count_); + EXPECT_EQ(PeerConnectionInterface::PeerConnectionState::kConnected, + combined_connection_state_); + EXPECT_EQ(2, combined_connection_state_signal_count_); } TEST_F(JsepTransportControllerTest, SignalConnectionStateComplete) { @@ -678,13 +726,27 @@ TEST_F(JsepTransportControllerTest, SignalConnectionStateComplete) { EXPECT_EQ_WAIT(cricket::kIceConnectionFailed, connection_state_, kTimeout); EXPECT_EQ(1, connection_state_signal_count_); + EXPECT_EQ(PeerConnectionInterface::kIceConnectionFailed, + ice_connection_state_); + EXPECT_EQ(1, ice_connection_state_signal_count_); + EXPECT_EQ(PeerConnectionInterface::PeerConnectionState::kFailed, + combined_connection_state_); + EXPECT_EQ(1, combined_connection_state_signal_count_); + fake_audio_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED); + fake_video_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED); // Set the connection count to be 1 and the cricket::FakeIceTransport will set // the transport state to be STATE_COMPLETED. fake_video_dtls->fake_ice_transport()->SetConnectionCount(1); fake_video_dtls->SetWritable(true); EXPECT_EQ_WAIT(cricket::kIceConnectionCompleted, connection_state_, kTimeout); EXPECT_EQ(2, connection_state_signal_count_); + EXPECT_EQ(PeerConnectionInterface::kIceConnectionCompleted, + ice_connection_state_); + EXPECT_EQ(2, ice_connection_state_signal_count_); + EXPECT_EQ(PeerConnectionInterface::PeerConnectionState::kConnected, + combined_connection_state_); + EXPECT_EQ(2, combined_connection_state_signal_count_); } TEST_F(JsepTransportControllerTest, SignalIceGatheringStateGathering) { @@ -758,6 +820,7 @@ TEST_F(JsepTransportControllerTest, fake_audio_dtls->SetWritable(true); fake_audio_dtls->fake_ice_transport()->SetCandidatesGatheringComplete(); fake_audio_dtls->fake_ice_transport()->SetConnectionCount(1); + fake_audio_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED); EXPECT_EQ(1, gathering_state_signal_count_); // Set the remote description and enable the bundle. @@ -770,6 +833,10 @@ TEST_F(JsepTransportControllerTest, transport_controller_->GetDtlsTransport(kVideoMid1)); EXPECT_EQ(fake_audio_dtls, fake_video_dtls); EXPECT_EQ_WAIT(cricket::kIceConnectionCompleted, connection_state_, kTimeout); + EXPECT_EQ(PeerConnectionInterface::kIceConnectionCompleted, + ice_connection_state_); + EXPECT_EQ(PeerConnectionInterface::PeerConnectionState::kConnected, + combined_connection_state_); EXPECT_EQ_WAIT(cricket::kIceGatheringComplete, gathering_state_, kTimeout); EXPECT_EQ(2, gathering_state_signal_count_); } diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index ecdec61c4c..9dc5e663dc 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -956,6 +956,10 @@ bool PeerConnection::Initialize( async_resolver_factory_.get(), config)); transport_controller_->SignalIceConnectionState.connect( this, &PeerConnection::OnTransportControllerConnectionState); + transport_controller_->SignalStandardizedIceConnectionState.connect( + this, &PeerConnection::SetStandardizedIceConnectionState); + transport_controller_->SignalConnectionState.connect( + this, &PeerConnection::SetConnectionState); transport_controller_->SignalIceGatheringState.connect( this, &PeerConnection::OnTransportControllerGatheringState); transport_controller_->SignalIceCandidatesGathered.connect( @@ -1707,6 +1711,16 @@ PeerConnection::ice_connection_state() { return ice_connection_state_; } +PeerConnectionInterface::IceConnectionState +PeerConnection::standardized_ice_connection_state() { + return standardized_ice_connection_state_; +} + +PeerConnectionInterface::PeerConnectionState +PeerConnection::peer_connection_state() { + return connection_state_; +} + PeerConnectionInterface::IceGatheringState PeerConnection::ice_gathering_state() { return ice_gathering_state_; @@ -3528,6 +3542,29 @@ void PeerConnection::SetIceConnectionState(IceConnectionState new_state) { Observer()->OnIceConnectionChange(ice_connection_state_); } +void PeerConnection::SetStandardizedIceConnectionState( + PeerConnectionInterface::IceConnectionState new_state) { + RTC_DCHECK(signaling_thread()->IsCurrent()); + if (standardized_ice_connection_state_ == new_state) + return; + if (IsClosed()) + return; + standardized_ice_connection_state_ = new_state; + // TODO(jonasolsson): Pass this value on to OnIceConnectionChange instead of + // the old one once disconnects are handled properly. +} + +void PeerConnection::SetConnectionState( + PeerConnectionInterface::PeerConnectionState new_state) { + RTC_DCHECK(signaling_thread()->IsCurrent()); + if (connection_state_ == new_state) + return; + if (IsClosed()) + return; + connection_state_ = new_state; + Observer()->OnConnectionChange(new_state); +} + void PeerConnection::OnIceGatheringChange( PeerConnectionInterface::IceGatheringState new_state) { RTC_DCHECK(signaling_thread()->IsCurrent()); @@ -3575,6 +3612,10 @@ void PeerConnection::ChangeSignalingState( if (signaling_state == kClosed) { ice_connection_state_ = kIceConnectionClosed; Observer()->OnIceConnectionChange(ice_connection_state_); + standardized_ice_connection_state_ = + PeerConnectionInterface::IceConnectionState::kIceConnectionClosed; + connection_state_ = PeerConnectionInterface::PeerConnectionState::kClosed; + Observer()->OnConnectionChange(connection_state_); if (ice_gathering_state_ != kIceGatheringComplete) { ice_gathering_state_ = kIceGatheringComplete; Observer()->OnIceGatheringChange(ice_gathering_state_); diff --git a/pc/peerconnection.h b/pc/peerconnection.h index c1d3d40c2a..604a5d3793 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -146,6 +146,8 @@ class PeerConnection : public PeerConnectionInternal, SignalingState signaling_state() override; IceConnectionState ice_connection_state() override; + IceConnectionState standardized_ice_connection_state(); + PeerConnectionState peer_connection_state() override; IceGatheringState ice_gathering_state() override; const SessionDescriptionInterface* local_description() const override; @@ -373,6 +375,11 @@ class PeerConnection : public PeerConnectionInternal, receiver); void SetIceConnectionState(IceConnectionState new_state); + void SetStandardizedIceConnectionState( + PeerConnectionInterface::IceConnectionState new_state); + void SetConnectionState( + PeerConnectionInterface::PeerConnectionState new_state); + // Called any time the IceGatheringState changes void OnIceGatheringChange(IceGatheringState new_state); // New ICE candidate has been gathered. @@ -950,6 +957,11 @@ class PeerConnection : public PeerConnectionInternal, SignalingState signaling_state_ = kStable; IceConnectionState ice_connection_state_ = kIceConnectionNew; + PeerConnectionInterface::IceConnectionState + standardized_ice_connection_state_ = kIceConnectionNew; + PeerConnectionInterface::PeerConnectionState connection_state_ = + PeerConnectionState::kNew; + IceGatheringState ice_gathering_state_ = kIceGatheringNew; PeerConnectionInterface::RTCConfiguration configuration_; diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index 36a832c0fb..b645ab1ad2 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -318,6 +318,12 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, ice_connection_state_history_.clear(); } + // Every PeerConnection state in order that has been seen by the observer. + std::vector + peer_connection_state_history() const { + return peer_connection_state_history_; + } + // Every ICE gathering state in order that has been seen by the observer. std::vector ice_gathering_state_history() const { @@ -914,6 +920,11 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, EXPECT_EQ(pc()->ice_connection_state(), new_state); ice_connection_state_history_.push_back(new_state); } + void OnConnectionChange( + webrtc::PeerConnectionInterface::PeerConnectionState new_state) override { + peer_connection_state_history_.push_back(new_state); + } + void OnIceGatheringChange( webrtc::PeerConnectionInterface::IceGatheringState new_state) override { EXPECT_EQ(pc()->ice_gathering_state(), new_state); @@ -1010,6 +1021,8 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, std::vector ice_connection_state_history_; + std::vector + peer_connection_state_history_; std::vector ice_gathering_state_history_; @@ -3562,6 +3575,17 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyIceStates) { ElementsAre(PeerConnectionInterface::kIceConnectionChecking, PeerConnectionInterface::kIceConnectionConnected, PeerConnectionInterface::kIceConnectionCompleted)); + // After the ice transport transitions from checking to connected we revert + // back to new as the standard requires, as at that point the DTLS transport + // is in the "new" state while no transports are "connecting", "checking", + // "failed" or disconnected. This is pretty unintuitive, and we might want to + // amend the spec to handle this case more gracefully. + EXPECT_THAT( + caller()->peer_connection_state_history(), + ElementsAre(PeerConnectionInterface::PeerConnectionState::kConnecting, + PeerConnectionInterface::PeerConnectionState::kNew, + PeerConnectionInterface::PeerConnectionState::kConnecting, + PeerConnectionInterface::PeerConnectionState::kConnected)); EXPECT_THAT(caller()->ice_gathering_state_history(), ElementsAre(PeerConnectionInterface::kIceGatheringGathering, PeerConnectionInterface::kIceGatheringComplete));