Remove timing limitation of SetOption invocation for PPAPI sockets.

content/browser/renderer_host/pepper/pepper_udp_socket_message_filter.cc

 // Copyright 2013 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 "content/browser/renderer_host/pepper/pepper_udp_socket_message_filter.h"
 
 #include <cstring>
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
@@ skipping 26 matching lines @@
 size_t g_num_instances = 0;
 
 }  // namespace
 
 namespace content {
 
 PepperUDPSocketMessageFilter::PepperUDPSocketMessageFilter(
     BrowserPpapiHostImpl* host,
     PP_Instance instance,
     bool private_api)
-    : allow_address_reuse_(false),
-      allow_broadcast_(false),
+    : socket_options_(0),
       closed_(false),
       remaining_recv_slots_(
           ppapi::proxy::UDPSocketResourceBase::kPluginReceiveBufferSlots),
       external_plugin_(host->external_plugin()),
       private_api_(private_api),
       render_process_id_(0),
       render_frame_id_(0) {
   ++g_num_instances;
   DCHECK(host);
 
@@ skipping 48 matching lines @@
 int32_t PepperUDPSocketMessageFilter::OnMsgSetOption(
     const ppapi::host::HostMessageContext* context,
     PP_UDPSocket_Option name,
     const ppapi::SocketOptionData& value) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (closed_)
     return PP_ERROR_FAILED;
 
   switch (name) {
-    case PP_UDPSOCKET_OPTION_ADDRESS_REUSE:
-    case PP_UDPSOCKET_OPTION_BROADCAST: {
-      if (socket_.get()) {
-        // They only take effect before the socket is bound.
-        return PP_ERROR_FAILED;
-      }
-
+    case PP_UDPSOCKET_OPTION_ADDRESS_REUSE: {
       bool boolean_value = false;
       if (!value.GetBool(&boolean_value))
         return PP_ERROR_BADARGUMENT;
 
-      if (name == PP_UDPSOCKET_OPTION_ADDRESS_REUSE)
-        allow_address_reuse_ = boolean_value;
-      else
-        allow_broadcast_ = boolean_value;
+      if (socket_.get()) {
+        return NetErrorToPepperError(
+            boolean_value ?
+            socket_->AllowAddressReuse() : socket_->DisallowAddressReuse());
+      }
+      if (boolean_value) {
+        socket_options_ |= SOCKET_OPTION_ADDRESS_REUSE;
+      } else {
+        socket_options_ &= ~SOCKET_OPTION_ADDRESS_REUSE;
+      }
+      return PP_OK;
+    }
+    case PP_UDPSOCKET_OPTION_BROADCAST: {
+      bool boolean_value = false;
+      if (!value.GetBool(&boolean_value))
+        return PP_ERROR_BADARGUMENT;
+
+      if (socket_.get()) {
+        return NetErrorToPepperError(
+            boolean_value ?
+            socket_->AllowBroadcast() : socket_->DisallowBroadcast());
+      }
+      if (boolean_value) {
+        socket_options_ |= SOCKET_OPTION_BROADCAST;
+      } else {
+        socket_options_ &= ~SOCKET_OPTION_BROADCAST;
+      }
       return PP_OK;
     }
     case PP_UDPSOCKET_OPTION_SEND_BUFFER_SIZE:
     case PP_UDPSOCKET_OPTION_RECV_BUFFER_SIZE: {
-      if (!socket_.get()) {
-        // They only take effect after the socket is bound.
-        return PP_ERROR_FAILED;
-      }
       int32_t integer_value = 0;
       if (!value.GetInt32(&integer_value) || integer_value <= 0)
         return PP_ERROR_BADARGUMENT;
 
-      int net_result = net::ERR_UNEXPECTED;
       if (name == PP_UDPSOCKET_OPTION_SEND_BUFFER_SIZE) {
         if (integer_value >
             ppapi::proxy::UDPSocketResourceBase::kMaxSendBufferSize) {
           return PP_ERROR_BADARGUMENT;
         }
-        net_result = socket_->SetSendBufferSize(integer_value);
+        if (socket_.get()) {
+          return NetErrorToPepperError(
+              socket_->SetSendBufferSize(integer_value));
+        }
+        socket_options_ |= SOCKET_OPTION_SNDBUF_SIZE;
+        sndbuf_size_ = integer_value;
+        return PP_OK;
       } else {
         if (integer_value >
             ppapi::proxy::UDPSocketResourceBase::kMaxReceiveBufferSize) {
           return PP_ERROR_BADARGUMENT;
         }
-        net_result = socket_->SetReceiveBufferSize(integer_value);
+        if (socket_.get()) {
+          return NetErrorToPepperError(
+              socket_->SetReceiveBufferSize(integer_value));
+        }
+        socket_options_ |= SOCKET_OPTION_RCVBUF_SIZE;
+        rcvbuf_size_ = integer_value;
+        return PP_OK;
       }
-      // TODO(wtc): Add error mapping code.
-      return (net_result == net::OK) ? PP_OK : PP_ERROR_FAILED;
     }
     default: {
       NOTREACHED();
       return PP_ERROR_BADARGUMENT;
     }
   }
 }
 
 int32_t PepperUDPSocketMessageFilter::OnMsgBind(
     const ppapi::host::HostMessageContext* context,
@@ skipping 86 matching lines @@
   scoped_ptr<net::UDPServerSocket> socket(
       new net::UDPServerSocket(NULL, net::NetLog::Source()));
 
   net::IPAddressNumber address;
   int port;
   if (!NetAddressPrivateImpl::NetAddressToIPEndPoint(addr, &address, &port)) {
     SendBindError(context, PP_ERROR_ADDRESS_INVALID);
     return;
   }
 
-  if (allow_address_reuse_)
-    socket->AllowAddressReuse();

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

For better or worse... it appears that the old code did not monitor return/error
values.

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

Yes... IMHO, we should handle the error, rather than just ignoring it... Any
thoughts?

-  if (allow_broadcast_)
-    socket->AllowBroadcast();
+  int32_t pp_result;
+  if (socket_options_ & SOCKET_OPTION_ADDRESS_REUSE) {
+    pp_result = NetErrorToPepperError(socket->AllowAddressReuse());
+    if (pp_result != PP_OK) {
+      SendBindError(context, pp_result);

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

It is valid to SendBindError an error which may previously never have been seen
by that function?

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

I think it is ok, as this function already returns an error on, like
GetLocalAddress failure, for example. Dave, Bill, any comments?

+      return;
+    }
+  }
+  if (socket_options_ & SOCKET_OPTION_BROADCAST) {
+    pp_result = NetErrorToPepperError(socket->AllowBroadcast());
+    if (pp_result != PP_OK) {
+      SendBindError(context, pp_result);
+      return;
+    }
+  }
+  if (socket_options_ & SOCKET_OPTION_RCVBUF_SIZE) {
+    pp_result = NetErrorToPepperError(
+        socket->SetReceiveBufferSize(rcvbuf_size_));
+    if (pp_result != PP_OK) {
+      SendBindError(context, pp_result);
+      return;
+    }
+  }
+  if (socket_options_ & SOCKET_OPTION_SNDBUF_SIZE) {
+    pp_result = NetErrorToPepperError(socket->SetSendBufferSize(sndbuf_size_));
+    if (pp_result != PP_OK) {
+      SendBindError(context, pp_result);
+      return;
+    }
+  }
 
-  int32_t pp_result =
+  pp_result =
       NetErrorToPepperError(socket->Listen(net::IPEndPoint(address, port)));
   if (pp_result != PP_OK) {
     SendBindError(context, pp_result);
     return;
   }
 
   net::IPEndPoint bound_address;
   pp_result = NetErrorToPepperError(socket->GetLocalAddress(&bound_address));
   if (pp_result != PP_OK) {
     SendBindError(context, pp_result);
     return;
   }
 
   PP_NetAddress_Private net_address = NetAddressPrivateImpl::kInvalidNetAddress;
   if (!NetAddressPrivateImpl::IPEndPointToNetAddress(
           bound_address.address(), bound_address.port(), &net_address)) {
     SendBindError(context, PP_ERROR_ADDRESS_INVALID);
     return;
   }
 
-  allow_address_reuse_ = false;
-  allow_broadcast_ = false;
+  socket_options_ = 0;
   socket_.swap(socket);
   SendBindReply(context, PP_OK, net_address);
 
   DoRecvFrom();
 }
 
 void PepperUDPSocketMessageFilter::DoRecvFrom() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!closed_);
   DCHECK(socket_.get());
@@ skipping 161 matching lines @@
                      NetAddressPrivateImpl::kInvalidNetAddress);
 }
 
 void PepperUDPSocketMessageFilter::SendSendToError(
     const ppapi::host::ReplyMessageContext& context,
     int32_t result) {
   SendSendToReply(context, result, 0);
 }
 
 }  // namespace content