Revert of CHECK() if Windows non-blocking connect returns 0. (patchset #1 id:1 of https://codereview.chromium.org/1055813006/)

Reason for revert:
It's crashing in Canary builds. We have probably gained enough data now, so I'm reverting.

Original issue's description:
> CHECK() if Windows non-blocking connect returns 0.
>
> Non-blocking connect() on Windows is documented as always returning
> WSAEWOULDBLOCK or another error.
>
> There has been code in place to handle a 0 return value, but we have
> no way to test whether it is correct or not.
>
> Replace the code to handle a 0 return with a CHECK() call, to find out
> whether a 0 result really can happen or not.
>
> BUG=436634
> TEST=net_unittests
>
> Committed: https://crrev.com/2d53a9381e88eab543cc4c9fd122b0b1ed4f41d7
> Cr-Commit-Position: refs/heads/master@{#326243}

TBR=mmenke@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=436634

Review URL: https://codereview.chromium.org/1076093006

Cr-Commit-Position: refs/heads/master@{#326507}

1 files changed, 24 insertions, 19 deletions

diff --git a/net/socket/tcp_socket_win.cc b/net/socket/tcp_socket_win.cc
index a7cc7d8..2620eba 100644
--- a/net/socket/tcp_socket_win.cc
+++ b/net/socket/tcp_socket_win.cc
@@ -801,25 +801,30 @@ int TCPSocketWin::DoConnect() {
     result = connect(socket_, storage.addr, storage.addr_len);
   }
 
-  // The MSDN page for connect says:
-  //   With a nonblocking socket, the connection attempt cannot be completed
-  //   immediately. In this case, connect will return SOCKET_ERROR, and
-  //   WSAGetLastError will return WSAEWOULDBLOCK.
-  // which implies that for a nonblocking socket, connect never returns 0.
-  // It's not clear what the correct thing to do is if connect()
-  // returns 0. It's not clear whether this ever happens in practice.
-  // This CHECK() is here to verify that it doesn't.
-  // TODO(ricea): If the CHECK() fires in canary or dev, revert this
-  // CL. If it doesn't, reconsider what we want to do here long-term.
-  CHECK(result);
-
-  int os_error = WSAGetLastError();
-  if (os_error != WSAEWOULDBLOCK) {
-    LOG(ERROR) << "connect failed: " << os_error;
-    connect_os_error_ = os_error;
-    int rv = MapConnectError(os_error);
-    CHECK_NE(ERR_IO_PENDING, rv);
-    return rv;
+  if (!result) {
+    // Connected without waiting!
+    //
+    // The MSDN page for connect says:
+    //   With a nonblocking socket, the connection attempt cannot be completed
+    //   immediately. In this case, connect will return SOCKET_ERROR, and
+    //   WSAGetLastError will return WSAEWOULDBLOCK.
+    // which implies that for a nonblocking socket, connect never returns 0.
+    // It's not documented whether the event object will be signaled or not
+    // if connect does return 0.  So the code below is essentially dead code
+    // and we don't know if it's correct.
+    NOTREACHED();
+
+    if (ResetEventIfSignaled(core_->read_overlapped_.hEvent))
+      return OK;
+  } else {
+    int os_error = WSAGetLastError();
+    if (os_error != WSAEWOULDBLOCK) {
+      LOG(ERROR) << "connect failed: " << os_error;
+      connect_os_error_ = os_error;
+      int rv = MapConnectError(os_error);
+      CHECK_NE(ERR_IO_PENDING, rv);
+      return rv;
+    }
   }
 
   // TODO(ricea): Remove ScopedTracker below once crbug.com/436634 is fixed.