From 3d564c10157d7de1d2d4236f4e2a13ff1363d52b Mon Sep 17 00:00:00 2001 From: Guo-wei Shieh Date: Wed, 19 Aug 2015 16:51:15 -0700 Subject: [PATCH] Add instrumentation to track the IceEndpointType. The IceEndpointType has the format of _. It is recorded on the BestConnection when we have the first OnTransportCompleted signaled. BUG=webrtc:4918 R=pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/1277263002 . Cr-Commit-Position: refs/heads/master@{#9737} --- talk/app/webrtc/fakemetricsobserver.cc | 23 ++++- talk/app/webrtc/fakemetricsobserver.h | 12 ++- talk/app/webrtc/objc/RTCEnumConverter.mm | 3 + talk/app/webrtc/objc/public/RTCTypes.h | 1 + talk/app/webrtc/peerconnectioninterface.h | 16 +++- talk/app/webrtc/umametrics.h | 50 ++++++++-- talk/app/webrtc/webrtcsession.cc | 91 +++++++++++++++++-- talk/app/webrtc/webrtcsession_unittest.cc | 12 ++- webrtc/p2p/base/candidate.h | 7 ++ webrtc/p2p/base/port.cc | 2 + webrtc/p2p/base/port.h | 10 +- webrtc/p2p/base/port_unittest.cc | 6 +- webrtc/p2p/base/relayport.cc | 6 +- webrtc/p2p/base/stunport.cc | 11 +-- webrtc/p2p/base/stunport_unittest.cc | 3 + webrtc/p2p/base/tcpport.cc | 17 ++-- webrtc/p2p/base/turnport.cc | 4 +- webrtc/p2p/base/turnport_unittest.cc | 4 + .../client/connectivitychecker_unittest.cc | 2 +- 19 files changed, 228 insertions(+), 52 deletions(-) diff --git a/talk/app/webrtc/fakemetricsobserver.cc b/talk/app/webrtc/fakemetricsobserver.cc index 66e1c51f2a..e833834d48 100644 --- a/talk/app/webrtc/fakemetricsobserver.cc +++ b/talk/app/webrtc/fakemetricsobserver.cc @@ -36,16 +36,26 @@ FakeMetricsObserver::FakeMetricsObserver() { void FakeMetricsObserver::Reset() { DCHECK(thread_checker_.CalledOnValidThread()); - memset(counters_, 0, sizeof(counters_)); + counters_ = std::vector>(); memset(int_histogram_samples_, 0, sizeof(int_histogram_samples_)); for (std::string& type : string_histogram_samples_) { type.clear(); } } -void FakeMetricsObserver::IncrementCounter(PeerConnectionMetricsCounter type) { +void FakeMetricsObserver::IncrementEnumCounter( + PeerConnectionEnumCounterType type, + int counter, + int counter_max) { DCHECK(thread_checker_.CalledOnValidThread()); - ++counters_[type]; + if (counters_.size() <= static_cast(type)) { + counters_.resize(type + 1); + } + auto& counters = counters_[type]; + if (counters.size() < static_cast(counter_max)) { + counters.resize(counter_max); + } + ++counters[counter]; } void FakeMetricsObserver::AddHistogramSample(PeerConnectionMetricsName type, @@ -61,9 +71,12 @@ void FakeMetricsObserver::AddHistogramSample(PeerConnectionMetricsName type, string_histogram_samples_[type].assign(value); } -int FakeMetricsObserver::GetCounter(PeerConnectionMetricsCounter type) const { +int FakeMetricsObserver::GetEnumCounter(PeerConnectionEnumCounterType type, + int counter) const { DCHECK(thread_checker_.CalledOnValidThread()); - return counters_[type]; + CHECK(counters_.size() > static_cast(type) && + counters_[type].size() > static_cast(counter)); + return counters_[type][counter]; } int FakeMetricsObserver::GetIntHistogramSample( diff --git a/talk/app/webrtc/fakemetricsobserver.h b/talk/app/webrtc/fakemetricsobserver.h index e9e49749bf..39454bf4db 100644 --- a/talk/app/webrtc/fakemetricsobserver.h +++ b/talk/app/webrtc/fakemetricsobserver.h @@ -41,14 +41,16 @@ class FakeMetricsObserver : public MetricsObserverInterface { FakeMetricsObserver(); void Reset(); - void IncrementCounter(PeerConnectionMetricsCounter type) override; + void IncrementEnumCounter(PeerConnectionEnumCounterType, + int counter, + int counter_max) override; void AddHistogramSample(PeerConnectionMetricsName type, int value) override; void AddHistogramSample(PeerConnectionMetricsName type, const std::string& value) override; // Accessors to be used by the tests. - int GetCounter(PeerConnectionMetricsCounter type) const; + int GetEnumCounter(PeerConnectionEnumCounterType type, int counter) const; int GetIntHistogramSample(PeerConnectionMetricsName type) const; const std::string& GetStringHistogramSample( PeerConnectionMetricsName type) const; @@ -58,8 +60,10 @@ class FakeMetricsObserver : public MetricsObserverInterface { private: rtc::ThreadChecker thread_checker_; - int counters_[kPeerConnectionMetricsCounter_Max]; - int int_histogram_samples_[kPeerConnectionMetricsCounter_Max]; + // This is a 2 dimension array. The first index is the enum counter type. The + // 2nd index is the counter of that particular enum counter type. + std::vector> counters_; + int int_histogram_samples_[kPeerConnectionMetricsName_Max]; std::string string_histogram_samples_[kPeerConnectionMetricsName_Max]; }; diff --git a/talk/app/webrtc/objc/RTCEnumConverter.mm b/talk/app/webrtc/objc/RTCEnumConverter.mm index 6254b95e80..fa4608a560 100644 --- a/talk/app/webrtc/objc/RTCEnumConverter.mm +++ b/talk/app/webrtc/objc/RTCEnumConverter.mm @@ -48,6 +48,9 @@ return RTCICEConnectionDisconnected; case webrtc::PeerConnectionInterface::kIceConnectionClosed: return RTCICEConnectionClosed; + case webrtc::PeerConnectionInterface::kIceConnectionMax: + NSAssert(NO, @"kIceConnectionMax not allowed"); + return RTCICEConnectionMax; } } diff --git a/talk/app/webrtc/objc/public/RTCTypes.h b/talk/app/webrtc/objc/public/RTCTypes.h index 946148afc3..99ac192c34 100644 --- a/talk/app/webrtc/objc/public/RTCTypes.h +++ b/talk/app/webrtc/objc/public/RTCTypes.h @@ -36,6 +36,7 @@ typedef enum { RTCICEConnectionFailed, RTCICEConnectionDisconnected, RTCICEConnectionClosed, + RTCICEConnectionMax, } RTCICEConnectionState; // RTCICEGatheringState the states in webrtc::ICEGatheringState. diff --git a/talk/app/webrtc/peerconnectioninterface.h b/talk/app/webrtc/peerconnectioninterface.h index bdbc6a7e38..ac6280fc24 100644 --- a/talk/app/webrtc/peerconnectioninterface.h +++ b/talk/app/webrtc/peerconnectioninterface.h @@ -126,7 +126,20 @@ class StatsObserver : public rtc::RefCountInterface { class MetricsObserverInterface : public rtc::RefCountInterface { public: - virtual void IncrementCounter(PeerConnectionMetricsCounter type) = 0; + // TODO(guoweis): Remove this function once IncrementEnumCounter gets into + // chromium. IncrementCounter only deals with one type of enumeration counter, + // i.e. PeerConnectionAddressFamilyCounter. Instead of creating a function for + // each enum type, IncrementEnumCounter is generalized with the enum type + // parameter. + virtual void IncrementCounter(PeerConnectionAddressFamilyCounter type) {} + + // |type| is the type of the enum counter to be incremented. |counter| + // is the particular counter in that type. |counter_max| is the next sequence + // number after the highest counter. + virtual void IncrementEnumCounter(PeerConnectionEnumCounterType type, + int counter, + int counter_max) {} + virtual void AddHistogramSample(PeerConnectionMetricsName type, int value) = 0; // TODO(jbauch): Make method abstract when it is implemented by Chromium. @@ -178,6 +191,7 @@ class PeerConnectionInterface : public rtc::RefCountInterface { kIceConnectionFailed, kIceConnectionDisconnected, kIceConnectionClosed, + kIceConnectionMax, }; struct IceServer { diff --git a/talk/app/webrtc/umametrics.h b/talk/app/webrtc/umametrics.h index bb9e05230d..37e7b6de42 100644 --- a/talk/app/webrtc/umametrics.h +++ b/talk/app/webrtc/umametrics.h @@ -32,11 +32,24 @@ namespace webrtc { +// Used to specify which enum counter type we're incrementing in +// MetricsObserverInterface::IncrementEnumCounter. +enum PeerConnectionEnumCounterType { + kEnumCounterAddressFamily, + // For the next 2 counters, we track them separately based on the "first hop" + // protocol used by the local candidate. "First hop" means the local candidate + // type in the case of non-TURN candidates, and the protocol used to connect + // to the TURN server in the case of TURN candidates. + kEnumCounterIceCandidatePairTypeUdp, + kEnumCounterIceCandidatePairTypeTcp, + kPeerConnectionEnumCounterMax +}; + // Currently this contains information related to WebRTC network/transport // information. -// The difference between PeerConnectionMetricsCounter and -// PeerConnectionMetricsName is that the "Counter" is only counting the +// The difference between PeerConnectionEnumCounter and +// PeerConnectionMetricsName is that the "EnumCounter" is only counting the // occurrences of events, while "Name" has a value associated with it which is // used to form a histogram. @@ -44,19 +57,19 @@ namespace webrtc { // chromium/src/tools/metrics/histograms/histograms.xml // Existing values cannot be re-ordered and new enums must be added // before kBoundary. -enum PeerConnectionMetricsCounter { +enum PeerConnectionAddressFamilyCounter { kPeerConnection_IPv4, kPeerConnection_IPv6, kBestConnections_IPv4, kBestConnections_IPv6, - kPeerConnectionMetricsCounter_Max, + kPeerConnectionAddressFamilyCounter_Max, }; // TODO(guoweis): Keep previous name here until all references are renamed. -#define kBoundary kPeerConnectionMetricsCounter_Max +#define kBoundary kPeerConnectionAddressFamilyCounter_Max // TODO(guoweis): Keep previous name here until all references are renamed. -typedef PeerConnectionMetricsCounter PeerConnectionUMAMetricsCounter; +typedef PeerConnectionAddressFamilyCounter PeerConnectionUMAMetricsCounter; // This enum defines types for UMA samples, which will have a range. enum PeerConnectionMetricsName { @@ -77,6 +90,29 @@ enum PeerConnectionMetricsName { // TODO(guoweis): Keep previous name here until all references are renamed. typedef PeerConnectionMetricsName PeerConnectionUMAMetricsName; +// The IceCandidatePairType has the format of +// _. It is recorded based on the +// type of candidate pair used when the PeerConnection first goes to a completed +// state. When BUNDLE is enabled, only the first transport gets recorded. +enum IceCandidatePairType { + kIceCandidatePairHostHost, + kIceCandidatePairHostSrflx, + kIceCandidatePairHostRelay, + kIceCandidatePairHostPrflx, + kIceCandidatePairSrflxHost, + kIceCandidatePairSrflxSrflx, + kIceCandidatePairSrflxRelay, + kIceCandidatePairSrflxPrflx, + kIceCandidatePairRelayHost, + kIceCandidatePairRelaySrflx, + kIceCandidatePairRelayRelay, + kIceCandidatePairRelayPrflx, + kIceCandidatePairPrflxHost, + kIceCandidatePairPrflxSrflx, + kIceCandidatePairPrflxRelay, + kIceCandidatePairMax +}; + } // namespace webrtc -#endif // TALK_APP_WEBRTC_UMA6METRICS_H_ +#endif // TALK_APP_WEBRTC_UMAMETRICS_H_ diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index c37eadb8a8..fde45104d6 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -57,6 +57,11 @@ using cricket::MediaContentDescription; using cricket::SessionDescription; using cricket::TransportInfo; +using cricket::LOCAL_PORT_TYPE; +using cricket::STUN_PORT_TYPE; +using cricket::RELAY_PORT_TYPE; +using cricket::PRFLX_PORT_TYPE; + namespace webrtc { // Error messages @@ -83,6 +88,48 @@ const char kDtlsSetupFailureRtcp[] = "Couldn't set up DTLS-SRTP on RTCP channel."; const int kMaxUnsignalledRecvStreams = 20; +IceCandidatePairType GetIceCandidatePairCounter( + const cricket::Candidate& local, + const cricket::Candidate& remote) { + const auto& l = local.type(); + const auto& r = remote.type(); + const auto& host = LOCAL_PORT_TYPE; + const auto& srflx = STUN_PORT_TYPE; + const auto& relay = RELAY_PORT_TYPE; + const auto& prflx = PRFLX_PORT_TYPE; + if (l == host && r == host) + return kIceCandidatePairHostHost; + if (l == host && r == srflx) + return kIceCandidatePairHostSrflx; + if (l == host && r == relay) + return kIceCandidatePairHostRelay; + if (l == host && r == prflx) + return kIceCandidatePairHostPrflx; + if (l == srflx && r == host) + return kIceCandidatePairSrflxHost; + if (l == srflx && r == srflx) + return kIceCandidatePairSrflxSrflx; + if (l == srflx && r == relay) + return kIceCandidatePairSrflxRelay; + if (l == srflx && r == prflx) + return kIceCandidatePairSrflxPrflx; + if (l == relay && r == host) + return kIceCandidatePairRelayHost; + if (l == relay && r == srflx) + return kIceCandidatePairRelaySrflx; + if (l == relay && r == relay) + return kIceCandidatePairRelayRelay; + if (l == relay && r == prflx) + return kIceCandidatePairRelayPrflx; + if (l == prflx && r == host) + return kIceCandidatePairPrflxHost; + if (l == prflx && r == srflx) + return kIceCandidatePairPrflxSrflx; + if (l == prflx && r == relay) + return kIceCandidatePairPrflxRelay; + return kIceCandidatePairMax; +} + // Compares |answer| against |offer|. Comparision is done // for number of m-lines in answer against offer. If matches true will be // returned otherwise false. @@ -1921,14 +1968,46 @@ void WebRtcSession::ReportBestConnectionState( if (!it_info->best_connection) { continue; } - if (it_info->local_candidate.address().family() == AF_INET) { - metrics_observer_->IncrementCounter(kBestConnections_IPv4); - } else if (it_info->local_candidate.address().family() == - AF_INET6) { - metrics_observer_->IncrementCounter(kBestConnections_IPv6); + + PeerConnectionEnumCounterType type = kPeerConnectionEnumCounterMax; + const cricket::Candidate& local = it_info->local_candidate; + const cricket::Candidate& remote = it_info->remote_candidate; + + // Increment the counter for IceCandidatePairType. + if (local.protocol() == cricket::TCP_PROTOCOL_NAME || + (local.type() == RELAY_PORT_TYPE && + local.relay_protocol() == cricket::TCP_PROTOCOL_NAME)) { + type = kEnumCounterIceCandidatePairTypeTcp; + } else if (local.protocol() == cricket::UDP_PROTOCOL_NAME) { + type = kEnumCounterIceCandidatePairTypeUdp; } else { - RTC_NOTREACHED(); + CHECK(0); } + metrics_observer_->IncrementEnumCounter( + type, GetIceCandidatePairCounter(local, remote), + kIceCandidatePairMax); + + // Increment the counter for IP type. + if (local.address().family() == AF_INET) { + // TODO(guoweis): Remove this next line once IncrementEnumCounter + // implemented for PeerConnectionMetrics. + metrics_observer_->IncrementCounter(kBestConnections_IPv4); + + metrics_observer_->IncrementEnumCounter( + kEnumCounterAddressFamily, kBestConnections_IPv4, + kPeerConnectionAddressFamilyCounter_Max); + + } else if (local.address().family() == AF_INET6) { + // TODO(guoweis): Remove this next line. + metrics_observer_->IncrementCounter(kBestConnections_IPv6); + + metrics_observer_->IncrementEnumCounter( + kEnumCounterAddressFamily, kBestConnections_IPv6, + kPeerConnectionAddressFamilyCounter_Max); + } else { + CHECK(0); + } + return; } } diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index ecbd5074cd..e442cf158b 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -1009,11 +1009,19 @@ class WebRtcSessionTest : public testing::Test { void Verify(const rtc::scoped_refptr metrics_observer, const ExpectedBestConnection& expected) const { EXPECT_EQ( - metrics_observer->GetCounter(webrtc::kBestConnections_IPv4), + metrics_observer->GetEnumCounter(webrtc::kEnumCounterAddressFamily, + webrtc::kBestConnections_IPv4), expected.ipv4_count_); EXPECT_EQ( - metrics_observer->GetCounter(webrtc::kBestConnections_IPv6), + metrics_observer->GetEnumCounter(webrtc::kEnumCounterAddressFamily, + webrtc::kBestConnections_IPv6), expected.ipv6_count_); + // This is used in the loopback call so there is only single host to host + // candidate pair. + EXPECT_EQ(metrics_observer->GetEnumCounter( + webrtc::kEnumCounterIceCandidatePairTypeUdp, + webrtc::kIceCandidatePairHostHost), + 1); } }; diff --git a/webrtc/p2p/base/candidate.h b/webrtc/p2p/base/candidate.h index c2b889541e..66b0ab04b3 100644 --- a/webrtc/p2p/base/candidate.h +++ b/webrtc/p2p/base/candidate.h @@ -70,6 +70,12 @@ class Candidate { const std::string & protocol() const { return protocol_; } void set_protocol(const std::string & protocol) { protocol_ = protocol; } + // The protocol used to talk to relay. + const std::string& relay_protocol() const { return relay_protocol_; } + void set_relay_protocol(const std::string& protocol) { + relay_protocol_ = protocol; + } + const rtc::SocketAddress & address() const { return address_; } void set_address(const rtc::SocketAddress & address) { address_ = address; @@ -214,6 +220,7 @@ class Candidate { std::string id_; int component_; std::string protocol_; + std::string relay_protocol_; rtc::SocketAddress address_; uint32 priority_; std::string username_; diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index 4023b322e7..02908cec24 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -249,6 +249,7 @@ void Port::AddAddress(const rtc::SocketAddress& address, const rtc::SocketAddress& base_address, const rtc::SocketAddress& related_address, const std::string& protocol, + const std::string& relay_protocol, const std::string& tcptype, const std::string& type, uint32 type_preference, @@ -263,6 +264,7 @@ void Port::AddAddress(const rtc::SocketAddress& address, c.set_component(component_); c.set_type(type); c.set_protocol(protocol); + c.set_relay_protocol(relay_protocol); c.set_tcptype(tcptype); c.set_address(address); c.set_priority(c.GetPriority(type_preference, network_->preference(), diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h index b95d985b21..a08e6b602a 100644 --- a/webrtc/p2p/base/port.h +++ b/webrtc/p2p/base/port.h @@ -313,9 +313,13 @@ class Port : public PortInterface, public rtc::MessageHandler, void AddAddress(const rtc::SocketAddress& address, const rtc::SocketAddress& base_address, const rtc::SocketAddress& related_address, - const std::string& protocol, const std::string& tcptype, - const std::string& type, uint32 type_preference, - uint32 relay_preference, bool final); + const std::string& protocol, + const std::string& relay_protocol, + const std::string& tcptype, + const std::string& type, + uint32 type_preference, + uint32 relay_preference, + bool final); // Adds the given connection to the list. (Deleting removes them.) void AddConnection(Connection* conn); diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc index 4877af3c4e..289e318eb2 100644 --- a/webrtc/p2p/base/port_unittest.cc +++ b/webrtc/p2p/base/port_unittest.cc @@ -135,13 +135,13 @@ class TestPort : public Port { virtual void PrepareAddress() { rtc::SocketAddress addr(ip(), min_port()); - AddAddress(addr, addr, rtc::SocketAddress(), "udp", "", Type(), + AddAddress(addr, addr, rtc::SocketAddress(), "udp", "", "", Type(), ICE_TYPE_PREFERENCE_HOST, 0, true); } // Exposed for testing candidate building. void AddCandidateAddress(const rtc::SocketAddress& addr) { - AddAddress(addr, addr, rtc::SocketAddress(), "udp", "", Type(), + AddAddress(addr, addr, rtc::SocketAddress(), "udp", "", "", Type(), type_preference_, 0, false); } void AddCandidateAddress(const rtc::SocketAddress& addr, @@ -149,7 +149,7 @@ class TestPort : public Port { const std::string& type, int type_preference, bool final) { - AddAddress(addr, base_address, rtc::SocketAddress(), "udp", "", type, + AddAddress(addr, base_address, rtc::SocketAddress(), "udp", "", "", type, type_preference, 0, final); } diff --git a/webrtc/p2p/base/relayport.cc b/webrtc/p2p/base/relayport.cc index e593f52ecf..fc077ff1e1 100644 --- a/webrtc/p2p/base/relayport.cc +++ b/webrtc/p2p/base/relayport.cc @@ -231,9 +231,9 @@ void RelayPort::SetReady() { // In case of Gturn, related address is set to null socket address. // This is due to as mapped address stun attribute is used for allocated // address. - AddAddress(iter->address, iter->address, rtc::SocketAddress(), - proto_name, "", RELAY_PORT_TYPE, - ICE_TYPE_PREFERENCE_RELAY, 0, false); + AddAddress(iter->address, iter->address, rtc::SocketAddress(), proto_name, + proto_name, "", RELAY_PORT_TYPE, ICE_TYPE_PREFERENCE_RELAY, 0, + false); } ready_ = true; SignalPortComplete(this); diff --git a/webrtc/p2p/base/stunport.cc b/webrtc/p2p/base/stunport.cc index 953e585444..925bd8e9de 100644 --- a/webrtc/p2p/base/stunport.cc +++ b/webrtc/p2p/base/stunport.cc @@ -280,9 +280,8 @@ int UDPPort::GetError() { void UDPPort::OnLocalAddressReady(rtc::AsyncPacketSocket* socket, const rtc::SocketAddress& address) { - AddAddress(address, address, rtc::SocketAddress(), - UDP_PROTOCOL_NAME, "", LOCAL_PORT_TYPE, - ICE_TYPE_PREFERENCE_HOST, 0, false); + AddAddress(address, address, rtc::SocketAddress(), UDP_PROTOCOL_NAME, "", "", + LOCAL_PORT_TYPE, ICE_TYPE_PREFERENCE_HOST, 0, false); MaybePrepareStunCandidate(); } @@ -396,9 +395,9 @@ void UDPPort::OnStunBindingRequestSucceeded( related_address.family()); } - AddAddress(stun_reflected_addr, socket_->GetLocalAddress(), - related_address, UDP_PROTOCOL_NAME, "", - STUN_PORT_TYPE, ICE_TYPE_PREFERENCE_SRFLX, 0, false); + AddAddress(stun_reflected_addr, socket_->GetLocalAddress(), related_address, + UDP_PROTOCOL_NAME, "", "", STUN_PORT_TYPE, + ICE_TYPE_PREFERENCE_SRFLX, 0, false); } MaybeSetPortCompleteOrError(); } diff --git a/webrtc/p2p/base/stunport_unittest.cc b/webrtc/p2p/base/stunport_unittest.cc index 8b6e81b6d0..8897dd4b98 100644 --- a/webrtc/p2p/base/stunport_unittest.cc +++ b/webrtc/p2p/base/stunport_unittest.cc @@ -250,6 +250,7 @@ TEST_F(StunPortTest, TestNoDuplicatedAddressWithTwoStunServers) { PrepareAddress(); EXPECT_TRUE_WAIT(done(), kTimeoutMs); EXPECT_EQ(1U, port()->Candidates().size()); + EXPECT_EQ(port()->Candidates()[0].relay_protocol(), ""); } // Test that candidates can be allocated for multiple STUN servers, one of which @@ -281,4 +282,6 @@ TEST_F(StunPortTest, TestTwoCandidatesWithTwoStunServersAcrossNat) { PrepareAddress(); EXPECT_TRUE_WAIT(done(), kTimeoutMs); EXPECT_EQ(2U, port()->Candidates().size()); + EXPECT_EQ(port()->Candidates()[0].relay_protocol(), ""); + EXPECT_EQ(port()->Candidates()[1].relay_protocol(), ""); } diff --git a/webrtc/p2p/base/tcpport.cc b/webrtc/p2p/base/tcpport.cc index 5c9e0425c8..6dc66d26cf 100644 --- a/webrtc/p2p/base/tcpport.cc +++ b/webrtc/p2p/base/tcpport.cc @@ -171,17 +171,16 @@ void TCPPort::PrepareAddress() { if (socket_->GetState() == rtc::AsyncPacketSocket::STATE_BOUND || socket_->GetState() == rtc::AsyncPacketSocket::STATE_CLOSED) AddAddress(socket_->GetLocalAddress(), socket_->GetLocalAddress(), - rtc::SocketAddress(), - TCP_PROTOCOL_NAME, TCPTYPE_PASSIVE_STR, LOCAL_PORT_TYPE, + rtc::SocketAddress(), TCP_PROTOCOL_NAME, "", + TCPTYPE_PASSIVE_STR, LOCAL_PORT_TYPE, ICE_TYPE_PREFERENCE_HOST_TCP, 0, true); } else { LOG_J(LS_INFO, this) << "Not listening due to firewall restrictions."; // Note: We still add the address, since otherwise the remote side won't // recognize our incoming TCP connections. - AddAddress(rtc::SocketAddress(ip(), 0), - rtc::SocketAddress(ip(), 0), rtc::SocketAddress(), - TCP_PROTOCOL_NAME, TCPTYPE_ACTIVE_STR, LOCAL_PORT_TYPE, - ICE_TYPE_PREFERENCE_HOST_TCP, 0, true); + AddAddress(rtc::SocketAddress(ip(), 0), rtc::SocketAddress(ip(), 0), + rtc::SocketAddress(), TCP_PROTOCOL_NAME, "", TCPTYPE_ACTIVE_STR, + LOCAL_PORT_TYPE, ICE_TYPE_PREFERENCE_HOST_TCP, 0, true); } } @@ -285,9 +284,9 @@ void TCPPort::OnReadyToSend(rtc::AsyncPacketSocket* socket) { void TCPPort::OnAddressReady(rtc::AsyncPacketSocket* socket, const rtc::SocketAddress& address) { - AddAddress(address, address, rtc::SocketAddress(), - TCP_PROTOCOL_NAME, TCPTYPE_PASSIVE_STR, LOCAL_PORT_TYPE, - ICE_TYPE_PREFERENCE_HOST_TCP, 0, true); + AddAddress(address, address, rtc::SocketAddress(), TCP_PROTOCOL_NAME, "", + TCPTYPE_PASSIVE_STR, LOCAL_PORT_TYPE, ICE_TYPE_PREFERENCE_HOST_TCP, + 0, true); } TCPConnection::TCPConnection(TCPPort* port, diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc index 2c9cd13045..2e4e26d0d1 100644 --- a/webrtc/p2p/base/turnport.cc +++ b/webrtc/p2p/base/turnport.cc @@ -675,11 +675,11 @@ void TurnPort::OnAllocateSuccess(const rtc::SocketAddress& address, address, // Base address. related_address, // Related address. UDP_PROTOCOL_NAME, + ProtoToString(server_address_.proto), // The first hop protocol. "", // TCP canddiate type, empty for turn candidates. RELAY_PORT_TYPE, GetRelayPreference(server_address_.proto, server_address_.secure), - server_priority_, - true); + server_priority_, true); } void TurnPort::OnAllocateError() { diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc index 00a62fa70b..b5fdfc1f44 100644 --- a/webrtc/p2p/base/turnport_unittest.cc +++ b/webrtc/p2p/base/turnport_unittest.cc @@ -734,6 +734,8 @@ TEST_F(TurnPortTest, TestTurnSendDataTurnUdpToUdp) { // Create ports and prepare addresses. CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr); TestTurnSendData(); + EXPECT_EQ(turn_port_->Candidates()[0].relay_protocol(), + cricket::UDP_PROTOCOL_NAME); } // Do a TURN allocation, establish a TCP connection, and send some data. @@ -742,6 +744,8 @@ TEST_F(TurnPortTest, TestTurnSendDataTurnTcpToUdp) { // Create ports and prepare addresses. CreateTurnPort(kTurnUsername, kTurnPassword, kTurnTcpProtoAddr); TestTurnSendData(); + EXPECT_EQ(turn_port_->Candidates()[0].relay_protocol(), + cricket::TCP_PROTOCOL_NAME); } // Test TURN fails to make a connection from IPv6 address to a server which has diff --git a/webrtc/p2p/client/connectivitychecker_unittest.cc b/webrtc/p2p/client/connectivitychecker_unittest.cc index 71c848397d..d590a7206f 100644 --- a/webrtc/p2p/client/connectivitychecker_unittest.cc +++ b/webrtc/p2p/client/connectivitychecker_unittest.cc @@ -81,7 +81,7 @@ class FakeStunPort : public StunPort { // Just set external address and signal that we are done. virtual void PrepareAddress() { AddAddress(kExternalAddr, kExternalAddr, rtc::SocketAddress(), "udp", "", - STUN_PORT_TYPE, ICE_TYPE_PREFERENCE_SRFLX, 0, true); + "", STUN_PORT_TYPE, ICE_TYPE_PREFERENCE_SRFLX, 0, true); SignalPortComplete(this); } };