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..f4303160b4090f7c372179b34b510adf847aee0c
--- /dev/null
+++ b/net/socket/tcp_server_socket.cc
@@ -0,0 +1,110 @@
+// 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;
+
+#if defined(WIN)
+  result = socket_.SetExclusiveAddrUse();

[akalin, 2013/08/26 23:48:51]

ideally this platform difference would be abstracted out. Are you planning to do
this? (perhaps in a future CL?)

[yzshen1, 2013/08/27 17:36:56]

Yeah, I wish to do it later. I added a TODO.

[yzshen1, 2013/08/27 22:54:26]

After a second thought, I just did it in this CL because it is trivial.

On 2013/08/27 17:36:56, yzshen1 wrote:

+#elif defined(POSIX)
+  result = socket_.SetAddressReuse(true);
+#endif
+  if (result != OK ||
+      (result = socket_.Bind(address)) != OK ||

[akalin, 2013/08/26 23:48:51]

assignments in conditionals are against the style guide, or at least not usual,
I believe.

You're doing this to avoid repeating the Close() call, right?

What about this:

Define a Swap() function for TCPSocket. Then use a local variable socket in this
function, and swap it with socket_ only at the very end (with OK). Assuming
TCPSocket's destructor does the same cleanup as Close(), this should work (and
be arguably more elegant).

[yzshen1, 2013/08/27 17:36:56]

It doesn't seem to worth the effort to add Swap() just for avoiding multiple
Close()s.

If you don't mind, I will just do multiple Closes(). :)

+      (result = socket_.Listen(backlog)) != 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_);
+
+  pending_accept_ = true;
+
+  // 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(
+      base::Bind(&TCPServerSocket::OnAcceptCompleted, base::Unretained(this),

[akalin, 2013/08/26 23:48:51]

you're doing Bind() twice!

[yzshen1, 2013/08/27 17:36:56]

Oops. :)

On 2013/08/26 23:48:51, akalin wrote:

+      socket, callback));

[akalin, 2013/08/26 23:48:51]

line up socket with &TCPServerSocket

[yzshen1, 2013/08/27 17:36:56]

Done.

+  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 OnAcceptCompleted()
+    // ourselves in order to do the conversion from |accepted_socket_| to
+    // |socket|. We don't want to run |callback| in this case, so we use a null
+    // CompletionCallback.
+    OnAcceptCompleted(socket, CompletionCallback(), result);
+  }
+
+  return result;
+}
+
+void TCPServerSocket::OnAcceptCompleted(
+    scoped_ptr<StreamSocket>* output_accepted_socket,
+    const CompletionCallback& forward_callback,
+    int result) {
+  do {
+    if (result != OK)

[akalin, 2013/08/26 23:48:51]

Can you decomp the body of the do/while loop into a separate function instead?
Something like:

int DoAccept(int result, scoped_ptr<StreamSocket>* accepted_socket) {
  ...
  if (result != OK)
    return result;
  ...
  *accepted_socket = ...;
  return OK;
}

Then you can do:

result = DoAccept(result, accepted_socket);
...
if (!forward_callback.is_null())
  forward_callback.Run(result);

In fact, you can probably work this a bit more and call DoAccept() from the
non-IO_PENDING case in Accept() directly.

[yzshen1, 2013/08/27 17:36:56]

Thanks, this is a good idea.
Done.

On 2013/08/26 23:48:51, akalin wrote:

+      break;
+
+    scoped_ptr<TCPClientSocket> client_socket(new TCPClientSocket(
+        AddressList(accepted_address_),
+        accepted_socket_->net_log().net_log(),
+        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 = accepted_socket_->Release();
+#elif defined(OS_POSIX)
+    int raw_socket = 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 |accepted_socket_| to close it.
+      accepted_socket_->Adopt(raw_socket);
+      break;
+    }
+
+    *output_accepted_socket = client_socket.Pass();
+  } while (false);
+
+  accepted_socket_.reset();
+  pending_accept_ = false;
+  if (!forward_callback.is_null())
+    forward_callback.Run(result);
+}
+
+}  // namespace net