Pepper: Allow plugins to call PPB_UDP_Socket::SendTo multiple times.

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 13 matching lines @@
 #include "ppapi/host/host_message_context.h"
 #include "ppapi/host/ppapi_host.h"
 #include "ppapi/host/resource_host.h"
 #include "ppapi/proxy/ppapi_messages.h"
 #include "ppapi/proxy/udp_socket_resource_base.h"
 #include "ppapi/shared_impl/private/net_address_private_impl.h"
 #include "ppapi/shared_impl/socket_option_data.h"
 
 using ppapi::NetAddressPrivateImpl;
 using ppapi::host::NetErrorToPepperError;
+using ppapi::proxy::UDPSocketResourceBase;
 
 namespace {
 
 size_t g_num_instances = 0;
 
 }  // namespace
 
 namespace content {
 
+PepperUDPSocketMessageFilter::PendingSend::PendingSend(
+    const net::IPAddressNumber& address,
+    int port,
+    const scoped_refptr<net::IOBufferWithSize>& buffer,
+    const ppapi::host::ReplyMessageContext& context)
+    : address(address), port(port), buffer(buffer), context(context) {
+}
+
+PepperUDPSocketMessageFilter::PendingSend::~PendingSend() {
+}
+
 PepperUDPSocketMessageFilter::PepperUDPSocketMessageFilter(
     BrowserPpapiHostImpl* host,
     PP_Instance instance,
     bool private_api)
     : allow_address_reuse_(false),
       allow_broadcast_(false),
       closed_(false),
-      remaining_recv_slots_(
-          ppapi::proxy::UDPSocketResourceBase::kPluginReceiveBufferSlots),
+      remaining_recv_slots_(UDPSocketResourceBase::kPluginReceiveBufferSlots),
+      send_in_progress_(false),
       external_plugin_(host->external_plugin()),
       private_api_(private_api),
       render_process_id_(0),
       render_frame_id_(0) {
   ++g_num_instances;
   DCHECK(host);
 
   if (!host->GetRenderFrameIDsForInstance(
           instance, &render_process_id_, &render_frame_id_)) {
     NOTREACHED();
@@ skipping 74 matching lines @@
       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) {
+        if (integer_value > UDPSocketResourceBase::kMaxSendBufferSize) {
           return PP_ERROR_BADARGUMENT;
         }
         net_result = socket_->SetSendBufferSize(integer_value);
       } else {
-        if (integer_value >
-            ppapi::proxy::UDPSocketResourceBase::kMaxReceiveBufferSize) {
+        if (integer_value > UDPSocketResourceBase::kMaxReceiveBufferSize) {
           return PP_ERROR_BADARGUMENT;
         }
         net_result = socket_->SetReceiveBufferSize(integer_value);
       }
       // TODO(wtc): Add error mapping code.
       return (net_result == net::OK) ? PP_OK : PP_ERROR_FAILED;
     }
     default: {
       NOTREACHED();
       return PP_ERROR_BADARGUMENT;
@@ skipping 60 matching lines @@
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   Close();
   return PP_OK;
 }
 
 int32_t PepperUDPSocketMessageFilter::OnMsgRecvSlotAvailable(
     const ppapi::host::HostMessageContext* context) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (remaining_recv_slots_ <
-          ppapi::proxy::UDPSocketResourceBase::kPluginReceiveBufferSlots) {
+      UDPSocketResourceBase::kPluginReceiveBufferSlots) {
     remaining_recv_slots_++;
   }
 
   if (!recvfrom_buffer_.get() && !closed_ && socket_.get()) {
     DCHECK_EQ(1u, remaining_recv_slots_);
     DoRecvFrom();
   }
 
   return PP_OK;
 }
@@ skipping 52 matching lines @@
   DoRecvFrom();
 }
 
 void PepperUDPSocketMessageFilter::DoRecvFrom() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!closed_);
   DCHECK(socket_.get());
   DCHECK(!recvfrom_buffer_.get());
   DCHECK_GT(remaining_recv_slots_, 0u);
 
-  recvfrom_buffer_ = new net::IOBuffer(
-      ppapi::proxy::UDPSocketResourceBase::kMaxReadSize);
+  recvfrom_buffer_ = new net::IOBuffer(UDPSocketResourceBase::kMaxReadSize);
 
   // Use base::Unretained(this), so that the lifespan of this object doesn't
   // have to last until the callback is called.
   // It is safe to do so because |socket_| is owned by this object. If this
   // object gets destroyed (and so does |socket_|), the callback won't be
   // called.
   int net_result = socket_->RecvFrom(
       recvfrom_buffer_.get(),
-      ppapi::proxy::UDPSocketResourceBase::kMaxReadSize,
+      UDPSocketResourceBase::kMaxReadSize,
       &recvfrom_address_,
       base::Bind(&PepperUDPSocketMessageFilter::OnRecvFromCompleted,
                  base::Unretained(this)));
   if (net_result != net::ERR_IO_PENDING)
     OnRecvFromCompleted(net_result);
 }
 
 void PepperUDPSocketMessageFilter::DoSendTo(
     const ppapi::host::ReplyMessageContext& context,
     const std::string& data,
     const PP_NetAddress_Private& addr) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(socket_.get());
 
   if (closed_ || !socket_.get()) {
     SendSendToError(context, PP_ERROR_FAILED);
     return;
   }
 
-  if (sendto_buffer_.get()) {
-    SendSendToError(context, PP_ERROR_INPROGRESS);
-    return;
-  }
-
   size_t num_bytes = data.size();
   if (num_bytes == 0 ||
-      num_bytes > static_cast<size_t>(
-                      ppapi::proxy::UDPSocketResourceBase::kMaxWriteSize)) {
+      num_bytes > static_cast<size_t>(UDPSocketResourceBase::kMaxWriteSize)) {
     // Size of |data| is checked on the plugin side.
     NOTREACHED();
     SendSendToError(context, PP_ERROR_BADARGUMENT);
     return;
   }
 
-  sendto_buffer_ = new net::IOBufferWithSize(num_bytes);
-  memcpy(sendto_buffer_->data(), data.data(), num_bytes);
-
   net::IPAddressNumber address;
   int port;
   if (!NetAddressPrivateImpl::NetAddressToIPEndPoint(addr, &address, &port)) {
     SendSendToError(context, PP_ERROR_ADDRESS_INVALID);
     return;
   }
 
-  // Please see OnMsgRecvFrom() for the reason why we use base::Unretained(this)
+  scoped_refptr<net::IOBufferWithSize> buffer(
+      new net::IOBufferWithSize(num_bytes));
+  memcpy(buffer->data(), data.data(), num_bytes);
+
+  // Make sure a malicious plugin can't queue up an unlimited number of buffers.
+  if (pending_sends_.size() ==
+      UDPSocketResourceBase::kPluginSendBufferSlots) {
+    SendSendToError(context, PP_ERROR_FAILED);
+    return;
+  }
+
+  pending_sends_.push(PendingSend(address, port, buffer, context));
+  if (!send_in_progress_)
+    DoPendingSend();

[dmichael (off chromium), 2014/10/08 22:42:40]

I think you can get rid of the if and just always call
DoPendingSend/FlushPendingSends
...and have it check internally if there's anything left in the queue

[bbudge, 2014/10/09 00:18:26]

Done.

+}
+
+void PepperUDPSocketMessageFilter::DoPendingSend() {
+  PendingSend pending_send = pending_sends_.front();
+  pending_sends_.pop();

[dmichael (off chromium), 2014/10/08 22:42:40]

...but to do my suggestion, you'd want to not pop pending_sends_ until
OnSendToCompleted

[bbudge, 2014/10/09 00:18:26]

Done.

+  // See OnMsgRecvFrom() for the reason why we use base::Unretained(this)
   // when calling |socket_| methods.
   int net_result = socket_->SendTo(
-      sendto_buffer_.get(),
-      sendto_buffer_->size(),
-      net::IPEndPoint(address, port),
+      pending_send.buffer.get(),
+      pending_send.buffer->size(),
+      net::IPEndPoint(pending_send.address, pending_send.port),
       base::Bind(&PepperUDPSocketMessageFilter::OnSendToCompleted,
                  base::Unretained(this),
-                 context));
+                 pending_send.buffer,
+                 pending_send.context));
   if (net_result != net::ERR_IO_PENDING)
-    OnSendToCompleted(context, net_result);
+    OnSendToCompleted(pending_send.buffer, pending_send.context, net_result);
+  else
+    send_in_progress_ = true;
 }
 
 void PepperUDPSocketMessageFilter::Close() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (socket_.get() && !closed_)
     socket_->Close();
   closed_ = true;
 }
 
 void PepperUDPSocketMessageFilter::OnRecvFromCompleted(int net_result) {
@@ skipping 21 matching lines @@
   recvfrom_buffer_ = NULL;
 
   DCHECK_GT(remaining_recv_slots_, 0u);
   remaining_recv_slots_--;
 
   if (remaining_recv_slots_ > 0 && !closed_ && socket_.get())
     DoRecvFrom();
 }
 
 void PepperUDPSocketMessageFilter::OnSendToCompleted(
+    scoped_refptr<net::IOBufferWithSize> buffer,

[dmichael (off chromium), 2014/10/08 22:42:40]

I think you won't need this anymore because the head of the queue is keeping the
buffer valid...

[bbudge, 2014/10/09 00:18:26]

Done.

     const ppapi::host::ReplyMessageContext& context,
     int net_result) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(sendto_buffer_.get());
 
   int32_t pp_result = NetErrorToPepperError(net_result);
   if (pp_result < 0)
     SendSendToError(context, pp_result);
   else
     SendSendToReply(context, PP_OK, pp_result);
-  sendto_buffer_ = NULL;
+
+  send_in_progress_ = false;
+
+  if (!pending_sends_.empty())
+    DoPendingSend();

[dmichael (off chromium), 2014/10/08 22:42:40]

And you'll want to pop() right before calling this to put the next thing on the
front

[bbudge, 2014/10/09 00:18:26]

Done. Very nice simplification. Thanks!

 }
 
 void PepperUDPSocketMessageFilter::SendBindReply(
     const ppapi::host::ReplyMessageContext& context,
     int32_t result,
     const PP_NetAddress_Private& addr) {
   ppapi::host::ReplyMessageContext reply_context(context);
   reply_context.params.set_result(result);
   SendReply(reply_context, PpapiPluginMsg_UDPSocket_BindReply(addr));
 }
@@ skipping 28 matching lines @@
                      NetAddressPrivateImpl::kInvalidNetAddress);
 }
 
 void PepperUDPSocketMessageFilter::SendSendToError(
     const ppapi::host::ReplyMessageContext& context,
     int32_t result) {
   SendSendToReply(context, result, 0);
 }
 
 }  // namespace content