Remove timing limitation to set Broadcast, ReceiveBuffer, and SendBuffer options from UDPSocket.

net/udp/udp_socket_win.cc

Index: net/udp/udp_socket_win.cc
diff --git a/net/udp/udp_socket_win.cc b/net/udp/udp_socket_win.cc
index 4c307fd3b2804f4cec8fcf370aafc875050150a3..a69a00974d7f994773586d85351586f913a763a2 100644
--- a/net/udp/udp_socket_win.cc
+++ b/net/udp/udp_socket_win.cc
@@ -484,45 +484,26 @@ int UDPSocketWin::CreateSocket(int addr_family) {
 
 int UDPSocketWin::SetReceiveBufferSize(int32 size) {
   DCHECK(CalledOnValidThread());
-  int rv = setsockopt(socket_, SOL_SOCKET, SO_RCVBUF,
-                      reinterpret_cast<const char*>(&size), sizeof(size));
-  if (rv != 0)
-    return MapSystemError(WSAGetLastError());
-
-  // According to documentation, setsockopt may succeed, but we need to check
-  // the results via getsockopt to be sure it works on Windows.
-  int32 actual_size = 0;
-  int option_size = sizeof(actual_size);
-  rv = getsockopt(socket_, SOL_SOCKET, SO_RCVBUF,
-                  reinterpret_cast<char*>(&actual_size), &option_size);
-  if (rv != 0)
-    return MapSystemError(WSAGetLastError());
-  if (actual_size >= size)
-    return OK;
-  UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SocketUnchangeableReceiveBuffer",
-                              actual_size, 1000, 1000000, 50);
-  return ERR_SOCKET_RECEIVE_BUFFER_SIZE_UNCHANGEABLE;
+  if (is_connected()) {

[rvargas (doing something else), 2014/11/14 02:44:43]

As I mentioned on the other review, I don't really see the purpose of this
pattern. If we are just forwarding the call to the OS we should just do it here.

(applies everywhere)

[hidehiko, 2014/11/19 06:11:24]

This is to address mmenke@'s comment
https://codereview.chromium.org/690903002/diff/40001/net/udp/udp_socket_libev...

There are two reasons of this pattern:
1) Now, the OS socket is created in Bind(), so to support setting options from
beginning of OS's bind(), we need to cache it.
2) According to mmenke@'s comment, it is necessary to support
Bind()->Close()->Bind() pattern. For that purpose, we need to remember the
option in the members.

Alternative approach would be:
1) Split CreateSocket() from Bind(), so that the API can have users call option
setting methods between CreateSocket() and Bind(). (This is what TCPSocket does,
IIUC).
2) Disallow rebinding. (Though, this still needs to remember the values before
CreateSocket()).

WDYT?

[rvargas (doing something else), 2014/11/19 22:18:56]

Both? I Agree with Matt in that rebinding is a weird pattern. I don't think we
should actively support that. I hope no one relies on this behavior today. 

It also makes sense to add an Open method to bring this class closer to
TCPSocket.

[hidehiko, 2014/12/01 16:06:24]

Ok, Done both.
Note: along the change, now UDPServerSocket has a bit more code rather than just
a proxy to UDPSocket. The semantics of the class should not change by this CL.
It is something similar to TCPServerSocket does, so I assume it's ok.
Also, Set{Receive,Send}BufferSize are reverted. Because Open() is split from
Connect() and Bind(), client code can set them before Connect() or Bind() with
the current implementation.

+    int result = SetReceiveBufferSizeInternal(size);
+    if (result != OK)
+      return result;
+  }
+  socket_options_ |= SOCKET_OPTION_RCVBUF_SIZE;
+  rcvbuf_size_ = size;
+  return OK;
 }
 
 int UDPSocketWin::SetSendBufferSize(int32 size) {
   DCHECK(CalledOnValidThread());
-  int rv = setsockopt(socket_, SOL_SOCKET, SO_SNDBUF,
-                      reinterpret_cast<const char*>(&size), sizeof(size));
-  if (rv != 0)
-    return MapSystemError(WSAGetLastError());
-  // According to documentation, setsockopt may succeed, but we need to check
-  // the results via getsockopt to be sure it works on Windows.
-  int32 actual_size = 0;
-  int option_size = sizeof(actual_size);
-  rv = getsockopt(socket_, SOL_SOCKET, SO_SNDBUF,
-                  reinterpret_cast<char*>(&actual_size), &option_size);
-  if (rv != 0)
-    return MapSystemError(WSAGetLastError());
-  if (actual_size >= size)
-    return OK;
-  UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SocketUnchangeableSendBuffer",
-                              actual_size, 1000, 1000000, 50);
-  return ERR_SOCKET_SEND_BUFFER_SIZE_UNCHANGEABLE;
+  if (is_connected()) {
+    int result = SetSendBufferSizeInternal(size);
+    if (result != OK)
+      return result;
+  }
+  socket_options_ |= SOCKET_OPTION_SNDBUF_SIZE;
+  sndbuf_size_ = size;
+  return OK;
 }
 
 void UDPSocketWin::AllowAddressReuse() {
@@ -532,11 +513,18 @@ void UDPSocketWin::AllowAddressReuse() {
   socket_options_ |= SOCKET_OPTION_REUSE_ADDRESS;
 }
 
-void UDPSocketWin::AllowBroadcast() {
+int UDPSocketWin::SetBroadcast(bool broadcast) {
   DCHECK(CalledOnValidThread());
-  DCHECK(!is_connected());
-
-  socket_options_ |= SOCKET_OPTION_BROADCAST;
+  if (is_connected()) {
+    int result = SetBroadcastInternal(broadcast);
+    if (result != OK)
+      return result;
+  }
+  if (broadcast)
+    socket_options_ |= SOCKET_OPTION_BROADCAST;
+  else
+    socket_options_ &= ~SOCKET_OPTION_BROADCAST;
+  return OK;
 }
 
 void UDPSocketWin::DoReadCallback(int rv) {
@@ -717,18 +705,14 @@ int UDPSocketWin::InternalSendTo(IOBuffer* buf, int buf_len,
 int UDPSocketWin::SetSocketOptions() {
   BOOL true_value = 1;
   if (socket_options_ & SOCKET_OPTION_REUSE_ADDRESS) {
-    int rv = setsockopt(socket_, SOL_SOCKET, SO_REUSEADDR,
-                        reinterpret_cast<const char*>(&true_value),
-                        sizeof(true_value));
-    if (rv < 0)
-      return MapSystemError(WSAGetLastError());
+    int rv = AllowAddressReuseInternal();
+    if (rv != OK)
+      return rv;
   }
   if (socket_options_ & SOCKET_OPTION_BROADCAST) {
-    int rv = setsockopt(socket_, SOL_SOCKET, SO_BROADCAST,
-                        reinterpret_cast<const char*>(&true_value),
-                        sizeof(true_value));
-    if (rv < 0)
-      return MapSystemError(WSAGetLastError());
+    int rv = SetBroadcastInternal(true);
+    if (rv != OK)
+      return rv;
   }
   if (!(socket_options_ & SOCKET_OPTION_MULTICAST_LOOP)) {
     DWORD loop = 0;
@@ -741,6 +725,17 @@ int UDPSocketWin::SetSocketOptions() {
     if (rv < 0)
       return MapSystemError(WSAGetLastError());
   }
+  if (socket_options_ & SOCKET_OPTION_RCVBUF_SIZE) {
+    int rv = SetReceiveBufferSizeInternal(rcvbuf_size_);
+    if (rv != OK)
+      return rv;
+  }
+  if (socket_options_ & SOCKET_OPTION_SNDBUF_SIZE) {
+    int rv = SetSendBufferSizeInternal(sndbuf_size_);
+    if (rv != OK)
+      return rv;
+  }
+
   if (multicast_time_to_live_ != 1) {
     DWORD hops = multicast_time_to_live_;
     int protocol_level =
@@ -781,6 +776,75 @@ int UDPSocketWin::SetSocketOptions() {
   return OK;
 }
 
+int UDPSocketWin::AllowAddressReuseInternal() {
+  DCHECK(CalledOnValidThread());
+  DCHECK(is_connected());
+
+  BOOL opt_value = TRUE;
+  int rv = setsockopt(
+      socket_, SOL_SOCKET, SO_REUSEADDR,
+      reinterpret_cast<const char*>(&opt_value), sizeof(opt_value));
+  return rv == 0 ? OK : MapSystemError(WSAGetLastError());
+}
+
+int UDPSocketWin::SetBroadcastInternal(bool broadcast) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(is_connected());
+
+  BOOL opt_value = broadcast ? TRUE : FALSE;
+  int rv = setsockopt(
+      socket_, SOL_SOCKET, SO_BROADCAST,
+      reinterpret_cast<const char*>(&opt_value), sizeof(opt_value));
+  return rv == 0 ? OK : MapSystemError(WSAGetLastError());
+}
+
+int UDPSocketWin::SetReceiveBufferSizeInternal(int32 size) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(is_connected());
+
+  int rv = setsockopt(socket_, SOL_SOCKET, SO_RCVBUF,
+                      reinterpret_cast<const char*>(&size), sizeof(size));
+  if (rv != 0)
+    return MapSystemError(WSAGetLastError());
+
+  // According to documentation, setsockopt may succeed, but we need to check
+  // the results via getsockopt to be sure it works on Windows.
+  int32 actual_size = 0;
+  int option_size = sizeof(actual_size);
+  rv = getsockopt(socket_, SOL_SOCKET, SO_RCVBUF,
+                  reinterpret_cast<char*>(&actual_size), &option_size);
+  if (rv != 0)
+    return MapSystemError(WSAGetLastError());
+  if (actual_size >= size)
+    return OK;
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SocketUnchangeableReceiveBuffer",
+                              actual_size, 1000, 1000000, 50);
+  return ERR_SOCKET_RECEIVE_BUFFER_SIZE_UNCHANGEABLE;
+}
+
+int UDPSocketWin::SetSendBufferSizeInternal(int32 size) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(is_connected());
+
+  int rv = setsockopt(socket_, SOL_SOCKET, SO_SNDBUF,
+                      reinterpret_cast<const char*>(&size), sizeof(size));
+  if (rv != 0)
+    return MapSystemError(WSAGetLastError());
+  // According to documentation, setsockopt may succeed, but we need to check
+  // the results via getsockopt to be sure it works on Windows.
+  int32 actual_size = 0;
+  int option_size = sizeof(actual_size);
+  rv = getsockopt(socket_, SOL_SOCKET, SO_SNDBUF,
+                  reinterpret_cast<char*>(&actual_size), &option_size);
+  if (rv != 0)
+    return MapSystemError(WSAGetLastError());
+  if (actual_size >= size)
+    return OK;
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SocketUnchangeableSendBuffer",
+                              actual_size, 1000, 1000000, 50);
+  return ERR_SOCKET_SEND_BUFFER_SIZE_UNCHANGEABLE;
+}
+
 int UDPSocketWin::DoBind(const IPEndPoint& address) {
   SockaddrStorage storage;
   if (!address.ToSockAddr(storage.addr, &storage.addr_len))