Remove 'Flash' from TCP/UDP Pepper interfaces.

Remove 'Flash' from TCP/UDP Pepper interfaces names. This CL preserves old idl and C/C++ headers for backward compatibility. Also TCP interface should be returned by old name.

BUG=none
TEST=build

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110265

[viettrungluu, 2011-11-09 18:50:19]

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/flash_tcp_soc...
File ppapi/cpp/private/flash_tcp_socket.h (left):

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/flash_tcp_soc...
ppapi/cpp/private/flash_tcp_socket.h:1: // Copyright (c) 2011 The Chromium
Authors. All rights reserved.
Please keep the existing Flash TCP stuff working.

[viettrungluu, 2011-11-09 18:53:20]

Also, it's probably better to split up this commit. I'll do the net address
changes. (It's new and Flash doesn't actually use it yet.)

[Dmitry Polukhin, 2011-11-09 19:00:31]

As for splitting this commit. I'm not sure that it is good idea because it will
add more iteration and PP_NetAddress is used everywhere.

Do we have trybots with Flash compilation?

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/flash_tcp_soc...
File ppapi/cpp/private/flash_tcp_socket.h (left):

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/flash_tcp_soc...
ppapi/cpp/private/flash_tcp_socket.h:1: // Copyright (c) 2011 The Chromium
Authors. All rights reserved.
On 2011/11/09 18:50:19, viettrungluu wrote:
> Please keep the existing Flash TCP stuff working.

It would be good to be able to test what is actually used.

[yzshen1, 2011-11-09 23:53:55]

One more thing: please make your change backward compatible.

Thanks!

http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_tcp_socke...
File ppapi/api/private/ppb_tcp_socket.idl (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_tcp_socke...
ppapi/api/private/ppb_tcp_socket.idl:18: interface PPB_TCPSocket {
Please use PPB_TCPSocket_Private.

http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_udp_socke...
File ppapi/api/private/ppb_udp_socket.idl (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_udp_socke...
ppapi/api/private/ppb_udp_socket.idl:14: interface PPB_UDPSocket {
Please use PPB_UDPSocket_Private.

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.cc
File ppapi/cpp/private/tcp_socket.cc (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.cc...
ppapi/cpp/private/tcp_socket.cc:5: // TODO(yzshen): See the comment in
corresponding .h file.
If you remove the TODO in the .h file, please remove this one, too.

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.h
File ppapi/cpp/private/tcp_socket.h (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.h#...
ppapi/cpp/private/tcp_socket.h:7: // (or even compiled with) Chromium.
Now we can compile it (as well as udp_socket) with Chromium, I think.

http://codereview.chromium.org/8506016/diff/1/ppapi/ppapi_shared.gypi
File ppapi/ppapi_shared.gypi (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/ppapi_shared.gypi#newcode134
ppapi/ppapi_shared.gypi:134: 'thunk/ppb_tcp_socket_api.h',
Sort, please.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc
File ppapi/proxy/interface_list.cc (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc#n...
ppapi/proxy/interface_list.cc:51: #include
"ppapi/c/trusted/ppb_broker_trusted.h"
Sort the list, please.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc#n...
ppapi/proxy/interface_list.cc:71: #include "ppapi/proxy/ppb_tcp_socket_proxy.h"
Sort the list, please.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc#n...
ppapi/proxy/interface_list.cc:164: AddProxy(API_ID_PPB_TCPSOCKET,
&ProxyFactory<PPB_TCPSocket_Proxy>);
I think they are not "special". Please use the normal way above.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
File ppapi/proxy/resource_creation_proxy.cc (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
ppapi/proxy/resource_creation_proxy.cc:22: #include
"ppapi/proxy/ppb_tcp_socket_proxy.h"
Sort, please.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
ppapi/proxy/resource_creation_proxy.cc:154: 
Please also sort method definitions.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
File ppapi/proxy/resource_creation_proxy.h (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
ppapi/proxy/resource_creation_proxy.h:73: virtual PP_Resource
CreateTCPSocket(PP_Instance instance) OVERRIDE;
Sort, please.

http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/api_id.h
File ppapi/shared_impl/api_id.h (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/api_id.h#newc...
ppapi/shared_impl/api_id.h:31: API_ID_PPB_TCPSOCKET,
Sort, please.

http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/resource.h
File ppapi/shared_impl/resource.h (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/resource.h#ne...
ppapi/shared_impl/resource.h:34: F(PPB_TCPSocket_API) \
Sort, please.

http://codereview.chromium.org/8506016/diff/1/ppapi/thunk/thunk.h
File ppapi/thunk/thunk.h (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/thunk/thunk.h#newcode39
ppapi/thunk/thunk.h:39: struct PPB_TCPSocket;
Sort, please.

http://codereview.chromium.org/8506016/diff/1/ppapi/thunk/thunk.h#newcode67
ppapi/thunk/thunk.h:67: PPAPI_THUNK_EXPORT const PPB_TCPSocket*
GetPPB_TCPSocket_Thunk();
Sort, please.

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/plugin_mod...
File webkit/plugins/ppapi/plugin_module.cc (right):

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/plugin_mod...
webkit/plugins/ppapi/plugin_module.cc:289: if (strcmp(name,
PPB_TCPSOCKET_INTERFACE) == 0)
Sort, please.

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
File webkit/plugins/ppapi/resource_creation_impl.cc (right):

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
webkit/plugins/ppapi/resource_creation_impl.cc:142: PP_Resource
ResourceCreationImpl::CreateTCPSocket(
Sort, please.

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
File webkit/plugins/ppapi/resource_creation_impl.h (right):

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
webkit/plugins/ppapi/resource_creation_impl.h:60: virtual PP_Resource
CreateTCPSocket(PP_Instance instance) OVERRIDE;
Sort, please.

[Dmitry Polukhin, 2011-11-10 15:10:10]

I tried to make my change backward compatible. But unfortunately I'm not sure
that I'm full understand what is and what is not part of ABI that I need to
preserve. Old and new interfaces are identical so it looks like the only thing
that I need to do is to return new interfaces if caller asks for old one. I did
it in two places plugin_module.cc and interface_list.cc. Also I preserved old C
and C++ headers. Do you know anything else that needs to be done for backward
compatibility (both binary and headers)?

http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_tcp_socke...
File ppapi/api/private/ppb_tcp_socket.idl (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_tcp_socke...
ppapi/api/private/ppb_tcp_socket.idl:18: interface PPB_TCPSocket {
On 2011/11/09 23:53:55, yzshen1 wrote:
> Please use PPB_TCPSocket_Private.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_udp_socke...
File ppapi/api/private/ppb_udp_socket.idl (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_udp_socke...
ppapi/api/private/ppb_udp_socket.idl:14: interface PPB_UDPSocket {
On 2011/11/09 23:53:55, yzshen1 wrote:
> Please use PPB_UDPSocket_Private.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/flash_tcp_soc...
File ppapi/cpp/private/flash_tcp_socket.h (left):

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/flash_tcp_soc...
ppapi/cpp/private/flash_tcp_socket.h:1: // Copyright (c) 2011 The Chromium
Authors. All rights reserved.
On 2011/11/09 18:50:19, viettrungluu wrote:
> Please keep the existing Flash TCP stuff working.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.cc
File ppapi/cpp/private/tcp_socket.cc (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.cc...
ppapi/cpp/private/tcp_socket.cc:5: // TODO(yzshen): See the comment in
corresponding .h file.
On 2011/11/09 23:53:55, yzshen1 wrote:
> If you remove the TODO in the .h file, please remove this one, too.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.h
File ppapi/cpp/private/tcp_socket.h (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.h#...
ppapi/cpp/private/tcp_socket.h:7: // (or even compiled with) Chromium.
On 2011/11/09 23:53:55, yzshen1 wrote:
> Now we can compile it (as well as udp_socket) with Chromium, I think.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/ppapi_shared.gypi
File ppapi/ppapi_shared.gypi (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/ppapi_shared.gypi#newcode134
ppapi/ppapi_shared.gypi:134: 'thunk/ppb_tcp_socket_api.h',
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort, please.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc
File ppapi/proxy/interface_list.cc (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc#n...
ppapi/proxy/interface_list.cc:51: #include
"ppapi/c/trusted/ppb_broker_trusted.h"
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort the list, please.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc#n...
ppapi/proxy/interface_list.cc:71: #include "ppapi/proxy/ppb_tcp_socket_proxy.h"
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort the list, please.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc#n...
ppapi/proxy/interface_list.cc:164: AddProxy(API_ID_PPB_TCPSOCKET,
&ProxyFactory<PPB_TCPSocket_Proxy>);
On 2011/11/09 23:53:55, yzshen1 wrote:
> I think they are not "special". Please use the normal way above.

They are special in name, they doesn't have _Private suffix. I think there is no
sens to add private everywhere it will just make names longer without real
benefits.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
File ppapi/proxy/resource_creation_proxy.cc (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
ppapi/proxy/resource_creation_proxy.cc:22: #include
"ppapi/proxy/ppb_tcp_socket_proxy.h"
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort, please.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
ppapi/proxy/resource_creation_proxy.cc:154: 
On 2011/11/09 23:53:55, yzshen1 wrote:
> Please also sort method definitions.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
File ppapi/proxy/resource_creation_proxy.h (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
ppapi/proxy/resource_creation_proxy.h:73: virtual PP_Resource
CreateTCPSocket(PP_Instance instance) OVERRIDE;
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort, please.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/api_id.h
File ppapi/shared_impl/api_id.h (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/api_id.h#newc...
ppapi/shared_impl/api_id.h:31: API_ID_PPB_TCPSOCKET,
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort, please.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/resource.h
File ppapi/shared_impl/resource.h (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/resource.h#ne...
ppapi/shared_impl/resource.h:34: F(PPB_TCPSocket_API) \
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort, please.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/thunk/thunk.h
File ppapi/thunk/thunk.h (right):

http://codereview.chromium.org/8506016/diff/1/ppapi/thunk/thunk.h#newcode39
ppapi/thunk/thunk.h:39: struct PPB_TCPSocket;
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort, please.

Done.

http://codereview.chromium.org/8506016/diff/1/ppapi/thunk/thunk.h#newcode67
ppapi/thunk/thunk.h:67: PPAPI_THUNK_EXPORT const PPB_TCPSocket*
GetPPB_TCPSocket_Thunk();
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort, please.

Done.

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/plugin_mod...
File webkit/plugins/ppapi/plugin_module.cc (right):

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/plugin_mod...
webkit/plugins/ppapi/plugin_module.cc:289: if (strcmp(name,
PPB_TCPSOCKET_INTERFACE) == 0)
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort, please.

Done.

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
File webkit/plugins/ppapi/resource_creation_impl.cc (right):

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
webkit/plugins/ppapi/resource_creation_impl.cc:142: PP_Resource
ResourceCreationImpl::CreateTCPSocket(
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort, please.

Done.

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
File webkit/plugins/ppapi/resource_creation_impl.h (right):

http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
webkit/plugins/ppapi/resource_creation_impl.h:60: virtual PP_Resource
CreateTCPSocket(PP_Instance instance) OVERRIDE;
On 2011/11/09 23:53:55, yzshen1 wrote:
> Sort, please.

Done.

[viettrungluu, 2011-11-11 01:13:51]

At a glance, it looks okay, but please wait until I write some tests (which I'm
busy doing). (I also need to verify this a bit more carefully.)

On 2011/11/10 15:10:10, Dmitry Polukhin wrote:
> I tried to make my change backward compatible. But unfortunately I'm not sure
> that I'm full understand what is and what is not part of ABI that I need to
> preserve. Old and new interfaces are identical so it looks like the only thing
> that I need to do is to return new interfaces if caller asks for old one. I
did
> it in two places plugin_module.cc and interface_list.cc. Also I preserved old
C
> and C++ headers. Do you know anything else that needs to be done for backward
> compatibility (both binary and headers)?
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_tcp_socke...
> File ppapi/api/private/ppb_tcp_socket.idl (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_tcp_socke...
> ppapi/api/private/ppb_tcp_socket.idl:18: interface PPB_TCPSocket {
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Please use PPB_TCPSocket_Private.
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_udp_socke...
> File ppapi/api/private/ppb_udp_socket.idl (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/api/private/ppb_udp_socke...
> ppapi/api/private/ppb_udp_socket.idl:14: interface PPB_UDPSocket {
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Please use PPB_UDPSocket_Private.
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/flash_tcp_soc...
> File ppapi/cpp/private/flash_tcp_socket.h (left):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/flash_tcp_soc...
> ppapi/cpp/private/flash_tcp_socket.h:1: // Copyright (c) 2011 The Chromium
> Authors. All rights reserved.
> On 2011/11/09 18:50:19, viettrungluu wrote:
> > Please keep the existing Flash TCP stuff working.
> 
> Done.
> 
> http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.cc
> File ppapi/cpp/private/tcp_socket.cc (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.cc...
> ppapi/cpp/private/tcp_socket.cc:5: // TODO(yzshen): See the comment in
> corresponding .h file.
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > If you remove the TODO in the .h file, please remove this one, too.
> 
> Done.
> 
> http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.h
> File ppapi/cpp/private/tcp_socket.h (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/cpp/private/tcp_socket.h#...
> ppapi/cpp/private/tcp_socket.h:7: // (or even compiled with) Chromium.
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Now we can compile it (as well as udp_socket) with Chromium, I think.
> 
> Done.
> 
> http://codereview.chromium.org/8506016/diff/1/ppapi/ppapi_shared.gypi
> File ppapi/ppapi_shared.gypi (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/ppapi_shared.gypi#newcode134
> ppapi/ppapi_shared.gypi:134: 'thunk/ppb_tcp_socket_api.h',
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort, please.
> 
> Done.
> 
> http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc
> File ppapi/proxy/interface_list.cc (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc#n...
> ppapi/proxy/interface_list.cc:51: #include
> "ppapi/c/trusted/ppb_broker_trusted.h"
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort the list, please.
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc#n...
> ppapi/proxy/interface_list.cc:71: #include
"ppapi/proxy/ppb_tcp_socket_proxy.h"
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort the list, please.
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/interface_list.cc#n...
> ppapi/proxy/interface_list.cc:164: AddProxy(API_ID_PPB_TCPSOCKET,
> &ProxyFactory<PPB_TCPSocket_Proxy>);
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > I think they are not "special". Please use the normal way above.
> 
> They are special in name, they doesn't have _Private suffix. I think there is
no
> sens to add private everywhere it will just make names longer without real
> benefits.
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
> File ppapi/proxy/resource_creation_proxy.cc (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
> ppapi/proxy/resource_creation_proxy.cc:22: #include
> "ppapi/proxy/ppb_tcp_socket_proxy.h"
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort, please.
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
> ppapi/proxy/resource_creation_proxy.cc:154: 
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Please also sort method definitions.
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
> File ppapi/proxy/resource_creation_proxy.h (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/proxy/resource_creation_p...
> ppapi/proxy/resource_creation_proxy.h:73: virtual PP_Resource
> CreateTCPSocket(PP_Instance instance) OVERRIDE;
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort, please.
> 
> Done.
> 
> http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/api_id.h
> File ppapi/shared_impl/api_id.h (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/api_id.h#newc...
> ppapi/shared_impl/api_id.h:31: API_ID_PPB_TCPSOCKET,
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort, please.
> 
> Done.
> 
> http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/resource.h
> File ppapi/shared_impl/resource.h (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/ppapi/shared_impl/resource.h#ne...
> ppapi/shared_impl/resource.h:34: F(PPB_TCPSocket_API) \
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort, please.
> 
> Done.
> 
> http://codereview.chromium.org/8506016/diff/1/ppapi/thunk/thunk.h
> File ppapi/thunk/thunk.h (right):
> 
> http://codereview.chromium.org/8506016/diff/1/ppapi/thunk/thunk.h#newcode39
> ppapi/thunk/thunk.h:39: struct PPB_TCPSocket;
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort, please.
> 
> Done.
> 
> http://codereview.chromium.org/8506016/diff/1/ppapi/thunk/thunk.h#newcode67
> ppapi/thunk/thunk.h:67: PPAPI_THUNK_EXPORT const PPB_TCPSocket*
> GetPPB_TCPSocket_Thunk();
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort, please.
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/plugin_mod...
> File webkit/plugins/ppapi/plugin_module.cc (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/plugin_mod...
> webkit/plugins/ppapi/plugin_module.cc:289: if (strcmp(name,
> PPB_TCPSOCKET_INTERFACE) == 0)
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort, please.
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
> File webkit/plugins/ppapi/resource_creation_impl.cc (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
> webkit/plugins/ppapi/resource_creation_impl.cc:142: PP_Resource
> ResourceCreationImpl::CreateTCPSocket(
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort, please.
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
> File webkit/plugins/ppapi/resource_creation_impl.h (right):
> 
>
http://codereview.chromium.org/8506016/diff/1/webkit/plugins/ppapi/resource_c...
> webkit/plugins/ppapi/resource_creation_impl.h:60: virtual PP_Resource
> CreateTCPSocket(PP_Instance instance) OVERRIDE;
> On 2011/11/09 23:53:55, yzshen1 wrote:
> > Sort, please.
> 
> Done.

[viettrungluu, 2011-11-11 01:15:10]

Also, Yuzhu, could you please patch this in and test Flash on it?

(Dmitry: sorry, no trybots or such ... the restricted Flash room is on a
separate network and such too.)

[yzshen1, 2011-11-11 19:05:19]

On 2011/11/11 01:15:10, viettrungluu wrote:
> Also, Yuzhu, could you please patch this in and test Flash on it?
> 
> (Dmitry: sorry, no trybots or such ... the restricted Flash room is on a
> separate network and such too.)
sync and compiling...

[yzshen1, 2011-11-11 21:04:28]

Patched and ran with Flash, it worked.
Thanks!

http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/tcp_socke...
File ppapi/cpp/private/tcp_socket_private.h (right):

http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/tcp_socke...
ppapi/cpp/private/tcp_socket_private.h:17: class TCPSocket : public Resource {
It should have "Private" in the name.
(So that it is consistent with other dev/private interfaces.)

http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/udp_socke...
File ppapi/cpp/private/udp_socket_private.h (right):

http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/udp_socke...
ppapi/cpp/private/udp_socket_private.h:17: class UDPSocket : public Resource {
It should have "Private" in the name.
(So that it is consistent with other dev/private interfaces.)

http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/interface_list.cc
File ppapi/proxy/interface_list.cc (right):

http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/interface_list....
ppapi/proxy/interface_list.cc:165: AddProxy(API_ID_PPB_TCPSOCKET,
&ProxyFactory<PPB_TCPSocket_Proxy>);
> They are special in name, they doesn't have _Private 
> suffix. I think there is no
> sens to add private everywhere it will just make names 
> longer without real benefits.

If the only concern is the _PRIVATE suffix being too long, I still think that we
should add it and use the 'normal' way. This ID is not used everywhere, and it
seems not too long to me.

Please note that this list is used for really special interfaces (see the
comment above).

http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/resource_creati...
File ppapi/proxy/resource_creation_proxy.cc (right):

http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/resource_creati...
ppapi/proxy/resource_creation_proxy.cc:26: #include
"ppapi/proxy/ppb_tcp_socket_proxy.h"
Sort it, please.

http://codereview.chromium.org/8506016/diff/17001/ppapi/shared_impl/api_id.h
File ppapi/shared_impl/api_id.h (right):

http://codereview.chromium.org/8506016/diff/17001/ppapi/shared_impl/api_id.h#...
ppapi/shared_impl/api_id.h:39: API_ID_PPB_TCPSOCKET,
As I mentioned in another file, please add _PRIVATE for them.

http://codereview.chromium.org/8506016/diff/17001/webkit/plugins/ppapi/plugin...
File webkit/plugins/ppapi/plugin_module.cc (right):

http://codereview.chromium.org/8506016/diff/17001/webkit/plugins/ppapi/plugin...
webkit/plugins/ppapi/plugin_module.cc:304: if (strcmp(name,
PPB_TCPSOCKET_PRIVATE_INTERFACE) == 0)
FYI, if you use 'normal' way (see my comments in interface_list), you don't need
these two if's.

[viettrungluu, 2011-11-12 02:45:13]

Yuzhu, thanks for patching it in and reviewing. Just to make sure, did you make
sure that things that use the TCP socket interface still work?

Anyway,  Dmitry, please go with Yuzhu's review. His comments seem good to me.
I've reconsidered, and you may as well get this in as soon as possible (and not
wait for me to finish writing tests). Thanks for doing this!

[yzshen1, 2011-11-14 06:16:45]

On 2011/11/12 02:45:13, viettrungluu wrote:
> Yuzhu, thanks for patching it in and reviewing. Just to make sure, did you
make
> sure that things that use the TCP socket interface still work?

Yes. I ran the test that we used before.
(But with some slight modification to get around the crash caused by
NOTIMPLEMENTED in converting platform-specific address to sockaddr. It has
nothing to do with this CL, I think.)

> 
> Anyway,  Dmitry, please go with Yuzhu's review. His comments seem good to me.
> I've reconsidered, and you may as well get this in as soon as possible (and
not
> wait for me to finish writing tests). Thanks for doing this!

[Dmitry Polukhin, 2011-11-14 13:03:02]

Please take a look again.

Yuzhu, thank you a lot for testing my patch!

http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/tcp_socke...
File ppapi/cpp/private/tcp_socket_private.h (right):

http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/tcp_socke...
ppapi/cpp/private/tcp_socket_private.h:17: class TCPSocket : public Resource {
On 2011/11/11 21:04:28, yzshen1 wrote:
> It should have "Private" in the name.
> (So that it is consistent with other dev/private interfaces.)

Done.

http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/udp_socke...
File ppapi/cpp/private/udp_socket_private.h (right):

http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/udp_socke...
ppapi/cpp/private/udp_socket_private.h:17: class UDPSocket : public Resource {
On 2011/11/11 21:04:28, yzshen1 wrote:
> It should have "Private" in the name.
> (So that it is consistent with other dev/private interfaces.)

Done.

http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/interface_list.cc
File ppapi/proxy/interface_list.cc (right):

http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/interface_list....
ppapi/proxy/interface_list.cc:165: AddProxy(API_ID_PPB_TCPSOCKET,
&ProxyFactory<PPB_TCPSocket_Proxy>);
On 2011/11/11 21:04:28, yzshen1 wrote:
> > They are special in name, they doesn't have _Private 
> > suffix. I think there is no
> > sens to add private everywhere it will just make names 
> > longer without real benefits.
> 
> If the only concern is the _PRIVATE suffix being too long, I still think that
we
> should add it and use the 'normal' way. This ID is not used everywhere, and it
> seems not too long to me.
> 
> Please note that this list is used for really special interfaces (see the
> comment above).

Done.

http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/resource_creati...
File ppapi/proxy/resource_creation_proxy.cc (right):

http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/resource_creati...
ppapi/proxy/resource_creation_proxy.cc:26: #include
"ppapi/proxy/ppb_tcp_socket_proxy.h"
On 2011/11/11 21:04:28, yzshen1 wrote:
> Sort it, please.

Done.

http://codereview.chromium.org/8506016/diff/17001/ppapi/shared_impl/api_id.h
File ppapi/shared_impl/api_id.h (right):

http://codereview.chromium.org/8506016/diff/17001/ppapi/shared_impl/api_id.h#...
ppapi/shared_impl/api_id.h:39: API_ID_PPB_TCPSOCKET,
On 2011/11/11 21:04:28, yzshen1 wrote:
> As I mentioned in another file, please add _PRIVATE for them.

Done.

[yzshen1, 2011-11-14 19:33:03]

Sorry, I think I haven't made it clear enough.
In the previous review, I commented that you need to change the API IDs and cpp
wrapper names to add 'private'. I didn't mean to ask you to add 'private'
everywhere (e.g., proxy/api/thunk names). You could take a look at other
dev/private to see where 'dev'/'private' is used in names.

I don't have a good reason why things are named this way. (I was confused when I
was adding APIs, too.) But it seems good to me to be consistent with existing
code. What do you think?

Thanks!

On 2011/11/14 13:03:02, Dmitry Polukhin wrote:
> Please take a look again.
> 
> Yuzhu, thank you a lot for testing my patch!
> 
>
http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/tcp_socke...
> File ppapi/cpp/private/tcp_socket_private.h (right):
> 
>
http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/tcp_socke...
> ppapi/cpp/private/tcp_socket_private.h:17: class TCPSocket : public Resource {
> On 2011/11/11 21:04:28, yzshen1 wrote:
> > It should have "Private" in the name.
> > (So that it is consistent with other dev/private interfaces.)
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/udp_socke...
> File ppapi/cpp/private/udp_socket_private.h (right):
> 
>
http://codereview.chromium.org/8506016/diff/17001/ppapi/cpp/private/udp_socke...
> ppapi/cpp/private/udp_socket_private.h:17: class UDPSocket : public Resource {
> On 2011/11/11 21:04:28, yzshen1 wrote:
> > It should have "Private" in the name.
> > (So that it is consistent with other dev/private interfaces.)
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/interface_list.cc
> File ppapi/proxy/interface_list.cc (right):
> 
>
http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/interface_list....
> ppapi/proxy/interface_list.cc:165: AddProxy(API_ID_PPB_TCPSOCKET,
> &ProxyFactory<PPB_TCPSocket_Proxy>);
> On 2011/11/11 21:04:28, yzshen1 wrote:
> > > They are special in name, they doesn't have _Private 
> > > suffix. I think there is no
> > > sens to add private everywhere it will just make names 
> > > longer without real benefits.
> > 
> > If the only concern is the _PRIVATE suffix being too long, I still think
that
> we
> > should add it and use the 'normal' way. This ID is not used everywhere, and
it
> > seems not too long to me.
> > 
> > Please note that this list is used for really special interfaces (see the
> > comment above).
> 
> Done.
> 
>
http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/resource_creati...
> File ppapi/proxy/resource_creation_proxy.cc (right):
> 
>
http://codereview.chromium.org/8506016/diff/17001/ppapi/proxy/resource_creati...
> ppapi/proxy/resource_creation_proxy.cc:26: #include
> "ppapi/proxy/ppb_tcp_socket_proxy.h"
> On 2011/11/11 21:04:28, yzshen1 wrote:
> > Sort it, please.
> 
> Done.
> 
> http://codereview.chromium.org/8506016/diff/17001/ppapi/shared_impl/api_id.h
> File ppapi/shared_impl/api_id.h (right):
> 
>
http://codereview.chromium.org/8506016/diff/17001/ppapi/shared_impl/api_id.h#...
> ppapi/shared_impl/api_id.h:39: API_ID_PPB_TCPSOCKET,
> On 2011/11/11 21:04:28, yzshen1 wrote:
> > As I mentioned in another file, please add _PRIVATE for them.
> 
> Done.

[Dmitry Polukhin, 2011-11-14 19:56:54]

On 2011/11/14 19:33:03, yzshen1 wrote:
> Sorry, I think I haven't made it clear enough.
> In the previous review, I commented that you need to change the API IDs and
cpp
> wrapper names to add 'private'. I didn't mean to ask you to add 'private'
> everywhere (e.g., proxy/api/thunk names). You could take a look at other
> dev/private to see where 'dev'/'private' is used in names.
> 
> I don't have a good reason why things are named this way. (I was confused when
I
> was adding APIs, too.) But it seems good to me to be consistent with existing
> code. What do you think?

Proxy names must have _Private suffix to be used with common macro. Thunk
creation functions too. I think current names are more or less consistent with
other private interfaces (taking into account that they are not always
consistent). If you don't have strong preference I would keep names "as is" but
I don't have strong opinion about names. One reason about skipping Private would
be that if we decide to make them stable at some point we will have to rename
everything again, it is sad :( Private suffix in IPC messages names doesn't have
much sense - perhaps better to remove.

[yzshen1, 2011-11-14 20:52:15]

On 2011/11/14 19:56:54, Dmitry Polukhin wrote:
> On 2011/11/14 19:33:03, yzshen1 wrote:
> > Sorry, I think I haven't made it clear enough.
> > In the previous review, I commented that you need to change the API IDs and
> cpp
> > wrapper names to add 'private'. I didn't mean to ask you to add 'private'
> > everywhere (e.g., proxy/api/thunk names). You could take a look at other
> > dev/private to see where 'dev'/'private' is used in names.
> > 
> > I don't have a good reason why things are named this way. (I was confused
when
> I
> > was adding APIs, too.) But it seems good to me to be consistent with
existing
> > code. What do you think?
> 
> Proxy names must have _Private suffix to be used with common macro. Thunk
> creation functions too. I think current names are more or less consistent with
> other private interfaces (taking into account that they are not always
> consistent). If you don't have strong preference I would keep names "as is"
but
> I don't have strong opinion about names. One reason about skipping Private
would
> be that if we decide to make them stable at some point we will have to rename
> everything again, it is sad :( Private suffix in IPC messages names doesn't
have
> much sense - perhaps better to remove.

Thanks for explaining. I am fine with keeping names "as is". :)

[yzshen1, 2011-11-14 20:52:24]

http://codereview.chromium.org/8506016/diff/21043/ppapi/ppapi_cpp.gypi
File ppapi/ppapi_cpp.gypi (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/ppapi_cpp.gypi#newcod...
ppapi/ppapi_cpp.gypi:441: 'cpp/private/instance_private.h',
[I haven't read the code the know what this lib is used for yet. Just to make
sure that you have considered it.]
Do you need to add *_socket_private.* here?

http://codereview.chromium.org/8506016/diff/21043/ppapi/proxy/interface_list.cc
File ppapi/proxy/interface_list.cc (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/proxy/interface_list....
ppapi/proxy/interface_list.cc:81: #include
"ppapi/proxy/ppb_tcp_socket_private_proxy.h"
sort, please.

http://codereview.chromium.org/8506016/diff/21043/ppapi/proxy/ppb_udp_socket_...
File ppapi/proxy/ppb_udp_socket_private_proxy.h (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/proxy/ppb_udp_socket_...
ppapi/proxy/ppb_udp_socket_private_proxy.h:19: // The maximum number of bytes
that each PpapiHostMsg_PPBUDPSocket_RecvFrom
Here and line 22: Please update the message name (if you decide to keep the
'private' in it.)

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/interfaces_ppb_...
File ppapi/thunk/interfaces_ppb_private.h (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/interfaces_ppb_...
ppapi/thunk/interfaces_ppb_private.h:18: PROXIED_IFACE(PPB_TCPSocket_Private,
PPB_TCPSOCKET_PRIVATE_INTERFACE,
Please sort them, please.

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/ppb_tcp_socket_...
File ppapi/thunk/ppb_tcp_socket_private_api.h (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/ppb_tcp_socket_...
ppapi/thunk/ppb_tcp_socket_private_api.h:5: #ifndef
PPAPI_THUNK_PPB_TCP_SOCKET_API_H_
Please update.

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/ppb_udp_socket_...
File ppapi/thunk/ppb_udp_socket_private_api.h (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/ppb_udp_socket_...
ppapi/thunk/ppb_udp_socket_private_api.h:5: #ifndef
PPAPI_THUNK_PPB_UDP_SOCKET_API_H_
Please update.

[viettrungluu, 2011-11-15 01:47:01]

FYI: http://codereview.chromium.org/8564034/ (which may land before your
change). You should be able to use this to test compatibility (by compiling the
test plugin my change but without your change, and then testing it on Chrome
with your change applied).

Note that if you want to run the test manually, you a) need to run it over http
(and out-of-process).

Also, if I manage to land my change before you land yours, you'll need to update
the test code; I've made that easy to do (I hope).

[Dmitry Polukhin, 2011-11-15 09:49:55]

I tested my changes on OutOfProcessPPAPITest.TCPSocketPrivate (libppapi_tests.so
built without my changes and Chrome built with my changes) it works. Also I
updated OutOfProcessPPAPITest.TCPSocketPrivate to test new interface after
renaming.

http://codereview.chromium.org/8506016/diff/21043/ppapi/ppapi_cpp.gypi
File ppapi/ppapi_cpp.gypi (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/ppapi_cpp.gypi#newcod...
ppapi/ppapi_cpp.gypi:441: 'cpp/private/instance_private.h',
On 2011/11/14 20:52:24, yzshen1 wrote:
> [I haven't read the code the know what this lib is used for yet. Just to make
> sure that you have considered it.]
> Do you need to add *_socket_private.* here?

No, theses interfaces are not supported in NaCl yet. They will be added in
Yuri's CL.

http://codereview.chromium.org/8506016/diff/21043/ppapi/proxy/interface_list.cc
File ppapi/proxy/interface_list.cc (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/proxy/interface_list....
ppapi/proxy/interface_list.cc:81: #include
"ppapi/proxy/ppb_tcp_socket_private_proxy.h"
On 2011/11/14 20:52:24, yzshen1 wrote:
> sort, please.

Done.

http://codereview.chromium.org/8506016/diff/21043/ppapi/proxy/ppb_udp_socket_...
File ppapi/proxy/ppb_udp_socket_private_proxy.h (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/proxy/ppb_udp_socket_...
ppapi/proxy/ppb_udp_socket_private_proxy.h:19: // The maximum number of bytes
that each PpapiHostMsg_PPBUDPSocket_RecvFrom
On 2011/11/14 20:52:24, yzshen1 wrote:
> Here and line 22: Please update the message name (if you decide to keep the
> 'private' in it.)
> 

Removed private from message name.

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/interfaces_ppb_...
File ppapi/thunk/interfaces_ppb_private.h (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/interfaces_ppb_...
ppapi/thunk/interfaces_ppb_private.h:18: PROXIED_IFACE(PPB_TCPSocket_Private,
PPB_TCPSOCKET_PRIVATE_INTERFACE,
On 2011/11/14 20:52:24, yzshen1 wrote:
> Please sort them, please.

I sorted them by interface name. Now resorted by first identifier after bracket
but I don't think it is really better.

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/ppb_tcp_socket_...
File ppapi/thunk/ppb_tcp_socket_private_api.h (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/ppb_tcp_socket_...
ppapi/thunk/ppb_tcp_socket_private_api.h:5: #ifndef
PPAPI_THUNK_PPB_TCP_SOCKET_API_H_
On 2011/11/14 20:52:24, yzshen1 wrote:
> Please update.

Done.

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/ppb_udp_socket_...
File ppapi/thunk/ppb_udp_socket_private_api.h (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/ppb_udp_socket_...
ppapi/thunk/ppb_udp_socket_private_api.h:5: #ifndef
PPAPI_THUNK_PPB_UDP_SOCKET_API_H_
On 2011/11/14 20:52:24, yzshen1 wrote:
> Please update.

Done.

[yzshen1, 2011-11-15 17:29:31]

lgtm

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/interfaces_ppb_...
File ppapi/thunk/interfaces_ppb_private.h (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/interfaces_ppb_...
ppapi/thunk/interfaces_ppb_private.h:18: PROXIED_IFACE(PPB_TCPSocket_Private,
PPB_TCPSOCKET_PRIVATE_INTERFACE,
On 2011/11/15 09:49:56, Dmitry Polukhin wrote:
> On 2011/11/14 20:52:24, yzshen1 wrote:
> > Please sort them, please.
> 
> I sorted them by interface name. Now resorted by first identifier after
bracket
> but I don't think it is really better.

Okay. What I suggested to move is the last one (i.e., PPB_FlashFullscreen).

[viettrungluu, 2011-11-15 18:30:10]

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/interfaces_ppb_...
File ppapi/thunk/interfaces_ppb_private.h (right):

http://codereview.chromium.org/8506016/diff/21043/ppapi/thunk/interfaces_ppb_...
ppapi/thunk/interfaces_ppb_private.h:18: PROXIED_IFACE(PPB_TCPSocket_Private,
PPB_TCPSOCKET_PRIVATE_INTERFACE,
On 2011/11/15 17:29:31, yzshen1 wrote:
> On 2011/11/15 09:49:56, Dmitry Polukhin wrote:
> > On 2011/11/14 20:52:24, yzshen1 wrote:
> > > Please sort them, please.
> > 
> > I sorted them by interface name. Now resorted by first identifier after
> bracket
> > but I don't think it is really better.
> 
> Okay. What I suggested to move is the last one (i.e., PPB_FlashFullscreen).

I wouldn't worry about that one. I should check to see if it can be removed.

[viettrungluu, 2011-11-15 18:32:35]

LGTM otherwise, btw.

http://codereview.chromium.org/8506016/diff/21056/ppapi/tests/test_tcp_socket...
File ppapi/tests/test_tcp_socket_private.h (right):

http://codereview.chromium.org/8506016/diff/21056/ppapi/tests/test_tcp_socket...
ppapi/tests/test_tcp_socket_private.h:16: typedef pp::TCPSocketPrivate
TCPSocketPrivate;
Oops, you should probably just change the instances in this header to
pp::TCPSocketPrivate. In the .cc file, you can do likewise (oops, sorry -- I
forgot that TCPSocketPrivate should be in the pp namespace) or add a "using
pp::TCPSocketPrivate;".

[Dmitry Polukhin, 2011-11-15 18:35:52]

Yuzhu and Trung, thank you a lot for review!
I'll resolve last comments and commit this CL tomorrow in my working hours.

[Dmitry Polukhin, 2011-11-16 06:55:11]

Once again thank you for reviewing and help with testing!

http://codereview.chromium.org/8506016/diff/21056/ppapi/tests/test_tcp_socket...
File ppapi/tests/test_tcp_socket_private.h (right):

http://codereview.chromium.org/8506016/diff/21056/ppapi/tests/test_tcp_socket...
ppapi/tests/test_tcp_socket_private.h:16: typedef pp::TCPSocketPrivate
TCPSocketPrivate;
On 2011/11/15 18:32:35, viettrungluu wrote:
> Oops, you should probably just change the instances in this header to
> pp::TCPSocketPrivate. In the .cc file, you can do likewise (oops, sorry -- I
> forgot that TCPSocketPrivate should be in the pp namespace) or add a "using
> pp::TCPSocketPrivate;".

Done.

[Dmitry Polukhin, 2011-11-16 07:05:44]

+ TBR: dmichael@ for OWNER review

But committing because it's basically simple renaming.