admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Update connection states to match spec changes.

Browse Source 
These changes simplify the code, and also fix the issue where the peerconnectionstate would sometimes return to "new" during connection setup.

Bug: webrtc:9308
Change-Id: I895cd2f94a2b9688c821cca64d1a077317b99d44
Reviewed-on: https://webrtc-review.googlesource.com/c/111964
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Olsson <jonasolsson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25942}
This commit is contained in:
Jonas Olsson 2018-12-07 13:11:44 +01:00 committed by Commit Bot
parent 10a58016ee
commit 6a8727bd2a
6 changed files with 75 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
p2p/BUILD.gn
View File

@ -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",
]
}

18
p2p/base/fakeicetransport.h
View File

@ -15,6 +15,7 @@
#include <string>
#include <utility>
#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<webrtc::IceTransportState> transport_state_;
absl::optional<IceTransportState> legacy_transport_state_;
IceGatheringState gathering_state_ = kIceGatheringNew;
bool had_connection_ = false;
bool writable_ = false;

77
pc/jseptransportcontroller.cc
View File

@ -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();
}

39
pc/jseptransportcontroller_unittest.cc
View File

@ -273,6 +273,8 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
void OnCombinedConnectionState(
PeerConnectionInterface::PeerConnectionState state) {
RTC_LOG(LS_INFO) << "OnCombinedConnectionState: "
<< static_cast<int>(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) {

2
pc/peerconnection_integrationtest.cc
View File

@ -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,

6
sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
View File

@ -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).