Remove timing limitation of SetOption invocation for PPAPI sockets.

content/browser/renderer_host/pepper/pepper_tcp_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_tcp_socket_message_filter.h"
 
 #include <cstring>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 45 matching lines @@
     : version_(version),
       external_plugin_(host->external_plugin()),
       render_process_id_(0),
       render_frame_id_(0),
       ppapi_host_(host->GetPpapiHost()),
       factory_(factory),
       instance_(instance),
       state_(TCPSocketState::INITIAL),
       end_of_file_reached_(false),
       bind_input_addr_(NetAddressPrivateImpl::kInvalidNetAddress),
+      socket_options_(SOCKET_OPTION_NODELAY),
       address_index_(0),
       socket_(new net::TCPSocket(NULL, net::NetLog::Source())),
       ssl_context_helper_(host->ssl_context_helper()),
       pending_accept_(false) {
   DCHECK(host);
   ++g_num_instances;
   if (!host->GetRenderFrameIDsForInstance(
           instance, &render_process_id_, &render_frame_id_)) {
     NOTREACHED();
   }
 }
 
 PepperTCPSocketMessageFilter::PepperTCPSocketMessageFilter(
     BrowserPpapiHostImpl* host,
     PP_Instance instance,
     TCPSocketVersion version,
     scoped_ptr<net::TCPSocket> socket)
     : version_(version),
       external_plugin_(host->external_plugin()),
       render_process_id_(0),
       render_frame_id_(0),
       ppapi_host_(host->GetPpapiHost()),
       factory_(NULL),
       instance_(instance),
       state_(TCPSocketState::CONNECTED),
       end_of_file_reached_(false),
       bind_input_addr_(NetAddressPrivateImpl::kInvalidNetAddress),
+      socket_options_(SOCKET_OPTION_NODELAY),
       address_index_(0),
       socket_(socket.Pass()),
       ssl_context_helper_(host->ssl_context_helper()),
       pending_accept_(false) {
   DCHECK(host);
   DCHECK_NE(version, ppapi::TCP_SOCKET_VERSION_1_0);
 
   ++g_num_instances;
   if (!host->GetRenderFrameIDsForInstance(
           instance, &render_process_id_, &render_frame_id_)) {
@@ skipping 346 matching lines @@
 }
 
 int32_t PepperTCPSocketMessageFilter::OnMsgSetOption(
     const ppapi::host::HostMessageContext* context,
     PP_TCPSocket_Option name,
     const ppapi::SocketOptionData& value) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   switch (name) {
     case PP_TCPSOCKET_OPTION_NO_DELAY: {
-      if (state_.state() != TCPSocketState::CONNECTED)
-        return PP_ERROR_FAILED;
-
       bool boolean_value = false;
       if (!value.GetBool(&boolean_value))
         return PP_ERROR_BADARGUMENT;
-      return socket_->SetNoDelay(boolean_value) ? PP_OK : PP_ERROR_FAILED;
+      if (state_.state() == TCPSocketState::CONNECTED)
+        return socket_->SetNoDelay(boolean_value) ? PP_OK : PP_ERROR_FAILED;
+
+      if (boolean_value) {
+        socket_options_ |= SOCKET_OPTION_NODELAY;
+      } else {
+        socket_options_ &= ~SOCKET_OPTION_NODELAY;
+      }
+      return PP_OK;
     }
     case PP_TCPSOCKET_OPTION_SEND_BUFFER_SIZE:
     case PP_TCPSOCKET_OPTION_RECV_BUFFER_SIZE: {
-      if (state_.state() != TCPSocketState::CONNECTED)
-        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_TCPSOCKET_OPTION_SEND_BUFFER_SIZE) {
         if (integer_value > TCPSocketResourceBase::kMaxSendBufferSize)
           return PP_ERROR_BADARGUMENT;
-        net_result = socket_->SetSendBufferSize(integer_value);
+        if (state_.state() == TCPSocketState::CONNECTED) {
+          return NetErrorToPepperError(
+              socket_->SetSendBufferSize(integer_value));
+        }
+        socket_options_ |= SOCKET_OPTION_SNDBUF_SIZE;
+        sndbuf_size_ = integer_value;
+        return PP_OK;
       } else {
         if (integer_value > TCPSocketResourceBase::kMaxReceiveBufferSize)
           return PP_ERROR_BADARGUMENT;
-        net_result = socket_->SetReceiveBufferSize(integer_value);
+        if (state_.state() == TCPSocketState::CONNECTED) {
+          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;
     }
   }
 }
 
 void PepperTCPSocketMessageFilter::DoBind(
     const ppapi::host::ReplyMessageContext& context,
@@ skipping 244 matching lines @@
             &local_addr) ||
         !NetAddressPrivateImpl::IPEndPointToNetAddress(
             ip_end_point_remote.address(),
             ip_end_point_remote.port(),
             &remote_addr)) {
       pp_result = PP_ERROR_ADDRESS_INVALID;
       break;
     }
 
     socket_->SetDefaultOptionsForClient();
+    // NODELAY is set by SetDefaultOptionsForClient().
+    if (!(socket_options_ & SOCKET_OPTION_NODELAY)) {
+      pp_result = NetErrorToPepperError(socket_->SetNoDelay(false));
+      if (pp_result != PP_OK)
+        break;
+    }
+    if (socket_options_ & SOCKET_OPTION_RCVBUF_SIZE) {
+      pp_result = NetErrorToPepperError(
+          socket_->SetReceiveBufferSize(rcvbuf_size_));
+      if (pp_result != PP_OK)
+        break;
+    }
+    if (socket_options_ & SOCKET_OPTION_SNDBUF_SIZE) {
+      pp_result = NetErrorToPepperError(
+          socket_->SetSendBufferSize(sndbuf_size_));
+      if (pp_result != PP_OK)
+        break;
+    }
+    socket_options_ = SOCKET_OPTION_NODELAY;
     SendConnectReply(context, PP_OK, local_addr, remote_addr);
     state_.CompletePendingTransition(true);
     return;
   } while (false);
 
   if (version_ == ppapi::TCP_SOCKET_VERSION_1_1_OR_ABOVE) {
     DCHECK_EQ(1u, address_list_.size());
 
     SendConnectError(context, pp_result);

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

What are the list of previously expectable error conditions from DoBind? Haven't
you expanded the list now?

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

IIUC, it is not managed, at least not-documented. Dave, Bill, any thoughts?
According to man, all errors possibly returned from setsockopt except
ENOPROTOOPT can already be returned from this function (with converting it to
the PP_ERROR). We do not expect ENOPROTOOPT, because level/optname of setsockopt
is fixed in current implementation.
Also, practically, at least on linux, setting options always succeeds, so it
seems ok. Though, I'm not an expert of other platforms. Do you know if it may
fail, like on Windows?

     state_.CompletePendingTransition(false);
   } else {
     // We have to recreate |socket_| because it doesn't allow a second connect
     // attempt. We won't lose any state such as bound address or set options,
     // because in the private or v1.0 API, connect must be the first operation.

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

This comment appears to be broken by this change.  

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

I think this is still correct comment? Could you kindly explain what is broken?

     socket_.reset(new net::TCPSocket(NULL, net::NetLog::Source()));
 
     if (address_index_ + 1 < address_list_.size()) {
       DCHECK_EQ(version_, ppapi::TCP_SOCKET_VERSION_PRIVATE);
       address_index_++;
       StartConnect(context);
     } else {
       SendConnectError(context, pp_result);
       // In order to maintain backward compatibility, allow further attempts to
       // connect the socket.
@@ skipping 217 matching lines @@
     const ppapi::host::ReplyMessageContext& context,
     int32_t pp_error) {
   SendAcceptReply(context,
                   pp_error,
                   0,
                   NetAddressPrivateImpl::kInvalidNetAddress,
                   NetAddressPrivateImpl::kInvalidNetAddress);
 }
 
 }  // namespace content