PPB_TCPSocket_Private/PPB_UDPSocket_Private are exposed to Browser

content/renderer/pepper_plugin_delegate_impl.cc

Index: content/renderer/pepper_plugin_delegate_impl.cc
diff --git a/content/renderer/pepper_plugin_delegate_impl.cc b/content/renderer/pepper_plugin_delegate_impl.cc
index 65645f1efd0b5f94813cbf985794532e5bd155b5..917a0ac0df723ba5afe6780b76dc1ca114b89a04 100644
--- a/content/renderer/pepper_plugin_delegate_impl.cc
+++ b/content/renderer/pepper_plugin_delegate_impl.cc
@@ -75,6 +75,8 @@
 #include "webkit/plugins/ppapi/ppb_broker_impl.h"
 #include "webkit/plugins/ppapi/ppb_flash_impl.h"
 #include "webkit/plugins/ppapi/ppb_flash_net_connector_impl.h"
+#include "webkit/plugins/ppapi/ppb_tcp_socket_private_impl.h"
+#include "webkit/plugins/ppapi/ppb_udp_socket_private_impl.h"
 #include "webkit/plugins/ppapi/resource_helper.h"
 #include "webkit/plugins/webplugininfo.h"
 
@@ -1679,6 +1681,180 @@ void PepperPluginDelegateImpl::OnConnectTcpACK(
   connector->CompleteConnectTcp(socket, local_addr, remote_addr);
 }
 
+uint32 PepperPluginDelegateImpl::TCPSocketCreate() {
+  if (!render_view_->CanUseSocketAPIs())
+    return 0;
+
+  uint32 socket_id = 0;
+  render_view_->Send(new PpapiHostMsg_PPBTCPSocket_Create(
+      render_view_->routing_id(), 0, &socket_id));
+  return socket_id;
+}
+
+void PepperPluginDelegateImpl::TCPSocketConnect(
+    webkit::ppapi::PPB_TCPSocket_Private_Impl* socket,
+    uint32 socket_id,
+    const std::string& host,
+    uint16_t port) {
+  tcp_sockets_.AddWithID(

[yzshen1, 2011/11/28 20:59:16]

Two issues here:
1) the following scenario is broken:
- create a socket object.
- without calling connect or connectwithnetaddress, destroy it.

In this case, a disconnect notification still needs to be sent to the browser to
release resources at the browser side. However, because the socket ID is not
added to |tcp_sockets_|, a DCHECK is hit.

2) it seems wrong to me to always hold a ref of PPB_TCP_Socket_Private_Impl. It
keeps the resource alive, unless Disconnect() is explicitly called. (Calling
Disconnect from destructor doesn't work anymore, because the object is alive as
long as Disconnect doesn't get called.)

UDP socket has the same problem, too.

[yzshen1, 2011/11/28 21:38:59]

I think one way to do it:
- don't hold a reference, just hold a raw pointer.
- when PPB_TCP_Socket_Private_Impl is destroyed, notify this object to remove
itself from the map.
- when receiving ACKs with a nonexistent socket ID, ignore them silently.

[ygorshenin, 2011/11/29 18:30:09]

Done.

[ygorshenin, 2011/11/29 18:30:09]

Done.

+      new scoped_refptr<webkit::ppapi::PPB_TCPSocket_Private_Impl>(socket),
+      socket_id);
+  render_view_->Send(
+      new PpapiHostMsg_PPBTCPSocket_Connect(socket_id, host, port));
+}
+
+void PepperPluginDelegateImpl::TCPSocketConnectWithNetAddress(
+      webkit::ppapi::PPB_TCPSocket_Private_Impl* socket,
+      uint32 socket_id,
+      const PP_NetAddress_Private& addr) {
+  tcp_sockets_.AddWithID(
+      new scoped_refptr<webkit::ppapi::PPB_TCPSocket_Private_Impl>(socket),
+      socket_id);
+  render_view_->Send(
+      new PpapiHostMsg_PPBTCPSocket_ConnectWithNetAddress(socket_id, addr));
+}
+
+void PepperPluginDelegateImpl::OnTCPSocketConnectACK(
+    uint32 socket_id,
+    bool succeeded,
+    const PP_NetAddress_Private& local_addr,
+    const PP_NetAddress_Private& remote_addr) {
+  DCHECK(tcp_sockets_.Lookup(socket_id));

[yzshen1, 2011/11/28 20:59:16]

For ALL ACK methods:

Please don't assume that socket_id is in the set.
It is possible that we receive ACKs after the socket is closed /disconnected
(e.g., the plugin may have explicitly requested to close/disconnect the socket).

[ygorshenin, 2011/11/29 18:30:09]

Done.

+  scoped_refptr<webkit::ppapi::PPB_TCPSocket_Private_Impl> socket =
+      *tcp_sockets_.Lookup(socket_id);

[yzshen1, 2011/11/28 20:59:16]

For ALL ACK methods:

Don't de-reference it directly. You should first check whether the return value
is NULL.

[ygorshenin, 2011/11/29 18:30:09]

Done.

+  if (socket.get())
+    socket->OnConnectCompleted(succeeded, local_addr, remote_addr);
+}
+
+void PepperPluginDelegateImpl::TCPSocketSSLHandshake(uint32 socket_id,
+                                     const std::string& server_name,

[yzshen1, 2011/11/28 20:59:16]

wrong indent.

[ygorshenin, 2011/11/29 18:30:09]

Done.

+                                     uint16_t server_port) {
+  DCHECK(tcp_sockets_.Lookup(socket_id));
+  render_view_->Send(new PpapiHostMsg_PPBTCPSocket_SSLHandshake(
+      socket_id, server_name, server_port));
+}
+
+void PepperPluginDelegateImpl::OnTCPSocketSSLHandshakeACK(uint32 socket_id,
+                                                          bool succeeded) {
+  DCHECK(tcp_sockets_.Lookup(socket_id));
+  scoped_refptr<webkit::ppapi::PPB_TCPSocket_Private_Impl> socket =
+      *tcp_sockets_.Lookup(socket_id);
+  if (socket.get())
+    socket->OnSSLHandshakeCompleted(succeeded);
+}
+
+void PepperPluginDelegateImpl::TCPSocketRead(uint32 socket_id,
+                                             int32_t bytes_to_read) {
+  DCHECK(tcp_sockets_.Lookup(socket_id));
+  render_view_->Send(
+      new PpapiHostMsg_PPBTCPSocket_Read(socket_id, bytes_to_read));
+}
+
+void PepperPluginDelegateImpl::OnTCPSocketReadACK(uint32 socket_id,
+                                                  bool succeeded,
+                                                  const std::string& data) {
+  DCHECK(tcp_sockets_.Lookup(socket_id));
+  scoped_refptr<webkit::ppapi::PPB_TCPSocket_Private_Impl> socket =
+      *tcp_sockets_.Lookup(socket_id);
+  if (socket.get())
+    socket->OnReadCompleted(succeeded, data);
+}
+
+void PepperPluginDelegateImpl::TCPSocketWrite(uint32 socket_id,
+                                              const std::string& buffer) {
+  DCHECK(tcp_sockets_.Lookup(socket_id));
+  render_view_->Send(new PpapiHostMsg_PPBTCPSocket_Write(socket_id, buffer));
+}
+
+void PepperPluginDelegateImpl::OnTCPSocketWriteACK(uint32 socket_id,
+                                                   bool succeeded,
+                                                   int32_t bytes_written) {
+  DCHECK(tcp_sockets_.Lookup(socket_id));
+  scoped_refptr<webkit::ppapi::PPB_TCPSocket_Private_Impl> socket =
+      *tcp_sockets_.Lookup(socket_id);
+  if (socket.get())
+    socket->OnWriteCompleted(succeeded, bytes_written);
+}
+
+void PepperPluginDelegateImpl::TCPSocketDisconnect(uint32 socket_id) {
+  DCHECK(tcp_sockets_.Lookup(socket_id));

[yzshen1, 2011/11/29 20:19:32]

Add comment about why we don't do DCHECK(tcp_sockets_.Lookup(socket_id)) here.

And do the same for UDPSocketClose().

[ygorshenin, 2011/11/30 11:50:54]

Done.

+  render_view_->Send(new PpapiHostMsg_PPBTCPSocket_Disconnect(socket_id));
+  tcp_sockets_.Remove(socket_id);
+}
+
+uint32 PepperPluginDelegateImpl::UDPSocketCreate() {
+  if (!render_view_->CanUseSocketAPIs())
+    return 0;
+
+  uint32 socket_id = 0;
+  render_view_->Send(new PpapiHostMsg_PPBUDPSocket_Create(
+      render_view_->routing_id(), 0, &socket_id));
+  return socket_id;
+}
+
+void PepperPluginDelegateImpl::UDPSocketBind(
+    webkit::ppapi::PPB_UDPSocket_Private_Impl* socket,
+    uint32 socket_id,
+    const PP_NetAddress_Private& addr) {
+  udp_sockets_.AddWithID(
+      new scoped_refptr<webkit::ppapi::PPB_UDPSocket_Private_Impl>(socket),
+      socket_id);
+  render_view_->Send(new PpapiHostMsg_PPBUDPSocket_Bind(socket_id, addr));
+}
+
+void PepperPluginDelegateImpl::OnUDPSocketBindACK(uint32 socket_id,
+                                                  bool succeeded) {
+  DCHECK(udp_sockets_.Lookup(socket_id));
+  scoped_refptr<webkit::ppapi::PPB_UDPSocket_Private_Impl> socket =
+      *udp_sockets_.Lookup(socket_id);
+  if (socket.get())
+    socket->OnBindCompleted(succeeded);
+}
+
+void PepperPluginDelegateImpl::UDPSocketRecvFrom(uint32 socket_id,
+                                                 int32_t num_bytes) {
+  DCHECK(udp_sockets_.Lookup(socket_id));
+  render_view_->Send(
+      new PpapiHostMsg_PPBUDPSocket_RecvFrom(socket_id, num_bytes));
+}
+
+void PepperPluginDelegateImpl::OnUDPSocketRecvFromACK(
+    uint32 socket_id,
+    bool succeeded,
+    const std::string& data,
+    const PP_NetAddress_Private& remote_addr) {
+  DCHECK(udp_sockets_.Lookup(socket_id));
+  scoped_refptr<webkit::ppapi::PPB_UDPSocket_Private_Impl> socket =
+      *udp_sockets_.Lookup(socket_id);
+  if (socket.get())
+    socket->OnRecvFromCompleted(succeeded, data, remote_addr);
+}
+
+void PepperPluginDelegateImpl::UDPSocketSendTo(
+    uint32 socket_id,
+    const std::string& buffer,
+    const PP_NetAddress_Private& net_addr) {
+  DCHECK(udp_sockets_.Lookup(socket_id));
+  render_view_->Send(
+      new PpapiHostMsg_PPBUDPSocket_SendTo(socket_id, buffer, net_addr));
+}
+
+void PepperPluginDelegateImpl::OnUDPSocketSendToACK(uint32 socket_id,
+                                                    bool succeeded,
+                                                    int32_t bytes_written) {
+  DCHECK(udp_sockets_.Lookup(socket_id));
+  scoped_refptr<webkit::ppapi::PPB_UDPSocket_Private_Impl> socket =
+      *udp_sockets_.Lookup(socket_id);
+  if (socket.get())
+    socket->OnSendToCompleted(succeeded, bytes_written);
+}
+
+void PepperPluginDelegateImpl::UDPSocketClose(uint32 socket_id) {
+  DCHECK(udp_sockets_.Lookup(socket_id));
+  render_view_->Send(new PpapiHostMsg_PPBUDPSocket_Close(socket_id));
+  udp_sockets_.Remove(socket_id);
+}
+
 int32_t PepperPluginDelegateImpl::ShowContextMenu(
     webkit::ppapi::PluginInstance* instance,
     webkit::ppapi::PPB_Flash_Menu_Impl* menu,