Wiring Libjingle network related metrics into Chrome

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 0c5c3737cf153f2ca680d981e502701bb4301220..5d9ccb852d7b45138a58869ab536d6fe3e004bd1 100644
--- a/content/renderer/media/rtc_peer_connection_handler.cc
+++ b/content/renderer/media/rtc_peer_connection_handler.cc
@@ -316,6 +316,41 @@ void LocalRTCStatsResponse::addStatistic(size_t report,
   impl_.addStatistic(report, name, value);
 }
 
+class PeerConnectionUMAObserver : public webrtc::UMAObserver {
+ public:
+  PeerConnectionUMAObserver() {}
+  ~PeerConnectionUMAObserver() {}
+
+  virtual void IncrementCounter(
+      webrtc::PeerConnectionUMAMetricsCounter counter) {

[Alexei Svitkine (slow), 2014/05/20 06:40:20]

Nit: OVERRIDE

[Mallinath (Gone from Chromium), 2014/05/21 19:02:18]

Done.

+    UMA_HISTOGRAM_ENUMERATION("WebRTC.PeerConnectionIPMetrics",
+                              counter,
+                              webrtc::kBoundary);
+  }
+  virtual void AddHistogramSample(
+      webrtc::PeerConnectionUMAMetricsName type, int value) {

[Alexei Svitkine (slow), 2014/05/20 06:40:20]

Nit: OVERRIDE and add an empty line before this.

[Mallinath (Gone from Chromium), 2014/05/21 19:02:18]

Done.

+    switch (type) {
+      case webrtc::kTimeToConnect:
+        UMA_HISTOGRAM_CUSTOM_COUNTS("WebRTC.PeerConnectionTimeToConnect",
+                                    value,
+                                    0, 100*1000, 50);  // Max 100 seconds.

[Alexei Svitkine (slow), 2014/05/20 06:40:20]

Is there a reason you chose a custom definition here instead of using e.g.
UMA_HISTOGRAM_MEDIUM_TIMES()?

[Mallinath (Gone from Chromium), 2014/05/21 19:02:18]

No reason, fact that I didn't knew such macro existed.
On 2014/05/20 06:40:20, Alexei Svitkine wrote:

+        break;
+      case webrtc::kNetworkInterfaces_IPv4:
+        UMA_HISTOGRAM_CUSTOM_COUNTS("WebRTC.PeerConnectionIPv4Interfaces",
+                                    value,
+                                    0, 100, 50);  // Max 100 interfaces.

[Alexei Svitkine (slow), 2014/05/20 06:40:20]

Can this be UMA_HISTOGRAM_COUNTS_100()? Same below.

[Mallinath (Gone from Chromium), 2014/05/21 19:02:18]

Done.

+        break;
+      case webrtc::kNetworkInterfaces_IPv6:
+        UMA_HISTOGRAM_CUSTOM_COUNTS("WebRTC.PeerConnectionIPv6Interfaces",
+                                    value,
+                                    0, 100, 50);  // Max 100 interfaces.
+        break;
+      default:
+        NOTREACHED();
+    }
+  }
+};
+
 RTCPeerConnectionHandler::RTCPeerConnectionHandler(
     blink::WebRTCPeerConnectionHandlerClient* client,
     PeerConnectionDependencyFactory* dependency_factory)
@@ -364,6 +399,10 @@ bool RTCPeerConnectionHandler::initialize(
     peer_connection_tracker_->RegisterPeerConnection(
         this, servers, constraints, frame_);
 
+  uma_observer_ = new talk_base::RefCountedObject<PeerConnectionUMAObserver>();
+  if (uma_observer_) {

[Alexei Svitkine (slow), 2014/05/20 06:40:20]

Nit: No {}'s

[Mallinath (Gone from Chromium), 2014/05/21 19:02:18]

Done.

+    native_peer_connection_->RegisterUMAObserver(uma_observer_.get());
+  }
   return true;
 }