diff --git a/webrtc/BUILD.gn b/webrtc/BUILD.gn index fe8760fed7..d47cdf9271 100644 --- a/webrtc/BUILD.gn +++ b/webrtc/BUILD.gn @@ -337,6 +337,7 @@ if (rtc_include_tests) { rtc_test("rtc_unittests") { testonly = true sources = [ + "api/fakemetricsobserver.cc", "base/array_view_unittest.cc", "base/atomicops_unittest.cc", "base/autodetectproxy_unittest.cc", diff --git a/webrtc/api/fakemetricsobserver.cc b/webrtc/api/fakemetricsobserver.cc index 71e71b3432..1b6265d7d5 100644 --- a/webrtc/api/fakemetricsobserver.cc +++ b/webrtc/api/fakemetricsobserver.cc @@ -45,7 +45,9 @@ void FakeMetricsObserver::AddHistogramSample(PeerConnectionMetricsName type, int FakeMetricsObserver::GetEnumCounter(PeerConnectionEnumCounterType type, int counter) const { RTC_DCHECK(thread_checker_.CalledOnValidThread()); - RTC_CHECK(counters_.size() > static_cast(type)); + if (counters_.size() <= static_cast(type)) { + return 0; + } const auto& it = counters_[type].find(counter); if (it == counters_[type].end()) { return 0; diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index 144b1c64b5..7cc5d00a30 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -1300,6 +1300,7 @@ void PeerConnection::RegisterUMAObserver(UMAObserver* observer) { // Send information about IPv4/IPv6 status. if (uma_observer_ && port_allocator_) { + port_allocator_->SetMetricsObserver(uma_observer_); if (port_allocator_->flags() & cricket::PORTALLOCATOR_ENABLE_IPV6) { uma_observer_->IncrementEnumCounter( kEnumCounterAddressFamily, kPeerConnection_IPv6, diff --git a/webrtc/api/umametrics.h b/webrtc/api/umametrics.h index 8dbfa22716..93c034f7c2 100644 --- a/webrtc/api/umametrics.h +++ b/webrtc/api/umametrics.h @@ -33,6 +33,8 @@ enum PeerConnectionEnumCounterType { kEnumCounterDataSrtpCipher, kEnumCounterDataSslCipher, kEnumCounterDtlsHandshakeError, + kEnumCounterIceRegathering, + kEnumCounterIceRestart, kPeerConnectionEnumCounterMax }; diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h index c62ea9984e..94f146ff57 100644 --- a/webrtc/api/webrtcsession.h +++ b/webrtc/api/webrtcsession.h @@ -293,6 +293,7 @@ class WebRtcSession : void set_metrics_observer( webrtc::MetricsObserverInterface* metrics_observer) { metrics_observer_ = metrics_observer; + transport_controller_->SetMetricsObserver(metrics_observer); } // Called when voice_channel_, video_channel_ and data_channel_ are created diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index a07c605e05..5ab3dca771 100644 --- a/webrtc/p2p/base/dtlstransportchannel.h +++ b/webrtc/p2p/base/dtlstransportchannel.h @@ -186,6 +186,10 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { channel_->RemoveRemoteCandidate(candidate); } + void SetMetricsObserver(webrtc::MetricsObserverInterface* observer) override { + channel_->SetMetricsObserver(observer); + } + void SetIceConfig(const IceConfig& config) override { channel_->SetIceConfig(config); } diff --git a/webrtc/p2p/base/faketransportcontroller.h b/webrtc/p2p/base/faketransportcontroller.h index 5d0ceb434d..573d571102 100644 --- a/webrtc/p2p/base/faketransportcontroller.h +++ b/webrtc/p2p/base/faketransportcontroller.h @@ -299,6 +299,9 @@ class FakeTransportChannel : public TransportChannelImpl, return ssl_max_version_; } + void SetMetricsObserver(webrtc::MetricsObserverInterface* observer) override { + } + private: void NegotiateSrtpCiphers() { for (std::vector::const_iterator it1 = srtp_ciphers_.begin(); diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index 3f68d6d6e0..6bcf0d7d2b 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -13,6 +13,7 @@ #include #include +#include "webrtc/api/peerconnectioninterface.h" #include "webrtc/base/common.h" #include "webrtc/base/crc32.h" #include "webrtc/base/logging.h" @@ -435,6 +436,11 @@ const IceConfig& P2PTransportChannel::config() const { return config_; } +void P2PTransportChannel::SetMetricsObserver( + webrtc::MetricsObserverInterface* observer) { + metrics_observer_ = observer; +} + void P2PTransportChannel::MaybeStartGathering() { if (ice_parameters_.ufrag.empty() || ice_parameters_.pwd.empty()) { LOG(LS_ERROR) << "Cannot gather candidates because ICE parameters are empty" @@ -451,6 +457,21 @@ void P2PTransportChannel::MaybeStartGathering() { gathering_state_ = kIceGatheringGathering; SignalGatheringState(this); } + + if (metrics_observer_ && !allocator_sessions_.empty()) { + IceRestartState state; + if (writable()) { + state = IceRestartState::CONNECTED; + } else if (IsGettingPorts()) { + state = IceRestartState::CONNECTING; + } else { + state = IceRestartState::DISCONNECTED; + } + metrics_observer_->IncrementEnumCounter( + webrtc::kEnumCounterIceRestart, static_cast(state), + static_cast(IceRestartState::MAX_VALUE)); + } + // Time for a new allocator. std::unique_ptr pooled_session = allocator_->TakePooledSession(transport_name(), component(), @@ -473,7 +494,6 @@ void P2PTransportChannel::MaybeStartGathering() { AddAllocatorSession(allocator_->CreateSession( transport_name(), component(), ice_parameters_.ufrag, ice_parameters_.pwd)); - LOG(LS_INFO) << "Start getting ports"; allocator_sessions_.back()->StartGettingPorts(); } } diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 7ffa430089..672abb7423 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -38,6 +38,10 @@ namespace cricket { +// Enum for UMA metrics, used to record whether the channel is +// connected/connecting/disconnected when ICE restart happens. +enum class IceRestartState { CONNECTING, CONNECTED, DISCONNECTED, MAX_VALUE }; + extern const int WEAK_PING_INTERVAL; extern const int STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL; extern const int STABLE_WRITABLE_CONNECTION_PING_INTERVAL; @@ -95,6 +99,7 @@ class P2PTransportChannel : public TransportChannelImpl, // TODO(deadbeef): Use rtc::Optional instead of negative values. void SetIceConfig(const IceConfig& config) override; const IceConfig& config() const; + void SetMetricsObserver(webrtc::MetricsObserverInterface* observer) override; // From TransportChannel: int SendPacket(const char* data, @@ -394,6 +399,8 @@ class P2PTransportChannel : public TransportChannelImpl, // connection. A zero-value indicates the connection will not be nominated. uint32_t nomination_ = 0; + webrtc::MetricsObserverInterface* metrics_observer_ = nullptr; + RTC_DISALLOW_COPY_AND_ASSIGN(P2PTransportChannel); }; diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index 70ef00701e..7e65b4baba 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -11,6 +11,7 @@ #include #include +#include "webrtc/api/fakemetricsobserver.h" #include "webrtc/p2p/base/fakeportallocator.h" #include "webrtc/p2p/base/p2ptransportchannel.h" #include "webrtc/p2p/base/testrelayserver.h" @@ -198,9 +199,15 @@ class P2PTransportChannelTestBase : public testing::Test, ep1_.allocator_.reset( CreateBasicPortAllocator(&ep1_.network_manager_, stun_servers, kTurnUdpIntAddr, rtc::SocketAddress())); + ep1_.metrics_observer_ = + new rtc::RefCountedObject(); + ep1_.allocator_->SetMetricsObserver(ep1_.metrics_observer_); ep2_.allocator_.reset( CreateBasicPortAllocator(&ep2_.network_manager_, stun_servers, kTurnUdpIntAddr, rtc::SocketAddress())); + ep2_.metrics_observer_ = + new rtc::RefCountedObject(); + ep2_.allocator_->SetMetricsObserver(ep2_.metrics_observer_); } protected: @@ -298,6 +305,9 @@ class P2PTransportChannelTestBase : public testing::Test, } rtc::FakeNetworkManager network_manager_; + // |metrics_observer_| should outlive |allocator_| as the former may be + // used by the latter. + rtc::scoped_refptr metrics_observer_; std::unique_ptr allocator_; ChannelData cd1_; ChannelData cd2_; @@ -334,6 +344,8 @@ class P2PTransportChannelTestBase : public testing::Test, ice_ep1_cd1_ch, ice_ep2_cd1_ch)); ep2_.cd1_.ch_.reset(CreateChannel(1, ICE_CANDIDATE_COMPONENT_DEFAULT, ice_ep2_cd1_ch, ice_ep1_cd1_ch)); + ep1_.cd1_.ch_->SetMetricsObserver(ep1_.metrics_observer_); + ep2_.cd1_.ch_->SetMetricsObserver(ep2_.metrics_observer_); ep1_.cd1_.ch_->SetIceConfig(ep1_config); ep2_.cd1_.ch_->SetIceConfig(ep2_config); ep1_.cd1_.ch_->MaybeStartGathering(); @@ -416,6 +428,9 @@ class P2PTransportChannelTestBase : public testing::Test, PortAllocator* GetAllocator(int endpoint) { return GetEndpoint(endpoint)->allocator_.get(); } + webrtc::FakeMetricsObserver* GetMetricsObserver(int endpoint) { + return GetEndpoint(endpoint)->metrics_observer_; + } void AddAddress(int endpoint, const SocketAddress& addr) { GetEndpoint(endpoint)->network_manager_.AddInterface(addr); } @@ -958,13 +973,12 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase { Config config2, int allocator_flags1, int allocator_flags2) { - int delay = kMinimumStepDelay; ConfigureEndpoint(0, config1); SetAllocatorFlags(0, allocator_flags1); - SetAllocationStepDelay(0, delay); + SetAllocationStepDelay(0, kMinimumStepDelay); ConfigureEndpoint(1, config2); SetAllocatorFlags(1, allocator_flags2); - SetAllocationStepDelay(1, delay); + SetAllocationStepDelay(1, kMinimumStepDelay); set_remote_ice_parameter_source(FROM_SETICEPARAMETERS); } @@ -1172,6 +1186,169 @@ TEST_F(P2PTransportChannelTest, GetStats) { DestroyChannels(); } +// Tests that UMAs are recorded when ICE restarts while the channel +// is disconnected. +TEST_F(P2PTransportChannelTest, TestUMAIceRestartWhileDisconnected) { + rtc::ScopedFakeClock clock; + ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); + + CreateChannels(); + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && + ep2_ch1()->receiving() && + ep2_ch1()->writable(), + kDefaultTimeout, clock); + + // Drop all packets so that both channels become not writable. + fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]); + const int kWriteTimeoutDelay = 6000; + EXPECT_TRUE_SIMULATED_WAIT(!ep1_ch1()->writable() && !ep2_ch1()->writable(), + kWriteTimeoutDelay, clock); + + ep1_ch1()->SetIceParameters(kIceParams[2]); + ep1_ch1()->SetRemoteIceParameters(kIceParams[3]); + ep1_ch1()->MaybeStartGathering(); + EXPECT_EQ(1, GetMetricsObserver(0)->GetEnumCounter( + webrtc::kEnumCounterIceRestart, + static_cast(IceRestartState::DISCONNECTED))); + + ep2_ch1()->SetIceParameters(kIceParams[3]); + ep2_ch1()->SetRemoteIceParameters(kIceParams[2]); + ep2_ch1()->MaybeStartGathering(); + EXPECT_EQ(1, GetMetricsObserver(1)->GetEnumCounter( + webrtc::kEnumCounterIceRestart, + static_cast(IceRestartState::DISCONNECTED))); + + DestroyChannels(); +} + +// Tests that UMAs are recorded when ICE restarts while the channel +// is connected. +TEST_F(P2PTransportChannelTest, TestUMAIceRestartWhileConnected) { + rtc::ScopedFakeClock clock; + ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); + + CreateChannels(); + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && + ep2_ch1()->receiving() && + ep2_ch1()->writable(), + kDefaultTimeout, clock); + + ep1_ch1()->SetIceParameters(kIceParams[2]); + ep1_ch1()->SetRemoteIceParameters(kIceParams[3]); + ep1_ch1()->MaybeStartGathering(); + EXPECT_EQ(1, GetMetricsObserver(0)->GetEnumCounter( + webrtc::kEnumCounterIceRestart, + static_cast(IceRestartState::CONNECTED))); + + ep2_ch1()->SetIceParameters(kIceParams[3]); + ep2_ch1()->SetRemoteIceParameters(kIceParams[2]); + ep2_ch1()->MaybeStartGathering(); + EXPECT_EQ(1, GetMetricsObserver(1)->GetEnumCounter( + webrtc::kEnumCounterIceRestart, + static_cast(IceRestartState::CONNECTED))); + + DestroyChannels(); +} + +// Tests that UMAs are recorded when ICE restarts while the channel +// is connecting. +TEST_F(P2PTransportChannelTest, TestUMAIceRestartWhileConnecting) { + rtc::ScopedFakeClock clock; + ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); + + // Create the channels without waiting for them to become connected. + CreateChannels(); + + ep1_ch1()->SetIceParameters(kIceParams[2]); + ep1_ch1()->SetRemoteIceParameters(kIceParams[3]); + ep1_ch1()->MaybeStartGathering(); + EXPECT_EQ(1, GetMetricsObserver(0)->GetEnumCounter( + webrtc::kEnumCounterIceRestart, + static_cast(IceRestartState::CONNECTING))); + + ep2_ch1()->SetIceParameters(kIceParams[3]); + ep2_ch1()->SetRemoteIceParameters(kIceParams[2]); + ep2_ch1()->MaybeStartGathering(); + EXPECT_EQ(1, GetMetricsObserver(1)->GetEnumCounter( + webrtc::kEnumCounterIceRestart, + static_cast(IceRestartState::CONNECTING))); + + DestroyChannels(); +} + +// Tests that a UMA on ICE regathering is recorded when there is a network +// change if and only if continual gathering is enabled. +TEST_F(P2PTransportChannelTest, + TestIceRegatheringReasonContinualGatheringByNetworkChange) { + rtc::ScopedFakeClock clock; + ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); + + // ep1 gathers continually but ep2 does not. + IceConfig continual_gathering_config = + CreateIceConfig(1000, GATHER_CONTINUALLY); + IceConfig default_config; + CreateChannels(continual_gathering_config, default_config); + + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && + ep2_ch1()->receiving() && + ep2_ch1()->writable(), + kDefaultTimeout, clock); + + // Adding address in ep1 will trigger continual gathering. + AddAddress(0, kAlternateAddrs[0]); + EXPECT_EQ_SIMULATED_WAIT( + 1, GetMetricsObserver(0)->GetEnumCounter( + webrtc::kEnumCounterIceRegathering, + static_cast(IceRegatheringReason::NETWORK_CHANGE)), + kDefaultTimeout, clock); + + ep2_ch1()->SetIceParameters(kIceParams[3]); + ep2_ch1()->SetRemoteIceParameters(kIceParams[2]); + ep2_ch1()->MaybeStartGathering(); + + AddAddress(1, kAlternateAddrs[1]); + SIMULATED_WAIT(false, kDefaultTimeout, clock); + // ep2 has not enabled continual gathering. + EXPECT_EQ(0, GetMetricsObserver(1)->GetEnumCounter( + webrtc::kEnumCounterIceRegathering, + static_cast(IceRegatheringReason::NETWORK_CHANGE))); + + DestroyChannels(); +} + +// Tests that a UMA on ICE regathering is recorded when there is a network +// failure if and only if continual gathering is enabled. +TEST_F(P2PTransportChannelTest, + TestIceRegatheringReasonContinualGatheringByNetworkFailure) { + rtc::ScopedFakeClock clock; + ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); + + // ep1 gathers continually but ep2 does not. + IceConfig config1 = CreateIceConfig(1000, GATHER_CONTINUALLY); + config1.regather_on_failed_networks_interval = rtc::Optional(2000); + IceConfig config2; + config2.regather_on_failed_networks_interval = rtc::Optional(2000); + CreateChannels(config1, config2); + + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && + ep2_ch1()->receiving() && + ep2_ch1()->writable(), + kDefaultTimeout, clock); + + fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]); + // Timeout value such that all connections are deleted. + const int kNetworkFailureTimeout = 35000; + SIMULATED_WAIT(false, kNetworkFailureTimeout, clock); + EXPECT_LE(1, GetMetricsObserver(0)->GetEnumCounter( + webrtc::kEnumCounterIceRegathering, + static_cast(IceRegatheringReason::NETWORK_FAILURE))); + EXPECT_EQ(0, GetMetricsObserver(1)->GetEnumCounter( + webrtc::kEnumCounterIceRegathering, + static_cast(IceRegatheringReason::NETWORK_FAILURE))); + + DestroyChannels(); +} + // Test that we properly create a connection on a STUN ping from unknown address // when the signaling is slow. TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) { diff --git a/webrtc/p2p/base/portallocator.h b/webrtc/p2p/base/portallocator.h index 8befbadcc7..3c81068060 100644 --- a/webrtc/p2p/base/portallocator.h +++ b/webrtc/p2p/base/portallocator.h @@ -23,6 +23,10 @@ #include "webrtc/base/sigslot.h" #include "webrtc/base/thread.h" +namespace webrtc { +class MetricsObserverInterface; +} + namespace cricket { // PortAllocator is responsible for allocating Port types for a given @@ -72,6 +76,9 @@ enum { PORTALLOCATOR_DISABLE_COSTLY_NETWORKS = 0x2000, }; +// Defines various reasons that have caused ICE regathering. +enum class IceRegatheringReason { NETWORK_CHANGE, NETWORK_FAILURE, MAX_VALUE }; + const uint32_t kDefaultPortAllocatorFlags = 0; const uint32_t kDefaultStepDelay = 1000; // 1 sec step delay. @@ -216,6 +223,9 @@ class PortAllocatorSession : public sigslot::has_slots<> { SignalCandidatesRemoved; sigslot::signal1 SignalCandidatesAllocationDone; + sigslot::signal2 + SignalIceRegathering; + virtual uint32_t generation() { return generation_; } virtual void set_generation(uint32_t generation) { generation_ = generation; } sigslot::signal1 SignalDestroyed; @@ -370,6 +380,10 @@ class PortAllocator : public sigslot::has_slots<> { const std::string& origin() const { return origin_; } void set_origin(const std::string& origin) { origin_ = origin; } + void SetMetricsObserver(webrtc::MetricsObserverInterface* observer) { + metrics_observer_ = observer; + } + protected: virtual PortAllocatorSession* CreateSessionInternal( const std::string& content_name, @@ -377,6 +391,14 @@ class PortAllocator : public sigslot::has_slots<> { const std::string& ice_ufrag, const std::string& ice_pwd) = 0; + webrtc::MetricsObserverInterface* metrics_observer() { + return metrics_observer_; + } + + const std::deque>& pooled_sessions() { + return pooled_sessions_; + } + uint32_t flags_; std::string agent_; rtc::ProxyInfo proxy_; @@ -397,6 +419,8 @@ class PortAllocator : public sigslot::has_slots<> { int allocated_pooled_session_count_ = 0; std::deque> pooled_sessions_; bool prune_turn_ports_ = false; + + webrtc::MetricsObserverInterface* metrics_observer_ = nullptr; }; } // namespace cricket diff --git a/webrtc/p2p/base/transportchannelimpl.h b/webrtc/p2p/base/transportchannelimpl.h index 0c7fa506b7..1d43e54d0a 100644 --- a/webrtc/p2p/base/transportchannelimpl.h +++ b/webrtc/p2p/base/transportchannelimpl.h @@ -18,6 +18,10 @@ namespace buzz { class XmlElement; } +namespace webrtc { +class MetricsObserverInterface; +} + namespace cricket { class Candidate; @@ -74,6 +78,9 @@ class TransportChannelImpl : public TransportChannel { // occurred. virtual void MaybeStartGathering() = 0; + virtual void SetMetricsObserver( + webrtc::MetricsObserverInterface* observer) = 0; + sigslot::signal1 SignalGatheringState; // Handles sending and receiving of candidates. The Transport diff --git a/webrtc/p2p/base/transportcontroller.cc b/webrtc/p2p/base/transportcontroller.cc index bd6ee66871..1dc7ca789a 100644 --- a/webrtc/p2p/base/transportcontroller.cc +++ b/webrtc/p2p/base/transportcontroller.cc @@ -187,6 +187,7 @@ TransportChannel* TransportController::CreateTransportChannel_n( // Need to create a new channel. Transport* transport = GetOrCreateTransport_n(transport_name); TransportChannelImpl* channel = transport->CreateChannel(component); + channel->SetMetricsObserver(metrics_observer_); channel->SignalWritableState.connect( this, &TransportController::OnChannelWritableState_n); channel->SignalReceivingState.connect( @@ -704,4 +705,12 @@ void TransportController::OnDtlsHandshakeError(rtc::SSLHandshakeError error) { SignalDtlsHandshakeError(error); } +void TransportController::SetMetricsObserver( + webrtc::MetricsObserverInterface* metrics_observer) { + metrics_observer_ = metrics_observer; + for (auto channel : channels_) { + channel->SetMetricsObserver(metrics_observer); + } +} + } // namespace cricket diff --git a/webrtc/p2p/base/transportcontroller.h b/webrtc/p2p/base/transportcontroller.h index b5d9ac7d40..5f3fccc868 100644 --- a/webrtc/p2p/base/transportcontroller.h +++ b/webrtc/p2p/base/transportcontroller.h @@ -25,6 +25,9 @@ namespace rtc { class Thread; } +namespace webrtc { +class MetricsObserverInterface; +} namespace cricket { @@ -129,6 +132,8 @@ class TransportController : public sigslot::has_slots<>, sigslot::signal1 SignalDtlsHandshakeError; + void SetMetricsObserver(webrtc::MetricsObserverInterface* metrics_observer); + protected: // Protected and virtual so we can override it in unit tests. virtual Transport* CreateTransport_n(const std::string& transport_name); @@ -238,6 +243,8 @@ class TransportController : public sigslot::has_slots<>, rtc::AsyncInvoker invoker_; // True if QUIC is used instead of DTLS. bool quic_ = false; + + webrtc::MetricsObserverInterface* metrics_observer_ = nullptr; }; } // namespace cricket diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index dba68f177e..dbac0d3822 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -14,6 +14,7 @@ #include #include +#include "webrtc/api/peerconnectioninterface.h" #include "webrtc/p2p/base/basicpacketsocketfactory.h" #include "webrtc/p2p/base/common.h" #include "webrtc/p2p/base/port.h" @@ -150,14 +151,35 @@ void BasicPortAllocator::Construct() { allow_tcp_listen_ = true; } +void BasicPortAllocator::OnIceRegathering(PortAllocatorSession* session, + IceRegatheringReason reason) { + if (!metrics_observer()) { + return; + } + // If the session has not been taken by an active channel, do not report the + // metric. + for (auto& allocator_session : pooled_sessions()) { + if (allocator_session.get() == session) { + return; + } + } + + metrics_observer()->IncrementEnumCounter( + webrtc::kEnumCounterIceRegathering, static_cast(reason), + static_cast(IceRegatheringReason::MAX_VALUE)); +} + BasicPortAllocator::~BasicPortAllocator() { } PortAllocatorSession* BasicPortAllocator::CreateSessionInternal( const std::string& content_name, int component, const std::string& ice_ufrag, const std::string& ice_pwd) { - return new BasicPortAllocatorSession( + PortAllocatorSession* session = new BasicPortAllocatorSession( this, content_name, component, ice_ufrag, ice_pwd); + session->SignalIceRegathering.connect(this, + &BasicPortAllocator::OnIceRegathering); + return session; } void BasicPortAllocator::AddTurnServer(const RelayServerConfig& turn_server) { @@ -247,7 +269,7 @@ void BasicPortAllocatorSession::StartGettingPorts() { network_thread_->Post(RTC_FROM_HERE, this, MSG_CONFIG_START); - LOG(LS_INFO) << "Pruning turn ports " + LOG(LS_INFO) << "Start getting ports with prune_turn_ports " << (prune_turn_ports_ ? "enabled" : "disabled"); } @@ -302,6 +324,8 @@ void BasicPortAllocatorSession::RegatherOnFailedNetworks() { return; } + LOG(LS_INFO) << "Regather candidates on failed networks"; + // Mark a sequence as "network failed" if its network is in the list of failed // networks, so that it won't be considered as equivalent when the session // regathers ports and candidates. @@ -321,7 +345,9 @@ void BasicPortAllocatorSession::RegatherOnFailedNetworks() { PrunePortsAndRemoveCandidates(ports_to_prune); } - if (allocation_started_ && network_manager_started_) { + if (allocation_started_ && network_manager_started_ && !IsStopped()) { + SignalIceRegathering(this, IceRegatheringReason::NETWORK_FAILURE); + DoAllocate(); } } @@ -501,7 +527,7 @@ void BasicPortAllocatorSession::AllocatePorts() { } void BasicPortAllocatorSession::OnAllocate() { - if (network_manager_started_) + if (network_manager_started_ && !IsStopped()) DoAllocate(); allocation_started_ = true; @@ -553,10 +579,6 @@ std::vector BasicPortAllocatorSession::GetNetworks() { void BasicPortAllocatorSession::DoAllocate() { bool done_signal_needed = false; std::vector networks = GetNetworks(); - - if (IsStopped()) { - return; - } if (networks.empty()) { LOG(LS_WARNING) << "Machine has no networks; no ports will be allocated"; done_signal_needed = true; @@ -627,12 +649,18 @@ void BasicPortAllocatorSession::OnNetworksChanged() { PrunePortsAndRemoveCandidates(ports_to_prune); } + if (allocation_started_ && !IsStopped()) { + if (network_manager_started_) { + // If the network manager has started, it must be regathering. + SignalIceRegathering(this, IceRegatheringReason::NETWORK_CHANGE); + } + DoAllocate(); + } + if (!network_manager_started_) { - LOG(LS_INFO) << "Network manager is started"; + LOG(LS_INFO) << "Network manager has started"; network_manager_started_ = true; } - if (allocation_started_) - DoAllocate(); } void BasicPortAllocatorSession::DisableEquivalentPhases( diff --git a/webrtc/p2p/client/basicportallocator.h b/webrtc/p2p/client/basicportallocator.h index 3044559a0b..d705fc528e 100644 --- a/webrtc/p2p/client/basicportallocator.h +++ b/webrtc/p2p/client/basicportallocator.h @@ -65,6 +65,9 @@ class BasicPortAllocator : public PortAllocator { private: void Construct(); + void OnIceRegathering(PortAllocatorSession* session, + IceRegatheringReason reason); + rtc::NetworkManager* network_manager_; rtc::PacketSocketFactory* socket_factory_; bool allow_tcp_listen_; diff --git a/webrtc/p2p/quic/quictransportchannel.h b/webrtc/p2p/quic/quictransportchannel.h index 22b69b7f0f..f5a7910721 100644 --- a/webrtc/p2p/quic/quictransportchannel.h +++ b/webrtc/p2p/quic/quictransportchannel.h @@ -199,6 +199,10 @@ class QuicTransportChannel : public TransportChannelImpl, void OnProofVerifyDetailsAvailable( const net::ProofVerifyDetails& verify_details) override; + void SetMetricsObserver(webrtc::MetricsObserverInterface* observer) override { + channel_->SetMetricsObserver(observer); + } + // Returns true if |quic_| has queued data which wasn't written due // to |channel_| being write blocked. bool HasDataToWrite() const;