Net: Add NET_API to the UDP code.

Net: Add NET_API to the UDP code.

BUG=76997
TEST=NONE
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86180

[Sergey Ulanov, 2011-05-18 22:54:07]

I think you also need to add NET_API for UDPSocket class.

http://codereview.chromium.org/7042019/diff/1/net/udp/udp_client_socket.h
File net/udp/udp_client_socket.h (right):

http://codereview.chromium.org/7042019/diff/1/net/udp/udp_client_socket.h#new...
net/udp/udp_client_socket.h:18: class NET_TEST UDPClientSocket : public
DatagramClientSocket {
Why is this NET_TEST instead of NET_API?

[rvargas (doing something else), 2011-05-19 22:02:27]

Regarding UDPSocket, that means that there are no unit tests for them...

I'll open a bug about it, and add NET_API as soon as I see the failure :)

http://codereview.chromium.org/7042019/diff/1/net/udp/udp_client_socket.h
File net/udp/udp_client_socket.h (right):

http://codereview.chromium.org/7042019/diff/1/net/udp/udp_client_socket.h#new...
net/udp/udp_client_socket.h:18: class NET_TEST UDPClientSocket : public
DatagramClientSocket {
On 2011/05/18 22:54:07, sergeyu wrote:
> Why is this NET_TEST instead of NET_API?

Because there is nobody outside net using this code (as far as I see)

[Sergey Ulanov, 2011-05-19 23:13:26]

+mbelshe
I think it makes sense to add NET_API to all of these classes even if they are
not used outside of net: they may be needed in the future. But I may not fully
understand the trade-offs associated with NET_API. What's the reason we want to
minimize number of exported APIs?

[rvargas (doing something else), 2011-05-19 23:18:28]

Thanks.

The purpose is not really to minimize exports, it is to minimize the exposed api
of net to external consumers.

[willchan no longer on Chromium, 2011-05-19 23:46:34]

Did I get added to comment on UDP stuff? I think Sergey's planning on using
this within chrome/, so it will need NET_API eventually. Not sure about now.

On Thu, May 19, 2011 at 4:18 PM, <rvargas@chromium.org> wrote:

> Thanks.
>
> The purpose is not really to minimize exports, it is to minimize the
> exposed api
> of net to external consumers.
>
>
> http://codereview.chromium.org/7042019/
>

[rvargas (doing something else), 2011-05-19 23:59:32]

I'm basically working with what we have today, but if you guys plan to make
UDPClientSocket (for instance) part of the "official" net api at this time
that's fine.

Going a little off topic, let's consider TCPClientSocket for a second.

It has NET_API because there is exactly one user outside of net. I would argue
that what we really want to do is remove that user
(content/browser/renderer_host/p2p/socket_host_tcp.cc) and make it so that
whatever p2p does right now is instead implemented at the network layer and the
exposed interface is at a higher level than the client socket. And of course,
I'm saying this without knowing what p2p really does.

[rvargas (doing something else), 2011-05-20 22:03:28]

ping

[Sergey Ulanov, 2011-05-20 22:29:03]

LGTM. You will need LGTM from mbelshe or willchan because I'm not a src/net
owner.

On 2011/05/19 23:59:32, rvargas wrote:
> I'm basically working with what we have today, but if you guys plan to make
> UDPClientSocket (for instance) part of the "official" net api at this time
> that's fine.
I don't plan to use UDPClientSocket for P2P code (UDPServerSocket provides all
functionality we need), so I'm fine with keeping it with NET_TEST, but I was
worried about other features that may need it in the future.

> Going a little off topic, let's consider TCPClientSocket for a second.
> 
> It has NET_API because there is exactly one user outside of net. I would argue
> that what we really want to do is remove that user
> (content/browser/renderer_host/p2p/socket_host_tcp.cc) and make it so that
> whatever p2p does right now is instead implemented at the network layer and
the
> exposed interface is at a higher level than the client socket. And of course,
> I'm saying this without knowing what p2p really does.
I don't think that moving this code to net makes much sense - it depends on
renderer IPC. Most other protocols (e.g. HTTP) are handled in the browser
process, but P2P is different because it is implemented in the renderer process,
and so the renderer host needs to lower-level access to UDP/TCP sockets to
implement IPC interface. Yes, this code can be split into two parts, but I don't
think it makes much sense: we would have export some other interface from net
anyway.

[willchan no longer on Chromium, 2011-05-20 22:35:04]

On Fri, May 20, 2011 at 3:29 PM, <sergeyu@chromium.org> wrote:

> LGTM. You will need LGTM from mbelshe or willchan because I'm not a src/net
> owner.


rvargas is an owner, so your approval should be enough. rvargas: let me know
if i should look too.


>
>
> On 2011/05/19 23:59:32, rvargas wrote:
>
>> I'm basically working with what we have today, but if you guys plan to
>> make
>> UDPClientSocket (for instance) part of the "official" net api at this time
>> that's fine.
>>
> I don't plan to use UDPClientSocket for P2P code (UDPServerSocket provides
> all
> functionality we need), so I'm fine with keeping it with NET_TEST, but I
> was
> worried about other features that may need it in the future.
>
>
>  Going a little off topic, let's consider TCPClientSocket for a second.
>>
>
>  It has NET_API because there is exactly one user outside of net. I would
>> argue
>> that what we really want to do is remove that user
>> (content/browser/renderer_host/p2p/socket_host_tcp.cc) and make it so that
>> whatever p2p does right now is instead implemented at the network layer
>> and
>>
> the
>
>> exposed interface is at a higher level than the client socket. And of
>> course,
>> I'm saying this without knowing what p2p really does.
>>
> I don't think that moving this code to net makes much sense - it depends on
> renderer IPC. Most other protocols (e.g. HTTP) are handled in the browser
> process, but P2P is different because it is implemented in the renderer
> process,
> and so the renderer host needs to lower-level access to UDP/TCP sockets to
> implement IPC interface. Yes, this code can be split into two parts, but I
> don't
> think it makes much sense: we would have export some other interface from
> net
> anyway.
>
>
>
> http://codereview.chromium.org/7042019/
>

[rvargas (doing something else), 2011-05-20 23:09:13]

Thanks.

Ooops. I picked TCPClientSocket and P2P in a random way, without realizing that
the example was actually somewhat related to this code... that was not my
intention. There are other examples that I could have picked up, because the
point that I was trying to make was a philosophical one about where to draw the
line for the net interface. Sorry about that.