Collect stats to investigate the viability of UDP

chrome/browser/net/network_stats.cc

Index: chrome/browser/net/network_stats.cc
===================================================================
--- chrome/browser/net/network_stats.cc	(revision 0)
+++ chrome/browser/net/network_stats.cc	(revision 0)
@@ -0,0 +1,321 @@
+// Copyright (c) 2011 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/callback_old.h"
+#include "base/logging.h"
+#include "base/message_loop.h"
+#include "base/metrics/field_trial.h"
+#include "base/metrics/histogram.h"
+#include "base/task.h"
+#include "base/threading/platform_thread.h"
+#include "base/time.h"
+#include "base/tuple.h"
+#include "content/browser/browser_thread.h"
+#include "net/base/address_list.h"
+#include "net/base/host_resolver.h"
+#include "net/base/net_errors.h"
+#include "net/base/net_util.h"
+#include "net/base/sys_addrinfo.h"
+#include "net/base/test_completion_callback.h"
+#include "net/socket/tcp_client_socket.h"
+#include "net/udp/udp_client_socket.h"
+#include "net/udp/udp_server_socket.h"
+
+namespace chrome_browser_net {
+
+// NetworkStats methods and members.
+NetworkStats::NetworkStats()
+    : socket_(NULL),
+      errors_(0),
+      bytes_to_read_(0),
+      bytes_to_send_(0),
+      ALLOW_THIS_IN_INITIALIZER_LIST(
+          read_callback_(this, &NetworkStats::OnReadComplete)),
+      ALLOW_THIS_IN_INITIALIZER_LIST(
+          write_callback_(this, &NetworkStats::OnWriteComplete)),
+      finished_callback_(NULL),
+      start_time_(base::TimeTicks::Now()) {
+}
+
+NetworkStats::~NetworkStats() {
+  socket_ = NULL;
+}
+
+void NetworkStats::OnReadComplete(int result) {
+  if (result <= 0) {
+    errors_++;
+    this->Finish(READ_FAIL, result);
+    return;
+  }
+
+  if (!received_stream_.VerifyBytes(read_buffer_->data(), result)) {
+    errors_++;
+    this->Finish(READ_VERIFY_FAIL, result);
+    return;
+  }
+
+  bytes_to_read_ -= result;
+
+  // Now read more data...
+  if (bytes_to_read_)
+    this->ReadData();
+  else
+    this->Finish(SUCCESS, net::OK);
+}
+
+void NetworkStats::OnWriteComplete(int result) {
+  if (result <= 0) {
+    errors_++;
+    this->Finish(WRITE_FAIL, result);
+    return;
+  }
+
+  write_buffer_->DidConsume(result);
+  bytes_to_send_ -= result;
+  if (!write_buffer_->BytesRemaining())
+      write_buffer_ = NULL;
+
+  if (bytes_to_send_)
+    this->SendData();
+}
+
+void NetworkStats::ReadData() {
+  DCHECK(!read_buffer_.get());
+  read_buffer_ = new net::IOBuffer(kMaxMessage);
+
+  int rv;
+  do {
+    rv = socket_->Read(read_buffer_, kMaxMessage, &read_callback_);
+    if (rv == net::ERR_IO_PENDING)
+      return;
+    this->OnReadComplete(rv);  // Complete the read manually.
+  } while (rv > 0);
+
+  read_buffer_ = NULL;
+}
+
+void NetworkStats::SendData() {
+  DCHECK(bytes_to_send_);   // We should have data to send.
+  const int kWriteChunkSize = 777;  // 777 is more abusive.

[Mike Belshe, 2011/05/27 21:27:00]

Do you need this chunking?  I had that in the unittest for fun, but its not
appropriate here?

[ramant (doing other things), 2011/05/31 21:25:36]

Done.

+
+  do {
+    if (!write_buffer_.get()) {
+      int bytes_to_send = std::min(kWriteChunkSize, bytes_to_send_);
+      scoped_refptr<net::IOBuffer> buffer(new net::IOBuffer(bytes_to_send));
+      sent_stream_.GetBytes(buffer->data(), bytes_to_send);
+      write_buffer_ = new net::DrainableIOBuffer(buffer, bytes_to_send);
+    }
+
+    int rv = socket_->Write(write_buffer_,
+                            write_buffer_->BytesRemaining(),
+                            &write_callback_);
+    if (rv == net::ERR_IO_PENDING)
+      return;
+

[Mike Belshe, 2011/05/27 21:27:00]

rv could be <0, indicating an error.  It should be rare.  You probably want this
function to return an int, which is either the status or the error.

[ramant (doing other things), 2011/05/31 21:25:36]

Done.

+    write_buffer_->DidConsume(rv);
+    bytes_to_send_ -= rv;
+    if (!write_buffer_->BytesRemaining())
+      write_buffer_ = NULL;
+  } while (bytes_to_send_);
+}
+
+// UDPStatsClient methods and members.
+UDPStatsClient::UDPStatsClient()
+    : NetworkStats() {
+}
+
+UDPStatsClient::~UDPStatsClient() {
+  socket_ = NULL;
+}
+
+bool UDPStatsClient::Start(const std::string& ip_str,
+                           int port,
+                           int bytes_to_send,
+                           net::CompletionCallback* callback) {
+  DCHECK(!socket_);
+  DCHECK(port);
+  DCHECK(bytes_to_send);   // We should have data to send.
+
+  finished_callback_ = callback;
+  bytes_to_read_ = bytes_to_send_ = bytes_to_send;

[Mike Belshe, 2011/05/27 21:27:00]

nit:  I think these compound assignments are against style?

[ramant (doing other things), 2011/05/31 21:25:36]

Done.

+
+  net::IPAddressNumber ip_number;
+  if (!net::ParseIPLiteralToNumber(ip_str, &ip_number)) {
+    errors_++;
+    this->Finish(IP_STRING_PARSE_FAIL, net::ERR_INVALID_ARGUMENT);

[Mike Belshe, 2011/05/27 21:27:00]

nit:  generally we don't do 'this->' in our code.

[ramant (doing other things), 2011/05/31 21:25:36]

Done.

+    return false;
+  }
+
+  net::IPEndPoint server_address = net::IPEndPoint(ip_number, port);
+
+  net::UDPClientSocket* socket =
+     new net::UDPClientSocket(NULL, net::NetLog::Source());
+
+  int rv = socket->Connect(server_address);
+  if (rv < 0) {
+    errors_++;
+    this->Finish(CONNECT_FAIL, rv);
+    return false;
+  }
+
+  socket_ = socket;
+
+  start_time_ = base::TimeTicks::Now();
+  this->SendData();

[Mike Belshe, 2011/05/27 21:27:00]

I think you'll need:

if (SendData() < 0) {
   Finish(SEND_FAILED, rv);
   return false;
}

[ramant (doing other things), 2011/05/31 21:25:36]

Done.

+
+  this->ReadData();
+
+  return true;
+}
+
+
+void UDPStatsClient::Finish(Status status, int result) {
+  base::TimeDelta duration = base::TimeTicks::Now() - start_time_;
+  if (result == net::OK) {
+    UMA_HISTOGRAM_TIMES("NetConnectivity.UDP.SuccessRTT", duration);
+  } else {
+    UMA_HISTOGRAM_TIMES("NetConnectivity.UDP.FailRTT", duration);
+  }
+
+  UMA_HISTOGRAM_ENUMERATION("NetConnectivity.UDP.Status", status, STATUS_MAX);
+
+  if (finished_callback_ != NULL) {
+    net::CompletionCallback* callback = finished_callback_;
+    finished_callback_ = NULL;
+    callback->Run(result);
+  }
+}
+
+// TCPStatsClient methods and members.
+TCPStatsClient::TCPStatsClient()
+  : NetworkStats(),
+    ALLOW_THIS_IN_INITIALIZER_LIST(
+        connect_callback_(this, &TCPStatsClient::OnConnectComplete)) {
+}
+
+TCPStatsClient::~TCPStatsClient() {
+  if (socket_) {
+    net::TCPClientSocket* socket = static_cast<net::TCPClientSocket*>(socket_);
+    socket->Disconnect();
+    socket_ = NULL;
+  }
+}
+
+bool TCPStatsClient::Start(const net::HostPortPair& server_host_port_pair,
+                           int bytes_to_send,
+                           net::CompletionCallback* callback) {
+  DCHECK(!socket_);
+  DCHECK(bytes_to_send);   // We should have data to send.
+
+  finished_callback_ = callback;
+  bytes_to_read_ = bytes_to_send_ = bytes_to_send;
+
+  scoped_ptr<net::HostResolver> system_host_resolver(
+      net::CreateSystemHostResolver(net::HostResolver::kDefaultParallelism,
+                                    net::HostResolver::kDefaultRetryAttempts,
+                                    NULL));
+  net::SingleRequestHostResolver host_resolver(system_host_resolver.get());
+  net::HostResolver::RequestInfo request(server_host_port_pair);
+  net::AddressList addresses;
+  int rv = host_resolver.Resolve(request, &addresses, NULL, net::BoundNetLog());
+  if (rv != net::OK) {
+    errors_++;
+    this->Finish(RESOLVE_FAIL, rv);
+    return false;
+  }
+
+  net::TCPClientSocket* socket =
+      new net::TCPClientSocket(addresses, NULL, net::NetLog::Source());
+  socket_ = socket;
+  rv = socket->Connect(&connect_callback_);
+  if (rv == net::ERR_IO_PENDING)
+    return true;
+  OnConnectComplete(rv);
+  return rv == net::OK;
+}
+
+void TCPStatsClient::OnConnectComplete(int result) {
+  if (result < 0) {
+    errors_++;
+    this->Finish(CONNECT_FAIL, result);
+    return;
+  }
+
+  DCHECK(bytes_to_send_);   // We should have data to send.
+
+  start_time_ = base::TimeTicks::Now();
+
+  this->SendData();

[Mike Belshe, 2011/05/27 21:27:00]

Same concern about Send() failing here.

[ramant (doing other things), 2011/05/31 21:25:36]

Done.

+
+  this->ReadData();
+}
+
+void TCPStatsClient::Finish(Status status, int result) {
+  base::TimeDelta duration = base::TimeTicks::Now() - start_time_;
+  if (result == net::OK) {
+    UMA_HISTOGRAM_TIMES("NetConnectivity.TCP.SuccessRTT", duration);
+  } else {
+    UMA_HISTOGRAM_TIMES("NetConnectivity.TCP.FailRTT", duration);
+  }
+
+  UMA_HISTOGRAM_ENUMERATION("NetConnectivity.TCP.Status", status, STATUS_MAX);
+
+  if (finished_callback_ != NULL) {
+    net::CompletionCallback* callback = finished_callback_;
+    finished_callback_ = NULL;
+    callback->Run(result);
+  }
+}
+
+// static
+void CollectNetworkStats(const std::string& network_stats_server) {
+  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,
+        NewRunnableFunction(&CollectNetworkStats, network_stats_server));
+    return;
+  }
+
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  static scoped_refptr<base::FieldTrial> trial = NULL;
+  static bool collect_stats = false;
+
+  if (!trial.get()) {
+    // Set up a field trial to collect network stats for UDP and TCP.
+    base::FieldTrial::Probability kDivisor = 1000;
+
+    // Enable the connectivity testing for 0.1% of the users.
+    base::FieldTrial::Probability kProbabilityPerGroup = 1;
+
+    // After October 30, 2011 builds, it will always be in default group
+    // (disable_network_stats).
+    trial = new base::FieldTrial("NetworkConnectivity", kDivisor,
+                                 "disable_network_stats", 2011, 10, 30);
+
+    // Add option to collect_stats for NetworkConnectivity.
+    int collect_stats_group = trial->AppendGroup("collect_stats",
+                                                 kProbabilityPerGroup);
+    if (trial->group() == collect_stats_group)
+      collect_stats = true;
+  }
+

[Mike Belshe, 2011/05/27 21:27:00]

Is there a way to add a check to see if the network is actually alive here?  

I believe this was a flaw with the WebSockets test - the test could kick off
when the user is offline (not connected to wifi on a laptop, for instance) and
it skewed the answers.  It would be fine to have an error state "NETWORK_DOWN"
if you want to track it.

[ramant (doing other things), 2011/05/31 21:25:36]

UMA calls only after it has successfully uploaded to the server. Added
net::NetworkChangeNotifier::IsOffline check.

Done.

+  if (collect_stats) {
+    TCPStatsClient* tcp_stats_client = new TCPStatsClient();
+    net::HostPortPair server_address(network_stats_server, 80);
+    tcp_stats_client->Start(server_address, 5, NULL);

[Mike Belshe, 2011/06/03 18:25:32]

One last thing - what do you think of doing a small write test and a large write
test?

I think 5 bytes is too small to be interesting.  Maybe 100 bytes for the small
test, 1200 bytes for the large test.

[ramant (doing other things), 2011/06/05 17:54:13]

Done.

+
+    UDPStatsClient* udp_stats_client = new UDPStatsClient();
+    udp_stats_client->Start(network_stats_server, 90, 5, NULL);

[Mike Belshe, 2011/05/27 21:27:00]

Port 90 is an IANA registered port for dnsix.  
http://www.iana.org/assignments/port-numbers

Recommend:
  HTTP:  use ports 80 (http), 6121 (spdy)
  UDP:   use ports 6121

[ramant (doing other things), 2011/05/31 21:25:36]

Done.

+  }
+}
+
+}  // namespace chrome_browser_net