NetConnectivity - Collect stats for TCP/UDP network

chrome/browser/net/network_stats.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/net/network_stats.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/rand_util.h"
 #include "base/stringprintf.h"
 #include "base/threading/platform_thread.h"
 #include "base/time.h"
 #include "base/tuple.h"
 #include "chrome/common/chrome_version_info.h"
 #include "content/public/browser/browser_thread.h"
+#include "googleurl/src/gurl.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/base/network_change_notifier.h"
 #include "net/base/sys_addrinfo.h"
 #include "net/base/test_completion_callback.h"
+#include "net/proxy/proxy_service.h"
 #include "net/socket/tcp_client_socket.h"
 #include "net/udp/udp_client_socket.h"
 #include "net/udp/udp_server_socket.h"
 
 using content::BrowserThread;
 
 namespace chrome_browser_net {
 
 // This specifies the number of bytes to be sent to the TCP/UDP servers as part
 // of small packet size test.
@@ skipping 53 matching lines @@
 
 // Maximum number of packets that can be sent to the server for packet loss
 // testing.
 static const uint32 kMaximumPackets = 4;
 
 // NetworkStats methods and members.
 NetworkStats::NetworkStats()
     : load_size_(0),
       bytes_to_read_(0),
       bytes_to_send_(0),
+      has_proxy_server_(false),
       packets_to_send_(0),
       packets_sent_(0),
       base_packet_number_(0),
       packets_received_mask_(0),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
 }
 
 NetworkStats::~NetworkStats() {
   socket_.reset();
 }
 
 bool NetworkStats::Start(net::HostResolver* host_resolver,
                          const net::HostPortPair& server_host_port_pair,
                          HistogramPortSelector histogram_port,
+                         bool has_proxy_server,
                          uint32 bytes_to_send,
                          uint32 packets_to_send,
                          const net::CompletionCallback& finished_callback) {
   DCHECK(bytes_to_send);   // We should have data to send.
   DCHECK_LE(packets_to_send, kMaximumPackets);
 
-  Initialize(bytes_to_send, histogram_port, packets_to_send, finished_callback);
+  Initialize(bytes_to_send,
+             histogram_port,
+             has_proxy_server,
+             packets_to_send,
+             finished_callback);
 
   net::HostResolver::RequestInfo request(server_host_port_pair);
   int rv = host_resolver->Resolve(
       request, &addresses_,
       base::Bind(&NetworkStats::OnResolveComplete,
                  base::Unretained(this)),
       NULL, net::BoundNetLog());
   if (rv == net::ERR_IO_PENDING)
     return true;
   return DoConnect(rv);
 }
 
 void NetworkStats::Initialize(
     uint32 bytes_to_send,
     HistogramPortSelector histogram_port,
+    bool has_proxy_server,
     uint32 packets_to_send,
     const net::CompletionCallback& finished_callback) {
   DCHECK(bytes_to_send);    // We should have data to send.
   DCHECK(packets_to_send);  // We should send at least 1 packet.
   DCHECK_LE(bytes_to_send, kLargeTestBytesToSend);
 
   load_size_ = bytes_to_send;
   packets_to_send_ = packets_to_send;
 
   histogram_port_ = histogram_port;
+  has_proxy_server_ = has_proxy_server;
   finished_callback_ = finished_callback;
 }
 
 uint32 SendingPacketSize(uint32 load_size) {
   return kVersionLength + kChecksumLength + kPayloadSizeLength + load_size;
 }
 
 uint32 ReceivingPacketSize(uint32 load_size) {
   return kVersionLength + kChecksumLength + kPayloadSizeLength + kKeyLength +
       load_size;
@@ skipping 378 matching lines @@
   if (result == net::OK) {
     *rtt_histogram_name = base::StringPrintf(
         "NetConnectivity.%s.Success.%d.%s.RTT",
         protocol_string,
         kPorts[port],
         load_size_string);
   }
 
   // Build "NetConnectivity.<protocol>.Status.<port>.<load_size>" histogram
   // name. Total number of histograms are 2*5*2.
-  *status_histogram_name =  base::StringPrintf(
+  *status_histogram_name = base::StringPrintf(
       "NetConnectivity.%s.Status.%d.%s",
       protocol_string,
       kPorts[port],
       load_size_string);
 
   // Build "NetConnectivity.<protocol>.PacketLoss.<port>.<load_size>" histogram
   // name. Total number of histograms are 5 (because we do this test for UDP
   // only).
-  *packet_loss_histogram_name =  base::StringPrintf(
+  *packet_loss_histogram_name = base::StringPrintf(
       "NetConnectivity.%s.PacketLoss.%d.%s",
       protocol_string,
       kPorts[port],
       load_size_string);
 }
 
 void NetworkStats::RecordHistograms(const ProtocolValue& protocol,
                                     const Status& status,
                                     int result) {
   base::TimeDelta duration = base::TimeTicks::Now() - start_time();
 
   std::string rtt_histogram_name;
   std::string status_histogram_name;
   std::string packet_loss_histogram_name;
   GetHistogramNames(protocol,
                     histogram_port_,
                     load_size_,
                     result,
                     &rtt_histogram_name,
                     &status_histogram_name,
                     &packet_loss_histogram_name);
 
-  // For packet loss test, just record packet loss data.
-  if (packets_to_send_ > 1) {
-    base::Histogram* packet_loss_histogram = base::LinearHistogram::FactoryGet(
-        packet_loss_histogram_name,
-        1,
-        2 << kMaximumPackets,
-        (2 << kMaximumPackets) + 1,
+  size_t histogram_count = has_proxy_server_ ? 1 : 2;
+  for (size_t i = 0; i < histogram_count; i++) {
+    // For packet loss test, just record packet loss data.
+    if (packets_to_send_ > 1) {
+      base::Histogram* histogram = base::LinearHistogram::FactoryGet(
+          packet_loss_histogram_name,
+          1,
+          2 << kMaximumPackets,
+          (2 << kMaximumPackets) + 1,
+          base::Histogram::kUmaTargetedHistogramFlag);
+      histogram->Add(packets_received_mask_);
+      packet_loss_histogram_name.append(".NoProxy");
+      continue;
+    }
+
+    if (result == net::OK) {
+      base::Histogram* rtt_histogram = base::Histogram::FactoryTimeGet(
+          rtt_histogram_name,
+          base::TimeDelta::FromMilliseconds(10),
+          base::TimeDelta::FromSeconds(60), 50,
+          base::Histogram::kUmaTargetedHistogramFlag);
+      rtt_histogram->AddTime(duration);
+      rtt_histogram_name.append(".NoProxy");
+    }
+
+    base::Histogram* status_histogram = base::LinearHistogram::FactoryGet(
+        status_histogram_name, 1, STATUS_MAX, STATUS_MAX+1,
         base::Histogram::kUmaTargetedHistogramFlag);
-    packet_loss_histogram->Add(packets_received_mask_);
-    return;
+    status_histogram->Add(status);
+    status_histogram_name.append(".NoProxy");
   }
-
-  if (result == net::OK) {
-    base::Histogram* rtt_histogram = base::Histogram::FactoryTimeGet(
-        rtt_histogram_name,
-        base::TimeDelta::FromMilliseconds(10),
-        base::TimeDelta::FromSeconds(60), 50,
-        base::Histogram::kUmaTargetedHistogramFlag);
-    rtt_histogram->AddTime(duration);
-  }
-
-  base::Histogram* status_histogram = base::LinearHistogram::FactoryGet(
-      status_histogram_name, 1, STATUS_MAX, STATUS_MAX+1,
-      base::Histogram::kUmaTargetedHistogramFlag);
-  status_histogram->Add(status);
 }
 
 // UDPStatsClient methods and members.
 UDPStatsClient::UDPStatsClient()
     : NetworkStats() {
 }
 
 UDPStatsClient::~UDPStatsClient() {
 }
 
@@ skipping 102 matching lines @@
 
   // Disconnect the socket so that there are no more IO operations.
   net::TCPClientSocket* tcp_socket =
       static_cast<net::TCPClientSocket*>(socket());
   if (tcp_socket)
     tcp_socket->Disconnect();
 
   delete this;
 }
 
+// ProxyDetector methods and members.
+ProxyDetector::ProxyDetector(const std::string& network_stats_server,
+                             NetworkStats::HistogramPortSelector histogram_port,
+                             IOThread* io_thread)
+    : network_stats_server_(network_stats_server),
+      histogram_port_(histogram_port),
+      io_thread_(io_thread) {
+}
+
+ProxyDetector::~ProxyDetector() {

[eroman, 2012/05/04 23:08:48]

When is ProxyDetector deleted? Note that if it is deleted while the
ResolveProxy() request is still in progress, that will lead to a use-after-free
when the callback is invoked.

I suggest adding one of two safety nets. Either:
  (a) assert to make sure there is no in-progress ProxyResolve request
  (b) Explicitly cancel the ProxyResolve request during deletion, if one is
outstanding. (ResolveProxy can take a request handle, and you can use this to
cancel it).

I recommend approach (a), and just making sure the lifetime is well understood.

[ramant (doing other things), 2012/05/07 23:35:47]

Implemented option a). Thanks for the above comment.

Done.

+}
+
+void ProxyDetector::ResolveProxy() {
+  std::string url = base::StringPrintf(
+      "https://%s:%d", network_stats_server_.c_str(), kPorts[histogram_port_]);

[eroman, 2012/05/04 23:08:48]

Assembling URLs in this fashion is generally not safe, since if the hostname is
an IPv6 literal concatenating ':' has different meaning.

There are several possible solutions:
  (a) Use HostPortPair::ToString() to stringize the host + port. In fact, I
suggest making the input to ProxyDetector be a HostPairPair regardless.

  (b) Use GURL::ReplaceComponents() to smuggle in a host and port into a URL.

The simplest strategy is probably (a), so I recommend that.

Lastly, is it intentional that the scheme be "https://" rather than "http://"?
Either is fine, just want to make sure this has been thought through (since PAC
scripts will sometimes give different results for HTTPS vs HTTP).

[ramant (doing other things), 2012/05/07 23:35:47]

Implemented option a). Thanks much for the suggestion.

Used https per following comment from rsleevi:
	
Ryan Sleevi 2012/04/28 02:06:32
  Would it make more sense to check for HTTPS here?

  The reason I mention it is that HTTPS will give you an indicator as to whether
or not the proxy server supports CONNECT, which is effectively how outbound
connections can be mediated by the proxy.

Should we use http?

Done.

+  GURL gurl(url);
+  net::ProxyService* service =
+      io_thread_->globals()->system_proxy_service.get();
+
+  int rv = service->ResolveProxy(
+      gurl,
+      &proxy_info_,
+      base::Bind(&ProxyDetector::OnResolveProxyComplete,
+                 base::Unretained(this)),
+      NULL,
+      net::BoundNetLog());
+  if (rv == net::OK)
+    OnResolveProxyComplete(rv);
+}
+
+void ProxyDetector::OnResolveProxyComplete(int result) {
+  bool has_proxy_server = (proxy_info_.proxy_server().is_valid() &&

[eroman, 2012/05/04 23:08:48]

One extra check to add is into that list is:
   result == net::OK

[ramant (doing other things), 2012/05/07 23:35:47]

Done.

+                           !proxy_info_.proxy_server().is_direct());
+  StartNetworkStatsTest(

[eroman, 2012/05/04 23:08:48]

[optional] I suggest de-coupling this from ProxyDetector. Instead, have
ProxyDetector take a Callback<boo> parameter. The caller can partially bind a
Callback to StartNetworkStatsTest with all the other parmaters already fixed
(hence don't need to mirror them in ProxyDetector).

[ramant (doing other things), 2012/05/07 23:35:47]

Done.

+      network_stats_server_, histogram_port_, has_proxy_server, io_thread_);
+}
+
 // static
 void CollectNetworkStats(const std::string& network_stats_server,
                          IOThread* io_thread) {
   if (network_stats_server.empty())
     return;
 
   // If we are not on IO Thread, then post a task to call CollectNetworkStats on
   // IO Thread.
   if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
     BrowserThread::PostTask(
         BrowserThread::IO,
         FROM_HERE,
         base::Bind(
             &CollectNetworkStats, network_stats_server, io_thread));
     return;
   }
 
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   // Check that there is a network connection. We get called only if UMA upload
   // to the server has succeeded.
   DCHECK(!net::NetworkChangeNotifier::IsOffline());
 
   CR_DEFINE_STATIC_LOCAL(scoped_refptr<base::FieldTrial>, trial, ());
   static bool collect_stats = false;
-
-  static uint32 port;
   static NetworkStats::HistogramPortSelector histogram_port;
 
   if (!trial.get()) {
     // Set up a field trial to collect network stats for UDP and TCP.
     const base::FieldTrial::Probability kDivisor = 1000;
 
     // Enable the connectivity testing for 0.5% of the users in stable channel.
     base::FieldTrial::Probability probability_per_group = 5;
 
     chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
@@ skipping 18 matching lines @@
     if (trial->group() == collect_stats_group)
       collect_stats = true;
 
     if (collect_stats) {
       // Pick a port randomly from the set of TCP/UDP echo server ports
       // specified in |kPorts|.
       histogram_port = static_cast<NetworkStats::HistogramPortSelector>(
           base::RandInt(NetworkStats::PORT_53, NetworkStats::PORT_8080));
       DCHECK_GE(histogram_port, NetworkStats::PORT_53);
       DCHECK_LE(histogram_port, NetworkStats::PORT_8080);
-      port = kPorts[histogram_port];
     }
   }
 
   if (!collect_stats)
     return;
 
   // Run test kMaxNumberOfTests times.
   const size_t kMaxNumberOfTests = INT_MAX;
   static size_t number_of_tests_done = 0;
   if (number_of_tests_done > kMaxNumberOfTests)
     return;
 
   ++number_of_tests_done;
 
+  ProxyDetector* proxy_client = new ProxyDetector(
+      network_stats_server, histogram_port, io_thread);
+  proxy_client->ResolveProxy();
+}
+
+// static
+void StartNetworkStatsTest(const std::string& network_stats_server,
+                           NetworkStats::HistogramPortSelector histogram_port,
+                           bool has_proxy_server,
+                           IOThread* io_thread) {
   net::HostResolver* host_resolver = io_thread->globals()->host_resolver.get();
   DCHECK(host_resolver);
 
-  net::HostPortPair server_address(network_stats_server, port);
+  DCHECK_GE(histogram_port, NetworkStats::PORT_53);
+  DCHECK_LE(histogram_port, NetworkStats::PORT_8080);
+  net::HostPortPair server_address(network_stats_server,
+                                   kPorts[histogram_port]);
 
   int experiment_to_run = base::RandInt(1, 5);
   switch (experiment_to_run) {
     case 1:
       {
         UDPStatsClient* small_udp_stats = new UDPStatsClient();
         small_udp_stats->Start(
-            host_resolver, server_address, histogram_port,
+            host_resolver, server_address, histogram_port, has_proxy_server,
             kSmallTestBytesToSend, 1, net::CompletionCallback());
       }
       break;
 
     case 2:
       {
         UDPStatsClient* large_udp_stats = new UDPStatsClient();
         large_udp_stats->Start(
-            host_resolver, server_address, histogram_port,
+            host_resolver, server_address, histogram_port, has_proxy_server,
             kLargeTestBytesToSend, 1, net::CompletionCallback());
       }
       break;
 
     case 3:
       {
         TCPStatsClient* small_tcp_client = new TCPStatsClient();
         small_tcp_client->Start(
-            host_resolver, server_address, histogram_port,
+            host_resolver, server_address, histogram_port, has_proxy_server,
             kSmallTestBytesToSend, 1, net::CompletionCallback());
       }
       break;
 
     case 4:
       {
         TCPStatsClient* large_tcp_client = new TCPStatsClient();
         large_tcp_client->Start(
-            host_resolver, server_address, histogram_port,
+            host_resolver, server_address, histogram_port, has_proxy_server,
             kLargeTestBytesToSend, 1, net::CompletionCallback());
       }
       break;
 
     case 5:
       {
         UDPStatsClient* packet_loss_udp_stats = new UDPStatsClient();
         packet_loss_udp_stats->Start(
-            host_resolver, server_address, histogram_port,
+            host_resolver, server_address, histogram_port, has_proxy_server,
             kLargeTestBytesToSend, kMaximumPackets, net::CompletionCallback());
       }
       break;
   }
 }
 
 }  // namespace chrome_browser_net