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

content/browser/renderer_host/pepper/pepper_udp_socket_message_filter.cc

Index: content/browser/renderer_host/pepper/pepper_udp_socket_message_filter.cc
diff --git a/content/browser/renderer_host/pepper/pepper_udp_socket_message_filter.cc b/content/browser/renderer_host/pepper/pepper_udp_socket_message_filter.cc
index 448599d23fb749305b612f60d0cd73e7bd093698..72b4b91319fff5424ffc43c8bf3375cdad53c770 100644
--- a/content/browser/renderer_host/pepper/pepper_udp_socket_message_filter.cc
+++ b/content/browser/renderer_host/pepper/pepper_udp_socket_message_filter.cc
@@ -31,6 +31,7 @@
 
 using ppapi::NetAddressPrivateImpl;
 using ppapi::host::NetErrorToPepperError;
+using ppapi::proxy::UDPSocketResourceBase;
 
 namespace {
 
@@ -40,6 +41,17 @@ size_t g_num_instances = 0;
 
 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,
@@ -47,8 +59,8 @@ PepperUDPSocketMessageFilter::PepperUDPSocketMessageFilter(
     : 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),
@@ -143,14 +155,12 @@ int32_t PepperUDPSocketMessageFilter::OnMsgSetOption(
 
       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);
@@ -231,7 +241,7 @@ int32_t PepperUDPSocketMessageFilter::OnMsgRecvSlotAvailable(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (remaining_recv_slots_ <
-          ppapi::proxy::UDPSocketResourceBase::kPluginReceiveBufferSlots) {
+      UDPSocketResourceBase::kPluginReceiveBufferSlots) {
     remaining_recv_slots_++;
   }
 
@@ -304,8 +314,7 @@ void PepperUDPSocketMessageFilter::DoRecvFrom() {
   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.
@@ -314,7 +323,7 @@ void PepperUDPSocketMessageFilter::DoRecvFrom() {
   // called.
   int net_result = socket_->RecvFrom(
       recvfrom_buffer_.get(),
-      ppapi::proxy::UDPSocketResourceBase::kMaxReadSize,
+      UDPSocketResourceBase::kMaxReadSize,
       &recvfrom_address_,
       base::Bind(&PepperUDPSocketMessageFilter::OnRecvFromCompleted,
                  base::Unretained(this)));
@@ -334,24 +343,15 @@ void PepperUDPSocketMessageFilter::DoSendTo(
     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)) {
@@ -359,17 +359,39 @@ void PepperUDPSocketMessageFilter::DoSendTo(
     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() {
@@ -411,17 +433,21 @@ void PepperUDPSocketMessageFilter::OnRecvFromCompleted(int net_result) {
 }
 
 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(