Remove timing limitation of SetOption invocation for PPAPI sockets.

net/udp/udp_socket_libevent.cc

Index: net/udp/udp_socket_libevent.cc
diff --git a/net/udp/udp_socket_libevent.cc b/net/udp/udp_socket_libevent.cc
index 9b1a9957189d968c3fed617ba02aa3ee7d13cb98..5fef2b9298b6161e8ecbdcb6ca16179f1d7ca2ef 100644
--- a/net/udp/udp_socket_libevent.cc
+++ b/net/udp/udp_socket_libevent.cc
@@ -317,34 +317,70 @@ int UDPSocketLibevent::Bind(const IPEndPoint& address) {
 
 int UDPSocketLibevent::SetReceiveBufferSize(int32 size) {
   DCHECK(CalledOnValidThread());
-  int rv = setsockopt(socket_, SOL_SOCKET, SO_RCVBUF,
-                      reinterpret_cast<const char*>(&size), sizeof(size));
-  int last_error = errno;
-  DCHECK(!rv) << "Could not set socket receive buffer size: " << last_error;
-  return rv == 0 ? OK : MapSystemError(last_error);
+  if (is_connected()) {
+    int result = SetReceiveBufferSizeInternal(size);
+    if (result != OK)
+      return result;
+  }
+  socket_options_ |= SOCKET_OPTION_RCVBUF_SIZE;
+  rcvbuf_size_ = size;
+  return OK;
 }
 
 int UDPSocketLibevent::SetSendBufferSize(int32 size) {
   DCHECK(CalledOnValidThread());
-  int rv = setsockopt(socket_, SOL_SOCKET, SO_SNDBUF,
-                      reinterpret_cast<const char*>(&size), sizeof(size));
-  int last_error = errno;
-  DCHECK(!rv) << "Could not set socket send buffer size: " << last_error;
-  return rv == 0 ? OK : MapSystemError(last_error);
+  if (is_connected()) {
+    int result = SetSendBufferSizeInternal(size);
+    if (result != OK)
+      return result;
+  }
+  socket_options_ |= SOCKET_OPTION_SNDBUF_SIZE;
+  sndbuf_size_ = size;
+  return OK;
 }
 
-void UDPSocketLibevent::AllowAddressReuse() {
+int UDPSocketLibevent::AllowAddressReuse() {
   DCHECK(CalledOnValidThread());
-  DCHECK(!is_connected());
-
+  if (is_connected()) {

[jar (doing other things), 2014/11/04 22:51:09]

What are the semantics for setting AllowAddressRuse *after* connecting?  

Is this just an invitation for an effective no-op, with no programmer warning?

[hidehiko, 2014/11/05 12:48:30]

It affects the behavior of other socket's for the same address.
I think following example illustrates the behavior.
Let be two processes, and each has a socket, s1 and s2.

case 1)
bind(s1, 'localhost', 12345)
bind(s2, 'localhost', 12345)  <- fails due to address conflicting.

case 2)
bind(s1, 'localhost', 12345)
setsockopt(s2, SO_REUSEADDR, 1)
bind(s2, 'localhost', 12345)  <- fails due to address conflicting.

case 3)
setsockopt(s1, SO_REUSEADDR, 1)
bind(s1, 'localhost', 12345)
setsockopt(s2, SO_REUSEADDR, 1)
bind(s2, 'localhost', 12345)  <- success

case 4)
bind(s1, 'localhost', 12345)
setsockopt(s1, SO_REUSEADDR, 1)
setsockopt(s2, SO_REUSEADDR, 1)
bind(s2, 'localhost', 12345)  <- success, too!

So, setting SO_REUSEADDR after bind() has effect, I think.

+    int result = SetAddressReuseInternal(true);
+    if (result != OK)
+      return result;
+  }
   socket_options_ |= SOCKET_OPTION_REUSE_ADDRESS;
+  return OK;
 }
 
-void UDPSocketLibevent::AllowBroadcast() {
+int UDPSocketLibevent::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 UDPSocketLibevent::AllowBroadcast() {

[jar (doing other things), 2014/11/04 22:51:09]

Why don't you use a single setter method, taking a boolean, rather than having
two nearly identical functions, with mostly duplicated code?

Same question for (Dis)AllowAddressReuse().

[hidehiko, 2014/11/05 12:48:30]

I just kept the current interface, as
UDPSocket{Libevent,Win}::Allow{Broadcast,AddressReuse} are used from several
places outside from net/... and ppapi/...

If unified version is better, I'm happy to work on unifying (Dis)Allow...
methods, but I'd like to do it in a separate CL as a refactoring, in order to
focus on fixing PPAPI in this CL.

+  DCHECK(CalledOnValidThread());
+  if (is_connected()) {
+    int result = SetBroadcastInternal(true);
+    if (result != OK)
+      return result;
+  }
   socket_options_ |= SOCKET_OPTION_BROADCAST;
+  return OK;
+}
+
+int UDPSocketLibevent::DisallowBroadcast() {
+  DCHECK(CalledOnValidThread());
+  if (is_connected()) {
+    int result = SetBroadcastInternal(false);
+    if (result != OK)
+      return result;
+  }
+  socket_options_ &= ~SOCKET_OPTION_BROADCAST;
+  return OK;
 }
 
 void UDPSocketLibevent::ReadWatcher::OnFileCanReadWithoutBlocking(int) {
@@ -516,29 +552,17 @@ int UDPSocketLibevent::InternalSendTo(IOBuffer* buf, int buf_len,
 }
 
 int UDPSocketLibevent::SetSocketOptions() {
-  int true_value = 1;
+  DCHECK(CalledOnValidThread());
   if (socket_options_ & SOCKET_OPTION_REUSE_ADDRESS) {
-    int rv = setsockopt(socket_, SOL_SOCKET, SO_REUSEADDR, &true_value,
-                        sizeof(true_value));
-    if (rv < 0)
-      return MapSystemError(errno);
+    int rv = SetAddressReuseInternal(true);
+    if (rv != OK)
+      return rv;
   }
   if (socket_options_ & SOCKET_OPTION_BROADCAST) {
-    int rv;
-#if defined(OS_MACOSX)
-    // SO_REUSEPORT on OSX permits multiple processes to each receive
-    // UDP multicast or broadcast datagrams destined for the bound
-    // port.
-    rv = setsockopt(socket_, SOL_SOCKET, SO_REUSEPORT, &true_value,
-                    sizeof(true_value));
-#else
-    rv = setsockopt(socket_, SOL_SOCKET, SO_BROADCAST, &true_value,
-                    sizeof(true_value));
-#endif  // defined(OS_MACOSX)
-    if (rv < 0)
-      return MapSystemError(errno);
+    int rv = SetBroadcastInternal(true);
+    if (rv != OK)
+      return rv;
   }
-
   if (!(socket_options_ & SOCKET_OPTION_MULTICAST_LOOP)) {
     int rv;
     if (addr_family_ == AF_INET) {
@@ -553,6 +577,17 @@ int UDPSocketLibevent::SetSocketOptions() {
     if (rv < 0)
       return MapSystemError(errno);
   }
+  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_ != IP_DEFAULT_MULTICAST_TTL) {
     int rv;
     if (addr_family_ == AF_INET) {
@@ -605,6 +640,52 @@ int UDPSocketLibevent::SetSocketOptions() {
   return OK;
 }
 
+int UDPSocketLibevent::SetAddressReuseInternal(bool value) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(is_connected());
+  int opt_value = value ? 1 : 0;
+  int rv = setsockopt(
+      socket_, SOL_SOCKET, SO_REUSEADDR, &opt_value, sizeof(opt_value));
+  return rv == 0 ? OK : MapSystemError(errno);
+}
+
+int UDPSocketLibevent::SetBroadcastInternal(bool value) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(is_connected());
+  int opt_value = value ? 1 : 0;
+#if defined(OS_MACOSX)
+  // SO_REUSEPORT on OSX permits multiple processes to each receive
+  // UDP multicast or broadcast datagrams destined for the bound
+  // port.
+  const int kOptName = SO_REUSEPORT;
+#else
+  const int kOptName = SO_BROADCAST;
+#endif
+  int rv = setsockopt(
+      socket_, SOL_SOCKET, kOptName, &opt_value, sizeof(opt_value));
+  return rv == 0 ? OK : MapSystemError(errno);
+}
+
+int UDPSocketLibevent::SetReceiveBufferSizeInternal(int32 size) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(is_connected());
+  int rv = setsockopt(socket_, SOL_SOCKET, SO_RCVBUF,
+                      reinterpret_cast<const char*>(&size), sizeof(size));
+  int last_error = errno;
+  DCHECK(!rv) << "Could not set socket receive buffer size: " << last_error;
+  return rv == 0 ? OK : MapSystemError(last_error);
+}
+
+int UDPSocketLibevent::SetSendBufferSizeInternal(int32 size) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(is_connected());
+  int rv = setsockopt(socket_, SOL_SOCKET, SO_SNDBUF,
+                      reinterpret_cast<const char*>(&size), sizeof(size));
+  int last_error = errno;
+  DCHECK(!rv) << "Could not set socket send buffer size: " << last_error;
+  return rv == 0 ? OK : MapSystemError(last_error);
+}
+
 int UDPSocketLibevent::DoBind(const IPEndPoint& address) {
   SockaddrStorage storage;
   if (!address.ToSockAddr(storage.addr, &storage.addr_len))