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

Make Ice Transports signal failures.

Browse Source 
The new iceTransportState depends on the transports to signal when they have disconnected, this change ensures that they do so.

The logic is similar to what the old iceConnectionState did, but it uses the ice transports writable() flag instead of the one from the containing dtls transport.

Bug: webrtc:10199, webrtc:9308
Change-Id: I8a2a71a689b2a7027fe9117c79144811367d2165
Reviewed-on: https://webrtc-review.googlesource.com/c/117565
Commit-Queue: Jonas Olsson <jonasolsson@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26269}
This commit is contained in:
Jonas Olsson 2019-01-15 16:31:55 +01:00 committed by Commit Bot
parent 4306a25dfc
commit 7a6739eda8
5 changed files with 53 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
p2p/base/p2p_transport_channel.cc
View File

@ -394,8 +394,6 @@ IceTransportState P2PTransportChannel::ComputeState() const {
// Compute the current RTCIceTransportState as described in
// https://www.w3.org/TR/webrtc/#dom-rtcicetransportstate
// TODO(bugs.webrtc.org/9308): Return IceTransportState::kDisconnected when it
// makes sense.
// TODO(bugs.webrtc.org/9218): Avoid prematurely signalling kFailed once we have
// implemented end-of-candidates signalling.
webrtc::IceTransportState P2PTransportChannel::ComputeIceTransportState()
@ -408,6 +406,13 @@ webrtc::IceTransportState P2PTransportChannel::ComputeIceTransportState()
}
}
if (!writable() && has_been_writable_) {
if (has_connection)
return webrtc::IceTransportState::kDisconnected;
else
return webrtc::IceTransportState::kFailed;
}
switch (gathering_state_) {
case kIceGatheringComplete:
if (has_connection)
@ -1878,6 +1883,23 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn) {
// change, it should be called after all the connection states have changed. For
// example, we call this at the end of SortConnectionsAndUpdateState.
void P2PTransportChannel::UpdateState() {
// If our selected connection is "presumed writable" (TURN-TURN with no
// CreatePermission required), act like we're already writable to the upper
// layers, so they can start media quicker.
bool writable =
selected_connection_ && (selected_connection_->writable() ||
PresumedWritable(selected_connection_));
SetWritable(writable);
bool receiving = false;
for (const Connection* connection : connections_) {
if (connection->receiving()) {
receiving = true;
break;
}
}
SetReceiving(receiving);
IceTransportState state = ComputeState();
webrtc::IceTransportState current_standardized_state =
ComputeIceTransportState();
@ -1927,22 +1949,6 @@ void P2PTransportChannel::UpdateState() {
standardized_state_ = current_standardized_state;
SignalIceTransportStateChanged(this);
}
// If our selected connection is "presumed writable" (TURN-TURN with no
// CreatePermission required), act like we're already writable to the upper
// layers, so they can start media quicker.
bool writable =
selected_connection_ && (selected_connection_->writable() ||
PresumedWritable(selected_connection_));
set_writable(writable);
bool receiving = false;
for (const Connection* connection : connections_) {
if (connection->receiving()) {
receiving = true;
break;
}
}
set_receiving(receiving);
}
void P2PTransportChannel::MaybeStopPortAllocatorSessions() {
@ -2490,7 +2496,7 @@ Connection* P2PTransportChannel::MorePingable(Connection* conn1,
}));
}
void P2PTransportChannel::set_writable(bool writable) {
void P2PTransportChannel::SetWritable(bool writable) {
if (writable_ == writable) {
return;
}
@ -2498,12 +2504,13 @@ void P2PTransportChannel::set_writable(bool writable) {
<< ": Changed writable_ to " << writable;
writable_ = writable;
if (writable_) {
has_been_writable_ = true;
SignalReadyToSend(this);
}
SignalWritableState(this);
}
void P2PTransportChannel::set_receiving(bool receiving) {
void P2PTransportChannel::SetReceiving(bool receiving) {
if (receiving_ == receiving) {
return;
}

5
p2p/base/p2p_transport_channel.h
View File

@ -370,9 +370,9 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
bool IsRemoteCandidatePruned(const Candidate& cand) const;
// Sets the writable state, signaling if necessary.
void set_writable(bool writable);
void SetWritable(bool writable);
// Sets the receiving state, signaling if necessary.
void set_receiving(bool receiving);
void SetReceiving(bool receiving);
std::string transport_name_;
int component_;
@ -429,6 +429,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
uint32_t nomination_ = 0;
bool receiving_ = false;
bool writable_ = false;
bool has_been_writable_ = false; // if writable_ has ever been true
rtc::AsyncInvoker invoker_;
absl::optional<rtc::NetworkRoute> network_route_;

2
pc/jsep_transport_controller.cc
View File

@ -457,6 +457,8 @@ JsepTransportController::CreateDtlsTransport(const std::string& transport_name,
this, &JsepTransportController::OnTransportRoleConflict_n);
dtls->ice_transport()->SignalStateChanged.connect(
this, &JsepTransportController::OnTransportStateChanged_n);
dtls->ice_transport()->SignalIceTransportStateChanged.connect(
this, &JsepTransportController::OnTransportStateChanged_n);
return dtls;
}

2
pc/jsep_transport_controller_unittest.cc
View File

@ -944,7 +944,7 @@ TEST_F(JsepTransportControllerTest, SignalConnectionStateComplete) {
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(3, connection_state_signal_count_);
EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
ice_connection_state_, kTimeout);
EXPECT_EQ(3, ice_connection_state_signal_count_);

27
pc/peer_connection_integrationtest.cc
View File

@ -512,6 +512,11 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver,
return pc()->ice_connection_state();
}
webrtc::PeerConnectionInterface::IceConnectionState
standardized_ice_connection_state() {
return pc()->standardized_ice_connection_state();
}
webrtc::PeerConnectionInterface::IceGatheringState ice_gathering_state() {
return pc()->ice_gathering_state();
}
@ -3811,6 +3816,8 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyIceStates) {
caller()->ice_gathering_state());
ASSERT_EQ(PeerConnectionInterface::kIceConnectionNew,
caller()->ice_connection_state());
ASSERT_EQ(PeerConnectionInterface::kIceConnectionNew,
caller()->standardized_ice_connection_state());
// Start the call by creating the offer, setting it as the local description,
// then sending it to the peer who will respond with an answer. This happens
@ -3818,19 +3825,16 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyIceStates) {
// background.
caller()->CreateAndSetAndSignalOffer();
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
caller()->ice_connection_state(), kDefaultTimeout);
ASSERT_EQ(PeerConnectionInterface::kIceConnectionCompleted,
caller()->ice_connection_state());
ASSERT_EQ(PeerConnectionInterface::kIceConnectionCompleted,
caller()->standardized_ice_connection_state());
// Verify that the observer was notified of the intermediate transitions.
EXPECT_THAT(caller()->ice_connection_state_history(),
ElementsAre(PeerConnectionInterface::kIceConnectionChecking,
PeerConnectionInterface::kIceConnectionConnected,
PeerConnectionInterface::kIceConnectionCompleted));
// After the ice transport transitions from checking to connected we revert
// back to new as the standard requires, as at that point the DTLS transport
// is in the "new" state while no transports are "connecting", "checking",
// "failed" or disconnected. This is pretty unintuitive, and we might want to
// amend the spec to handle this case more gracefully.
EXPECT_THAT(
caller()->peer_connection_state_history(),
ElementsAre(PeerConnectionInterface::PeerConnectionState::kConnecting,
@ -3847,12 +3851,18 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyIceStates) {
RTC_LOG(LS_INFO) << "Firewall rules applied";
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionDisconnected,
caller()->ice_connection_state(), kDefaultTimeout);
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionDisconnected,
caller()->standardized_ice_connection_state(),
kDefaultTimeout);
// Let ICE re-establish by removing the firewall rules.
firewall()->ClearRules();
RTC_LOG(LS_INFO) << "Firewall rules cleared";
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
caller()->ice_connection_state(), kDefaultTimeout);
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
caller()->standardized_ice_connection_state(),
kDefaultTimeout);
// According to RFC7675, if there is no response within 30 seconds then the
// peer should consider the other side to have rejected the connection. This
@ -3864,6 +3874,9 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyIceStates) {
RTC_LOG(LS_INFO) << "Firewall rules applied again";
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionFailed,
caller()->ice_connection_state(), kConsentTimeout);
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionFailed,
caller()->standardized_ice_connection_state(),
kConsentTimeout);
}
// Tests that the best connection is set to the appropriate IPv4/IPv6 connection