Move server socket functionality from TCPServerSocket into TCPSocket.

net/socket/tcp_socket_libevent.h

Index: net/socket/tcp_socket_libevent.h
diff --git a/net/socket/tcp_socket_libevent.h b/net/socket/tcp_socket_libevent.h
new file mode 100644
index 0000000000000000000000000000000000000000..2267a1960d82b83bbe4748571736f680dd9eebc8
--- /dev/null
+++ b/net/socket/tcp_socket_libevent.h
@@ -0,0 +1,68 @@
+// 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.
+
+#ifndef NET_SOCKET_TCP_SOCKET_LIBEVENT_H_
+#define NET_SOCKET_TCP_SOCKET_LIBEVENT_H_
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "base/threading/non_thread_safe.h"
+#include "net/base/address_family.h"
+#include "net/base/completion_callback.h"
+#include "net/base/net_export.h"
+#include "net/base/net_log.h"
+
+namespace net {
+
+class IPEndPoint;
+
+class NET_EXPORT TCPSocketLibevent : public base::NonThreadSafe,
+                                     public base::MessageLoopForIO::Watcher {
+ public:
+  TCPSocketLibevent(NetLog* net_log, const NetLog::Source& source);
+  virtual ~TCPSocketLibevent();
+
+  int Create(AddressFamily family);
+  // Takes ownership of |socket|.
+  int Adopt(int socket);
+  // Returns a socket file descriptor. The ownership is transferred to the
+  // caller.
+  int Release();
+  int Bind(const IPEndPoint& address);
+  int GetLocalAddress(IPEndPoint* address) const;
+  int Listen(int backlog);

[akalin, 2013/08/26 18:53:21]

can you explain again why TCPSocket needs to have server-specific methods like
Listen and Accept? I would have thought they'd stay with TCPServerSocket, which
would adopt TCPSocket's socket once Listen and Accept are needed.

[yzshen1, 2013/08/26 23:09:57]

In this discussion thread
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/UbAhFjxpDRk

I listed the following two options:
------------------------------------
option 2:
class TCPSocket(Win|Libevent) {
  // Provides both client and server operations. It is more POSIX-like.
  // Pepper will directly use this class.
};

class TCPClientSocket {
  // Wrapper around TCPSocket. Exposes a client-only interface.
 private:
  TCPSocket socket_impl_;
};
class TCPServerSocket {
  // Wrapper around TCPSocket. Exposes a server-only interface.
 private:
  TCPSocket socket_impl_;
};
-------------------------------------
and
-------------------------------------
option 3:
class TCPSocket(Win|Libevent) {
  // All the common operations before we know whether it is a client
  // or server socket.
  int Bind(const IPEndPoint& address);
  int GetLocalAddress(IPEndPoint* address);
  void AllowAddressReuse();
  ...
}

class TCPClientSocket(Win|Libevent) {
  int AdoptTCPSocket(TCPSocket* socket);
  ...
}

class TCPServerSocket(Win|Libevent) {
  int AdoptTCPSocket(TCPSocket* socket);
  ...
}
-------------------------------------

Here I am using #2 and I think you mean #3, right?

I think option #2 is better because:
In option #2, TCPSocket is on a different level than TCPClient/ServerSocket. The
interface of TCPSocket doesn't depend on that of TCPClient/ServerSocket (and
vice versa). We have the flexibility to control how "POSIX-like" it is. On the
other hand, in option #3 the three interfaces are tied together. Here is one
example:

The existing net::TCPServerSocket has the following method:
int Listen(const net::IPEndPoint& address, int backlog)
which is basically POSIX bind() + listen().

If we adopt option #3, because Bind() is an operation that can be performed on
both client and server socket, therefore Bind() belongs to TCPSocket, we will
need to provide a different signature for TCPServerSocket::Listen():
int Listen(int backlog)
Otherwise the following sequence doesn't make sense:
tcp_socket->Bind(some_addr);
tcp_server_socket->AdoptTCPSocket(tcp_socket);
tcp_server_socket->Listen(another_addr, backlog);  // |another_addr| is not
needed!

Option #2 has clear layering:
- net::TCPSocket hides OS API differences, supports watching I/O readiness and
completion callback, etc.
- net::TCP(Client|Server)Socket and Pepper socket are built on top of
net::TCPSocket, but independent of each other.

I think the design guidelines of net::TCP(Client|Server)Socket are different
from those of the Pepper socket API. net::TCP(Client|Server)Socket provide a
clear, high-level API that is sufficient for upper layers of Chrome. The Pepper
socket API tries to provide a flexible, slightly lower level API for external
developers. Option #3 tries to unify the two and will probably ends up with a
confusing mixture.

What do you think? I would be happy to listen to your suggestions and have
further discussions.

+  int Accept(scoped_ptr<TCPSocketLibevent>* socket,
+             IPEndPoint* address,
+             const CompletionCallback& callback);
+  int SetAddressReuse(bool allow);
+  void Close();
+
+  const BoundNetLog& net_log() const { return net_log_; }
+
+  // MessageLoopForIO::Watcher implementation.
+  virtual void OnFileCanReadWithoutBlocking(int fd) OVERRIDE;
+  virtual void OnFileCanWriteWithoutBlocking(int fd) OVERRIDE;
+
+ private:
+  int AcceptInternal(scoped_ptr<TCPSocketLibevent>* socket,
+                     IPEndPoint* address);
+
+  int socket_;
+
+  base::MessageLoopForIO::FileDescriptorWatcher accept_socket_watcher_;
+
+  scoped_ptr<TCPSocketLibevent>* accept_socket_;
+  IPEndPoint* accept_address_;
+  CompletionCallback accept_callback_;
+
+  BoundNetLog net_log_;
+
+  DISALLOW_COPY_AND_ASSIGN(TCPSocketLibevent);
+};
+
+}  // namespace net
+
+#endif  // NET_SOCKET_TCP_SOCKET_LIBEVENT_H_