Windows only: Move client socket functionality from TCPClientSocket into TCPSocket.

net/socket/tcp_socket_win.h

Index: net/socket/tcp_socket_win.h
diff --git a/net/socket/tcp_socket_win.h b/net/socket/tcp_socket_win.h
index 044e5e058520b1aea6a6dfc8337b1c0c92b643e7..0bede404164ec06dd3ca82beee23d4d12c0a9e67 100644
--- a/net/socket/tcp_socket_win.h
+++ b/net/socket/tcp_socket_win.h
@@ -9,22 +9,21 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/message_loop/message_loop.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/win/object_watcher.h"
 #include "net/base/address_family.h"
 #include "net/base/completion_callback.h"
+#include "net/base/ip_endpoint.h"
 #include "net/base/net_export.h"
 #include "net/base/net_log.h"
 
 namespace net {
 
-class IPEndPoint;
+class AddressList;
+class IOBuffer;
 
-// TODO(yzshen): This class is incomplete. TCP client operations (Connect/Read/
-// Write/etc.) will be added. And TCPClientSocket will be changed to be a
-// wrapper around TCPSocket.
 class NET_EXPORT TCPSocketWin : NON_EXPORTED_BASE(public base::NonThreadSafe),
                                 public base::win::ObjectWatcher::Delegate  {
  public:
@@ -33,37 +32,104 @@ class NET_EXPORT TCPSocketWin : NON_EXPORTED_BASE(public base::NonThreadSafe),
 
   int Create(AddressFamily family);

[wtc, 2013/09/06 21:55:42]

"Open" is a better name. "Create" is usually a method that creates an
object and returns it.

[yzshen1, 2013/09/09 23:14:48]

I slightly prefer Create(). The document of OS API socket() uses "create a
socket" instead of "open". What do you think?

On 2013/09/06 21:55:42, wtc wrote:

[wtc, 2013/09/12 01:27:16]

The socket() function is more similar to a static Create
method:

  class TCPSocketWin {
   public:
    static TCPSocketWin* Create(AddressFamily family);
  ...

But the function you specified here is invoked on an
existing TCPSocketWin object, so "Create" sounds strange.
Also, "Open" would match the "Close" method, which performs
the reverse operation.

[yzshen1, 2013/09/12 20:07:50]

Done. I have changed the name to Open.

On 2013/09/12 01:27:16, wtc wrote:

   // Takes ownership of |socket|.
-  int Adopt(SOCKET socket);
-  // Returns a socket descriptor. The ownership is transferred to the caller.
-  SOCKET Release();
+  int AdoptConnectedSocket(SOCKET socket, const IPEndPoint& peer_address);
+
   int Bind(const IPEndPoint& address);
   int GetLocalAddress(IPEndPoint* address) const;
+
   int Listen(int backlog);
   int Accept(scoped_ptr<TCPSocketWin>* socket,
              IPEndPoint* address,
              const CompletionCallback& callback);
+
+  int Connect(const IPEndPoint& address, const CompletionCallback& callback);
+  bool IsConnected() const;
+  bool IsConnectedAndIdle() const;
+  int GetPeerAddress(IPEndPoint* address) const;

[wtc, 2013/09/06 21:55:42]

I suggest declaring GetPeerAddress next to the related GetLocalAddress function.

[yzshen1, 2013/09/09 23:14:48]

Done.

+
+  // Multiple outstanding requests are not supported.
+  // Full duplex mode (reading and writing at the same time) is supported.
+  int Read(IOBuffer* buf, int buf_len, const CompletionCallback& callback);
+  int Write(IOBuffer* buf, int buf_len, const CompletionCallback& callback);
+
   int SetDefaultOptionsForServer();
+  int SetDefaultOptionsForClient();
   int SetExclusiveAddrUse();
+  bool SetReceiveBufferSize(int32 size);
+  bool SetSendBufferSize(int32 size);
+  bool SetKeepAlive(bool enable, int delay);
+  bool SetNoDelay(bool no_delay);
+
   void Close();
 
+  bool UsingTCPFastOpen() const;
+  bool IsValid() const { return socket_ != INVALID_SOCKET; }

[wtc, 2013/09/06 21:55:42]

You can consider renaming this method "IsOpen".

[yzshen1, 2013/09/09 23:14:48]

Please see my comment about Create(). Thanks!

On 2013/09/06 21:55:42, wtc wrote:

+
+  // Marks the start/end of a series of connect attempts for logging purpose.
+  //
+  // TCPClientSocket may attempt to connect to multiple addresses until it

[wtc, 2013/09/06 21:55:42]

TCPClientSocket => TCPSocket

or did you really mean TCPClientSocket?

Do the StartLoggingMultipleConnectAttempts and EndLoggingMultipleConnectAttempts
methods belong in this class?

[yzshen1, 2013/09/09 23:14:48]

I did really mean TCPClientSocket. I agree that it seems awkward.
I would like to talk with you in person and listen to your suggestions.

On 2013/09/06 21:55:42, wtc wrote:

+  // succeeds in establishing a connection. The corresponding log will be

[wtc, 2013/09/06 21:55:42]

Nit: will be => will have ?

[yzshen1, 2013/09/09 23:14:48]

Done.

+  // multiple NetLog::TYPE_TCP_CONNECT_ATTEMPT entries nested within a
+  // NetLog::TYPE_TCP_CONNECT. These methods set the start/end of
+  // NetLog::TYPE_TCP_CONNECT.
+  void StartLoggingMultipleConnectAttempts(const AddressList& addresses);
+  void EndLoggingMultipleConnectAttempts(int net_error);
+
   const BoundNetLog& net_log() const { return net_log_; }
 
+ private:
+  class Core;
+
   // base::ObjectWatcher::Delegate implementation.
   virtual void OnObjectSignaled(HANDLE object) OVERRIDE;
 
- private:
   int AcceptInternal(scoped_ptr<TCPSocketWin>* socket,
                      IPEndPoint* address);
 
+  int DoConnect();
+  void DoConnectComplete(int result);
+
+  void LogConnectStart(const AddressList& addresses);
+  void LogConnectCompletion(int net_error);
+
+  int DoRead(IOBuffer* buf, int buf_len, const CompletionCallback& callback);
+  void DoReadCallback(int rv);
+  void DoWriteCallback(int rv);
+  void DidCompleteConnect();
+  void DidCompleteWrite();
+  void DidSignalRead();
+
   SOCKET socket_;
-  HANDLE socket_event_;
 
+  HANDLE accept_event_;
   base::win::ObjectWatcher accept_watcher_;
 
   scoped_ptr<TCPSocketWin>* accept_socket_;
   IPEndPoint* accept_address_;
   CompletionCallback accept_callback_;
 
+  // The various states that the socket could be in.
+  bool waiting_connect_;
+  bool waiting_read_;
+  bool waiting_write_;
+
+  // The core of the socket that can live longer than the socket itself. We pass
+  // resources to the Windows async IO functions and we have to make sure that
+  // they are not destroyed while the OS still references them.
+  scoped_refptr<Core> core_;
+
+  // External callback; called when connect or read is complete.
+  CompletionCallback read_callback_;
+
+  // External callback; called when write is complete.
+  CompletionCallback write_callback_;
+
+  IPEndPoint peer_address_;
+  // The OS error that a connect attempt last completed with.
+  int connect_os_error_;
+
+  bool logging_multiple_connect_attempts_;
+
   BoundNetLog net_log_;
 
   DISALLOW_COPY_AND_ASSIGN(TCPSocketWin);
@@ -72,3 +138,4 @@ class NET_EXPORT TCPSocketWin : NON_EXPORTED_BASE(public base::NonThreadSafe),
 }  // namespace net
 
 #endif  // NET_SOCKET_TCP_SOCKET_WIN_H_
+