NetConnectivity - Collect stats for TCP/UDP network

chrome/browser/net/network_stats.h

Index: chrome/browser/net/network_stats.h
===================================================================
--- chrome/browser/net/network_stats.h	(revision 135105)
+++ chrome/browser/net/network_stats.h	(working copy)
@@ -21,6 +21,7 @@
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/test_data_stream.h"
+#include "net/proxy/proxy_info.h"
 #include "net/socket/socket.h"
 namespace chrome_browser_net {
 
@@ -101,6 +102,7 @@
   bool Start(net::HostResolver* host_resolver,
              const net::HostPortPair& server,
              HistogramPortSelector histogram_port,
+             bool has_proxy_server,
              uint32 bytes_to_send,
              uint32 packets_to_send,
              const net::CompletionCallback& callback);
@@ -109,6 +111,7 @@
   // Constructs an NetworkStats object that collects metrics for network
   // connectivity (either TCP or UDP).
   NetworkStats();
+  // NetworkStats is deleted when Finish() is called.
   virtual ~NetworkStats();
 
   // Initializes |finished_callback_| and the number of bytes to send to the
@@ -116,6 +119,7 @@
   // |finished_callback| is mainly useful for unittests.
   void Initialize(uint32 bytes_to_send,
                   HistogramPortSelector histogram_port,
+                  bool has_proxy_server,
                   uint32 packets_to_send,
                   const net::CompletionCallback& finished_callback);
 
@@ -226,25 +230,37 @@
 
   // Returns the histogram names for collecting network connectivity stats.
   // This is called by RecordHistograms. It sets the histogram names in
-  // |rtt_histogram_name| and |status_histogram_name|.
-  // If |result| equals to net::OK, it returns
-  // "NetConnectivity.<protocol>.Success.<port>.<load_size>.RTT" as histogram
-  // name for RTT histogram,
+  // |rtt_histogram_name|, |status_histogram_name| and
+  // |packet_loss_histogram_name| and the corresponding no_proxy versions if
+  // there is no proxy server. It returns
   // "NetConnectivity.<protocol>.Status.<port>.<load_size>" as histogram name
-  // for status histogram and
+  // for status histogram in |status_histogram_name| and
   // "NetConnectivity.<protocol>.PacketLoss.<port>.<load_size>" as histogram
-  // name for packet loss histogram.
+  // name for packet loss histogram. If |result| equals to net::OK, it returns
+  // "NetConnectivity.<protocol>.Success.<port>.<load_size>.RTT" as histogram
+  // name in |rtt_histogram_name|. If |has_proxy_server| is false, it returns
+  // "NetConnectivity.<protocol>.NoProxy.Success.<port>.<load_size>.RTT" as
+  // no proxy RTT histogram,
+  // "NetConnectivity.<protocol>.NoProxy.Status.<port>.<load_size>" as no proxy

[jar (doing other things), 2012/05/03 16:41:34]

I expect that you'll be most interesting in comparing otherwise identical stats,
with and without the proxy.  As a result, it would be "nice" if they
alphabetized next to each other.  To make that happen, I'd suggest putting
"NoProxy" at the absolute end of the new name.
NetConnectivity.<protocol>.Status.<port>.load_size>.NoProxy

[ramant (doing other things), 2012/05/04 03:53:11]

Done.

+  // status histogram and
+  // "NetConnectivity.<protocol>.NoProxy.PacketLoss.<port>.<load_size>" as no
+  // proxy packet loss histogram.
   // |protocol| argument sets <protocol> in the histogram name. It would be
   // either TCP or UDP. <port> is the string representation of |histogram_port|.
   // |load_size| argument determines <load_size> in the histogram name. It would
   // be either 100B or 1K.
-  static void GetHistogramNames(const ProtocolValue& protocol,
-                                HistogramPortSelector histogram_port,
-                                uint32 load_size,
-                                int result,
-                                std::string* rtt_histogram_name,
-                                std::string* status_histogram_name,
-                                std::string* packet_loss_histogram_name);
+  static void GetHistogramNames(
+      const ProtocolValue& protocol,
+      HistogramPortSelector histogram_port,
+      bool has_proxy_server,
+      uint32 load_size,
+      int result,
+      std::string* rtt_histogram_name,
+      std::string* rtt_no_proxy_histogram_name,
+      std::string* status_histogram_name,
+      std::string* status_no_proxy_histogram_name,
+      std::string* packet_loss_histogram_name,
+      std::string* packet_loss_no_proxy_histogram_name);
 
   // The socket handle for this session.
   scoped_ptr<net::Socket> socket_;
@@ -271,6 +287,9 @@
   // connectivity.
   HistogramPortSelector histogram_port_;
 
+  // |has_proxy_server_| specifies if there is a proxy server or not.
+  bool has_proxy_server_;
+
   // HostResolver fills out the |addresses_| after host resolution is completed.
   net::AddressList addresses_;
 
@@ -298,6 +317,7 @@
   // Constructs an UDPStatsClient object that collects metrics for UDP
   // connectivity.
   UDPStatsClient();
+  // UDPStatsClient is deleted when Finish() is called.
   virtual ~UDPStatsClient();
 
  protected:
@@ -315,7 +335,8 @@
   virtual bool ReadComplete(int result) OVERRIDE;
 
   // Collects stats for UDP connectivity. This is called when all the data from
-  // server is read or when there is a failure during connect/read/write.
+  // server is read or when there is a failure during connect/read/write. This
+  // object is deleted at the end of this method.
   virtual void Finish(Status status, int result) OVERRIDE;
 };
 
@@ -324,6 +345,7 @@
   // Constructs a TCPStatsClient object that collects metrics for TCP
   // connectivity.
   TCPStatsClient();
+  // TCPStatsClient is deleted when Finish() is called.
   virtual ~TCPStatsClient();
 
  protected:
@@ -339,7 +361,8 @@
   virtual bool ReadComplete(int result) OVERRIDE;
 
   // Collects stats for TCP connectivity. This is called when all the data from
-  // server is read or when there is a failure during connect/read/write.
+  // server is read or when there is a failure during connect/read/write. This
+  // object is deleted at the end of this method.
   virtual void Finish(Status status, int result) OVERRIDE;
 
  private:
@@ -348,6 +371,40 @@
   void OnConnectComplete(int result);
 };
 
+class ProxyClient {

[jar (doing other things), 2012/05/03 16:41:34]

I'm surprised to see this much work.  Is the intent to run the tests "through"
the proxy?

I was expecting to see proxy detection, and then a see the histograms that are
currently emitted split into with/without a proxy (tangentially) present
(without changing the "test").

[ramant (doing other things), 2012/05/04 03:53:11]

No. This code to detect the proxy runs before we run the test.

Renamed ProxyClient to ProxyDetector. Should we rename to a different name?
thanks.

+ public:
+  // Constructs a ProxyClient object that resolves if access to
+  // network_stats_server goes through a proxy server or not.
+  ProxyClient(const std::string& network_stats_server,
+              NetworkStats::HistogramPortSelector histogram_port,
+              IOThread* io_thread);
+  virtual ~ProxyClient();
+
+  // This method uses system ProxyService to resolve the proxy for the given
+  // |network_stats_server| and |histogram_port|.
+  void ResolveProxy();
+
+  // Collects the network connectivity stats for UDP and TCP protocols.
+  void OnResolveProxyComplete(int result);
+
+ private:
+  // |network_stats_server_| specifies the TCP/UDP echo server name.
+  std::string network_stats_server_;
+
+  // |histogram_port_| specifies the port for which we are testing the network
+  // connectivity.
+  NetworkStats::HistogramPortSelector histogram_port_;
+
+  // |proxy_info_| holds proxy information returned by ResolveProxy.
+  net::ProxyInfo proxy_info_;
+
+  // The IOThread for accessing global HostResolver to resolve
+  // network_stats_server_ host and for accessing global system ProxyService.
+  // |io_thread_| is accessed on IO thread and it is safe to access it on IO
+  // thread.
+  IOThread* io_thread_;
+};
+
 // This collects the network connectivity stats for UDP and TCP for small
 // percentage of users who are participating in the experiment. All users must
 // have enabled "UMA upload". This method gets called only if UMA upload to the