Add GetBoundAddress to PPB_UDPSocket_Private

content/browser/renderer_host/pepper_udp_socket.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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 "content/browser/renderer_host/pepper_udp_socket.h"
 
 #include <string.h>
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "content/browser/renderer_host/pepper_message_filter.h"
@@ skipping 24 matching lines @@
   if (socket_.get())
     socket_->Close();
 }
 
 void PepperUDPSocket::Bind(const PP_NetAddress_Private& addr) {
   socket_.reset(new net::UDPServerSocket(NULL, net::NetLog::Source()));
 
   net::IPEndPoint address;
   if (!socket_.get() ||
       !NetAddressPrivateImpl::NetAddressToIPEndPoint(addr, &address)) {
-    SendBindACK(false);
+    SendBindACKError();
     return;
   }
 
   int result = socket_->Listen(address);
 

[viettrungluu, 2012/02/16 18:47:10]

Probably you want to reset the socket in the failure cases.

[mtilburg1, 2012/02/17 12:14:23]

Trung - I'm not sure this is necessary, since the bind has not succeeded, no
other operations will be allowed to occur.  The next time that Bind is called,
it will be reset.

-  SendBindACK(result == net::OK);
+  if (result == net::OK &&

[viettrungluu, 2012/02/16 18:47:10]

(See comment below. I guess you'll need to modify SendBindACK() to take a
PP_NetAddress_Private.)

Do something like:

scoped_ptr<WHATEVER socket_ points to> socket;
PP_NetAddress_Private bound_netaddr = NetAddressPrivateImpl::kInvalidNetAddress;

... replacing socket_ with socket ...

bool success =
    result != net::OK ||
    socket_->GetLockAddress(&bound_address_) != net::OK ||
    NetAddressPrivateImpl::IPEndPointToNetAddress(bound_address_,
bound_netaddr));
if (success)
  socket_.reset(socket.release());
SendBindACK(success, bound_netaddr);


Maybe you need to do something more in the case that IPEndPointToNetAddress()
failed (but Listen() didn't). Or, more likely, that really shouldn't happen, and
you should just CHECK/DCHECK that it succeeded (in which case you can put it in
SendBindACK() again, and SendBindACK() need only take a bool).

[mtilburg1, 2012/02/17 12:14:23]

Trung-

While this method will also work, I don't agree that this is any better than
what I have.  See my response to the comment below

+      socket_->GetLocalAddress(&bound_address_) != net::OK) {
+    SendBindACKError();
+    return;
+  }
+
+  OnBindCompleted(result);
 }
 
 void PepperUDPSocket::RecvFrom(int32_t num_bytes) {
   if (recvfrom_buffer_.get()) {
     SendRecvFromACKError();
     return;
   }
 
   recvfrom_buffer_ = new net::IOBuffer(num_bytes);
   int result = socket_->RecvFrom(
@@ skipping 35 matching lines @@
   manager_->Send(new PpapiMsg_PPBUDPSocket_RecvFromACK(
       routing_id_, plugin_dispatcher_id_, socket_id_, false, std::string(),
       addr));
 }
 
 void PepperUDPSocket::SendSendToACKError() {
   manager_->Send(new PpapiMsg_PPBUDPSocket_SendToACK(
       routing_id_, plugin_dispatcher_id_, socket_id_, false, 0));
 }
 
-void PepperUDPSocket::SendBindACK(bool result) {
+void PepperUDPSocket::SendBindACKError() {

[viettrungluu, 2012/02/16 18:47:10]

This is now needlessly redundant and convoluted. I think you should stick with
the old |SendBindACK()|, and do the additional |IPEndPointToNetAddress()| in the
body of |Bind()|.

[mtilburg1, 2012/02/17 12:14:23]

I don't agree... we need an Error case (as we have for RecvFrom) because in the
error case we need to make sure that we don't send a garbage addr back(as you
pointed out in the last review)

+  PP_NetAddress_Private addr = NetAddressPrivateImpl::kInvalidNetAddress;
   manager_->Send(new PpapiMsg_PPBUDPSocket_BindACK(
-      routing_id_, plugin_dispatcher_id_, socket_id_, result));
+      routing_id_, plugin_dispatcher_id_, socket_id_, false, addr));
+}
+
+void PepperUDPSocket::OnBindCompleted(int result) {
+  PP_NetAddress_Private addr = NetAddressPrivateImpl::kInvalidNetAddress;
+  if (result < 0 ||
+      !NetAddressPrivateImpl::IPEndPointToNetAddress(bound_address_,
+                                                     &addr)) {
+    SendBindACKError();
+  } else {
+    manager_->Send(new PpapiMsg_PPBUDPSocket_BindACK(
+        routing_id_, plugin_dispatcher_id_, socket_id_, true, addr));
+  }
 }
 
 void PepperUDPSocket::OnRecvFromCompleted(int result) {
   DCHECK(recvfrom_buffer_.get());
 
   // Convert IPEndPoint we get back from RecvFrom to a PP_NetAddress_Private,
   // to send back.
   PP_NetAddress_Private addr = NetAddressPrivateImpl::kInvalidNetAddress;
   if (result < 0 ||
       !NetAddressPrivateImpl::IPEndPointToNetAddress(recvfrom_address_,
                                                      &addr)) {
     SendRecvFromACKError();
   } else {
     manager_->Send(new PpapiMsg_PPBUDPSocket_RecvFromACK(
         routing_id_, plugin_dispatcher_id_, socket_id_, true,
         std::string(recvfrom_buffer_->data(), result), addr));
   }
 
   recvfrom_buffer_ = NULL;
 }
 
 void PepperUDPSocket::OnSendToCompleted(int result) {
   DCHECK(sendto_buffer_.get());
 
   manager_->Send(new PpapiMsg_PPBUDPSocket_SendToACK(
       routing_id_, plugin_dispatcher_id_, socket_id_, true, result));
 
   sendto_buffer_ = NULL;
 }