Remove timing limitation of SetOption invocation for PPAPI sockets.

net/udp/udp_socket_libevent.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_libevent.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <netdb.h>
 #include <net/if.h>
@@ skipping 299 matching lines @@
   if (rv < 0) {
     Close();
     return rv;
   }
   local_address_.reset();
   return rv;
 }
 
 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) {
   if (!socket_->read_callback_.is_null())
     socket_->DidCompleteRead();
 }
 
 void UDPSocketLibevent::WriteWatcher::OnFileCanWriteWithoutBlocking(int) {
   if (!socket_->write_callback_.is_null())
     socket_->DidCompleteWrite();
@@ skipping 151 matching lines @@
                             addr,
                             storage.addr_len));
   if (result < 0)
     result = MapSystemError(errno);
   if (result != ERR_IO_PENDING)
     LogWrite(result, buf->data(), address);
   return result;
 }
 
 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) {
       u_char loop = 0;
       rv = setsockopt(socket_, IPPROTO_IP, IP_MULTICAST_LOOP,
                       &loop, sizeof(loop));
     } else {
       u_int loop = 0;
       rv = setsockopt(socket_, IPPROTO_IPV6, IPV6_MULTICAST_LOOP,
                       &loop, sizeof(loop));
     }
     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) {
       u_char ttl = multicast_time_to_live_;
       rv = setsockopt(socket_, IPPROTO_IP, IP_MULTICAST_TTL,
                       &ttl, sizeof(ttl));
     } else {
       // Signed integer. -1 to use route default.
       int ttl = multicast_time_to_live_;
       rv = setsockopt(socket_, IPPROTO_IPV6, IPV6_MULTICAST_HOPS,
@@ skipping 32 matching lines @@
         break;
       }
       default:
         NOTREACHED() << "Invalid address family";
         return ERR_ADDRESS_INVALID;
     }
   }
   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))
     return ERR_ADDRESS_INVALID;
   int rv = bind(socket_, storage.addr, storage.addr_len);
   if (rv == 0)
     return OK;
   int last_error = errno;
   UMA_HISTOGRAM_SPARSE_SLOWLY("Net.UdpSocketBindErrorFromPosix", last_error);
 #if defined(OS_CHROMEOS)
@@ skipping 149 matching lines @@
     return MapSystemError(errno);
 
   return OK;
 }
 
 void UDPSocketLibevent::DetachFromThread() {
   base::NonThreadSafe::DetachFromThread();
 }
 
 }  // namespace net