Add UMA metrics for ICE regathering reasons.

webrtc/p2p/base/p2ptransportchannel_unittest.cc

Index: webrtc/p2p/base/p2ptransportchannel_unittest.cc
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index d7ee87300d9ed5f9d886bb7a0c8190d2a2e9c080..bae38277d0454ad1c91ac76a184e9629870299c6 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -11,6 +11,7 @@
 #include <algorithm>
 #include <memory>
 
+#include "webrtc/api/fakemetricsobserver.h"
 #include "webrtc/p2p/base/fakeportallocator.h"
 #include "webrtc/p2p/base/p2ptransportchannel.h"
 #include "webrtc/p2p/base/testrelayserver.h"
@@ -954,10 +955,13 @@ const P2PTransportChannelTestBase::Result
 class P2PTransportChannelTest : public P2PTransportChannelTestBase {
  protected:
   static const Result* kMatrix[NUM_CONFIGS][NUM_CONFIGS];
-  void ConfigureEndpoints(Config config1,
-                          Config config2,
-                          int allocator_flags1,
-                          int allocator_flags2) {
+  void ConfigureEndpoints(
+      Config config1,
+      Config config2,
+      int allocator_flags1,
+      int allocator_flags2,
+      webrtc::MetricsObserverInterface* observer1 = nullptr,
+      webrtc::MetricsObserverInterface* observer2 = nullptr) {

[Taylor Brandstetter, 2016/10/03 20:33:35]

I'd prefer if the Endpoint owned its own metric observer and hooked it up by
default. Then the individual test cases don't need to do anything but access it.

This is what other tests such as WebRtcSessionTest do.

[honghaiz3, 2016/10/04 01:03:55]

Done.

     int delay = kMinimumStepDelay;
     ConfigureEndpoint(0, config1);
     SetAllocatorFlags(0, allocator_flags1);
@@ -967,6 +971,12 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase {
     SetAllocationStepDelay(1, delay);
 
     set_remote_ice_parameter_source(FROM_SETICEPARAMETERS);
+    if (observer1) {
+      GetAllocator(0)->set_metrics_observer(observer1);
+    }
+    if (observer2) {
+      GetAllocator(1)->set_metrics_observer(observer2);
+    }
   }
   void ConfigureEndpoint(int endpoint, Config config) {
     switch (config) {
@@ -1172,6 +1182,199 @@ TEST_F(P2PTransportChannelTest, GetStats) {
   DestroyChannels();
 }
 
+TEST_F(P2PTransportChannelTest,
+       TestIceRegatheringReasonIceRestartWhenDisconnected) {
+  rtc::ScopedFakeClock clock;
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts, observer,
+                     observer);
+
+  CreateChannels();
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                                 ep2_ch1()->receiving() &&
+                                 ep2_ch1()->writable(),
+                             3000, clock);

[Taylor Brandstetter, 2016/10/03 20:33:35]

Could we use constants here instead of 3000 and 6000?

[honghaiz3, 2016/10/04 01:03:55]

Done.

+
+  // Drop all packets so that both channels become not writable.
+  fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]);
+  EXPECT_TRUE_SIMULATED_WAIT(!ep1_ch1()->writable() && !ep2_ch1()->writable(),
+                             6000, clock);
+
+  ep1_ch1()->SetIceParameters(kIceParams[2]);
+  ep1_ch1()->SetRemoteIceParameters(kIceParams[3]);
+  ep1_ch1()->MaybeStartGathering();
+  EXPECT_EQ(1, observer->GetEnumCounter(
+                   webrtc::kEnumCounterIceRegatheringReason,
+                   static_cast<int>(
+                       IceRegatheringReason::ICE_RESTART_WHEN_DISCONNECTED)));
+
+  ep2_ch1()->SetIceParameters(kIceParams[3]);
+  ep2_ch1()->SetRemoteIceParameters(kIceParams[2]);
+  ep2_ch1()->MaybeStartGathering();
+  EXPECT_EQ(2, observer->GetEnumCounter(
+                   webrtc::kEnumCounterIceRegatheringReason,
+                   static_cast<int>(
+                       IceRegatheringReason::ICE_RESTART_WHEN_DISCONNECTED)));
+
+  DestroyChannels();
+}
+
+TEST_F(P2PTransportChannelTest,
+       TestIceRegatheringReasonIceRestartWhenConnected) {
+  rtc::ScopedFakeClock clock;
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts, observer,
+                     observer);
+
+  CreateChannels();
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                                 ep2_ch1()->receiving() &&
+                                 ep2_ch1()->writable(),
+                             3000, clock);
+
+  ep1_ch1()->SetIceParameters(kIceParams[2]);
+  ep1_ch1()->SetRemoteIceParameters(kIceParams[3]);
+  ep1_ch1()->MaybeStartGathering();
+  EXPECT_EQ(
+      1,
+      observer->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(IceRegatheringReason::ICE_RESTART_WHEN_CONNECTED)));
+
+  ep2_ch1()->SetIceParameters(kIceParams[3]);
+  ep2_ch1()->SetRemoteIceParameters(kIceParams[2]);
+  ep2_ch1()->MaybeStartGathering();
+  EXPECT_EQ(
+      2,
+      observer->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(IceRegatheringReason::ICE_RESTART_WHEN_CONNECTED)));
+
+  DestroyChannels();
+}
+
+TEST_F(P2PTransportChannelTest,
+       TestIceRegatheringReasonIceRestartWhenConnecting) {
+  rtc::ScopedFakeClock clock;
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts, observer,
+                     observer);
+
+  // 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,
+      observer->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(IceRegatheringReason::ICE_RESTART_WHEN_CONNECTING)));
+
+  ep2_ch1()->SetIceParameters(kIceParams[3]);
+  ep2_ch1()->SetRemoteIceParameters(kIceParams[2]);
+  ep2_ch1()->MaybeStartGathering();
+  EXPECT_EQ(
+      2,
+      observer->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(IceRegatheringReason::ICE_RESTART_WHEN_CONNECTING)));
+
+  DestroyChannels();
+}
+

[Taylor Brandstetter, 2016/10/03 20:33:35]

I think it would be helpful to add a comment explaining that this tests one
channel with continual gathering enabled and one with it disabled. Same with the
test below.

[honghaiz3, 2016/10/04 01:03:54]

Done.

+TEST_F(P2PTransportChannelTest,
+       TestIceRegatheringReasonContinualGatheringByNetworkChange) {
+  rtc::ScopedFakeClock clock;
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer1(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer2(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts, observer1,
+                     observer2);
+
+  // 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(),
+                             3000, clock);
+
+  // Add address in ep1 will trigger continual gathering.
+  AddAddress(0, kAlternateAddrs[0]);
+  EXPECT_EQ_SIMULATED_WAIT(
+      1, observer1->GetEnumCounter(
+             webrtc::kEnumCounterIceRegatheringReason,
+             static_cast<int>(
+                 IceRegatheringReason::CONTINUAL_GATHERING_BY_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, observer2->GetEnumCounter(
+             webrtc::kEnumCounterIceRegatheringReason,
+             static_cast<int>(
+                 IceRegatheringReason::CONTINUAL_GATHERING_BY_NETWORK_CHANGE)));
+
+  DestroyChannels();
+}
+
+TEST_F(P2PTransportChannelTest,
+       TestIceRegatheringReasonContinualGatheringByNetworkFailure) {
+  rtc::ScopedFakeClock clock;
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer1(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer2(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts, observer1,
+                     observer2);
+
+  // ep1 gathers continually but ep2 does not.
+  IceConfig config1 = CreateIceConfig(1000, GATHER_CONTINUALLY);
+  config1.regather_on_failed_networks_interval = rtc::Optional<int>(2000);
+  IceConfig config2;
+  config2.regather_on_failed_networks_interval = rtc::Optional<int>(2000);
+  CreateChannels(config1, config2);
+
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                                 ep2_ch1()->receiving() &&
+                                 ep2_ch1()->writable(),
+                             3000, clock);
+
+  fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]);
+
+  SIMULATED_WAIT(false, 35000, clock);

[Taylor Brandstetter, 2016/10/03 20:33:35]

Could you replace 35000 with a constant?

[honghaiz3, 2016/10/04 01:03:54]

Done.

+  EXPECT_LE(
+      1,
+      observer1->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(
+              IceRegatheringReason::CONTINUAL_GATHERING_BY_NETWORK_FAILURE)));
+  LOG(LS_INFO) << "x1";

[Taylor Brandstetter, 2016/10/03 20:33:35]

Remember to remove this log statement

[honghaiz3, 2016/10/04 01:03:54]

Done.

+  EXPECT_EQ(
+      0,
+      observer2->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(
+              IceRegatheringReason::CONTINUAL_GATHERING_BY_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) {