diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 8611a657b2..ce390bc6bf 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -149,6 +149,7 @@ if (rtc_include_tests) { "../rtc_base/third_party/sigslot", "../test:test_support", "//third_party/abseil-cpp/absl/memory", + "//third_party/abseil-cpp/absl/types:optional", ] } diff --git a/p2p/base/fakeicetransport.h b/p2p/base/fakeicetransport.h index 88afc382ec..c36dd7fad8 100644 --- a/p2p/base/fakeicetransport.h +++ b/p2p/base/fakeicetransport.h @@ -15,6 +15,7 @@ #include #include +#include "absl/types/optional.h" #include "p2p/base/icetransportinternal.h" #include "rtc_base/asyncinvoker.h" #include "rtc_base/copyonwritebuffer.h" @@ -69,6 +70,13 @@ class FakeIceTransport : public IceTransportInternal { } } + void SetTransportState(webrtc::IceTransportState state, + IceTransportState legacy_state) { + transport_state_ = state; + legacy_transport_state_ = legacy_state; + SignalIceTransportStateChanged(this); + } + void SetConnectionCount(size_t connection_count) { size_t old_connection_count = connection_count_; connection_count_ = connection_count; @@ -107,6 +115,10 @@ class FakeIceTransport : public IceTransportInternal { const std::string& remote_ice_pwd() const { return remote_ice_pwd_; } IceTransportState GetState() const override { + if (legacy_transport_state_) { + return *legacy_transport_state_; + } + if (connection_count_ == 0) { return had_connection_ ? IceTransportState::STATE_FAILED : IceTransportState::STATE_INIT; @@ -120,6 +132,10 @@ class FakeIceTransport : public IceTransportInternal { } webrtc::IceTransportState GetIceTransportState() const override { + if (transport_state_) { + return *transport_state_; + } + if (connection_count_ == 0) { return had_connection_ ? webrtc::IceTransportState::kFailed : webrtc::IceTransportState::kNew; @@ -293,6 +309,8 @@ class FakeIceTransport : public IceTransportInternal { std::string remote_ice_pwd_; IceMode remote_ice_mode_ = ICEMODE_FULL; size_t connection_count_ = 0; + absl::optional transport_state_; + absl::optional legacy_transport_state_; IceGatheringState gathering_state_ = kIceGatheringNew; bool had_connection_ = false; bool writable_ = false; diff --git a/pc/jseptransportcontroller.cc b/pc/jseptransportcontroller.cc index 03899c3ceb..ea2451df3d 100644 --- a/pc/jseptransportcontroller.cc +++ b/pc/jseptransportcontroller.cc @@ -1368,36 +1368,30 @@ void JsepTransportController::UpdateAggregateStates_n() { int total_ice = dtls_transports.size(); if (total_ice_failed > 0) { - // Any of the RTCIceTransports are in the "failed" state. + // Any 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. + // None of the previous states apply and any RTCIceTransports are in the + // "disconnected" 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. + } else if (total_ice_new + total_ice_closed == total_ice) { + // None of the previous states apply and all RTCIceTransports are in the + // "new" or "closed" state, or there are no transports. + new_ice_connection_state = PeerConnectionInterface::kIceConnectionNew; + } else if (total_ice_new + total_ice_checking > 0) { + // None of the previous states apply and any RTCIceTransports are in the + // "new" or "checking" 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. + } else if (total_ice_completed + total_ice_closed == total_ice) { + // None of the previous states apply and all RTCIceTransports are in the + // "completed" or "closed" 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. + total_ice) { + // None of the previous states apply and all RTCIceTransports are in the + // "connected", "completed" or "closed" 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(); } @@ -1431,38 +1425,27 @@ void JsepTransportController::UpdateAggregateStates_n() { 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. + } else if (total_ice_disconnected > 0) { + // None of the previous states apply and any RTCIceTransports or + // RTCDtlsTransports are in the "disconnected" 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. + } else if (total_new + total_closed == total_transports) { + // None of the previous states apply and all RTCIceTransports and + // RTCDtlsTransports are in the "new" or "closed" state, or there are no + // transports. + new_combined_state = PeerConnectionInterface::PeerConnectionState::kNew; + } else if (total_new + total_dtls_connecting + total_ice_checking > 0) { + // None of the previous states apply and all RTCIceTransports or + // RTCDtlsTransports are in the "new", "connecting" or "checking" 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. + total_transports) { + // None of the previous states apply and all RTCIceTransports and + // RTCDtlsTransports are in the "connected", "completed" or "closed" 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(); } diff --git a/pc/jseptransportcontroller_unittest.cc b/pc/jseptransportcontroller_unittest.cc index fd8a779c78..3031b8df7a 100644 --- a/pc/jseptransportcontroller_unittest.cc +++ b/pc/jseptransportcontroller_unittest.cc @@ -273,6 +273,8 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, void OnCombinedConnectionState( PeerConnectionInterface::PeerConnectionState state) { + RTC_LOG(LS_INFO) << "OnCombinedConnectionState: " + << static_cast(state); if (!signaling_thread_->IsCurrent()) { signaled_on_non_signaling_thread_ = true; } @@ -783,10 +785,10 @@ TEST_F(JsepTransportControllerTest, EXPECT_EQ(1, connection_state_signal_count_); EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionFailed, ice_connection_state_, kTimeout); - EXPECT_EQ(1, ice_connection_state_signal_count_); + EXPECT_EQ(2, ice_connection_state_signal_count_); EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kFailed, combined_connection_state_, kTimeout); - EXPECT_EQ(1, combined_connection_state_signal_count_); + EXPECT_EQ(2, combined_connection_state_signal_count_); fake_audio_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED); fake_video_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED); @@ -798,10 +800,10 @@ TEST_F(JsepTransportControllerTest, EXPECT_EQ(2, connection_state_signal_count_); EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionConnected, ice_connection_state_, kTimeout); - EXPECT_EQ(2, ice_connection_state_signal_count_); + EXPECT_EQ(3, ice_connection_state_signal_count_); EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnected, combined_connection_state_, kTimeout); - EXPECT_EQ(2, combined_connection_state_signal_count_); + EXPECT_EQ(3, combined_connection_state_signal_count_); } TEST_F(JsepTransportControllerTest, @@ -906,37 +908,48 @@ TEST_F(JsepTransportControllerTest, SignalConnectionStateComplete) { // First, have one transport connect, and another fail, to ensure that // the first transport connecting didn't trigger a "connected" state signal. // We should only get a signal when all are connected. - fake_audio_dtls->fake_ice_transport()->SetConnectionCount(1); + fake_audio_dtls->fake_ice_transport()->SetTransportState( + IceTransportState::kCompleted, + cricket::IceTransportState::STATE_COMPLETED); fake_audio_dtls->SetWritable(true); fake_audio_dtls->fake_ice_transport()->SetCandidatesGatheringComplete(); - // Decrease the number of the connection to trigger the signal. - fake_video_dtls->fake_ice_transport()->SetConnectionCount(1); - fake_video_dtls->fake_ice_transport()->SetConnectionCount(0); + + EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionChecking, + ice_connection_state_, kTimeout); + EXPECT_EQ(1, ice_connection_state_signal_count_); + EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnecting, + combined_connection_state_, kTimeout); + EXPECT_EQ(1, combined_connection_state_signal_count_); + + fake_video_dtls->fake_ice_transport()->SetTransportState( + IceTransportState::kFailed, cricket::IceTransportState::STATE_FAILED); fake_video_dtls->fake_ice_transport()->SetCandidatesGatheringComplete(); EXPECT_EQ_WAIT(cricket::kIceConnectionFailed, connection_state_, kTimeout); EXPECT_EQ(1, connection_state_signal_count_); EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionFailed, ice_connection_state_, kTimeout); - EXPECT_EQ(1, ice_connection_state_signal_count_); + EXPECT_EQ(2, ice_connection_state_signal_count_); EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kFailed, combined_connection_state_, kTimeout); - EXPECT_EQ(1, combined_connection_state_signal_count_); + EXPECT_EQ(2, 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->fake_ice_transport()->SetTransportState( + IceTransportState::kCompleted, + cricket::IceTransportState::STATE_COMPLETED); fake_video_dtls->SetWritable(true); EXPECT_EQ_WAIT(cricket::kIceConnectionCompleted, connection_state_, kTimeout); EXPECT_EQ(2, connection_state_signal_count_); EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted, ice_connection_state_, kTimeout); - EXPECT_EQ(2, ice_connection_state_signal_count_); + EXPECT_EQ(3, ice_connection_state_signal_count_); EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnected, combined_connection_state_, kTimeout); - EXPECT_EQ(2, combined_connection_state_signal_count_); + EXPECT_EQ(3, combined_connection_state_signal_count_); } TEST_F(JsepTransportControllerTest, SignalIceGatheringStateGathering) { diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index f0184f1327..fda34fe263 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -3883,8 +3883,6 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyIceStates) { 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, diff --git a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java index 2063199e72..58553223ea 100644 --- a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java +++ b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java @@ -836,8 +836,6 @@ public class PeerConnectionTest { offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING); offeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED); - offeringExpectations.expectConnectionChange(PeerConnectionState.NEW); - offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING); offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED); // TODO(bemasc): uncomment once delivery of ICECompleted is reliable // (https://code.google.com/p/webrtc/issues/detail?id=3021). @@ -1071,8 +1069,6 @@ public class PeerConnectionTest { offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING); offeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED); - offeringExpectations.expectConnectionChange(PeerConnectionState.NEW); - offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING); offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED); // TODO(bemasc): uncomment once delivery of ICECompleted is reliable // (https://code.google.com/p/webrtc/issues/detail?id=3021). @@ -1255,8 +1251,6 @@ public class PeerConnectionTest { offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING); offeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED); - offeringExpectations.expectConnectionChange(PeerConnectionState.NEW); - offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING); offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED); // TODO(bemasc): uncomment once delivery of ICECompleted is reliable // (https://code.google.com/p/webrtc/issues/detail?id=3021).