Provide a way to disable Nagle's algorithm on Pepper TCP sockets.

Provide a way to disable Nagle's algorithm on Pepper TCP sockets.

This CL adds a SetSocketFeature() API to PPB_TCPSocket_Private, supporting one
option, to control Nagle's algorithim on the underlying socket.

BUG=170248
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=184087

[Wez, 2013-02-07 00:15:03]

yzshen: PTAL :)

dmichael: We don't seem to have any tests for PPB_TCPSocket_Private... am I just
missing something?

[dmichael (off chromium), 2013-02-07 18:23:10]

It looks like there are tests to me: ppapi/tests/test_tcp_socket_private*
...though I haven't looked carefully at their contents.

[Wez, 2013-02-07 19:48:14]

On 2013/02/07 18:23:10, dmichael wrote:
> It looks like there are tests to me: ppapi/tests/test_tcp_socket_private*
> ...though I haven't looked carefully at their contents.

Ummm... yeah, not entirely sure how I missed those!

I've updated the C++ wrappers and added a simple test.

Updating the wrappers was messy, now that there are three versions of the
interface.  It's a shame we can't manipulate resources of allocated via one
version of an interface via APIs from an earlier version.

[dmichael (off chromium), 2013-02-07 20:00:12]

On Thu, Feb 7, 2013 at 12:48 PM, <wez@chromium.org> wrote:

> On 2013/02/07 18:23:10, dmichael wrote:
>
>> It looks like there are tests to me: ppapi/tests/test_tcp_socket_**
>> private*
>> ...though I haven't looked carefully at their contents.
>>
>
> Ummm... yeah, not entirely sure how I missed those!
>
> I've updated the C++ wrappers and added a simple test.
>
> Updating the wrappers was messy, now that there are three versions of the
> interface.  It's a shame we can't manipulate resources of allocated via one
> version of an interface via APIs from an earlier version.
>
We can. See, e.g., var.cc. We only check the more recent version of an API
when the particular function has changed. When the function has *not *changed,
we only use the oldest version of the API that has the function. There's no
need to do the complicated fallback from 0.5 to 0.4 to 0.3 in most cases.

Our general strategy is that we'll only have 1 backing implementation for a
given resource on a particular version of Chrome. The same object handles
all the valid versions for that API. If we ever encounter a place where
this won't work, we're likely to define a new API instead of breaking this
rule of 1 object per API (regardless of version).


>
https://chromiumcodereview.**appspot.com/12220050/<https://chromiumcodereview...
>

[Wez, 2013-02-07 21:23:10]

On 2013/02/07 20:00:12, dmichael wrote:
> On Thu, Feb 7, 2013 at 12:48 PM, <mailto:wez@chromium.org> wrote:
> 
> > On 2013/02/07 18:23:10, dmichael wrote:
> >
> >> It looks like there are tests to me: ppapi/tests/test_tcp_socket_**
> >> private*
> >> ...though I haven't looked carefully at their contents.
> >>
> >
> > Ummm... yeah, not entirely sure how I missed those!
> >
> > I've updated the C++ wrappers and added a simple test.
> >
> > Updating the wrappers was messy, now that there are three versions of the
> > interface.  It's a shame we can't manipulate resources of allocated via one
> > version of an interface via APIs from an earlier version.
> >
> We can. See, e.g., var.cc. We only check the more recent version of an API
> when the particular function has changed. When the function has *not *changed,
> we only use the oldest version of the API that has the function. There's no
> need to do the complicated fallback from 0.5 to 0.4 to 0.3 in most cases.
> 
> Our general strategy is that we'll only have 1 backing implementation for a
> given resource on a particular version of Chrome. The same object handles
> all the valid versions for that API. If we ever encounter a place where
> this won't work, we're likely to define a new API instead of breaking this
> rule of 1 object per API (regardless of version).

Right, I remember looking at var.cc previously, so I was a little surprised by
the complexity in TCPSocket_Private.  I've trimmed it, and the UDP socket
wrapper, down.

[yzshen1, 2013-02-08 21:51:11]

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
File content/browser/renderer_host/pepper/pepper_message_filter.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_message_filter.cc:364: 
Do you plan to add implementation later?
If yes, please add a TODO.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
File content/browser/renderer_host/pepper/pepper_tcp_socket.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_tcp_socket.cc:204: case
PP_TCPSOCKETFEATURE_NO_DELAY:
wrong indent.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_tcp_socket.cc:431: connection_state_
== SSL_CONNECTED ||
wrong indent.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
File content/browser/renderer_host/pepper/pepper_tcp_socket.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_tcp_socket.h:63: void
SetBoolFeature(uint32_t name, bool value);
I noticed that in pepper_message_filter, |name| is int32_t.
Is it intentional to be different?

And I think it is better to pass PP_TCPSocketFeature_Private directly.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/renderer/p...
File content/renderer/pepper/pepper_plugin_delegate_impl.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/renderer/p...
content/renderer/pepper/pepper_plugin_delegate_impl.cc:1187: uint32 socket_id,
int32_t name, bool value) {
- wrong indent.
- From chromium style guide:
"For function declarations and definitions, put each argument on a separate line
unless the whole declaration fits on one line."

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/renderer/p...
content/renderer/pepper/pepper_plugin_delegate_impl.cc:1508: uint32
plugin_dispatcher_id, uint32 socket_id, bool succeeded) {
From chromium style guide:
"For function declarations and definitions, put each argument on a separate line
unless the whole declaration fits on one line."

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/renderer/p...
File content/renderer/pepper/pepper_plugin_delegate_impl.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/renderer/p...
content/renderer/pepper/pepper_plugin_delegate_impl.h:271: virtual void
TCPSocketSetBoolFeature(uint32 socket_id, int32_t name,
From chromium style guide:
"For function declarations and definitions, put each argument on a separate line
unless the whole declaration fits on one line."

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/api/private/...
File ppapi/api/private/ppb_tcp_socket_private.idl (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/api/private/...
ppapi/api/private/ppb_tcp_socket_private.idl:13: M26 = 0.5
nit: you probably cannot get it into 26, so this should be changed to 27.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/api/private/...
ppapi/api/private/ppb_tcp_socket_private.idl:20: PP_TCPSOCKETFEATURE_NO_DELAY =
0,
it seems good to comment that setting this feature after a SSL handshake is
started will fail.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/api/private/...
ppapi/api/private/ppb_tcp_socket_private.idl:23: PP_TCPSOCKETFEATURE_INVALID =
-1
Please order them by their values.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/api/private/...
ppapi/api/private/ppb_tcp_socket_private.idl:156: int32_t SetSocketFeature([in]
PP_Resource udp_socket,
- nit: I think SetFeature() is clear enough.
- udp_socket -> tcp_socket.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/cpp/private/...
File ppapi/cpp/private/tcp_socket_private.cc (left):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/cpp/private/...
ppapi/cpp/private/tcp_socket_private.cc:51: if
(has_interface<PPB_TCPSocket_Private_0_4>()) {
I am not sure whether we want to make this change.
This implies that v0.5 method X is able to work with v0.3 method Y. It may be
the case now.  v0.5 SetSocketFeature() is able to work with v0.3
Connect/Write()/Read()/etc. However, what if later we introduce a v0.6
Disconnect() which cannot work with v0.3 Connect()?

Unless we want to make this a promise, we may not want to encourage this usage.

Maybe we could send a mail to the pepper-team@ for discussion.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/cpp/private/...
File ppapi/cpp/private/udp_socket_private.cc (left):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/cpp/private/...
ppapi/cpp/private/udp_socket_private.cc:56: if
(has_interface<PPB_UDPSocket_Private_0_4>()) {
Please see my comment in tcp_socket_private.cc

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/proxy/ppb_in...
File ppapi/proxy/ppb_instance_proxy.cc (left):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/proxy/ppb_in...
ppapi/proxy/ppb_instance_proxy.cc:1148: if
(!dispatcher()->permissions().HasPermission(PERMISSION_DEV))
Is this caused by a sync? It seems to have nothing to do with the other parts of
your CL.
(And if it is, I wonder why this file is included.)

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/proxy/ppb_tc...
File ppapi/proxy/ppb_tcp_socket_private_proxy.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/proxy/ppb_tc...
ppapi/proxy/ppb_tcp_socket_private_proxy.cc:49: virtual void
SendBoolSocketFeature(int32_t name, bool value) OVERRIDE;
Please use the enum type instead of int32_t.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/shared_impl/...
File ppapi/shared_impl/private/tcp_socket_private_impl.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/shared_impl/...
ppapi/shared_impl/private/tcp_socket_private_impl.cc:230: case
PP_TCPSOCKETFEATURE_NO_DELAY:
wrong indent.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/shared_impl/...
File ppapi/shared_impl/private/tcp_socket_private_impl.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/shared_impl/...
ppapi/shared_impl/private/tcp_socket_private_impl.h:69: virtual int32_t
SetSocketFeature(PP_TCPSocketFeature_Private name,
For function declarations and definitions, put each argument on a separate line
unless the whole declaration fits on one line.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/shared_impl/...
ppapi/shared_impl/private/tcp_socket_private_impl.h:70: struct PP_Var value,
scoped_refptr<TrackedCallback> callback) OVERRIDE;
you don't need to have 'struct' here.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/thunk/ppb_tc...
File ppapi/thunk/ppb_tcp_socket_private_api.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/thunk/ppb_tc...
ppapi/thunk/ppb_tcp_socket_private_api.h:44: PP_Var value,
wrong indent.

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
File webkit/plugins/ppapi/mock_plugin_delegate.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
webkit/plugins/ppapi/mock_plugin_delegate.cc:267: void
MockPluginDelegate::TCPSocketSetBoolFeature(uint32 socket_id, int32_t name,
For function declarations and definitions, put each argument on a separate line
unless the whole declaration fits on one line.

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
File webkit/plugins/ppapi/mock_plugin_delegate.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
webkit/plugins/ppapi/mock_plugin_delegate.h:127: virtual void
TCPSocketSetBoolFeature(uint32 socket_id, int32_t name,
For function declarations and definitions, put each argument on a separate line
unless the whole declaration fits on one line.

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
File webkit/plugins/ppapi/plugin_delegate.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
webkit/plugins/ppapi/plugin_delegate.h:547: virtual void
TCPSocketSetBoolFeature(uint32 socket_id, int32_t name,
For function declarations and definitions, put each argument on a separate line
unless the whole declaration fits on one line.

[Wez, 2013-02-10 04:47:02]

PTAL; let's discuss the C++ wrapper versioning issue off-CL.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
File content/browser/renderer_host/pepper/pepper_message_filter.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_message_filter.cc:364: 
On 2013/02/08 21:51:11, yzshen1 wrote:
> Do you plan to add implementation later?
> If yes, please add a TODO.

Nope, it's supposed to be added in this CL! Looks like I got my branches mixed
up.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
File content/browser/renderer_host/pepper/pepper_tcp_socket.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_tcp_socket.cc:204: case
PP_TCPSOCKETFEATURE_NO_DELAY:
On 2013/02/08 21:51:11, yzshen1 wrote:
> wrong indent.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_tcp_socket.cc:431: connection_state_
== SSL_CONNECTED ||
On 2013/02/08 21:51:11, yzshen1 wrote:
> wrong indent.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
File content/browser/renderer_host/pepper/pepper_tcp_socket.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_tcp_socket.h:63: void
SetBoolFeature(uint32_t name, bool value);
On 2013/02/08 21:51:11, yzshen1 wrote:
> I noticed that in pepper_message_filter, |name| is int32_t.
> Is it intentional to be different?

No; I just followed the style of the UDP SetBoolSocketFeature implementation. 
Fixed.

> And I think it is better to pass PP_TCPSocketFeature_Private directly.

I based this on the UDP SetSocketFeature implementation, which uses int32_t
here, but I've switched these to use the enum.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/renderer/p...
File content/renderer/pepper/pepper_plugin_delegate_impl.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/renderer/p...
content/renderer/pepper/pepper_plugin_delegate_impl.cc:1187: uint32 socket_id,
int32_t name, bool value) {
On 2013/02/08 21:51:11, yzshen1 wrote:
> - wrong indent.
> - From chromium style guide:
> "For function declarations and definitions, put each argument on a separate
line
> unless the whole declaration fits on one line."

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/renderer/p...
content/renderer/pepper/pepper_plugin_delegate_impl.cc:1508: uint32
plugin_dispatcher_id, uint32 socket_id, bool succeeded) {
On 2013/02/08 21:51:11, yzshen1 wrote:
> From chromium style guide:
> "For function declarations and definitions, put each argument on a separate
line
> unless the whole declaration fits on one line."

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/renderer/p...
File content/renderer/pepper/pepper_plugin_delegate_impl.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/renderer/p...
content/renderer/pepper/pepper_plugin_delegate_impl.h:271: virtual void
TCPSocketSetBoolFeature(uint32 socket_id, int32_t name,
On 2013/02/08 21:51:11, yzshen1 wrote:
> From chromium style guide:
> "For function declarations and definitions, put each argument on a separate
line
> unless the whole declaration fits on one line."

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/api/private/...
File ppapi/api/private/ppb_tcp_socket_private.idl (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/api/private/...
ppapi/api/private/ppb_tcp_socket_private.idl:13: M26 = 0.5
On 2013/02/08 21:51:11, yzshen1 wrote:
> nit: you probably cannot get it into 26, so this should be changed to 27.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/api/private/...
ppapi/api/private/ppb_tcp_socket_private.idl:20: PP_TCPSOCKETFEATURE_NO_DELAY =
0,
On 2013/02/08 21:51:11, yzshen1 wrote:
> it seems good to comment that setting this feature after a SSL handshake is
> started will fail.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/api/private/...
ppapi/api/private/ppb_tcp_socket_private.idl:23: PP_TCPSOCKETFEATURE_INVALID =
-1
On 2013/02/08 21:51:11, yzshen1 wrote:
> Please order them by their values.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/api/private/...
ppapi/api/private/ppb_tcp_socket_private.idl:156: int32_t SetSocketFeature([in]
PP_Resource udp_socket,
On 2013/02/08 21:51:11, yzshen1 wrote:
> - nit: I think SetFeature() is clear enough.
> - udp_socket -> tcp_socket.

SGTM; I'd chosen the function name to match the one in UDPSocket_Private.  Since
we don't care about consistency between the APIs, I've renamed this to
SetOption() for consistency with BSD socket terminology.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/cpp/private/...
File ppapi/cpp/private/tcp_socket_private.cc (left):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/cpp/private/...
ppapi/cpp/private/tcp_socket_private.cc:51: if
(has_interface<PPB_TCPSocket_Private_0_4>()) {
On 2013/02/08 21:51:11, yzshen1 wrote:
> I am not sure whether we want to make this change.
> This implies that v0.5 method X is able to work with v0.3 method Y. It may be
> the case now.  v0.5 SetSocketFeature() is able to work with v0.3
> Connect/Write()/Read()/etc. However, what if later we introduce a v0.6
> Disconnect() which cannot work with v0.3 Connect()?
> 
> Unless we want to make this a promise, we may not want to encourage this
usage.
> 
> Maybe we could send a mail to the pepper-team@ for discussion.

This change was suggested by dmichael@ - let's discuss offline.  We should make
sure we have clear guidelines on stuff like this.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/proxy/ppb_in...
File ppapi/proxy/ppb_instance_proxy.cc (left):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/proxy/ppb_in...
ppapi/proxy/ppb_instance_proxy.cc:1148: if
(!dispatcher()->permissions().HasPermission(PERMISSION_DEV))
On 2013/02/08 21:51:11, yzshen1 wrote:
> Is this caused by a sync? It seems to have nothing to do with the other parts
of
> your CL.
> (And if it is, I wonder why this file is included.)

Mix up w/ a branch for an un-related CL. :(

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/proxy/ppb_tc...
File ppapi/proxy/ppb_tcp_socket_private_proxy.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/proxy/ppb_tc...
ppapi/proxy/ppb_tcp_socket_private_proxy.cc:49: virtual void
SendBoolSocketFeature(int32_t name, bool value) OVERRIDE;
On 2013/02/08 21:51:11, yzshen1 wrote:
> Please use the enum type instead of int32_t.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/shared_impl/...
File ppapi/shared_impl/private/tcp_socket_private_impl.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/shared_impl/...
ppapi/shared_impl/private/tcp_socket_private_impl.cc:230: case
PP_TCPSOCKETFEATURE_NO_DELAY:
On 2013/02/08 21:51:11, yzshen1 wrote:
> wrong indent.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/shared_impl/...
File ppapi/shared_impl/private/tcp_socket_private_impl.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/shared_impl/...
ppapi/shared_impl/private/tcp_socket_private_impl.h:69: virtual int32_t
SetSocketFeature(PP_TCPSocketFeature_Private name,
On 2013/02/08 21:51:11, yzshen1 wrote:
> For function declarations and definitions, put each argument on a separate
line
> unless the whole declaration fits on one line.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/shared_impl/...
ppapi/shared_impl/private/tcp_socket_private_impl.h:70: struct PP_Var value,
scoped_refptr<TrackedCallback> callback) OVERRIDE;
On 2013/02/08 21:51:11, yzshen1 wrote:
> you don't need to have 'struct' here.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/thunk/ppb_tc...
File ppapi/thunk/ppb_tcp_socket_private_api.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/thunk/ppb_tc...
ppapi/thunk/ppb_tcp_socket_private_api.h:44: PP_Var value,
On 2013/02/08 21:51:11, yzshen1 wrote:
> wrong indent.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
File webkit/plugins/ppapi/mock_plugin_delegate.cc (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
webkit/plugins/ppapi/mock_plugin_delegate.cc:267: void
MockPluginDelegate::TCPSocketSetBoolFeature(uint32 socket_id, int32_t name,
On 2013/02/08 21:51:11, yzshen1 wrote:
> For function declarations and definitions, put each argument on a separate
line
> unless the whole declaration fits on one line.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
File webkit/plugins/ppapi/mock_plugin_delegate.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
webkit/plugins/ppapi/mock_plugin_delegate.h:127: virtual void
TCPSocketSetBoolFeature(uint32 socket_id, int32_t name,
On 2013/02/08 21:51:11, yzshen1 wrote:
> For function declarations and definitions, put each argument on a separate
line
> unless the whole declaration fits on one line.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
File webkit/plugins/ppapi/plugin_delegate.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/webkit/plugins/ppa...
webkit/plugins/ppapi/plugin_delegate.h:547: virtual void
TCPSocketSetBoolFeature(uint32 socket_id, int32_t name,
On 2013/02/08 21:51:11, yzshen1 wrote:
> For function declarations and definitions, put each argument on a separate
line
> unless the whole declaration fits on one line.

Done.

[Wez, 2013-02-10 09:20:48]

FYI, I've added thunking for the new version of the interface - I notice the
previous version wasn't thunked but I think that was just because the methods it
added were private-only?

[yzshen1, 2013-02-12 17:39:58]

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
File content/browser/renderer_host/pepper/pepper_tcp_socket.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_tcp_socket.h:63: void
SetBoolFeature(uint32_t name, bool value);
On 2013/02/10 04:47:02, Wez wrote:
> On 2013/02/08 21:51:11, yzshen1 wrote:
> > I noticed that in pepper_message_filter, |name| is int32_t.
> > Is it intentional to be different?
> 
> No; I just followed the style of the UDP SetBoolSocketFeature implementation. 
> Fixed.
> 
> > And I think it is better to pass PP_TCPSocketFeature_Private directly.
> 
> I based this on the UDP SetSocketFeature implementation, which uses int32_t
> here, but I've switched these to use the enum.

I didn't see that you have switched to enum. Have you uploaded the latest patch?

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/cpp/private/...
File ppapi/cpp/private/tcp_socket_private.cc (left):

https://chromiumcodereview.appspot.com/12220050/diff/12001/ppapi/cpp/private/...
ppapi/cpp/private/tcp_socket_private.cc:51: if
(has_interface<PPB_TCPSocket_Private_0_4>()) {
Sounds good.
 
On 2013/02/10 04:47:02, Wez wrote:
> On 2013/02/08 21:51:11, yzshen1 wrote:
> > I am not sure whether we want to make this change.
> > This implies that v0.5 method X is able to work with v0.3 method Y. It may
be
> > the case now.  v0.5 SetSocketFeature() is able to work with v0.3
> > Connect/Write()/Read()/etc. However, what if later we introduce a v0.6
> > Disconnect() which cannot work with v0.3 Connect()?
> > 
> > Unless we want to make this a promise, we may not want to encourage this
> usage.
> > 
> > Maybe we could send a mail to the pepper-team@ for discussion.
> 
> This change was suggested by dmichael@ - let's discuss offline.  We should
make
> sure we have clear guidelines on stuff like this.

https://chromiumcodereview.appspot.com/12220050/diff/6005/content/browser/ren...
File content/browser/renderer_host/pepper/pepper_message_filter.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/6005/content/browser/ren...
content/browser/renderer_host/pepper/pepper_message_filter.h:155: void
OnTCPSetBoolOption(uint32 socket_id, uint32_t name, bool value);
I still think that using the enum type is better than uint32_t.

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
File ppapi/proxy/ppapi_messages.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
ppapi/proxy/ppapi_messages.h:1221: uint32_t /* socket_id */,
Please be consistent with other messages about the type of |socket_id|.

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
ppapi/proxy/ppapi_messages.h:1222: uint32_t /* name */,
Please use the enum type.

[yzshen, 2013-02-12 18:02:46]

> The previous version wasn't thunked
What do you mean by that? I thought we had const
GetPPB_TCPSocket_Private_0_4_Thunk() and
GetPPB_TCPSocket_Private_0_3_Thunk()?


On Sun, Feb 10, 2013 at 1:20 AM, <wez@chromium.org> wrote:

>
> FYI, I've added thunking for the new version of the interface - I notice
> the
> previous version wasn't thunked but I think that was just because the
> methods it
> added were private-only?
>
>
https://chromiumcodereview.**appspot.com/12220050/<https://chromiumcodereview...
>



-- 
Best regards,
Yuzhu Shen.

[Wez, 2013-02-12 18:45:07]

On 2013/02/12 18:02:46, yzshen wrote:
> > The previous version wasn't thunked
> What do you mean by that? I thought we had const
> GetPPB_TCPSocket_Private_0_4_Thunk() and
> GetPPB_TCPSocket_Private_0_3_Thunk()?

See the old-style thunking definitions in ppapi/thunk/thunk.h.

> 
> 
> On Sun, Feb 10, 2013 at 1:20 AM, <mailto:wez@chromium.org> wrote:
> 
> >
> > FYI, I've added thunking for the new version of the interface - I notice
> > the
> > previous version wasn't thunked but I think that was just because the
> > methods it
> > added were private-only?
> >
> >
>
https://chromiumcodereview.**appspot.com/12220050/%3Chttps://chromiumcoderevi...>
> >
> 
> 
> 
> -- 
> Best regards,
> Yuzhu Shen.

[Wez, 2013-02-12 18:45:20]

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
File content/browser/renderer_host/pepper/pepper_tcp_socket.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_tcp_socket.h:63: void
SetBoolFeature(uint32_t name, bool value);
On 2013/02/12 17:39:58, yzshen1 wrote:
> On 2013/02/10 04:47:02, Wez wrote:
> > On 2013/02/08 21:51:11, yzshen1 wrote:
> > > I noticed that in pepper_message_filter, |name| is int32_t.
> > > Is it intentional to be different?
> > 
> > No; I just followed the style of the UDP SetBoolSocketFeature
implementation. 
> > Fixed.
> > 
> > > And I think it is better to pass PP_TCPSocketFeature_Private directly.
> > 
> > I based this on the UDP SetSocketFeature implementation, which uses int32_t
> > here, but I've switched these to use the enum.
> 
> I didn't see that you have switched to enum. Have you uploaded the latest
patch?

Yes; see PepperPluginDelegateImpl.  This SetBoolFeature() is called by
PepperMessageFilter w/ a uint32_t "name" received via IPC, so to switch this to
an enum would require duplicating knowledge of the valid values.

https://chromiumcodereview.appspot.com/12220050/diff/6005/content/browser/ren...
File content/browser/renderer_host/pepper/pepper_message_filter.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/6005/content/browser/ren...
content/browser/renderer_host/pepper/pepper_message_filter.h:155: void
OnTCPSetBoolOption(uint32 socket_id, uint32_t name, bool value);
On 2013/02/12 17:39:58, yzshen1 wrote:
> I still think that using the enum type is better than uint32_t.

This is an IPC handler.

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
File ppapi/proxy/ppapi_messages.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
ppapi/proxy/ppapi_messages.h:1221: uint32_t /* socket_id */,
On 2013/02/12 17:39:58, yzshen1 wrote:
> Please be consistent with other messages about the type of |socket_id|.

Done.

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
ppapi/proxy/ppapi_messages.h:1222: uint32_t /* name */,
On 2013/02/12 17:39:58, yzshen1 wrote:
> Please use the enum type.

This is an IPC message definition. Using the enum type here would imply that
recipients can rely on the field having one of the defined enum values, which is
not the case.

[yzshen, 2013-02-13 07:57:05]

I think this is not used but somebody forgot to remove it.
Please see
interfaces_ppb_private_no_permissions.h<https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/thunk/interfaces_ppb_private_no_permissions.h>


On Tue, Feb 12, 2013 at 10:45 AM, <wez@chromium.org> wrote:

> On 2013/02/12 18:02:46, yzshen wrote:
>
>> > The previous version wasn't thunked
>> What do you mean by that? I thought we had const
>> GetPPB_TCPSocket_Private_0_4_**Thunk() and
>> GetPPB_TCPSocket_Private_0_3_**Thunk()?
>>
>
> See the old-style thunking definitions in ppapi/thunk/thunk.h.
>
>
>
>
>  On Sun, Feb 10, 2013 at 1:20 AM, <mailto:wez@chromium.org> wrote:
>>
>
>  >
>> > FYI, I've added thunking for the new version of the interface - I notice
>> > the
>> > previous version wasn't thunked but I think that was just because the
>> > methods it
>> > added were private-only?
>> >
>> >
>>
>
> https://chromiumcodereview.**a**ppspot.com/12220050/%3Chttps:/**
>
/chromiumcodereview.appspot.**com/12220050/<http://appspot.com/12220050/%3Chttps://chromiumcodereview.appspot.com/12220050/>
> >
>
>  >
>>
>
>
>
>  --
>> Best regards,
>> Yuzhu Shen.
>>
>
>
>
>
https://chromiumcodereview.**appspot.com/12220050/<https://chromiumcodereview...
>



-- 
Best regards,
Yuzhu Shen.

[yzshen1, 2013-02-13 08:07:14]

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
File content/browser/renderer_host/pepper/pepper_tcp_socket.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/12001/content/browser/re...
content/browser/renderer_host/pepper/pepper_tcp_socket.h:63: void
SetBoolFeature(uint32_t name, bool value);
You could directly use the enum over IPC, and use it at the host side.
For example, see:
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/proxy/ppapi_...

On 2013/02/12 18:45:21, Wez wrote:
> On 2013/02/12 17:39:58, yzshen1 wrote:
> > On 2013/02/10 04:47:02, Wez wrote:
> > > On 2013/02/08 21:51:11, yzshen1 wrote:
> > > > I noticed that in pepper_message_filter, |name| is int32_t.
> > > > Is it intentional to be different?
> > > 
> > > No; I just followed the style of the UDP SetBoolSocketFeature
> implementation. 
> > > Fixed.
> > > 
> > > > And I think it is better to pass PP_TCPSocketFeature_Private directly.
> > > 
> > > I based this on the UDP SetSocketFeature implementation, which uses
int32_t
> > > here, but I've switched these to use the enum.
> > 
> > I didn't see that you have switched to enum. Have you uploaded the latest
> patch?
> 
> Yes; see PepperPluginDelegateImpl.  This SetBoolFeature() is called by
> PepperMessageFilter w/ a uint32_t "name" received via IPC, so to switch this
to
> an enum would require duplicating knowledge of the valid values.

https://chromiumcodereview.appspot.com/12220050/diff/6005/content/browser/ren...
File content/browser/renderer_host/pepper/pepper_message_filter.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/6005/content/browser/ren...
content/browser/renderer_host/pepper/pepper_message_filter.h:155: void
OnTCPSetBoolOption(uint32 socket_id, uint32_t name, bool value);
Please see my comment in pepper_tcp_socket.h
You can use enum over IPC.
On 2013/02/12 18:45:21, Wez wrote:
> On 2013/02/12 17:39:58, yzshen1 wrote:
> > I still think that using the enum type is better than uint32_t.
> 
> This is an IPC handler.

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
File ppapi/proxy/ppapi_messages.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
ppapi/proxy/ppapi_messages.h:1222: uint32_t /* name */,
On 2013/02/12 18:45:21, Wez wrote:
> On 2013/02/12 17:39:58, yzshen1 wrote:
> > Please use the enum type.
> 
> This is an IPC message definition. Using the enum type here would imply that
> recipients can rely on the field having one of the defined enum values, which
is
> not the case.
I don't understand, why the recipients shouldn't rely on the field having one of
the defined enum values?

[Wez, 2013-02-13 22:06:22]

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
File ppapi/proxy/ppapi_messages.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
ppapi/proxy/ppapi_messages.h:1222: uint32_t /* name */,
On 2013/02/13 08:07:14, yzshen1 wrote:
> On 2013/02/12 18:45:21, Wez wrote:
> > On 2013/02/12 17:39:58, yzshen1 wrote:
> > > Please use the enum type.
> > 
> > This is an IPC message definition. Using the enum type here would imply that
> > recipients can rely on the field having one of the defined enum values,
which
> is
> > not the case.
> I don't understand, why the recipients shouldn't rely on the field having one
of
> the defined enum values?

Because over IPC an enum is transferred as an int. If the peer is buggy or
compromised then the int's value may fall outside the enum's range. Since the
IPC layer simply static_cast<>s the int back to the enum, the resulting value is
undefined for values outside the enum's range.

[yzshen1, 2013-02-14 08:35:38]

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
File ppapi/proxy/ppapi_messages.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
ppapi/proxy/ppapi_messages.h:1222: uint32_t /* name */,
Your concern makes sense. I am okay to do it your way (for now).

However, we need to find a better way to fix the general problem. Using integer
as a replacement of enum doesn't seem to be a real fix:
- It hurts readability.
- It is still cast back to the enum type (in pepper_tcp_socket.cc). And I think
you rely on the 'default' branch to catch out-of-range values. There are two
problems:
    (1) the validation is not in the IPC layer, instead it happens long after we
receive the IPC message. Imagine that we have a lot of messages passing enum
types as integers. The validation will be scattered in different parts of our
codebase. And it relies on every user of those messages to do the validation,
which is error-prone.
    (2) Since the result of the cast is undefined, theoretically it can be any
value. Therefore, it may run any branch of the switch statement. That
uncertainty bothers me as well. :)

 
On 2013/02/13 22:06:22, Wez wrote:
> On 2013/02/13 08:07:14, yzshen1 wrote:
> > On 2013/02/12 18:45:21, Wez wrote:
> > > On 2013/02/12 17:39:58, yzshen1 wrote:
> > > > Please use the enum type.
> > > 
> > > This is an IPC message definition. Using the enum type here would imply
that
> > > recipients can rely on the field having one of the defined enum values,
> which
> > is
> > > not the case.
> > I don't understand, why the recipients shouldn't rely on the field having
one
> of
> > the defined enum values?
> 
> Because over IPC an enum is transferred as an int. If the peer is buggy or
> compromised then the int's value may fall outside the enum's range. Since the
> IPC layer simply static_cast<>s the int back to the enum, the resulting value
is
> undefined for values outside the enum's range.

[Wez, 2013-02-14 08:57:53]

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
File ppapi/proxy/ppapi_messages.h (right):

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
ppapi/proxy/ppapi_messages.h:1222: uint32_t /* name */,
On 2013/02/14 08:35:38, yzshen1 wrote:
> Your concern makes sense. I am okay to do it your way (for now).
> 
> However, we need to find a better way to fix the general problem. Using
integer
> as a replacement of enum doesn't seem to be a real fix:
> - It hurts readability.
> - It is still cast back to the enum type (in pepper_tcp_socket.cc). And I
think
> you rely on the 'default' branch to catch out-of-range values. There are two
> problems:
>     (1) the validation is not in the IPC layer, instead it happens long after
we
> receive the IPC message. Imagine that we have a lot of messages passing enum
> types as integers. The validation will be scattered in different parts of our
> codebase. And it relies on every user of those messages to do the validation,
> which is error-prone.
>     (2) Since the result of the cast is undefined, theoretically it can be any
> value. Therefore, it may run any branch of the switch statement. That
> uncertainty bothers me as well. :)
> 
>  
> On 2013/02/13 22:06:22, Wez wrote:
> > On 2013/02/13 08:07:14, yzshen1 wrote:
> > > On 2013/02/12 18:45:21, Wez wrote:
> > > > On 2013/02/12 17:39:58, yzshen1 wrote:
> > > > > Please use the enum type.
> > > > 
> > > > This is an IPC message definition. Using the enum type here would imply
> that
> > > > recipients can rely on the field having one of the defined enum values,
> > which
> > > is
> > > > not the case.
> > > I don't understand, why the recipients shouldn't rely on the field having
> one
> > of
> > > the defined enum values?
> > 
> > Because over IPC an enum is transferred as an int. If the peer is buggy or
> > compromised then the int's value may fall outside the enum's range. Since
the
> > IPC layer simply static_cast<>s the int back to the enum, the resulting
value
> is
> > undefined for values outside the enum's range.
> 

Agreed; I've filed crbug.com/176110 for fixing the IPC issue.

https://chromiumcodereview.appspot.com/12220050/diff/6005/ppapi/proxy/ppapi_m...
ppapi/proxy/ppapi_messages.h:1222: uint32_t /* name */,
On 2013/02/14 08:35:38, yzshen1 wrote:
> Your concern makes sense. I am okay to do it your way (for now).
> 
> However, we need to find a better way to fix the general problem. Using
integer
> as a replacement of enum doesn't seem to be a real fix:
> - It hurts readability.
> - It is still cast back to the enum type (in pepper_tcp_socket.cc). And I
think
> you rely on the 'default' branch to catch out-of-range values.

D'oh!  Yes, I should remove the cast.

 There are two
> problems:
>     (1) the validation is not in the IPC layer, instead it happens long after
we
> receive the IPC message. Imagine that we have a lot of messages passing enum
> types as integers. The validation will be scattered in different parts of our
> codebase. And it relies on every user of those messages to do the validation,
> which is error-prone.
>     (2) Since the result of the cast is undefined, theoretically it can be any
> value. Therefore, it may run any branch of the switch statement. That
> uncertainty bothers me as well. :)

Yup.  See above.

I'll update the CL to remove the cast - if you're otherwise happy with it then
please wield your awesome OWNER powers. ;)

> 
>  
> On 2013/02/13 22:06:22, Wez wrote:
> > On 2013/02/13 08:07:14, yzshen1 wrote:
> > > On 2013/02/12 18:45:21, Wez wrote:
> > > > On 2013/02/12 17:39:58, yzshen1 wrote:
> > > > > Please use the enum type.
> > > > 
> > > > This is an IPC message definition. Using the enum type here would imply
> that
> > > > recipients can rely on the field having one of the defined enum values,
> > which
> > > is
> > > > not the case.
> > > I don't understand, why the recipients shouldn't rely on the field having
> one
> > of
> > > the defined enum values?
> > 
> > Because over IPC an enum is transferred as an int. If the peer is buggy or
> > compromised then the int's value may fall outside the enum's range. Since
the
> > IPC layer simply static_cast<>s the int back to the enum, the resulting
value
> is
> > undefined for values outside the enum's range.
>

[yzshen1, 2013-02-14 09:07:34]

LGTM

Thanks!

[commit-bot: I haz the power, 2013-02-15 03:07:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/12220050/15034

[commit-bot: I haz the power, 2013-02-15 03:07:57]

Presubmit check for 12220050-15034 failed and returned exit status 1.

INFO:root:Found 28 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/ppapi/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    ppapi/proxy/ppapi_messages.h

Missing PPAPI IDL for stable interface:

***************
  ppapi/c/pp_macros.h
***************

Presubmit checks took 4.2s to calculate.

[Wez, 2013-02-15 03:21:49]

Cris, can you review the IPC message & handling, plz?

[Tom Sepez, 2013-02-15 22:48:28]

Messages LGTM.

[Wez, 2013-02-15 22:49:13]

On 2013/02/15 22:48:28, Tom Sepez wrote:
> Messages LGTM.

Thanks!

[commit-bot: I haz the power, 2013-02-15 22:51:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/12220050/18002

[commit-bot: I haz the power, 2013-02-15 22:51:32]

Presubmit check for 12220050-18002 failed and returned exit status 1.

INFO:root:Found 28 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/ppapi/PRESUBMIT.py

** Presubmit ERRORS **
Missing PPAPI IDL for stable interface:

***************
  ppapi/c/pp_macros.h
***************

Presubmit checks took 4.4s to calculate.

[Cris Neckar, 2013-02-19 20:00:15]

Sorry I missed this guy, I was on vacation :)

[commit-bot: I haz the power, 2013-02-21 03:43:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/12220050/18002

[commit-bot: I haz the power, 2013-02-21 03:44:08]

Presubmit check for 12220050-18002 failed and returned exit status 1.

INFO:root:Found 28 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/ppapi/PRESUBMIT.py

** Presubmit Warnings **
PPAPI_RELEASE has changed

***************
#define PPAPI_RELEASE 27
***************

Presubmit checks took 4.2s to calculate.

[commit-bot: I haz the power, 2013-02-21 07:25:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/12220050/18002

[commit-bot: I haz the power, 2013-02-21 07:25:33]

Presubmit check for 12220050-18002 failed and returned exit status 1.

INFO:root:Found 28 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/ppapi/PRESUBMIT.py

** Presubmit Warnings **
PPAPI_RELEASE has changed

***************
#define PPAPI_RELEASE 27
***************

Presubmit checks took 4.8s to calculate.

[commit-bot: I haz the power, 2013-02-22 03:49:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/12220050/18002

[commit-bot: I haz the power, 2013-02-22 03:49:57]

Failed to apply patch for ppapi/thunk/thunk.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ppapi/thunk/thunk.h
  Hunk #1 FAILED at 68.
  1 out of 1 hunk FAILED -- saving rejects to file ppapi/thunk/thunk.h.rej

Patch:       ppapi/thunk/thunk.h
Index: ppapi/thunk/thunk.h
diff --git a/ppapi/thunk/thunk.h b/ppapi/thunk/thunk.h
index
3fbe66576d77ea9079a0d1328096d48a7ec4addc..d9af97ee3d8f37097d4cdcc6db87d40eba0d6e8b
100644
--- a/ppapi/thunk/thunk.h
+++ b/ppapi/thunk/thunk.h
@@ -68,6 +68,8 @@ PPAPI_THUNK_EXPORT const PPB_Instance_Private_0_1*
     GetPPB_Instance_Private_0_1_Thunk();
 PPAPI_THUNK_EXPORT const PPB_TCPServerSocket_Private_0_1*
     GetPPB_TCPServerSocket_Private_0_1_Thunk();
+PPAPI_THUNK_EXPORT const PPB_TCPSocket_Private_0_5*
+    GetPPB_TCPSocket_Private_0_5_Thunk();
 PPAPI_THUNK_EXPORT const PPB_TCPSocket_Private_0_3*
     GetPPB_TCPSocket_Private_0_3_Thunk();
 PPAPI_THUNK_EXPORT const PPB_UDPSocket_Private_0_2*

[commit-bot: I haz the power, 2013-02-22 06:04:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/12220050/33001

[commit-bot: I haz the power, 2013-02-22 11:17:10]

Change committed as 184087