Remove timing limitation to set Broadcast, ReceiveBuffer, and SendBuf…

…fer options from UDPSocket. Currently, we have timing limitation to set these options. This CL splits Open() from Connect() and Bind(), so that those options can be set anytime after Open(). This is the preparation to remove the same limitation from PPAPI's socket APIs. BUG=425563, 420697 TEST=Ran trybots. Review URL: https://codereview.chromium.org/721273002 Cr-Commit-Position: refs/heads/master@{#307204}
dednal · Dec 8, 2014 · 17fac55 · 17fac55
1 parent cdbebd0
commit 17fac55
Show file tree

Hide file tree

Showing 18 changed files with 290 additions and 265 deletions.
diff --git a/chrome/browser/extensions/api/cast_streaming/cast_streaming_apitest.cc b/chrome/browser/extensions/api/cast_streaming/cast_streaming_apitest.cc
@@ -28,7 +28,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/base/rand_callback.h"
-#include "net/udp/udp_socket.h"
+#include "net/udp/udp_server_socket.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using media::cast::test::GetFreeLocalPort;
@@ -341,13 +341,10 @@ class CastStreamingApiTestWithPixelOutput : public CastStreamingApiTest {
 #define MAYBE_EndToEnd DISABLED_EndToEnd
 #endif
 IN_PROC_BROWSER_TEST_F(CastStreamingApiTestWithPixelOutput, MAYBE_EndToEnd) {
-  scoped_ptr<net::UDPSocket> receive_socket(
-      new net::UDPSocket(net::DatagramSocket::DEFAULT_BIND,
-                         net::RandIntCallback(),
-                         NULL,
-                         net::NetLog::Source()));
+  scoped_ptr<net::UDPServerSocket> receive_socket(
+      new net::UDPServerSocket(NULL, net::NetLog::Source()));
   receive_socket->AllowAddressReuse();
-  ASSERT_EQ(net::OK, receive_socket->Bind(GetFreeLocalPort()));
+  ASSERT_EQ(net::OK, receive_socket->Listen(GetFreeLocalPort()));
   net::IPEndPoint receiver_end_point;
   ASSERT_EQ(net::OK, receive_socket->GetLocalAddress(&receiver_end_point));
   receive_socket.reset();

diff --git a/chrome/browser/extensions/api/cast_streaming/performance_test.cc b/chrome/browser/extensions/api/cast_streaming/performance_test.cc
@@ -48,7 +48,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/base/rand_callback.h"
-#include "net/udp/udp_socket.h"
+#include "net/udp/udp_server_socket.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/perf/perf_test.h"
 #include "ui/compositor/compositor_switches.h"
@@ -343,13 +343,10 @@ class CastV2PerformanceTest
     localhost.push_back(0);
     localhost.push_back(0);
     localhost.push_back(1);
-    scoped_ptr<net::UDPSocket> receive_socket(
-        new net::UDPSocket(net::DatagramSocket::DEFAULT_BIND,
-                           net::RandIntCallback(),
-                           NULL,
-                           net::NetLog::Source()));
+    scoped_ptr<net::UDPServerSocket> receive_socket(
+        new net::UDPServerSocket(NULL, net::NetLog::Source()));
     receive_socket->AllowAddressReuse();
-    CHECK_EQ(net::OK, receive_socket->Bind(net::IPEndPoint(localhost, 0)));
+    CHECK_EQ(net::OK, receive_socket->Listen(net::IPEndPoint(localhost, 0)));
     net::IPEndPoint endpoint;
     CHECK_EQ(net::OK, receive_socket->GetLocalAddress(&endpoint));
     return endpoint;

diff --git a/chrome/browser/extensions/api/dial/dial_service.cc b/chrome/browser/extensions/api/dial/dial_service.cc
@@ -172,13 +172,16 @@ bool DialServiceImpl::DialSocket::CreateAndBindSocket(
                               rand_cb,
                               net_log,
                               net_log_source));
-  socket_->AllowBroadcast();
 
   // 0 means bind a random port
   net::IPEndPoint address(bind_ip_address, 0);
 
-  if (!CheckResult("Bind", socket_->Bind(address)))
+  if (socket_->Open(address.GetFamily()) != net::OK ||
+      socket_->SetBroadcast(true) != net::OK ||
+      !CheckResult("Bind", socket_->Bind(address))) {
+    socket_.reset();
     return false;
+  }
 
   DCHECK(socket_.get());
 

diff --git a/cloud_print/gcp20/prototype/dns_sd_server.cc b/cloud_print/gcp20/prototype/dns_sd_server.cc
@@ -123,9 +123,7 @@ bool DnsSdServer::CreateSocket() {
   DCHECK(success);
 
 
-  socket_.reset(new net::UDPSocket(net::DatagramSocket::DEFAULT_BIND,
-                                   net::RandIntCallback(), NULL,
-                                   net::NetLog::Source()));
+  socket_.reset(new net::UDPServerSocket(NULL, net::NetLog::Source()));
 
   net::IPEndPoint local_address = net::IPEndPoint(local_ip_any,
                                                   kDefaultPortMulticast);
@@ -134,7 +132,7 @@ bool DnsSdServer::CreateSocket() {
 
   socket_->AllowAddressReuse();
 
-  int status = socket_->Bind(local_address);
+  int status = socket_->Listen(local_address);
   if (status < 0)
     return false;
 
@@ -143,9 +141,6 @@ bool DnsSdServer::CreateSocket() {
 
   if (status < 0)
     return false;
-
-  DCHECK(socket_->is_connected());
-
   return true;
 }
 

diff --git a/cloud_print/gcp20/prototype/dns_sd_server.h b/cloud_print/gcp20/prototype/dns_sd_server.h
@@ -13,7 +13,7 @@
 #include "base/memory/weak_ptr.h"
 #include "cloud_print/gcp20/prototype/service_parameters.h"
 #include "net/base/ip_endpoint.h"
-#include "net/udp/udp_socket.h"
+#include "net/udp/udp_server_socket.h"
 
 namespace net {
 
@@ -79,7 +79,7 @@ class DnsSdServer : public base::SupportsWeakPtr<DnsSdServer> {
   uint32 GetCurrentTLL() const;
 
   // Stores socket to multicast address.
-  scoped_ptr<net::UDPSocket> socket_;
+  scoped_ptr<net::UDPServerSocket> socket_;
 
   // Stores multicast address end point.
   net::IPEndPoint multicast_address_;
@@ -106,4 +106,3 @@ class DnsSdServer : public base::SupportsWeakPtr<DnsSdServer> {
 };
 
 #endif  // CLOUD_PRINT_GCP20_PROTOTYPE_DNS_SD_SERVER_H_
-
diff --git a/extensions/browser/api/socket/udp_socket.cc b/extensions/browser/api/socket/udp_socket.cc
@@ -49,8 +49,16 @@ void UDPSocket::Connect(const std::string& address,
       break;
     }
 
+    result = socket_.Open(ip_end_point.GetFamily());
+    if (result != net::OK)
+      break;
+
     result = socket_.Connect(ip_end_point);
-    is_connected_ = (result == net::OK);
+    if (result != net::OK) {
+      socket_.Close();
+      break;
+    }
+    is_connected_ = true;
   } while (false);
 
   callback.Run(result);
@@ -64,7 +72,14 @@ int UDPSocket::Bind(const std::string& address, uint16 port) {
   if (!StringAndPortToIPEndPoint(address, port, &ip_end_point))
     return net::ERR_INVALID_ARGUMENT;
 
-  return socket_.Bind(ip_end_point);
+  int result = socket_.Open(ip_end_point.GetFamily());
+  if (result != net::OK)
+    return result;
+
+  result = socket_.Bind(ip_end_point);
+  if (result != net::OK)
+    socket_.Close();
+  return result;
 }
 
 void UDPSocket::Disconnect() {

diff --git a/media/cast/net/udp_transport.cc b/media/cast/net/udp_transport.cc
@@ -69,16 +69,21 @@ void UdpTransport::StartReceiving(
   DCHECK(io_thread_proxy_->RunsTasksOnCurrentThread());
 
   packet_receiver_ = packet_receiver;
-  udp_socket_->AllowAddressReuse();
   udp_socket_->SetMulticastLoopbackMode(true);
   if (!IsEmpty(local_addr_)) {
-    if (udp_socket_->Bind(local_addr_) < 0) {
+    if (udp_socket_->Open(local_addr_.GetFamily()) < 0 ||
+        udp_socket_->AllowAddressReuse() < 0 ||
+        udp_socket_->Bind(local_addr_) < 0) {
+      udp_socket_->Close();
       status_callback_.Run(TRANSPORT_SOCKET_ERROR);
       LOG(ERROR) << "Failed to bind local address.";
       return;
     }
   } else if (!IsEmpty(remote_addr_)) {
-    if (udp_socket_->Connect(remote_addr_) < 0) {
+    if (udp_socket_->Open(remote_addr_.GetFamily()) < 0 ||
+        udp_socket_->AllowAddressReuse() < 0 ||
+        udp_socket_->Connect(remote_addr_) < 0) {
+      udp_socket_->Close();
       status_callback_.Run(TRANSPORT_SOCKET_ERROR);
       LOG(ERROR) << "Failed to connect to remote address.";
       return;

diff --git a/media/cast/test/utility/net_utility.cc b/media/cast/test/utility/net_utility.cc
@@ -6,7 +6,7 @@
 
 #include "base/basictypes.h"
 #include "net/base/net_errors.h"
-#include "net/udp/udp_socket.h"
+#include "net/udp/udp_server_socket.h"
 
 namespace media {
 namespace cast {
@@ -19,13 +19,10 @@ net::IPEndPoint GetFreeLocalPort() {
   localhost.push_back(0);
   localhost.push_back(0);
   localhost.push_back(1);
-  scoped_ptr<net::UDPSocket> receive_socket(
-      new net::UDPSocket(net::DatagramSocket::DEFAULT_BIND,
-                         net::RandIntCallback(),
-                         NULL,
-                         net::NetLog::Source()));
+  scoped_ptr<net::UDPServerSocket> receive_socket(
+      new net::UDPServerSocket(NULL, net::NetLog::Source()));
   receive_socket->AllowAddressReuse();
-  CHECK_EQ(net::OK, receive_socket->Bind(net::IPEndPoint(localhost, 0)));
+  CHECK_EQ(net::OK, receive_socket->Listen(net::IPEndPoint(localhost, 0)));
   net::IPEndPoint endpoint;
   CHECK_EQ(net::OK, receive_socket->GetLocalAddress(&endpoint));
   return endpoint;

diff --git a/media/cast/test/utility/udp_proxy.cc b/media/cast/test/utility/udp_proxy.cc
@@ -15,7 +15,7 @@
 #include "base/time/default_tick_clock.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
-#include "net/udp/udp_socket.h"
+#include "net/udp/udp_server_socket.h"
 
 namespace media {
 namespace cast {
@@ -738,10 +738,7 @@ class UDPProxyImpl : public UDPProxy {
  private:
   void Start(base::WaitableEvent* start_event,
              net::NetLog* net_log) {
-    socket_.reset(new net::UDPSocket(net::DatagramSocket::DEFAULT_BIND,
-                                     net::RandIntCallback(),
-                                     net_log,
-                                     net::NetLog::Source()));
+    socket_.reset(new net::UDPServerSocket(net_log, net::NetLog::Source()));
     BuildPipe(&to_dest_pipe_, new PacketSender(this, &destination_));
     BuildPipe(&from_dest_pipe_, new PacketSender(this, &return_address_));
     to_dest_pipe_->InitOnIOThread(base::MessageLoopProxy::current(),
@@ -753,7 +750,7 @@ class UDPProxyImpl : public UDPProxy {
     if (!destination_is_mutable_)
       VLOG(0) << "To:" << destination_.ToString();
 
-    CHECK_GE(socket_->Bind(local_port_), 0);
+    CHECK_GE(socket_->Listen(local_port_), 0);
 
     start_event->Signal();
     PollRead();
@@ -830,7 +827,7 @@ class UDPProxyImpl : public UDPProxy {
 
   base::DefaultTickClock tick_clock_;
   base::Thread proxy_thread_;
-  scoped_ptr<net::UDPSocket> socket_;
+  scoped_ptr<net::UDPServerSocket> socket_;
   scoped_ptr<PacketPipe> to_dest_pipe_;
   scoped_ptr<PacketPipe> from_dest_pipe_;
 

diff --git a/net/udp/udp_client_socket.cc b/net/udp/udp_client_socket.cc
@@ -4,6 +4,7 @@
 
 #include "net/udp/udp_client_socket.h"
 
+#include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 
 namespace net {
@@ -19,6 +20,9 @@ UDPClientSocket::~UDPClientSocket() {
 }
 
 int UDPClientSocket::Connect(const IPEndPoint& address) {
+  int rv = socket_.Open(address.GetFamily());
+  if (rv != OK)
+    return rv;
   return socket_.Connect(address);
 }
 

diff --git a/net/udp/udp_server_socket.cc b/net/udp/udp_server_socket.cc
@@ -4,6 +4,7 @@
 
 #include "net/udp/udp_server_socket.h"
 
+#include "net/base/net_errors.h"
 #include "net/base/rand_callback.h"
 
 namespace net {
@@ -13,13 +14,35 @@ UDPServerSocket::UDPServerSocket(net::NetLog* net_log,
     : socket_(DatagramSocket::DEFAULT_BIND,
               RandIntCallback(),
               net_log,
-              source) {
+              source),
+      allow_address_reuse_(false),
+      allow_broadcast_(false) {
 }
 
 UDPServerSocket::~UDPServerSocket() {
 }
 
 int UDPServerSocket::Listen(const IPEndPoint& address) {
+  int rv = socket_.Open(address.GetFamily());
+  if (rv != OK)
+    return rv;
+
+  if (allow_address_reuse_) {
+    rv = socket_.AllowAddressReuse();
+    if (rv != OK) {
+      socket_.Close();
+      return rv;
+    }
+  }
+
+  if (allow_broadcast_) {
+    rv = socket_.SetBroadcast(true);
+    if (rv != OK) {
+      socket_.Close();
+      return rv;
+    }
+  }
+
   return socket_.Bind(address);
 }
 
@@ -62,11 +85,11 @@ const BoundNetLog& UDPServerSocket::NetLog() const {
 }
 
 void UDPServerSocket::AllowAddressReuse() {
-  socket_.AllowAddressReuse();
+  allow_address_reuse_ = true;
 }
 
 void UDPServerSocket::AllowBroadcast() {
-  socket_.AllowBroadcast();
+  allow_broadcast_ = true;
 }
 
 int UDPServerSocket::JoinGroup(const IPAddressNumber& group_address) const {

diff --git a/net/udp/udp_server_socket.h b/net/udp/udp_server_socket.h
@@ -49,6 +49,8 @@ class NET_EXPORT UDPServerSocket : public DatagramServerSocket {
 
  private:
   UDPSocket socket_;
+  bool allow_address_reuse_;
+  bool allow_broadcast_;
   DISALLOW_COPY_AND_ASSIGN(UDPServerSocket);
 };
 

diff --git a/net/udp/udp_socket.h b/net/udp/udp_socket.h
@@ -21,18 +21,20 @@ namespace net {
 // Client form:
 // In this case, we're connecting to a specific server, so the client will
 // usually use:
-//       Connect(address)    // Connect to a UDP server
-//       Read/Write          // Reads/Writes all go to a single destination
+//       Open(address_family)  // Open a socket.
+//       Connect(address)      // Connect to a UDP server
+//       Read/Write            // Reads/Writes all go to a single destination
 //
 // Server form:
 // In this case, we want to read/write to many clients which are connecting
 // to this server.  First the server 'binds' to an addres, then we read from
 // clients and write responses to them.
 // Example:
-//       Bind(address/port)  // Binds to port for reading from clients
-//       RecvFrom/SendTo     // Each read can come from a different client
-//                           // Writes need to be directed to a specific
-//                           // address.
+//       Open(address_family)  // Open a socket.
+//       Bind(address/port)    // Binds to port for reading from clients
+//       RecvFrom/SendTo       // Each read can come from a different client
+//                             // Writes need to be directed to a specific
+//                             // address.
 #if defined(OS_WIN)
 typedef UDPSocketWin UDPSocket;
 #elif defined(OS_POSIX)