Revert "Replace the IceConnectionState implementation."
This reverts commit 1e87b4f32b73526f9caaae2a7bccfbd0cd84dcb9.
Reason for revert: Breaks internal project
Original change's description:
> 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}
TBR=kwiberg@webrtc.org,hbos@webrtc.org,hta@webrtc.org,jonasolsson@webrtc.org
Change-Id: Icc4368d120a4167286fa6ba2e884a3650b453eff
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:6145
Reviewed-on: https://webrtc-review.googlesource.com/c/111925
Reviewed-by: Alex Loiko <aleloi@webrtc.org>
Commit-Queue: Alex Loiko <aleloi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25775}diff --git a/api/peerconnectionproxy.h b/api/peerconnectionproxy.h
index 7105a78..6b66bcc 100644
--- a/api/peerconnectionproxy.h
+++ b/api/peerconnectionproxy.h
@@ -125,7 +125,6 @@
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 e64a3b4..099cea7 100644
--- a/p2p/base/icetransportinternal.h
+++ b/p2p/base/icetransportinternal.h
@@ -26,6 +26,14 @@
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 8f4da3d..f61291c 100644
--- a/p2p/base/p2ptransportchannel.cc
+++ b/p2p/base/p2ptransportchannel.cc
@@ -2481,7 +2481,8 @@
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 e9d9265..6d4c4fd 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_->SignalIceTransportStateChanged.connect(
+ ice_transport_->SignalStateChanged.connect(
this, &BasicRegatheringController::OnIceTransportStateChanged);
ice_transport->SignalWritableState.connect(
this, &BasicRegatheringController::OnIceTransportWritableState);
diff --git a/pc/jseptransportcontroller.cc b/pc/jseptransportcontroller.cc
index 66758e3..78ecaf3 100644
--- a/pc/jseptransportcontroller.cc
+++ b/pc/jseptransportcontroller.cc
@@ -446,8 +446,6 @@
this, &JsepTransportController::OnTransportRoleConflict_n);
dtls->ice_transport()->SignalStateChanged.connect(
this, &JsepTransportController::OnTransportStateChanged_n);
- dtls->ice_transport()->SignalIceTransportStateChanged.connect(
- this, &JsepTransportController::OnTransportStateChanged_n);
return dtls;
}
@@ -1265,12 +1263,20 @@
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_ice_connected = false;
+ bool any_failed = false;
+
+ // TODO(http://bugs.webrtc.org/9719) If(when) media_transport disables
+ // dtls_transports entirely, the below line will have to be changed to account
+ // for the fact that dtls transports might be absent.
+ bool all_connected = !dtls_transports.empty();
+ bool all_completed = !dtls_transports.empty();
bool any_gathering = false;
bool all_done_gathering = !dtls_transports.empty();
@@ -1278,8 +1284,16 @@
std::map<cricket::DtlsTransportState, int> dtls_state_counts;
for (const auto& dtls : dtls_transports) {
- any_ice_connected |= dtls->ice_transport()->writable();
-
+ 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_gathering = any_gathering || dtls->ice_transport()->gathering_state() !=
cricket::kIceGatheringNew;
all_done_gathering =
@@ -1290,6 +1304,45 @@
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.
@@ -1306,24 +1359,9 @@
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_ice_connected &&
- (ice_connection_state_ ==
- PeerConnectionInterface::kIceConnectionCompleted ||
- ice_connection_state_ ==
- PeerConnectionInterface::kIceConnectionDisconnected))) {
+ } else if (total_ice_disconnected > 0) {
// 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) {
@@ -1353,23 +1391,11 @@
RTC_NOTREACHED();
}
- 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;
+ if (standardized_ice_connection_state_ != new_ice_connection_state) {
+ standardized_ice_connection_state_ = new_ice_connection_state;
invoker_.AsyncInvoke<void>(
RTC_FROM_HERE, signaling_thread_, [this, new_ice_connection_state] {
- SignalIceConnectionState(new_ice_connection_state);
+ SignalStandardizedIceConnectionState(new_ice_connection_state);
});
}
@@ -1391,35 +1417,6 @@
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 b703012..3ed7f5f 100644
--- a/pc/jseptransportcontroller.h
+++ b/pc/jseptransportcontroller.h
@@ -181,11 +181,12 @@
// Else if all completed => completed,
// Else if all connected => connected,
// Else => connecting
- sigslot::signal1<PeerConnectionInterface::IceConnectionState>
- SignalIceConnectionState;
+ sigslot::signal1<cricket::IceConnectionState> SignalIceConnectionState;
sigslot::signal1<PeerConnectionInterface::PeerConnectionState>
SignalConnectionState;
+ sigslot::signal1<PeerConnectionInterface::IceConnectionState>
+ SignalStandardizedIceConnectionState;
// If all transports done gathering => complete,
// Else if any are gathering => gathering,
@@ -340,8 +341,13 @@
std::map<std::string, cricket::JsepTransport*> mid_to_transport_;
// Aggregate states for Transports.
- PeerConnectionInterface::IceConnectionState ice_connection_state_ =
- PeerConnectionInterface::kIceConnectionNew;
+ // 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;
diff --git a/pc/jseptransportcontroller_unittest.cc b/pc/jseptransportcontroller_unittest.cc
index 66532be..129d22a 100644
--- a/pc/jseptransportcontroller_unittest.cc
+++ b/pc/jseptransportcontroller_unittest.cc
@@ -99,6 +99,8 @@
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(
@@ -252,7 +254,7 @@
}
protected:
- void OnConnectionState(PeerConnectionInterface::IceConnectionState state) {
+ void OnConnectionState(cricket::IceConnectionState state) {
if (!signaling_thread_->IsCurrent()) {
signaled_on_non_signaling_thread_ = true;
}
@@ -260,6 +262,15 @@
++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()) {
@@ -299,7 +310,9 @@
}
// Information received from signals from transport controller.
- PeerConnectionInterface::IceConnectionState connection_state_ =
+ cricket::IceConnectionState connection_state_ =
+ cricket::kIceConnectionConnecting;
+ PeerConnectionInterface::IceConnectionState ice_connection_state_ =
PeerConnectionInterface::kIceConnectionNew;
PeerConnectionInterface::PeerConnectionState combined_connection_state_ =
PeerConnectionInterface::PeerConnectionState::kNew;
@@ -309,6 +322,7 @@
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;
@@ -713,9 +727,11 @@
fake_ice->SetConnectionCount(1);
// The connection stats will be failed if there is no active connection.
fake_ice->SetConnectionCount(0);
- EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionFailed,
- connection_state_, kTimeout);
+ 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_WAIT(PeerConnectionInterface::PeerConnectionState::kFailed,
combined_connection_state_, kTimeout);
EXPECT_EQ(1, combined_connection_state_signal_count_);
@@ -745,9 +761,11 @@
fake_video_dtls->fake_ice_transport()->SetConnectionCount(0);
fake_video_dtls->fake_ice_transport()->SetCandidatesGatheringComplete();
- EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionFailed,
- connection_state_, kTimeout);
+ 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_WAIT(PeerConnectionInterface::PeerConnectionState::kFailed,
combined_connection_state_, kTimeout);
EXPECT_EQ(1, combined_connection_state_signal_count_);
@@ -758,9 +776,11 @@
// the transport state to be STATE_CONNECTING.
fake_video_dtls->fake_ice_transport()->SetConnectionCount(2);
fake_video_dtls->SetWritable(true);
- EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionConnected,
- connection_state_, kTimeout);
+ 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_);
EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnected,
combined_connection_state_, kTimeout);
EXPECT_EQ(2, combined_connection_state_signal_count_);
@@ -793,23 +813,22 @@
fake_video_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED);
// Still not connected, because we are waiting for media transport.
- EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnecting,
- combined_connection_state_, kTimeout);
+ EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_,
+ kTimeout);
FakeMediaTransport* media_transport = static_cast<FakeMediaTransport*>(
transport_controller_->GetMediaTransport(kAudioMid1));
media_transport->SetState(webrtc::MediaTransportState::kWritable);
- EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnecting,
- combined_connection_state_, kTimeout);
+ EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, 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(PeerConnectionInterface::PeerConnectionState::kConnected,
- combined_connection_state_, kTimeout);
+ EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout);
}
TEST_F(JsepTransportControllerTest,
@@ -848,12 +867,10 @@
media_transport->SetState(webrtc::MediaTransportState::kWritable);
- EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kConnected,
- combined_connection_state_, kTimeout);
+ EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout);
media_transport->SetState(webrtc::MediaTransportState::kClosed);
- EXPECT_EQ_WAIT(PeerConnectionInterface::PeerConnectionState::kFailed,
- combined_connection_state_, kTimeout);
+ EXPECT_EQ_WAIT(cricket::kIceConnectionFailed, connection_state_, kTimeout);
}
TEST_F(JsepTransportControllerTest, SignalConnectionStateComplete) {
@@ -879,9 +896,11 @@
fake_video_dtls->fake_ice_transport()->SetConnectionCount(0);
fake_video_dtls->fake_ice_transport()->SetCandidatesGatheringComplete();
- EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionFailed,
- connection_state_, kTimeout);
+ 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_WAIT(PeerConnectionInterface::PeerConnectionState::kFailed,
combined_connection_state_, kTimeout);
EXPECT_EQ(1, combined_connection_state_signal_count_);
@@ -892,9 +911,11 @@
// the transport state to be STATE_COMPLETED.
fake_video_dtls->fake_ice_transport()->SetConnectionCount(1);
fake_video_dtls->SetWritable(true);
- EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
- connection_state_, kTimeout);
+ 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_WAIT(PeerConnectionInterface::PeerConnectionState::kConnected,
combined_connection_state_, kTimeout);
EXPECT_EQ(2, combined_connection_state_signal_count_);
@@ -983,8 +1004,9 @@
fake_video_dtls = static_cast<FakeDtlsTransport*>(
transport_controller_->GetDtlsTransport(kVideoMid1));
EXPECT_EQ(fake_audio_dtls, fake_video_dtls);
- EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
- connection_state_, kTimeout);
+ 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);
@@ -1016,8 +1038,7 @@
CreateLocalDescriptionAndCompleteConnectionOnNetworkThread();
// connecting --> connected --> completed
- EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
- connection_state_, kTimeout);
+ EXPECT_EQ_WAIT(cricket::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 6a2e370..e60474c 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -998,7 +998,9 @@
signaling_thread(), network_thread(), port_allocator_.get(),
async_resolver_factory_.get(), config));
transport_controller_->SignalIceConnectionState.connect(
- this, &PeerConnection::SetIceConnectionState);
+ this, &PeerConnection::OnTransportControllerConnectionState);
+ transport_controller_->SignalStandardizedIceConnectionState.connect(
+ this, &PeerConnection::SetStandardizedIceConnectionState);
transport_controller_->SignalConnectionState.connect(
this, &PeerConnection::SetConnectionState);
transport_controller_->SignalIceGatheringState.connect(
@@ -1760,6 +1762,11 @@
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_;
@@ -3625,17 +3632,22 @@
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());
@@ -3694,6 +3706,8 @@
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) {
@@ -5493,6 +5507,51 @@
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 7945aca..7e97afa 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -147,6 +147,7 @@
SignalingState signaling_state() override;
IceConnectionState ice_connection_state() override;
+ IceConnectionState standardized_ice_connection_state();
PeerConnectionState peer_connection_state() override;
IceGatheringState ice_gathering_state() override;
@@ -876,8 +877,7 @@
bool SrtpRequired() const;
// JsepTransportController signal handlers.
- void OnTransportControllerConnectionState(
- PeerConnectionInterface::IceConnectionState state);
+ void OnTransportControllerConnectionState(cricket::IceConnectionState state);
void OnTransportControllerGatheringState(cricket::IceGatheringState state);
void OnTransportControllerCandidatesGathered(
const std::string& transport_name,
@@ -983,6 +983,8 @@
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 7cd03a4..a7f7aad 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -553,10 +553,6 @@
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();
}
@@ -1205,11 +1201,17 @@
}
bool DtlsConnected() {
- return callee()->peer_connection_state() ==
- webrtc::PeerConnectionInterface::PeerConnectionState::
- kConnected &&
- caller()->peer_connection_state() ==
- webrtc::PeerConnectionInterface::PeerConnectionState::kConnected;
+ // 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);
}
// When |event_log_factory| is null, the default implementation of the event
@@ -1599,10 +1601,7 @@
EXPECT_EQ_WAIT(rtc::SrtpCryptoSuiteToName(expected_cipher_suite),
caller()->OldGetStats()->SrtpCipher(), kDefaultTimeout);
// TODO(bugs.webrtc.org/9456): Fix it.
- EXPECT_LE(1, webrtc::metrics::NumEvents(
- "WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
- expected_cipher_suite));
- EXPECT_GE(2, webrtc::metrics::NumEvents(
+ EXPECT_EQ(1, webrtc::metrics::NumEvents(
"WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
expected_cipher_suite));
}
@@ -2825,10 +2824,7 @@
EXPECT_EQ_WAIT(rtc::SrtpCryptoSuiteToName(kDefaultSrtpCryptoSuite),
caller()->OldGetStats()->SrtpCipher(), kDefaultTimeout);
// TODO(bugs.webrtc.org/9456): Fix it.
- EXPECT_LE(1, webrtc::metrics::NumEvents(
- "WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
- kDefaultSrtpCryptoSuite));
- EXPECT_GE(2, webrtc::metrics::NumEvents(
+ EXPECT_EQ(1, webrtc::metrics::NumEvents(
"WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
kDefaultSrtpCryptoSuite));
}
@@ -2850,10 +2846,7 @@
EXPECT_EQ_WAIT(rtc::SrtpCryptoSuiteToName(kDefaultSrtpCryptoSuite),
caller()->OldGetStats()->SrtpCipher(), kDefaultTimeout);
// TODO(bugs.webrtc.org/9456): Fix it.
- EXPECT_LE(1, webrtc::metrics::NumEvents(
- "WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
- kDefaultSrtpCryptoSuite));
- EXPECT_GE(2, webrtc::metrics::NumEvents(
+ EXPECT_EQ(1, webrtc::metrics::NumEvents(
"WebRTC.PeerConnection.SrtpCryptoSuite.Audio",
kDefaultSrtpCryptoSuite));
}
@@ -3555,13 +3548,8 @@
// fixed, this test should be updated.
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
caller()->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();
+ EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
+ callee()->ice_connection_state(), kDefaultTimeout);
}
// Replaces the first candidate with a static address and configures a
@@ -3876,13 +3864,8 @@
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
caller()->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();
+ EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
+ callee()->ice_connection_state(), kMaxWaitForFramesMs);
// To verify that the ICE restart actually occurs, get
// ufrag/password/candidates before and after restart.
@@ -3913,7 +3896,7 @@
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
caller()->ice_connection_state(), kMaxWaitForFramesMs);
- EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
+ EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
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 e0ee176..2063199 100644
--- a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
+++ b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
@@ -820,7 +820,6 @@
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());
@@ -828,7 +827,6 @@
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());
@@ -836,6 +834,7 @@
offeringExpectations.expectSignalingChange(SignalingState.STABLE);
offeringExpectations.expectAddStream("answeredMediaStream");
+ offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
offeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED);
offeringExpectations.expectConnectionChange(PeerConnectionState.NEW);
offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
@@ -845,9 +844,8 @@
//
// 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);
@@ -1058,7 +1056,6 @@
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());
@@ -1066,22 +1063,21 @@
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);
@@ -1216,7 +1212,6 @@
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());
@@ -1249,7 +1244,6 @@
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());
@@ -1259,6 +1253,7 @@
offeringExpectations.expectSignalingChange(SignalingState.STABLE);
offeringExpectations.expectAddStream("answeredMediaStream");
+ offeringExpectations.expectIceConnectionChange(IceConnectionState.CHECKING);
offeringExpectations.expectIceConnectionChange(IceConnectionState.CONNECTED);
offeringExpectations.expectConnectionChange(PeerConnectionState.NEW);
offeringExpectations.expectConnectionChange(PeerConnectionState.CONNECTING);
@@ -1268,9 +1263,8 @@
//
// 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);
