Remove timing limitation of SetOption invocation for PPAPI sockets.

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 b18aa0a53693403d727acee938022bb7b7012239..35885a82c8fbeaad39c98d073c247e3643c8848f 100644
--- a/net/udp/udp_socket_win.cc
+++ b/net/udp/udp_socket_win.cc
@@ -484,59 +484,70 @@ 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()) {
+    int result = SetReceiveBufferSizeInternal(size);

[rvargas (doing something else), 2014/11/08 02:17:20]

I actually don't see why we want to embrace this model of caching everything
only to call setsockopt multiple times in a row later on. Is it to protect
against caller calling multiple times this method in a row before bind? It looks
more twisted and there are other ways to prevent that (*if* we really care)

+    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 = SetReceiveBufferSizeInternal(size);
+    if (result != OK)
+      return result;
+  }
+  socket_options_ |= SOCKET_OPTION_SNDBUF_SIZE;
+  sndbuf_size_ = size;
+  return OK;
 }
 
-void UDPSocketWin::AllowAddressReuse() {
+int UDPSocketWin::AllowAddressReuse() {
   DCHECK(CalledOnValidThread());
-  DCHECK(!is_connected());
-
+  if (is_connected()) {
+    int result = SetAddressReuseInternal(true);

[rvargas (doing something else), 2014/11/08 02:17:20]

This path should fail so there is no reason to continue.

"Note  When using bind with the SO_EXCLUSIVEADDRUSE or SO_REUSEADDR socket
option, the socket option must be set prior to executing bind to have any
affect."[sic]

http://msdn.microsoft.com/en-us/library/windows/desktop/ms737550(v=vs.85).aspx

It probably doesn't make sense to change our API unless there's a strong reason
to allow clients to attempt this later only to receive a failure.

+    if (result != OK)
+      return result;
+  }
   socket_options_ |= SOCKET_OPTION_REUSE_ADDRESS;
+  return OK;
 }
 
-void UDPSocketWin::AllowBroadcast() {
+int UDPSocketWin::DisallowAddressReuse() {
   DCHECK(CalledOnValidThread());
-  DCHECK(!is_connected());
+  if (is_connected()) {
+    int result = SetAddressReuseInternal(false);
+    if (result != OK)
+      return result;
+  }
+  socket_options_ &= ~SOCKET_OPTION_REUSE_ADDRESS;
+  return OK;
+}
 
+int UDPSocketWin::AllowBroadcast() {

[rvargas (doing something else), 2014/11/08 02:17:20]

I'd prefer not having two independent methods to control the same thing,
especially considering the behavior of SetMulticastLoopbackMode

+  DCHECK(CalledOnValidThread());
+  if (is_connected()) {
+    int result = SetBroadcastInternal(true);
+    if (result != OK)
+      return result;
+  }
   socket_options_ |= SOCKET_OPTION_BROADCAST;
+  return OK;
+}
+
+int UDPSocketWin::DisallowBroadcast() {
+  DCHECK(CalledOnValidThread());
+  if (is_connected()) {
+    int result = SetBroadcastInternal(false);
+    if (result != OK)
+      return result;
+  }
+  socket_options_ &= ~SOCKET_OPTION_BROADCAST;
+  return OK;
 }
 
 void UDPSocketWin::DoReadCallback(int rv) {
@@ -715,20 +726,18 @@ int UDPSocketWin::InternalSendTo(IOBuffer* buf, int buf_len,
 }
 
 int UDPSocketWin::SetSocketOptions() {
-  BOOL true_value = 1;
+  DCHECK(CalledOnValidThread());
+  DCHECK(is_connected());
+
   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 = SetAddressReuseInternal(true);
+    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 +750,16 @@ 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 +800,75 @@ int UDPSocketWin::SetSocketOptions() {
   return OK;
 }
 
+int UDPSocketWin::SetAddressReuseInternal(bool address_reuse) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(is_connected());
+
+  int opt_value = address_reuse ? 1 : 0;
+  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());
+
+  int opt_value = broadcast ? 1 : 0;

[rvargas (doing something else), 2014/11/08 02:17:20]

BOOL, 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))