UDP connectivity - call host resolver for UDP echo (server, port)

before connecting to the server.

Moved the host resolved code from TCPClientStats to NetworkStats
so that TCP and UDP client code can share it.

Minor restructuring of the code to avoid code duplication.

TEST=network_stats_unittest
BUG=93463
R=jar
Review URL: http://codereview.chromium.org/7631065

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@97726 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 113 insertions, 106 deletions

diff --git a/chrome/browser/net/network_stats.cc b/chrome/browser/net/network_stats.cc
index 5b394b1..6b600b2 100644
--- a/chrome/browser/net/network_stats.cc
+++ b/chrome/browser/net/network_stats.cc
@@ -80,6 +80,8 @@ NetworkStats::NetworkStats()
       bytes_to_send_(0),
       encoded_message_(""),
       ALLOW_THIS_IN_INITIALIZER_LIST(
+          resolve_callback_(this, &NetworkStats::OnResolveComplete)),
+      ALLOW_THIS_IN_INITIALIZER_LIST(
           read_callback_(this, &NetworkStats::OnReadComplete)),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           write_callback_(this, &NetworkStats::OnWriteComplete)),
@@ -92,6 +94,25 @@ NetworkStats::~NetworkStats() {
   socket_.reset();
 }
 
+bool NetworkStats::Start(net::HostResolver* host_resolver,
+                         const net::HostPortPair& server_host_port_pair,
+                         uint32 bytes_to_send,
+                         net::CompletionCallback* finished_callback) {
+  DCHECK(bytes_to_send);   // We should have data to send.
+
+  Initialize(bytes_to_send, finished_callback);
+
+  net::HostResolver::RequestInfo request(server_host_port_pair);
+  int rv = host_resolver->Resolve(request,
+                                  &addresses_,
+                                  &resolve_callback_,
+                                  NULL,
+                                  net::BoundNetLog());
+  if (rv == net::ERR_IO_PENDING)
+    return true;
+  return DoConnect(rv);
+}
+
 void NetworkStats::Initialize(uint32 bytes_to_send,
                               net::CompletionCallback* finished_callback) {
   DCHECK(bytes_to_send);   // We should have data to send.
@@ -104,7 +125,7 @@ void NetworkStats::Initialize(uint32 bytes_to_send,
   finished_callback_ = finished_callback;
 }
 
-bool NetworkStats::DoStart(int result) {
+bool NetworkStats::ConnectComplete(int result) {
   if (result < 0) {
     Finish(CONNECT_FAILED, result);
     return false;
@@ -171,6 +192,10 @@ bool NetworkStats::ReadComplete(int result) {
   return false;
 }
 
+void NetworkStats::OnResolveComplete(int result) {
+  DoConnect(result);
+}
+
 void NetworkStats::OnReadComplete(int result) {
   if (!ReadComplete(result)) {
     // Called ReadData() via PostDelayedTask() to avoid recursion. Added a delay
@@ -343,21 +368,11 @@ UDPStatsClient::UDPStatsClient()
 UDPStatsClient::~UDPStatsClient() {
 }
 
-bool UDPStatsClient::Start(const std::string& ip_str,
-                           int port,
-                           uint32 bytes_to_send,
-                           net::CompletionCallback* finished_callback) {
-  DCHECK(port);
-  DCHECK(bytes_to_send);   // We should have data to send.
-
-  Initialize(bytes_to_send, finished_callback);
-
-  net::IPAddressNumber ip_number;
-  if (!net::ParseIPLiteralToNumber(ip_str, &ip_number)) {
-    Finish(IP_STRING_PARSE_FAILED, net::ERR_INVALID_ARGUMENT);
+bool UDPStatsClient::DoConnect(int result) {
+  if (result != net::OK) {
+    Finish(RESOLVE_FAILED, result);
     return false;
   }
-  net::IPEndPoint server_address = net::IPEndPoint(ip_number, port);
 
   net::UDPClientSocket* udp_socket =
       new net::UDPClientSocket(net::DatagramSocket::DEFAULT_BIND,
@@ -370,8 +385,20 @@ bool UDPStatsClient::Start(const std::string& ip_str,
   }
   set_socket(udp_socket);
 
-  int rv = udp_socket->Connect(server_address);
-  return DoStart(rv);
+  const addrinfo* address = GetAddressList().head();
+  if (!address) {
+    Finish(RESOLVE_FAILED, net::ERR_INVALID_ARGUMENT);
+    return false;
+  }
+
+  net::IPEndPoint endpoint;
+  endpoint.FromSockAddr(address->ai_addr, address->ai_addrlen);
+  int rv = udp_socket->Connect(endpoint);
+  if (rv < 0) {
+    Finish(CONNECT_FAILED, rv);
+    return false;
+  }
+  return NetworkStats::ConnectComplete(rv);
 }
 
 bool UDPStatsClient::ReadComplete(int result) {
@@ -416,39 +443,14 @@ void UDPStatsClient::Finish(Status status, int result) {
 
 // TCPStatsClient methods and members.
 TCPStatsClient::TCPStatsClient()
-  : NetworkStats(),
-    ALLOW_THIS_IN_INITIALIZER_LIST(
-        resolve_callback_(this, &TCPStatsClient::OnResolveComplete)),
-    ALLOW_THIS_IN_INITIALIZER_LIST(
-        connect_callback_(this, &TCPStatsClient::OnConnectComplete)) {
+    : NetworkStats(),
+      ALLOW_THIS_IN_INITIALIZER_LIST(
+          connect_callback_(this, &TCPStatsClient::OnConnectComplete)) {
 }
 
 TCPStatsClient::~TCPStatsClient() {
 }
 
-bool TCPStatsClient::Start(net::HostResolver* host_resolver,
-                           const net::HostPortPair& server_host_port_pair,
-                           uint32 bytes_to_send,
-                           net::CompletionCallback* finished_callback) {
-  DCHECK(bytes_to_send);   // We should have data to send.
-
-  Initialize(bytes_to_send, finished_callback);
-
-  net::HostResolver::RequestInfo request(server_host_port_pair);
-  int rv = host_resolver->Resolve(request,
-                                  &addresses_,
-                                  &resolve_callback_,
-                                  NULL,
-                                  net::BoundNetLog());
-  if (rv == net::ERR_IO_PENDING)
-    return true;
-  return DoConnect(rv);
-}
-
-void TCPStatsClient::OnResolveComplete(int result) {
-  DoConnect(result);
-}
-
 bool TCPStatsClient::DoConnect(int result) {
   if (result != net::OK) {
     Finish(RESOLVE_FAILED, result);
@@ -456,7 +458,7 @@ bool TCPStatsClient::DoConnect(int result) {
   }
 
   net::TCPClientSocket* tcp_socket =
-      new net::TCPClientSocket(addresses_, NULL, net::NetLog::Source());
+      new net::TCPClientSocket(GetAddressList(), NULL, net::NetLog::Source());
   if (!tcp_socket) {
     Finish(SOCKET_CREATE_FAILED, net::ERR_INVALID_ARGUMENT);
     return false;
@@ -467,11 +469,11 @@ bool TCPStatsClient::DoConnect(int result) {
   if (rv == net::ERR_IO_PENDING)
     return true;
 
-  return DoStart(rv);
+  return NetworkStats::ConnectComplete(rv);
 }
 
 void TCPStatsClient::OnConnectComplete(int result) {
-  DoStart(result);
+  NetworkStats::ConnectComplete(result);
 }
 
 bool TCPStatsClient::ReadComplete(int result) {
@@ -573,29 +575,31 @@ void CollectNetworkStats(const std::string& network_stats_server,
   // Use SPDY's UDP port per http://www.iana.org/assignments/port-numbers.
   // |network_stats_server| echo TCP and UDP servers listen on the following
   // ports.
-  uint32 kTCPTestingPort = 80;
+  uint32 kTCPTestingPort = 8081;
   uint32 kUDPTestingPort = 6121;
 
+  net::HostResolver* host_resolver = io_thread->globals()->host_resolver.get();
+  DCHECK(host_resolver);
+
+  net::HostPortPair udp_server_address(network_stats_server, kUDPTestingPort);
+
   UDPStatsClient* small_udp_stats = new UDPStatsClient();
   small_udp_stats->Start(
-      network_stats_server, kUDPTestingPort, kSmallTestBytesToSend, NULL);
+      host_resolver, udp_server_address, kSmallTestBytesToSend, NULL);
 
   UDPStatsClient* large_udp_stats = new UDPStatsClient();
   large_udp_stats->Start(
-      network_stats_server, kUDPTestingPort, kLargeTestBytesToSend, NULL);
-
-  net::HostResolver* host_resolver = io_thread->globals()->host_resolver.get();
-  DCHECK(host_resolver);
+      host_resolver, udp_server_address, kLargeTestBytesToSend, NULL);
 
-  net::HostPortPair server_address(network_stats_server, kTCPTestingPort);
+  net::HostPortPair tcp_server_address(network_stats_server, kTCPTestingPort);
 
   TCPStatsClient* small_tcp_client = new TCPStatsClient();
-  small_tcp_client->Start(host_resolver, server_address, kSmallTestBytesToSend,
-                          NULL);
+  small_tcp_client->Start(
+      host_resolver, tcp_server_address, kSmallTestBytesToSend, NULL);
 
   TCPStatsClient* large_tcp_client = new TCPStatsClient();
-  large_tcp_client->Start(host_resolver, server_address, kLargeTestBytesToSend,
-                          NULL);
+  large_tcp_client->Start(
+      host_resolver, tcp_server_address, kLargeTestBytesToSend, NULL);
 }
 
 }  // namespace chrome_browser_net
diff --git a/chrome/browser/net/network_stats.h b/chrome/browser/net/network_stats.h
index bbc449f..59106d2 100644
--- a/chrome/browser/net/network_stats.h
+++ b/chrome/browser/net/network_stats.h
@@ -64,6 +64,18 @@ class NetworkStats {
     STATUS_MAX,              // Bounding value.
   };
 
+  // Starts the client, connecting to |server|.
+  // Client will send |bytes_to_send| bytes to |server|.
+  // When client has received all echoed bytes from the server, or
+  // when an error occurs causing the client to stop, |Finish| will be
+  // called with a net status code.
+  // |Finish| will collect histogram stats.
+  // Returns true if successful in starting the client.
+  bool Start(net::HostResolver* host_resolver,
+             const net::HostPortPair& server,
+             uint32 bytes_to_send,
+             net::CompletionCallback* callback);
+
  protected:
   // Constructs an NetworkStats object that collects metrics for network
   // connectivity (either TCP or UDP).
@@ -76,11 +88,15 @@ class NetworkStats {
   void Initialize(uint32 bytes_to_send,
                   net::CompletionCallback* finished_callback);
 
+  // Called after host is resolved. UDPStatsClient and TCPStatsClient implement
+  // this method. They create the socket and connect to the server.
+  virtual bool DoConnect(int result) = 0;
+
   // This method is called after socket connection is completed. It will send
   // |bytes_to_send| bytes to |server| by calling SendData(). After successfully
   // sending data to the |server|, it calls ReadData() to read/verify the data
   // from the |server|. Returns true if successful.
-  bool DoStart(int result);
+  bool ConnectComplete(int result);
 
   // Collects network connectivity stats. This is called when all the data from
   // server is read or when there is a failure during connect/read/write.
@@ -104,7 +120,13 @@ class NetworkStats {
   // Returns |start_time_| (used by histograms).
   base::TimeTicks start_time() const { return start_time_; }
 
+  // Returns |addresses_|.
+  net::AddressList GetAddressList() const { return addresses_; }
+
  private:
+  // Callback that is called when host resolution is completed.
+  void OnResolveComplete(int result);
+
   // Callbacks when an internal IO is completed.
   void OnReadComplete(int result);
   void OnWriteComplete(int result);
@@ -151,6 +173,12 @@ class NetworkStats {
   // used to verify the data received from the server.
   net::TestDataStream stream_;
 
+  // HostResolver fills out the |addresses_| after host resolution is completed.
+  net::AddressList addresses_;
+
+  // Callback to call when host resolution is completed.
+  net::CompletionCallbackImpl<NetworkStats> resolve_callback_;
+
   // Callback to call when data is read from the server.
   net::CompletionCallbackImpl<NetworkStats> read_callback_;
 
@@ -176,30 +204,23 @@ class UDPStatsClient : public NetworkStats {
   UDPStatsClient();
   virtual ~UDPStatsClient();
 
-  // Starts the client, connecting to |server|.
-  // Client will send |bytes_to_send| bytes to |server|.
-  // When client has received all echoed bytes from the server, or
-  // when an error occurs causing the client to stop, |Finish| will be
-  // called with a net status code.
-  // |Finish| will collect histogram stats.
-  // Returns true if successful in starting the client.
-  bool Start(const std::string& ip_str,
-             int port,
-             uint32 bytes_to_send,
-             net::CompletionCallback* callback);
-
  protected:
   // Allow tests to access our innards for testing purposes.
   friend class NetworkStatsTestUDP;
 
-  // 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.
-  virtual void Finish(Status status, int result);
+  // Called after host is resolved. Creates UDClientSocket and connects to the
+  // server. If successfully connected, then calls ConnectComplete() to start
+  // the echo protocol. Returns |false| if there is any error.
+  virtual bool DoConnect(int result);
 
   // This method calls NetworkStats::ReadComplete() to verify the data and calls
   // Finish() if there is an error or if read callback didn't return any data
   // (|result| is less than or equal to 0).
   virtual bool ReadComplete(int result);
+
+  // 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.
+  virtual void Finish(Status status, int result);
 };
 
 class TCPStatsClient : public NetworkStats {
@@ -209,50 +230,29 @@ class TCPStatsClient : public NetworkStats {
   TCPStatsClient();
   virtual ~TCPStatsClient();
 
-  // Starts the client, connecting to |server|.
-  // Client will send |bytes_to_send| bytes.
-  // When the client has received all echoed bytes from the server, or
-  // when an error occurs causing the client to stop, |Finish| will be
-  // called with a net status code.
-  // |Finish| will collect histogram stats.
-  // Returns true if successful in starting the client.
-  bool Start(net::HostResolver* host_resolver,
-             const net::HostPortPair& server,
-             uint32 bytes_to_send,
-             net::CompletionCallback* callback);
-
  protected:
   // Allow tests to access our innards for testing purposes.
   friend class NetworkStatsTestTCP;
 
-  // 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.
-  virtual void Finish(Status status, int result);
+  // Called after host is resolved. Creates TCPClientSocket and connects to the
+  // server.
+  virtual bool DoConnect(int result);
 
   // This method calls NetworkStats::ReadComplete() to verify the data and calls
   // Finish() if there is an error (|result| is less than 0).
   virtual bool ReadComplete(int result);
 
- private:
-  // Callback that is called when host resolution is completed.
-  void OnResolveComplete(int result);
-
-  // Called after host is resolved. Creates TCPClientSocket and connects to the
-  // server.
-  bool DoConnect(int result);
+  // 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.
+  virtual void Finish(Status status, int result);
 
-  // Callback that is called when connect is completed and calls DoStart() to
-  // start the echo protocl.
+ private:
+  // Callback that is called when connect is completed and calls
+  // ConnectComplete() to start the echo protocol.
   void OnConnectComplete(int result);
 
-  // Callback to call when host resolution is completed.
-  net::CompletionCallbackImpl<TCPStatsClient> resolve_callback_;
-
   // Callback to call when connect is completed.
   net::CompletionCallbackImpl<TCPStatsClient> connect_callback_;
-
-  // HostResolver fills out the |addresses_| after host resolution is completed.
-  net::AddressList addresses_;
 };
 
 // This collects the network connectivity stats for UDP and TCP for small
diff --git a/chrome/browser/net/network_stats_unittest.cc b/chrome/browser/net/network_stats_unittest.cc
index cbe9625..260e24a 100644
--- a/chrome/browser/net/network_stats_unittest.cc
+++ b/chrome/browser/net/network_stats_unittest.cc
@@ -36,9 +36,12 @@ class NetworkStatsTestUDP : public NetworkStatsTest {
   void RunUDPEchoTest(int bytes) {
     TestCompletionCallback cb;
 
+    scoped_ptr<net::MockHostResolver> host_resolver(
+        new net::MockHostResolver());
+
     UDPStatsClient* udp_stats_client = new UDPStatsClient();
-    EXPECT_TRUE(udp_stats_client->Start(test_server_.host_port_pair().host(),
-                                        test_server_.host_port_pair().port(),
+    EXPECT_TRUE(udp_stats_client->Start(host_resolver.get(),
+                                        test_server_.host_port_pair(),
                                         bytes,
                                         &cb));
     int rv = cb.WaitForResult();