Add instrumentation to track the IceCandidatePairType and IceConnectionState

content/renderer/media/rtc_peer_connection_handler.cc

Index: content/renderer/media/rtc_peer_connection_handler.cc
diff --git a/content/renderer/media/rtc_peer_connection_handler.cc b/content/renderer/media/rtc_peer_connection_handler.cc
index 2de508789f2b1050a5f9e62ffef495cf89cd1de8..542ef73bff44f0f34461174ab56a0a367cd233b9 100644
--- a/content/renderer/media/rtc_peer_connection_handler.cc
+++ b/content/renderer/media/rtc_peer_connection_handler.cc
@@ -505,13 +505,24 @@ class PeerConnectionUMAObserver : public webrtc::UMAObserver {
  public:
   PeerConnectionUMAObserver() {}
   ~PeerConnectionUMAObserver() override {}
-
-  void IncrementCounter(
-      webrtc::PeerConnectionUMAMetricsCounter counter) override {
-    // Runs on libjingle's signaling thread.
-    UMA_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.IPMetrics",
-                              counter,
-                              webrtc::kBoundary);
+  void IncrementEnumCounter(webrtc::PeerConnectionEnumCounterType counter_type,
+                            int counter,
+                            int counter_max) override {
+    switch (counter_type) {
+      case webrtc::kEnumCounterAddressFamily:

[pthatcher2, 2015/08/19 02:59:06]

Would it have made more sense to pass in a string to IncrementEnumCounter than
an enum?

[guoweis_left_chromium, 2015/08/19 18:42:49]

Can you elaborate?

+        UMA_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.IPMetrics", counter,
+                                  counter_max);
+      case webrtc::kEnumCounterIceEndpointTypeUdp:
+        UMA_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.CandidatePairType_UDP",
+                                  counter, counter_max);
+        break;
+      case webrtc::kEnumCounterIceEndpointTypeTcp:
+        UMA_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.CandidatePairType_TCP",
+                                  counter, counter_max);
+        break;
+      default:
+        NOTREACHED();
+    }
   }
 
   void AddHistogramSample(webrtc::PeerConnectionUMAMetricsName type,
@@ -728,6 +739,7 @@ RTCPeerConnectionHandler::RTCPeerConnectionHandler(
       num_local_candidates_ipv6_(0),
       weak_factory_(this) {
   g_peer_connection_handlers.Get().insert(this);
+  ResetUMAStats();
 }
 
 RTCPeerConnectionHandler::~RTCPeerConnectionHandler() {
@@ -1316,6 +1328,7 @@ void RTCPeerConnectionHandler::OnSignalingChange(
 void RTCPeerConnectionHandler::OnIceConnectionChange(
     webrtc::PeerConnectionInterface::IceConnectionState new_state) {
   TRACE_EVENT0("webrtc", "RTCPeerConnectionHandler::OnIceConnectionChange");
+  ReportICEState(new_state);
   DCHECK(thread_checker_.CalledOnValidThread());
   if (new_state == webrtc::PeerConnectionInterface::kIceConnectionChecking) {
     ice_connection_checking_start_ = base::TimeTicks::Now();
@@ -1370,8 +1383,7 @@ void RTCPeerConnectionHandler::OnIceGatheringChange(
              webrtc::PeerConnectionInterface::kIceGatheringGathering) {
     // ICE restarts will change gathering state back to "gathering",
     // reset the counter.
-    num_local_candidates_ipv6_ = 0;
-    num_local_candidates_ipv4_ = 0;
+    ResetUMAStats();

[pthatcher2, 2015/08/19 02:59:06]

This makes it so that we get a UMA for ICE every state for every ICE restart. 
Is that what we want?

[guoweis_left_chromium, 2015/08/19 18:42:49]

I think we should also track the stat after restart.

   }
 
   blink::WebRTCPeerConnectionHandlerClient::ICEGatheringState state =
@@ -1526,4 +1538,20 @@ void RTCPeerConnectionHandler::RunSynchronousClosureOnSignalingThread(
   }
 }
 
+void RTCPeerConnectionHandler::ReportICEState(
+    webrtc::PeerConnectionInterface::IceConnectionState new_state) {
+  if (ice_state_tracking_[new_state])

[pthatcher2, 2015/08/19 02:59:06]

I'd rename ice_state_tracking_ to ice_state_seen_.

[guoweis_left_chromium, 2015/08/19 18:42:49]

Done.

+    return;
+  ice_state_tracking_[new_state] = true;
+  UMA_HISTOGRAM_ENUMERATION(
+      "WebRTC.PeerConnection.ConnectionState", new_state,
+      webrtc::PeerConnectionInterface::kIceConnectionStateMax);
+}
+
+void RTCPeerConnectionHandler::ResetUMAStats() {
+  num_local_candidates_ipv6_ = 0;
+  num_local_candidates_ipv4_ = 0;
+  ice_connection_checking_start_ = base::TimeTicks();
+  memset(ice_state_tracking_, 0, sizeof(ice_state_tracking_));
+}
 }  // namespace content