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

net/udp/udp_socket_win.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 "net/udp/udp_socket_win.h"
 
 #include <mstcpip.h>
 
 #include "base/callback.h"
 #include "base/lazy_instance.h"
@@ skipping 466 matching lines @@
   addr_family_ = addr_family;
   socket_ = CreatePlatformSocket(addr_family_, SOCK_DGRAM, IPPROTO_UDP);
   if (socket_ == INVALID_SOCKET)
     return MapSystemError(WSAGetLastError());
   core_ = new Core(this);
   return OK;
 }
 
 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() {
   DCHECK(CalledOnValidThread());
   DCHECK(!is_connected());
 
   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) {
   DCHECK_NE(rv, ERR_IO_PENDING);
   DCHECK(!read_callback_.is_null());
 
   // since Run may result in Read being called, clear read_callback_ up front.
   CompletionCallback c = read_callback_;
   read_callback_.Reset();
   c.Run(rv);
@@ skipping 160 matching lines @@
   }
 
   core_->WatchForWrite();
   core_->write_iobuffer_ = buf;
   return ERR_IO_PENDING;
 }
 
 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;
     int protocol_level =
         addr_family_ == AF_INET ? IPPROTO_IP : IPPROTO_IPV6;
     int option =
         addr_family_ == AF_INET ? IP_MULTICAST_LOOP: IPV6_MULTICAST_LOOP;
     int rv = setsockopt(socket_, protocol_level, option,
                         reinterpret_cast<const char*>(&loop), sizeof(loop));
     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 =
         addr_family_ == AF_INET ? IPPROTO_IP : IPPROTO_IPV6;
     int option =
         addr_family_ == AF_INET ? IP_MULTICAST_TTL: IPV6_MULTICAST_HOPS;
     int rv = setsockopt(socket_, protocol_level, option,
                         reinterpret_cast<const char*>(&hops), sizeof(hops));
     if (rv < 0)
       return MapSystemError(WSAGetLastError());
@@ skipping 20 matching lines @@
         break;
       }
       default:
         NOTREACHED() << "Invalid address family";
         return ERR_ADDRESS_INVALID;
     }
   }
   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))
     return ERR_ADDRESS_INVALID;
   int rv = bind(socket_, storage.addr, storage.addr_len);
   if (rv == 0)
     return OK;
   int last_error = WSAGetLastError();
   UMA_HISTOGRAM_SPARSE_SLOWLY("Net.UdpSocketBindErrorFromWinOS", last_error);
   // Map some codes that are special to bind() separately.
@@ skipping 222 matching lines @@
               NULL);
 
   return OK;
 }
 
 void UDPSocketWin::DetachFromThread() {
   base::NonThreadSafe::DetachFromThread();
 }
 
 }  // namespace net