Cleanup the Win overlapped reads vs. non-blocking reads by removing the overlapped read code path

Cleaned up the Overlapped Reads field trial where we
compared the existing method for reading from sockets on
Windows (using overlapped I/O) with using non-blocking I/O.  
This was the "OverlappedReadImpact" field trial that started
in November 2012 and ended in June of 2013.

The original patch for implementing the trial was here:
https://chromiumcodereview.appspot.com/10916016

The non-blocking I/O code path resolved the issue we were
seeing in lab testing but did not show a marked improvement
in the field (nor did it show a degradation).  The results
for 3 months of data from the field trial showed effectively
no change in the page load time or HTTP Job times
(differences are within the noise variation).

Since the non-blocking code path is a little cleaner,
doesn't rely on growing buffer sizes and fixed the issue we
were seeing in the lab, the decision was made to keep the
non-blocking implementation and eliminate the overlapped
read code path.

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

[Ryan Sleevi, 2013-08-06 19:43:38]

lgtm

https://codereview.chromium.org/22188002/diff/1/net/socket/tcp_client_socket_...
File net/socket/tcp_client_socket_win.cc (right):

https://codereview.chromium.org/22188002/diff/1/net/socket/tcp_client_socket_...
net/socket/tcp_client_socket_win.cc:798: FD_READ | FD_CLOSE);
nit: alignment

https://codereview.chromium.org/22188002/diff/1/net/socket/tcp_client_socket_...
net/socket/tcp_client_socket_win.cc:807:
CreateNetLogSocketErrorCallback(net_error, os_error));
nit: Style is wrong here, per
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

NetLog::TYPE_SOCKET_READ_ERROR should be on a new line

[rvargas (doing something else), 2013-08-06 20:14:36]

was this up for review? (if so, I missed the email)

[Pat Meenan, 2013-08-06 20:58:45]

On 2013/08/06 20:14:36, rvargas wrote:
> was this up for review? (if so, I missed the email)

Yes please.  It's entirely possible that I messed up and an e-mail didn't go out
(wasn't sure if adding reviewers automatically triggered an email).

[rvargas (doing something else), 2013-08-06 21:43:31]

lgtm

https://codereview.chromium.org/22188002/diff/1/net/socket/tcp_client_socket_...
File net/socket/tcp_client_socket_win.cc (right):

https://codereview.chromium.org/22188002/diff/1/net/socket/tcp_client_socket_...
net/socket/tcp_client_socket_win.cc:843: // Since Run may result in Write being
called,
nit: This should still fit in one line.

[Pat Meenan, 2013-08-07 13:54:17]

https://codereview.chromium.org/22188002/diff/1/net/socket/tcp_client_socket_...
File net/socket/tcp_client_socket_win.cc (right):

https://codereview.chromium.org/22188002/diff/1/net/socket/tcp_client_socket_...
net/socket/tcp_client_socket_win.cc:798: FD_READ | FD_CLOSE);
On 2013/08/06 19:43:38, Ryan Sleevi wrote:
> nit: alignment

Fixed

https://codereview.chromium.org/22188002/diff/1/net/socket/tcp_client_socket_...
net/socket/tcp_client_socket_win.cc:807:
CreateNetLogSocketErrorCallback(net_error, os_error));
On 2013/08/06 19:43:38, Ryan Sleevi wrote:
> nit: Style is wrong here, per
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...
> 
> NetLog::TYPE_SOCKET_READ_ERROR should be on a new line

Fixed

https://codereview.chromium.org/22188002/diff/1/net/socket/tcp_client_socket_...
net/socket/tcp_client_socket_win.cc:843: // Since Run may result in Write being
called,
On 2013/08/06 21:43:31, rvargas wrote:
> nit: This should still fit in one line.

Thanks.  Looks like my guidelines in my editor are off by one.  Fixed.

[Pat Meenan, 2013-08-12 16:59:33]

stevet@chromium.org: Please review changes in 

chrome/browser/chrome_browser_field_trials_desktop.cc

The net code itself has already been looked at, just need you to look over the
removal of the overlapped read field trial.

[Ryan Sleevi, 2013-08-22 02:10:40]

On 2013/08/12 16:59:33, Pat Meenan wrote:
> stevet@chromium.org: Please review changes in 
> 
> chrome/browser/chrome_browser_field_trials_desktop.cc
> 
> The net code itself has already been looked at, just need you to look over the
> removal of the overlapped read field trial.

Ping on this?

[SteveT, 2013-08-26 11:22:29]

Alexei, could you take a look?

[Pat Meenan, 2013-08-26 12:29:53]

jhawkins@chromium.org: Please review changes in
chrome/renderer/page_load_histograms.cc (it doesn't look like there are more
specific OWNERS available).

This CL is removing the field trial and the renderer changes specifically are
removing the UMA histograms that were specific to the field trial.

[Alexei Svitkine (slow), 2013-08-26 13:32:33]

LGTM!

[James Hawkins, 2013-08-26 17:23:20]

lgtm

[commit-bot: I haz the power, 2013-08-26 17:24:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pmeenan@chromium.org/22188002/13001

[commit-bot: I haz the power, 2013-08-26 21:05:18]

Change committed as 219607