Replace the IceConnectionState implementation.
PeerConnection::ice_connection_state() used to return a value based on both DTLS and ICE transports.
Now that we have PeerConnection::peer_connection_state() to fill that role we can change the implementation of ice_connection_state over to match the spec.
Bug: webrtc:6145
Change-Id: Ia4f348f728f24faf4b976c63dea2187bb1f01ef0
Reviewed-on: https://webrtc-review.googlesource.com/c/108780
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Olsson <jonasolsson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25773}diff --git a/api/peerconnectionproxy.h b/api/peerconnectionproxy.h
index 6b66bcc..7105a78 100644
--- a/api/peerconnectionproxy.h
+++ b/api/peerconnectionproxy.h
@@ -125,6 +125,7 @@
std::unique_ptr<rtc::BitrateAllocationStrategy>);
PROXY_METHOD0(SignalingState, signaling_state)
PROXY_METHOD0(IceConnectionState, ice_connection_state)
+PROXY_METHOD0(PeerConnectionState, peer_connection_state)
PROXY_METHOD0(IceGatheringState, ice_gathering_state)
PROXY_METHOD2(bool, StartRtcEventLog, rtc::PlatformFile, int64_t)
PROXY_METHOD2(bool,
diff --git a/p2p/base/icetransportinternal.h b/p2p/base/icetransportinternal.h
index 099cea7..e64a3b4 100644
--- a/p2p/base/icetransportinternal.h
+++ b/p2p/base/icetransportinternal.h
@@ -26,14 +26,6 @@
typedef std::vector<Candidate> Candidates;
-enum IceConnectionState {
- kIceConnectionConnecting = 0,
- kIceConnectionFailed,
- kIceConnectionConnected, // Writable, but still checking one or more
- // connections
- kIceConnectionCompleted,
-};
-
// TODO(deadbeef): Unify with PeerConnectionInterface::IceConnectionState
// once /talk/ and /webrtc/ are combined, and also switch to ENUM_NAME naming
// style.
diff --git a/p2p/base/p2ptransportchannel.cc b/p2p/base/p2ptransportchannel.cc
index f61291c..8f4da3d 100644
--- a/p2p/base/p2ptransportchannel.cc
+++ b/p2p/base/p2ptransportchannel.cc
@@ -2481,8 +2481,7 @@
if (writable_ == writable) {
return;
}
- RTC_LOG(LS_VERBOSE) << ToString()
- << ": Changed writable_ to " << writable;
+ RTC_LOG(LS_VERBOSE) << ToString() << ": Changed writable_ to " << writable;
writable_ = writable;
if (writable_) {
SignalReadyToSend(this);
diff --git a/p2p/base/regatheringcontroller.cc b/p2p/base/regatheringcontroller.cc
index 6d4c4fd..e9d9265 100644
--- a/p2p/base/regatheringcontroller.cc
+++ b/p2p/base/regatheringcontroller.cc
@@ -37,7 +37,7 @@
rand_(rtc::SystemTimeNanos()) {
RTC_DCHECK(ice_transport_);
RTC_DCHECK(thread_);
- ice_transport_->SignalStateChanged.connect(
+ ice_transport_->SignalIceTransportStateChanged.connect(
this, &BasicRegatheringController::OnIceTransportStateChanged);
ice_transport->SignalWritableState.connect(
this, &BasicRegatheringController::OnIceTransportWritableState);
diff --git a/pc/jseptransportcontroller.cc b/pc/jseptransportcontroller.cc
index 78ecaf3..66758e3 100644
--- a/pc/jseptransportcontroller.cc
+++ b/pc/jseptransportcontroller.cc
@@ -446,6 +446,8 @@
this, &JsepTransportController::OnTransportRoleConflict_n);
dtls->ice_transport()->SignalStateChanged.connect(
this, &JsepTransportController::OnTransportStateChanged_n);
+ dtls->ice_transport()->SignalIceTransportStateChanged.connect(
+ this, &JsepTransportController::OnTransportStateChanged_n);
return dtls;
}
@@ -1263,20 +1265,12 @@
RTC_DCHECK(network_thread_->IsCurrent());
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;
-
- // 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_ice_connected = false;
bool any_gathering = false;
bool all_done_gathering = !dtls_transports.empty();
@@ -1284,16 +1278,8 @@
std::map<cricket::DtlsTransportState, int> dtls_state_counts;
for (const auto& dtls : dtls_transports) {
- any_failed = any_failed || dtls->ice_transport()->GetState() ==
- cricket::IceTransportState::STATE_FAILED;
- all_connected = all_connected && dtls->writable();
- all_completed =
- all_completed && dtls->writable() &&
- dtls->ice_transport()->GetState() ==
- cricket::IceTransportState::STATE_COMPLETED &&
- dtls->ice_transport()->GetIceRole() == cricket::ICEROLE_CONTROLLING &&
- dtls->ice_transport()->gathering_state() ==
- cricket::kIceGatheringComplete;
+ any_ice_connected |= dtls->ice_transport()->writable();
+
any_gathering = any_gathering || dtls->ice_transport()->gathering_state() !=
cricket::kIceGatheringNew;
all_done_gathering =
@@ -1304,45 +1290,6 @@
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) {
- new_connection_state = cricket::kIceConnectionCompleted;
- } else if (all_connected) {
- new_connection_state = cricket::kIceConnectionConnected;
- }
- if (ice_connection_state_ != new_connection_state) {
- ice_connection_state_ = new_connection_state;
- invoker_.AsyncInvoke<void>(RTC_FROM_HERE, signaling_thread_,
- [this, new_connection_state] {
- SignalIceConnectionState(new_connection_state);
- });
- }
-
// Compute the current RTCIceConnectionState as described in
// https://www.w3.org/TR/webrtc/#dom-rtciceconnectionstate.
// The PeerConnection is responsible for handling the "closed" state.
@@ -1359,9 +1306,24 @@
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) {
+ } else if (total_ice_disconnected > 0 ||
+ (!any_ice_connected &&
+ (ice_connection_state_ ==
+ PeerConnectionInterface::kIceConnectionCompleted ||
+ ice_connection_state_ ==
+ PeerConnectionInterface::kIceConnectionDisconnected))) {
// Any of the RTCIceTransports are in the "disconnected" state and none of
// them are in the "failed" state.
+ //
+ // As a hack we also mark the connection as disconnected if it used to be
+ // completed but our connections are no longer writable.
+ if (total_ice_disconnected == 0) {
+ // If the IceConnectionState is disconnected the DtlsConnectionState has
+ // to be failed or disconnected. Setting total_ice_disconnected ensures
+ // that is the case, even if we got here by following the
+ // !any_ice_connected branch.
+ total_ice_disconnected = 1;
+ }
new_ice_connection_state =
PeerConnectionInterface::kIceConnectionDisconnected;
} else if (total_ice_checking > 0) {
@@ -1391,11 +1353,23 @@
RTC_NOTREACHED();
}
- if (standardized_ice_connection_state_ != new_ice_connection_state) {
- standardized_ice_connection_state_ = new_ice_connection_state;
+ if (ice_connection_state_ != new_ice_connection_state) {
+ if (ice_connection_state_ ==
+ PeerConnectionInterface::kIceConnectionChecking &&
+ new_ice_connection_state ==
+ PeerConnectionInterface::kIceConnectionCompleted) {
+ // Make sure we always signal Connected. It's not clear from the spec if
+ // this is required, but there's little harm and it's what we used to do.
+ invoker_.AsyncInvoke<void>(RTC_FROM_HERE, signaling_thread_, [this] {
+ SignalIceConnectionState(
+ PeerConnectionInterface::kIceConnectionConnected);
+ });
+ }
+
+ ice_connection_state_ = new_ice_connection_state;
invoker_.AsyncInvoke<void>(
RTC_FROM_HERE, signaling_thread_, [this, new_ice_connection_state] {
- SignalStandardizedIceConnectionState(new_ice_connection_state);
+ SignalIceConnectionState(new_ice_connection_state);
});
}
@@ -1417,6 +1391,35 @@
total_ice_new + dtls_state_counts[cricket::DTLS_TRANSPORT_NEW];
int total_transports = total_ice * 2;
+ // We'll factor any media transports into the combined connection state if
+ // they exist. Media transports aren't a concept that exist in the spec, but
+ // since the combined state is supposed to answer "can we send data over this
+ // peerconnection" it's arguably following the letter if not the spirit of the
+ // spec.
+ 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;
+ }
+
+ if (jsep_transport->media_transport_state() ==
+ webrtc::MediaTransportState::kPending) {
+ total_transports++;
+ total_dtls_connecting++;
+ } else if (jsep_transport->media_transport_state() ==
+ webrtc::MediaTransportState::kWritable) {
+ total_transports++;
+ total_connected++;
+ } else if (jsep_transport->media_transport_state() ==
+ webrtc::MediaTransportState::kClosed) {
+ // We treat kClosed as failed, because if it happens before shutting down
+ // media transports it means that there was a failure.
+ total_transports++;
+ total_failed++;
+ }
+ }
+
if (total_failed > 0) {
// Any of the RTCIceTransports or RTCDtlsTransports are in a "failed" state.
new_combined_state = PeerConnectionInterface::PeerConnectionState::kFailed;
diff --git a/pc/jseptransportcontroller.h b/pc/jseptransportcontroller.h
index 3ed7f5f..b703012 100644
--- a/pc/jseptransportcontroller.h
+++ b/pc/jseptransportcontroller.h
@@ -181,12 +181,11 @@
// Else if all completed => completed,
// Else if all connected => connected,
// Else => connecting
- sigslot::signal1<cricket::IceConnectionState> SignalIceConnectionState;
+ sigslot::signal1<PeerConnectionInterface::IceConnectionState>
+ SignalIceConnectionState;
sigslot::signal1<PeerConnectionInterface::PeerConnectionState>
SignalConnectionState;
- sigslot::signal1<PeerConnectionInterface::IceConnectionState>
- SignalStandardizedIceConnectionState;
// If all transports done gathering => complete,
// Else if any are gathering => gathering,
@@ -341,13 +340,8 @@
std::map<std::string, cricket::JsepTransport*> mid_to_transport_;
// 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::IceConnectionState ice_connection_state_ =
+ PeerConnectionInterface::kIceConnectionNew;
PeerConnectionInterface::PeerConnectionState combined_connection_state_ =
PeerConnectionInterface::PeerConnectionState::kNew;
cricket::IceGatheringState ice_gathering_state_ = cricket::kIceGatheringNew;
diff --git a/pc/jseptransportcontroller_unittest.cc b/pc/jseptransportcontroller_unittest.cc
index 129d22a..66532be 100644
--- a/pc/jseptransportcontroller_unittest.cc
+++ b/pc/jseptransportcontroller_unittest.cc
@@ -99,8 +99,6 @@
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(
@@ -254,7 +252,7 @@
}
protected:
- void OnConnectionState(cricket::IceConnectionState state) {
+ void OnConnectionState(PeerConnectionInterface::IceConnectionState state) {
if (!signaling_thread_->IsCurrent()) {
signaled_on_non_signaling_thread_ = true;
}
@@ -262,15 +260,6 @@
++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()) {
@@ -310,9 +299,7 @@
}
// Information received from signals from transport controller.
- cricket::IceConnectionState connection_state_ =
- cricket::kIceConnectionConnecting;
- PeerConnectionInterface::IceConnectionState ice_connection_state_ =
+ PeerConnectionInterface::IceConnectionState connection_state_ =
PeerConnectionInterface::kIceConnectionNew;
PeerConnectionInterface::PeerConnectionState combined_connection_state_ =
PeerConnectionInterface::PeerConnectionState::kNew;
@@ -322,7 +309,6 @@
std::map<std::string, Candidates> 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;
@@ -727,11 +713,9 @@
fake_ice->SetConnectionCount(1);
// The connection stats will be failed if there is no active connection.
fake_ice->SetConnectionCount(0);
- 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_);
+ connection_state_, kTimeout);
+ EXPECT_EQ(1, connection_state_signal_count_);
EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kFailed,
combined_connection_state_, kTimeout);
EXPECT_EQ(1, combined_connection_state_signal_count_);
@@ -761,11 +745,9 @@
fake_video_dtls->fake_ice_transport()->SetConnectionCount(0);
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_);
+ connection_state_, kTimeout);
+ EXPECT_EQ(1, connection_state_signal_count_);
EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kFailed,
combined_connection_state_, kTimeout);
EXPECT_EQ(1, combined_connection_state_signal_count_);
@@ -776,11 +758,9 @@
// 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_WAIT(PeerConnectionInterface::kIceConnectionConnected,
- ice_connection_state_, kTimeout);
- EXPECT_EQ(2, ice_connection_state_signal_count_);
+ connection_state_, kTimeout);
+ EXPECT_EQ(2, connection_state_signal_count_);
EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnected,
combined_connection_state_, kTimeout);
EXPECT_EQ(2, combined_connection_state_signal_count_);
@@ -813,22 +793,23 @@
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);
+ EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnecting,
+ combined_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);
+ EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnecting,
+ combined_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);
+ EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnected,
+ combined_connection_state_, kTimeout);
}
TEST_F(JsepTransportControllerTest,
@@ -867,10 +848,12 @@
media_transport->SetState(webrtc::MediaTransportState::kWritable);
- EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout);
+ EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnected,
+ combined_connection_state_, kTimeout);
media_transport->SetState(webrtc::MediaTransportState::kClosed);
- EXPECT_EQ_WAIT(cricket::kIceConnectionFailed, connection_state_, kTimeout);
+ EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kFailed,
+ combined_connection_state_, kTimeout);
}
TEST_F(JsepTransportControllerTest, SignalConnectionStateComplete) {
@@ -896,11 +879,9 @@
fake_video_dtls->fake_ice_transport()->SetConnectionCount(0);
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_);
+ connection_state_, kTimeout);
+ EXPECT_EQ(1, connection_state_signal_count_);
EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kFailed,
combined_connection_state_, kTimeout);
EXPECT_EQ(1, combined_connection_state_signal_count_);
@@ -911,11 +892,9 @@
// 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_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
- ice_connection_state_, kTimeout);
- EXPECT_EQ(2, ice_connection_state_signal_count_);
+ connection_state_, kTimeout);
+ EXPECT_EQ(2, connection_state_signal_count_);
EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnected,
combined_connection_state_, kTimeout);
EXPECT_EQ(2, combined_connection_state_signal_count_);
@@ -1004,9 +983,8 @@
fake_video_dtls = static_cast<FakeDtlsTransport*>(
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_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
+ connection_state_, kTimeout);
EXPECT_EQ(PeerConnectionInterface::PeerConnectionState::kConnected,
combined_connection_state_);
EXPECT_EQ_WAIT(cricket::kIceGatheringComplete, gathering_state_, kTimeout);
@@ -1038,7 +1016,8 @@
CreateLocalDescriptionAndCompleteConnectionOnNetworkThread();
// connecting --> connected --> completed
- EXPECT_EQ_WAIT(cricket::kIceConnectionCompleted, connection_state_, kTimeout);
+ EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
+ connection_state_, kTimeout);
EXPECT_EQ(2, connection_state_signal_count_);
// new --> gathering --> complete
diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index e60474c..6a2e370 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -998,9 +998,7 @@
signaling_thread(), network_thread(), port_allocator_.get(),
async_resolver_factory_.get(), config));
transport_controller_->SignalIceConnectionState.connect(
- this, &PeerConnection::OnTransportControllerConnectionState);
- transport_controller_->SignalStandardizedIceConnectionState.connect(
- this, &PeerConnection::SetStandardizedIceConnectionState);
+ this, &PeerConnection::SetIceConnectionState);
transport_controller_->SignalConnectionState.connect(
this, &PeerConnection::SetConnectionState);
transport_controller_->SignalIceGatheringState.connect(
@@ -1762,11 +1760,6 @@
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_;
@@ -3632,22 +3625,17 @@
RTC_DCHECK(ice_connection_state_ !=
PeerConnectionInterface::kIceConnectionClosed);
+ if (new_state == PeerConnectionInterface::kIceConnectionConnected) {
+ NoteUsageEvent(UsageEvent::ICE_STATE_CONNECTED);
+ } else if (new_state == PeerConnectionInterface::kIceConnectionCompleted) {
+ NoteUsageEvent(UsageEvent::ICE_STATE_CONNECTED);
+ ReportTransportStats();
+ }
+
ice_connection_state_ = 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());
@@ -3706,8 +3694,6 @@
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) {
@@ -5507,51 +5493,6 @@
rtcp ? kDtlsSrtpSetupFailureRtcp : kDtlsSrtpSetupFailureRtp);
}
-void PeerConnection::OnTransportControllerConnectionState(
- cricket::IceConnectionState state) {
- switch (state) {
- case cricket::kIceConnectionConnecting:
- // If the current state is Connected or Completed, then there were
- // writable channels but now there are not, so the next state must
- // be Disconnected.
- // kIceConnectionConnecting is currently used as the default,
- // un-connected state by the TransportController, so its only use is
- // detecting disconnections.
- if (ice_connection_state_ ==
- PeerConnectionInterface::kIceConnectionConnected ||
- ice_connection_state_ ==
- PeerConnectionInterface::kIceConnectionCompleted) {
- SetIceConnectionState(
- PeerConnectionInterface::kIceConnectionDisconnected);
- }
- break;
- case cricket::kIceConnectionFailed:
- SetIceConnectionState(PeerConnectionInterface::kIceConnectionFailed);
- break;
- case cricket::kIceConnectionConnected:
- RTC_LOG(LS_INFO) << "Changing to ICE connected state because "
- "all transports are writable.";
- SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
- NoteUsageEvent(UsageEvent::ICE_STATE_CONNECTED);
- break;
- case cricket::kIceConnectionCompleted:
- RTC_LOG(LS_INFO) << "Changing to ICE completed state because "
- "all transports are complete.";
- if (ice_connection_state_ !=
- PeerConnectionInterface::kIceConnectionConnected) {
- // If jumping directly from "checking" to "connected",
- // signal "connected" first.
- SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
- }
- SetIceConnectionState(PeerConnectionInterface::kIceConnectionCompleted);
- NoteUsageEvent(UsageEvent::ICE_STATE_CONNECTED);
- ReportTransportStats();
- break;
- default:
- RTC_NOTREACHED();
- }
-}
-
void PeerConnection::OnTransportControllerCandidatesGathered(
const std::string& transport_name,
const cricket::Candidates& candidates) {
diff --git a/pc/peerconnection.h b/pc/peerconnection.h
index 7e97afa..7945aca 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -147,7 +147,6 @@
SignalingState signaling_state() override;
IceConnectionState ice_connection_state() override;
- IceConnectionState standardized_ice_connection_state();
PeerConnectionState peer_connection_state() override;
IceGatheringState ice_gathering_state() override;
@@ -877,7 +876,8 @@
bool SrtpRequired() const;
// JsepTransportController signal handlers.
- void OnTransportControllerConnectionState(cricket::IceConnectionState state);
+ void OnTransportControllerConnectionState(
+ PeerConnectionInterface::IceConnectionState state);
void OnTransportControllerGatheringState(cricket::IceGatheringState state);
void OnTransportControllerCandidatesGathered(
const std::string& transport_name,
@@ -983,8 +983,6 @@
SignalingState signaling_state_ = kStable;
IceConnectionState ice_connection_state_ = kIceConnectionNew;
- PeerConnectionInterface::IceConnectionState
- standardized_ice_connection_state_ = kIceConnectionNew;
PeerConnectionInterface::PeerConnectionState connection_state_ =
PeerConnectionState::kNew;
diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc
index a7f7aad..7cd03a4 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -553,6 +553,10 @@
return pc()->ice_connection_state();
}
+ webrtc::PeerConnectionInterface::PeerConnectionState peer_connection_state() {
+ return pc()->peer_connection_state();
+ }
+
webrtc::PeerConnectionInterface::IceGatheringState ice_gathering_state() {
return pc()->ice_gathering_state();
}
@@ -1201,17 +1205,11 @@
}
bool DtlsConnected() {
- // TODO(deadbeef): kIceConnectionConnected currently means both ICE and DTLS
- // are connected. This is an important distinction. Once we have separate
- // ICE and DTLS state, this check needs to use the DTLS state.
- return (callee()->ice_connection_state() ==
- webrtc::PeerConnectionInterface::kIceConnectionConnected ||
- callee()->ice_connection_state() ==
- webrtc::PeerConnectionInterface::kIceConnectionCompleted) &&
- (caller()->ice_connection_state() ==
- webrtc::PeerConnectionInterface::kIceConnectionConnected ||
- caller()->ice_connection_state() ==
- webrtc::PeerConnectionInterface::kIceConnectionCompleted);
+ return callee()->peer_connection_state() ==
+ webrtc::PeerConnectionInterface::PeerConnectionState::
+ kConnected &&
+ caller()->peer_connection_state() ==
+ webrtc::PeerConnectionInterface::PeerConnectionState::kConnected;
}
// When |event_log_factory| is null, the default implementation of the event
@@ -1601,7 +1599,10 @@
EXPECT_EQ_WAIT(rtc::SrtpCryptoSuiteToName(expected_cipher_suite),
caller()->OldGetStats()->SrtpCipher(), kDefaultTimeout);
// TODO(bugs.webrtc.org/9456): Fix it.
- EXPECT_EQ(1, webrtc::metrics::NumEvents(
+ EXPECT_LE(1, webrtc::metrics::NumEvents(
+ "WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
+ expected_cipher_suite));
+ EXPECT_GE(2, webrtc::metrics::NumEvents(
"WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
expected_cipher_suite));
}
@@ -2824,7 +2825,10 @@
EXPECT_EQ_WAIT(rtc::SrtpCryptoSuiteToName(kDefaultSrtpCryptoSuite),
caller()->OldGetStats()->SrtpCipher(), kDefaultTimeout);
// TODO(bugs.webrtc.org/9456): Fix it.
- EXPECT_EQ(1, webrtc::metrics::NumEvents(
+ EXPECT_LE(1, webrtc::metrics::NumEvents(
+ "WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
+ kDefaultSrtpCryptoSuite));
+ EXPECT_GE(2, webrtc::metrics::NumEvents(
"WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
kDefaultSrtpCryptoSuite));
}
@@ -2846,7 +2850,10 @@
EXPECT_EQ_WAIT(rtc::SrtpCryptoSuiteToName(kDefaultSrtpCryptoSuite),
caller()->OldGetStats()->SrtpCipher(), kDefaultTimeout);
// TODO(bugs.webrtc.org/9456): Fix it.
- EXPECT_EQ(1, webrtc::metrics::NumEvents(
+ EXPECT_LE(1, webrtc::metrics::NumEvents(
+ "WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
+ kDefaultSrtpCryptoSuite));
+ EXPECT_GE(2, webrtc::metrics::NumEvents(
"WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
kDefaultSrtpCryptoSuite));
}
@@ -3548,8 +3555,13 @@
// fixed, this test should be updated.
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
caller()->ice_connection_state(), kDefaultTimeout);
- EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
- callee()->ice_connection_state(), kDefaultTimeout);
+ EXPECT_TRUE_WAIT(
+ callee()->ice_connection_state() ==
+ webrtc::PeerConnectionInterface::kIceConnectionConnected ||
+ callee()->ice_connection_state() ==
+ webrtc::PeerConnectionInterface::kIceConnectionCompleted,
+ kDefaultTimeout)
+ << callee()->ice_connection_state();
}
// Replaces the first candidate with a static address and configures a
@@ -3864,8 +3876,13 @@
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
caller()->ice_connection_state(), kMaxWaitForFramesMs);
- EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
- callee()->ice_connection_state(), kMaxWaitForFramesMs);
+ EXPECT_TRUE_WAIT(
+ callee()->ice_connection_state() ==
+ webrtc::PeerConnectionInterface::kIceConnectionConnected ||
+ callee()->ice_connection_state() ==
+ webrtc::PeerConnectionInterface::kIceConnectionCompleted,
+ kDefaultTimeout)
+ << callee()->ice_connection_state();
// To verify that the ICE restart actually occurs, get
// ufrag/password/candidates before and after restart.
@@ -3896,7 +3913,7 @@
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
caller()->ice_connection_state(), kMaxWaitForFramesMs);
- EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
+ EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
callee()->ice_connection_state(), kMaxWaitForFramesMs);
// Grab the ufrags/candidates again.
diff --git a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
index 2063199..e0ee176 100644
--- a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
+++ b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
@@ -820,6 +820,7 @@
sdpLatch = new SdpObserverLatch();
answeringExpectations.expectSignalingChange(SignalingState.STABLE);
answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
+ answeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
answeringPC.setLocalDescription(sdpLatch, answerSdp);
assertTrue(sdpLatch.await());
assertNull(sdpLatch.getSdp());
@@ -827,6 +828,7 @@
sdpLatch = new SdpObserverLatch();
offeringExpectations.expectSignalingChange(SignalingState.HAVE_LOCAL_OFFER);
offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
+ offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
offeringPC.setLocalDescription(sdpLatch, offerSdp);
assertTrue(sdpLatch.await());
assertNull(sdpLatch.getSdp());
@@ -834,7 +836,6 @@
offeringExpectations.expectSignalingChange(SignalingState.STABLE);
offeringExpectations.expectAddStream("answeredMediaStream");
- offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
offeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED);
offeringExpectations.expectConnectionChange(PeerConnectionState.NEW);
offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
@@ -844,8 +845,9 @@
//
// offeringExpectations.expectIceConnectionChange(
// IceConnectionState.COMPLETED);
- answeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
answeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED);
+ answeringExpectations.expectConnectionChange(PeerConnectionState.NEW);
+ answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED);
offeringPC.setRemoteDescription(sdpLatch, answerSdp);
@@ -1056,6 +1058,7 @@
sdpLatch = new SdpObserverLatch();
answeringExpectations.expectSignalingChange(SignalingState.STABLE);
answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
+ answeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
answeringPC.setLocalDescription(sdpLatch, answerSdp);
assertTrue(sdpLatch.await());
assertNull(sdpLatch.getSdp());
@@ -1063,21 +1066,22 @@
sdpLatch = new SdpObserverLatch();
offeringExpectations.expectSignalingChange(SignalingState.HAVE_LOCAL_OFFER);
offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
+ offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
offeringPC.setLocalDescription(sdpLatch, offerSdp);
assertTrue(sdpLatch.await());
assertNull(sdpLatch.getSdp());
sdpLatch = new SdpObserverLatch();
offeringExpectations.expectSignalingChange(SignalingState.STABLE);
- 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).
- answeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
answeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED);
+ answeringExpectations.expectConnectionChange(PeerConnectionState.NEW);
+ answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED);
offeringPC.setRemoteDescription(sdpLatch, answerSdp);
@@ -1212,6 +1216,7 @@
offeringExpectations.expectIceCandidates(2);
offeringExpectations.expectIceGatheringChange(IceGatheringState.COMPLETE);
offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
+ offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
offeringPC.setLocalDescription(sdpLatch, offerSdp);
assertTrue(sdpLatch.await());
assertNull(sdpLatch.getSdp());
@@ -1244,6 +1249,7 @@
answeringExpectations.expectIceCandidates(2);
answeringExpectations.expectIceGatheringChange(IceGatheringState.COMPLETE);
answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
+ answeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
answeringPC.setLocalDescription(sdpLatch, answerSdp);
assertTrue(sdpLatch.await());
assertNull(sdpLatch.getSdp());
@@ -1253,7 +1259,6 @@
offeringExpectations.expectSignalingChange(SignalingState.STABLE);
offeringExpectations.expectAddStream("answeredMediaStream");
- offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
offeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED);
offeringExpectations.expectConnectionChange(PeerConnectionState.NEW);
offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
@@ -1263,8 +1268,9 @@
//
// offeringExpectations.expectIceConnectionChange(
// IceConnectionState.COMPLETED);
- answeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
answeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED);
+ answeringExpectations.expectConnectionChange(PeerConnectionState.NEW);
+ answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
answeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTED);
offeringPC.setRemoteDescription(sdpLatch, answerSdp);