Move server socket functionality from TCPServerSocket into TCPSocket.

net/socket/tcp_server_socket.cc

Index: net/socket/tcp_server_socket.cc
diff --git a/net/socket/tcp_server_socket.cc b/net/socket/tcp_server_socket.cc
new file mode 100644
index 0000000000000000000000000000000000000000..99cab7d1159c68516a68322ad77ee8c99d5d095d
--- /dev/null
+++ b/net/socket/tcp_server_socket.cc
@@ -0,0 +1,120 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/socket/tcp_server_socket.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/logging.h"
+#include "build/build_config.h"
+#include "net/base/net_errors.h"
+#include "net/socket/tcp_client_socket.h"
+
+namespace net {
+
+TCPServerSocket::TCPServerSocket(NetLog* net_log, const NetLog::Source& source)
+    : socket_(net_log, source),
+      pending_accept_(false) {
+}
+
+TCPServerSocket::~TCPServerSocket() {
+}
+
+int TCPServerSocket::Listen(const IPEndPoint& address, int backlog) {
+  int result = socket_.Create(address.GetFamily());
+  if (result != OK)
+    return result;
+
+  result = socket_.SetDefaultOptionsForServer();
+  if (result != OK) {
+    socket_.Close();
+    return result;
+  }
+
+  result = socket_.Bind(address);
+  if (result != OK) {
+    socket_.Close();
+    return result;
+  }
+
+  result = socket_.Listen(backlog);
+  if (result != OK) {
+    socket_.Close();
+    return result;
+  }
+
+  return OK;
+}
+
+int TCPServerSocket::GetLocalAddress(IPEndPoint* address) const {
+  return socket_.GetLocalAddress(address);
+}
+
+int TCPServerSocket::Accept(scoped_ptr<StreamSocket>* socket,
+                            const CompletionCallback& callback) {
+  DCHECK(socket);
+  DCHECK(!callback.is_null());
+  DCHECK(!pending_accept_);

[akalin, 2013/08/29 22:08:33]

would prefer:

if (pending_accept_) {
  NOTREACHED();
  return ERR_UNEXPECTED;
}

[yzshen1, 2013/08/29 23:04:18]

I thought this is more consistent with the existing code. (I may be wrong.)

For example, TCPClientSocketWin::Read() has
  DCHECK(!waiting_read_);

The old TCPServerSocketWin::Accept() (which this CL removes) has
  DCHECK(accept_callback_.is_null());
It is equivalent to DCHECK(!pending_accept_);

By reading the code, I think TCP.*Socket expect callers to do right things:
using valid parameters and correct calling sequences. Therefore they only use
DCHECK()s. It is a design decision IMO. It would be quite some changes to
check-and-return-failure everywhere.

If you think it is okay, I am going to leave it as it is. What do you think?

On 2013/08/29 22:08:33, akalin wrote:

[akalin, 2013/08/29 23:33:26]

You know that NOTREACHED() is equivalent to DCHECK(false), not CHECK(false),
right? I feel like failing early in release mode (by returning ERR_UNEXPECTED,
which is the error code for this purpose) is worth doing here, even if other
code doesn't do it.

+
+
+  // It is safe to use base::Unretained(this). |socket_| is owned by this class,
+  // and the callback won't be run after |socket_| is destroyed.
+  CompletionCallback accept_callback =
+      base::Bind(&TCPServerSocket::OnAcceptCompleted, base::Unretained(this),
+                 socket, callback);
+  int result = socket_.Accept(&accepted_socket_, &accepted_address_,
+                              accept_callback);
+  if (result != ERR_IO_PENDING) {
+    // |accept_callback| won't be called so we need to run
+    // ConvertAcceptedSocket() ourselves in order to do the conversion from
+    // |accepted_socket_| to |socket|.
+    result = ConvertAcceptedSocket(result, socket);
+  } else {
+    pending_accept_ = true;
+  }
+
+  return result;
+}
+
+int TCPServerSocket::ConvertAcceptedSocket(
+    int result,
+    scoped_ptr<StreamSocket>* output_accepted_socket) {
+  // Make sure the TCPSocket object is destructed in any case.

[akalin, 2013/08/29 22:08:33]

destructed -> destroyed

[yzshen1, 2013/08/29 23:04:18]

Done.

+  scoped_ptr<TCPSocket> temp_accepted_socket(accepted_socket_.Pass());
+  if (result != OK)
+    return result;
+
+  scoped_ptr<TCPClientSocket> client_socket(new TCPClientSocket(
+      AddressList(accepted_address_),
+      temp_accepted_socket->net_log().net_log(),
+      temp_accepted_socket->net_log().source()));
+  // TODO(yzshen): Once we switch TCPClientSocket::AdoptSocket() to take a
+  // TCPSocket object, we don't need to do platform-specific handling.
+#if defined(OS_WIN)
+  SOCKET raw_socket = temp_accepted_socket->Release();
+#elif defined(OS_POSIX)
+  int raw_socket = temp_accepted_socket->Release();
+#endif
+  result = client_socket->AdoptSocket(raw_socket);
+  if (result != OK) {
+    // |client_socket| won't take ownership of |raw_socket| on failure.
+    // Therefore, we put it back into |temp_accepted_socket| to close it.
+    temp_accepted_socket->Adopt(raw_socket);
+    return result;
+  }
+
+  *output_accepted_socket = client_socket.Pass();
+  return OK;
+}
+
+void TCPServerSocket::OnAcceptCompleted(
+    scoped_ptr<StreamSocket>* output_accepted_socket,
+    const CompletionCallback& forward_callback,
+    int result) {
+  result = ConvertAcceptedSocket(result, output_accepted_socket);
+  pending_accept_ = false;
+  forward_callback.Run(result);
+}
+
+}  // namespace net