net: merge two versions of SetTCPNoDelay() function into one

net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from tcp_socket_util.cc and DisableNagle() function
from tcp_socket_win.cc, into a single slightly improved one in tcp_socket.cc
with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org

Committed: https://crrev.com/2d1f2621d8e6dd10feba6cab380fb46d60cb3098
Cr-Commit-Position: refs/heads/master@{#378129}

[tfarina, 2016-02-26 15:47:35]

Description was changed from

==========
net: merge to versions of SetTCPNoDelay() function into one

This patch merges the function from tcp_socket_posix.cc and the one from
tcp_socket_util.cc into a single slightly improved one in tcp_socket.cc.

The third one, DisableNagle() from tcp_socket_win.cc will be merged into
this new one later on.

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

to

==========
net: merge two versions of SetTCPNoDelay() function into one

This patch merges the function from tcp_socket_posix.cc and the one from
tcp_socket_util.cc into a single slightly improved one in tcp_socket.cc.

The third one, DisableNagle() from tcp_socket_win.cc will be merged into
this new one later on.

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

[tfarina, 2016-02-26 15:51:09]

I think it will be easier to get this in (and thus reviewed) if we leave the
DisableNagle() function from tcp_socket_win.cc for a follow up. Do you prefer
that I
do this change now? WDYT?

[eroman, 2016-02-26 17:00:14]

This change will make more sense if it handles the Windows version as well,
since that will motivate whether it makes sense to put it in a new
tcp_socket.cc, or whether there should just be two separate implementations (one
in tcp_socket_posix.cc, one in tcp_socket_win.cc).

(1) In the generic header instead of:
        int fd
     Use:
        SocketDescriptor socket

The difference is that on Windows it uses the type SOCKET instead of int.

(2) Not sure if we should have a separate implementation for Windows or not.
Probably fine having one unified. My main concern is that MSDN describes it as
taking a BOOL not an int. In practice BOOL is a typedef of int so should be
identical, and I don't think this would ever change. But would be good to either
drop a comment to that effect or have an ifdef that uses BOOL on Windows.

[tfarina, 2016-02-26 18:25:49]

On 2016/02/26 17:00:14, eroman wrote:
> This change will make more sense if it handles the Windows version as well,
> since that will motivate whether it makes sense to put it in a new
> tcp_socket.cc, or whether there should just be two separate implementations
(one
> in tcp_socket_posix.cc, one in tcp_socket_win.cc).
> 
That is true. I think looking at it again it makes more sense to do it now
rather than later, I'm just not sure what to do with its huge documentation.

> (1) In the generic header instead of:
>         int fd
>      Use:
>         SocketDescriptor socket
> 
Yup. Done. I almost forgot about doing it.

> The difference is that on Windows it uses the type SOCKET instead of int.
> 
> (2) Not sure if we should have a separate implementation for Windows or not.
> Probably fine having one unified. My main concern is that MSDN describes it as
> taking a BOOL not an int. In practice BOOL is a typedef of int so should be
> identical, and I don't think this would ever change. But would be good to
either
> drop a comment to that effect or have an ifdef that uses BOOL on Windows.
I went with #ifdef, hopefully it is clearer.

[eroman, 2016-02-26 20:21:24]

Update the CL description:

> The third one, DisableNagle() from tcp_socket_win.cc will be merged into
> this new one later on.

(This has been addressed now).

Otherwise LGTM

[tfarina, 2016-02-26 20:28:49]

Description was changed from

==========
net: merge two versions of SetTCPNoDelay() function into one

This patch merges the function from tcp_socket_posix.cc and the one from
tcp_socket_util.cc into a single slightly improved one in tcp_socket.cc.

The third one, DisableNagle() from tcp_socket_win.cc will be merged into
this new one later on.

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

to

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from
tcp_socket_util.cc and DisableNagle() function from tcp_socket_win.cc, into a
single slightly improved one in tcp_socket.cc with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

[tfarina, 2016-02-26 20:29:05]

Description was changed from

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from
tcp_socket_util.cc and DisableNagle() function from tcp_socket_win.cc, into a
single slightly improved one in tcp_socket.cc with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

to

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from
tcp_socket_util.cc and DisableNagle() function
from tcp_socket_win.cc, into a single slightly improved one in tcp_socket.cc
with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

[tfarina, 2016-02-26 20:29:22]

Description was changed from

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from
tcp_socket_util.cc and DisableNagle() function
from tcp_socket_win.cc, into a single slightly improved one in tcp_socket.cc
with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

to

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from tcp_socket_util.cc and DisableNagle() function
from tcp_socket_win.cc, into a single slightly improved one in tcp_socket.cc
with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

[tfarina, 2016-02-26 20:29:38]

Description was changed from

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from tcp_socket_util.cc and DisableNagle() function
from tcp_socket_win.cc, into a single slightly improved one in tcp_socket.cc
with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

to

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from tcp_socket_util.cc and DisableNagle() function
from tcp_socket_win.cc, into a single slightly improved one in tcp_socket.cc
with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

[tfarina, 2016-02-26 20:30:38]

On 2016/02/26 20:21:24, eroman wrote:
> Update the CL description:
> 
Tried :)

> > The third one, DisableNagle() from tcp_socket_win.cc will be merged into
> > this new one later on.
> 
> (This has been addressed now).
> 
> Otherwise LGTM

Thanks for the direction!

[eroman, 2016-02-26 20:40:04]

lgtm

[tfarina, 2016-02-27 08:21:26]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2016-02-27 08:21:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1728853006/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1728853006/60001

[commit-bot: I haz the power, 2016-02-27 08:45:34]

Description was changed from

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from tcp_socket_util.cc and DisableNagle() function
from tcp_socket_win.cc, into a single slightly improved one in tcp_socket.cc
with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

to

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from tcp_socket_util.cc and DisableNagle() function
from tcp_socket_win.cc, into a single slightly improved one in tcp_socket.cc
with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

[commit-bot: I haz the power, 2016-02-27 08:45:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-02-27 08:47:21]

Description was changed from

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from tcp_socket_util.cc and DisableNagle() function
from tcp_socket_win.cc, into a single slightly improved one in tcp_socket.cc
with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org
==========

to

==========
net: merge DisableNagle() with two other SetTCPNoDelay() implementations

This patch merges SetTCPNoDelay() function from tcp_socket_posix.cc,
SetTCPNoDelay() function from tcp_socket_util.cc and DisableNagle() function
from tcp_socket_win.cc, into a single slightly improved one in tcp_socket.cc
with the following prototype:

bool SetTCPNoDelay(SocketDescriptor socket, bool no_delay);

BUG=None
TEST=net_unittests
R=eroman@chromium.org

Committed: https://crrev.com/2d1f2621d8e6dd10feba6cab380fb46d60cb3098
Cr-Commit-Position: refs/heads/master@{#378129}
==========

[commit-bot: I haz the power, 2016-02-27 08:47:22]

Patchset 4 (id:??) landed as
https://crrev.com/2d1f2621d8e6dd10feba6cab380fb46d60cb3098
Cr-Commit-Position: refs/heads/master@{#378129}

[nektarios, 2016-02-29 21:03:01]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1744153003/ by nektar@chromium.org.

The reason for reverting is: net-unittests failure on Mac.
https://build.chromium.org/p/chromium.mac/builders/Mac10.9%20Tests/builds/169....