zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Allow IceConnectionState to become failed without ever connecting.

Browse Source 
As a unintended consequence of the changed iceConnectionState implementation we won't ever transition to the "failed" iceConnection state unless a connection has first been established. This CL fixes that and adds a integration test for this scenario.

Bug: chromium:933786
Change-Id: I45effd7411959ac0e5b16a13d7568756dbeff4d1
Reviewed-on: https://webrtc-review.googlesource.com/c/123785
Commit-Queue: Jonas Olsson <jonasolsson@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26811}
This commit is contained in:
Jonas Olsson
2019-02-22 10:33:29 +01:00
committed by Commit Bot
parent d209cd1754
commit b75d9e91f4
2 changed files with 70 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
p2p/base/p2p_transport_channel.cc
View File

@ -406,13 +406,14 @@ webrtc::IceTransportState P2PTransportChannel::ComputeIceTransportState()
} }
} }
if (!writable() && has_been_writable_) { if (had_connection_ && !has_connection) {
if (has_connection)
return webrtc::IceTransportState::kDisconnected;
else
return webrtc::IceTransportState::kFailed; return webrtc::IceTransportState::kFailed;
} }
if (!writable() && has_been_writable_) {
return webrtc::IceTransportState::kDisconnected;
}
if (gathering_state_ == kIceGatheringNew) if (gathering_state_ == kIceGatheringNew)
return webrtc::IceTransportState::kNew; return webrtc::IceTransportState::kNew;
else if (has_connection) else if (has_connection)

77
pc/peer_connection_integrationtest.cc
View File

@ -60,6 +60,7 @@
#include "pc/test/fake_rtc_certificate_generator.h" #include "pc/test/fake_rtc_certificate_generator.h"
#include "pc/test/fake_video_track_renderer.h" #include "pc/test/fake_video_track_renderer.h"
#include "pc/test/mock_peer_connection_observers.h" #include "pc/test/mock_peer_connection_observers.h"
#include "rtc_base/fake_clock.h"
#include "rtc_base/fake_network.h" #include "rtc_base/fake_network.h"
#include "rtc_base/firewall_socket_server.h" #include "rtc_base/firewall_socket_server.h"
#include "rtc_base/gunit.h" #include "rtc_base/gunit.h"
@ -3808,9 +3809,10 @@ class PeerConnectionIntegrationIceStatesTest
// states, induced by putting a firewall between the peers and waiting for them // states, induced by putting a firewall between the peers and waiting for them
// to time out. // to time out.
TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyIceStates) { TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyIceStates) {
// TODO(bugs.webrtc.org/8295): When using a ScopedFakeClock, this test will rtc::ScopedFakeClock fake_clock;
// sometimes hit a DCHECK in platform_thread.cc about the PacerThread being // Some things use a time of "0" as a special value, so we need to start out
// too busy. For now, revert to running without a fake clock. // the fake clock at a nonzero time.
fake_clock.AdvanceTime(TimeDelta::seconds(1));
const SocketAddress kStunServerAddress = const SocketAddress kStunServerAddress =
SocketAddress("99.99.99.1", cricket::STUN_SERVER_PORT); SocketAddress("99.99.99.1", cricket::STUN_SERVER_PORT);
@ -3868,20 +3870,22 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyIceStates) {
firewall()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, caller_address); firewall()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, caller_address);
} }
RTC_LOG(LS_INFO) << "Firewall rules applied"; RTC_LOG(LS_INFO) << "Firewall rules applied";
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionDisconnected, ASSERT_EQ_SIMULATED_WAIT(PeerConnectionInterface::kIceConnectionDisconnected,
caller()->ice_connection_state(), kDefaultTimeout); caller()->ice_connection_state(), kDefaultTimeout,
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionDisconnected, fake_clock);
ASSERT_EQ_SIMULATED_WAIT(PeerConnectionInterface::kIceConnectionDisconnected,
caller()->standardized_ice_connection_state(), caller()->standardized_ice_connection_state(),
kDefaultTimeout); kDefaultTimeout, fake_clock);
// Let ICE re-establish by removing the firewall rules. // Let ICE re-establish by removing the firewall rules.
firewall()->ClearRules(); firewall()->ClearRules();
RTC_LOG(LS_INFO) << "Firewall rules cleared"; RTC_LOG(LS_INFO) << "Firewall rules cleared";
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted, ASSERT_EQ_SIMULATED_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
caller()->ice_connection_state(), kDefaultTimeout); caller()->ice_connection_state(), kDefaultTimeout,
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionConnected, fake_clock);
ASSERT_EQ_SIMULATED_WAIT(PeerConnectionInterface::kIceConnectionConnected,
caller()->standardized_ice_connection_state(), caller()->standardized_ice_connection_state(),
kDefaultTimeout); kDefaultTimeout, fake_clock);
// According to RFC7675, if there is no response within 30 seconds then the // 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 // peer should consider the other side to have rejected the connection. This
@ -3891,11 +3895,54 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyIceStates) {
firewall()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, caller_address); firewall()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, caller_address);
} }
RTC_LOG(LS_INFO) << "Firewall rules applied again"; RTC_LOG(LS_INFO) << "Firewall rules applied again";
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionFailed, ASSERT_EQ_SIMULATED_WAIT(PeerConnectionInterface::kIceConnectionFailed,
caller()->ice_connection_state(), kConsentTimeout); caller()->ice_connection_state(), kConsentTimeout,
ASSERT_EQ_WAIT(PeerConnectionInterface::kIceConnectionFailed, fake_clock);
ASSERT_EQ_SIMULATED_WAIT(PeerConnectionInterface::kIceConnectionFailed,
caller()->standardized_ice_connection_state(), caller()->standardized_ice_connection_state(),
kConsentTimeout); kConsentTimeout, fake_clock);
// We need to manually close the peerconnections before the fake clock goes
// out of scope, or we trigger a DCHECK in rtp_sender.cc when we briefly
// return to using non-faked time.
delete SetCallerPcWrapperAndReturnCurrent(nullptr);
delete SetCalleePcWrapperAndReturnCurrent(nullptr);
}
// Tests that if the connection doesn't get set up properly we eventually reach
// the "failed" iceConnectionState.
TEST_P(PeerConnectionIntegrationIceStatesTest, IceStateSetupFailure) {
rtc::ScopedFakeClock fake_clock;
// Some things use a time of "0" as a special value, so we need to start out
// the fake clock at a nonzero time.
fake_clock.AdvanceTime(TimeDelta::seconds(1));
// Block connections to/from the caller and wait for ICE to become
// disconnected.
for (const auto& caller_address : CallerAddresses()) {
firewall()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, caller_address);
}
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();
SetPortAllocatorFlags();
SetUpNetworkInterfaces();
caller()->AddAudioVideoTracks();
caller()->CreateAndSetAndSignalOffer();
// 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
// is signaled by the state transitioning to "failed".
constexpr int kConsentTimeout = 30000;
ASSERT_EQ_SIMULATED_WAIT(PeerConnectionInterface::kIceConnectionFailed,
caller()->standardized_ice_connection_state(),
kConsentTimeout, fake_clock);
// We need to manually close the peerconnections before the fake clock goes
// out of scope, or we trigger a DCHECK in rtp_sender.cc when we briefly
// return to using non-faked time.
delete SetCallerPcWrapperAndReturnCurrent(nullptr);
delete SetCalleePcWrapperAndReturnCurrent(nullptr);
} }
// Tests that the best connection is set to the appropriate IPv4/IPv6 connection // Tests that the best connection is set to the appropriate IPv4/IPv6 connection