Refactor CastSocket code for the following:

chrome/browser/extensions/api/cast_channel/cast_socket.h

Index: chrome/browser/extensions/api/cast_channel/cast_socket.h
===================================================================
--- chrome/browser/extensions/api/cast_channel/cast_socket.h	(revision 236353)
+++ chrome/browser/extensions/api/cast_channel/cast_socket.h	(working copy)
@@ -115,10 +115,6 @@
   // is in cert error state.
   // Returns whether certificate is successfully extracted.
   virtual bool ExtractPeerCert(std::string* cert);
-  // Sends a challenge request to the receiver.
-  virtual int SendAuthChallenge();
-  // Reads auth challenge reply from the receiver.
-  virtual int ReadAuthChallengeReply();
   // Verifies whether the challenge reply received from the peer is valid:
   // 1. Signature in the reply is valid.
   // 2. Certificate is rooted to a trusted CA.
@@ -147,6 +143,22 @@
     CONN_STATE_AUTH_CHALLENGE_REPLY_COMPLETE,
   };
 
+  // Internal write states.
+  enum WriteState {
+    WRITE_STATE_NONE,
+    WRITE_STATE_WRITE,
+    WRITE_STATE_WRITE_COMPLETE,
+    WRITE_STATE_ERROR,
+  };
+
+  // Internal read states.
+  enum ReadState {
+    READ_STATE_NONE,
+    READ_STATE_READ,
+    READ_STATE_READ_COMPLETE,
+    READ_STATE_ERROR,
+  };
+
   /////////////////////////////////////////////////////////////////////////////
   // Following methods work together to implement the following flow:
   // 1. Create a new TCP socket and connect to it
@@ -157,9 +169,9 @@
   // 5. If SSL socket is connected successfully, and if protocol is casts://
   //    then issue an auth challenge request.
   // 6. Validate the auth challenge response.
-
+  //
   // Main method that performs connection state transitions.
-  int DoConnectLoop(int result);
+  void DoConnectLoop(int result);
   // Each of the below Do* method is executed in the corresponding
   // connection state. For e.g. when connection state is TCP_CONNECT
   // DoTcpConnect is called, and so on.
@@ -172,33 +184,53 @@
   int DoAuthChallengeReplyComplete(int result);
   /////////////////////////////////////////////////////////////////////////////
 
-  // Callback method for callbacks from underlying sockets.
-  void OnConnectComplete(int result);
-
-  // Callback method when a challenge request is sent or a reply is received.
-  void OnChallengeEvent(int result);
-
+  // Internal implementation of the Connect method.
+  void ConnectInternal();

[Wez, 2013/11/26 02:45:56]

So is this not part of DoConnectLoop, then?

[Munjal (Google), 2013/11/26 20:06:14]

No. But now I removed it anyway.

   // Runs the external connection callback and resets it.
   void DoConnectCallback(int result);
-
   // Verifies that the URL is a valid cast:// or casts:// URL and sets url_ to
   // the result.
   bool ParseChannelUrl(const GURL& url);

[Wez, 2013/11/26 02:45:56]

nit: Why is this mixed-in in between the Connect and write impl methods?

[Munjal (Google), 2013/11/26 20:06:14]

This is a helper used by connection methods.

[Wez, 2013/11/27 04:15:19]

OK; I think it would help to clarify that, e.g. "Called by <connect method> to
verify |url| and, if valid, assign it to |url_|. Returns true if |url| is a
valid cast[s]:// URL."

 
-  // Sends the given |message| and invokes the given callback when done.
-  int SendMessageInternal(const CastMessage& message,
-                          const net::CompletionCallback& callback);
+  /////////////////////////////////////////////////////////////////////////////
+  // Following methods work together to implement write flow.
+  //
+  // Main method that performs write flow state transitions.
+  void DoWriteLoop(int result);
+  // Each of the below Do* method is executed in the corresponding
+  // write state. For e.g. when write state is WRITE_STATE_WRITE_COMPLETE
+  // DowriteComplete is called, and so on.
+  int DoWrite();
+  int DoWriteComplete(int result);
+  int DoWriteError(int result);
+  /////////////////////////////////////////////////////////////////////////////
 
-  // Writes data to the socket from the WriteRequest at the head of the queue.
-  // Calls OnWriteData() on completion.
-  int WriteData();
-  void OnWriteData(int result);
+  // Posts a task to send the given message.

[Wez, 2013/11/26 02:45:56]

Why are these methods mixed-in between the write & read impls?

[Munjal (Google), 2013/11/26 20:06:14]

These are all helper methods using during write flow.

[Wez, 2013/11/27 04:15:19]

OK; as above, I think it'd help to have a block comment introducing them as
such. (But that's a nit ;)

+  void PostTaskToSendCastMessage(const CastMessage& message,
+                                 const net::CompletionCallback& callback);
+  // Adds |message| to the write queue and starts the write loop if needed.
+  void SendCastMessageInternal(const CastMessage& message,
+                               const net::CompletionCallback& callback);
+  // Removes all messages from the write queue.
+  void ClearWriteQueue();
 
-  // Reads data from the socket into one of the read buffers. Calls
-  // OnReadData() on completion.
-  int ReadData();
-  void OnReadData(int result);
+  /////////////////////////////////////////////////////////////////////////////
+  // Following methods work together to implement read flow.
+  //
+  // Main method that performs write flow state transitions.
+  void DoReadLoop(int result);
+  // Each of the below Do* method is executed in the corresponding
+  // write state. For e.g. when write state is READ_STATE_READ_COMPLETE
+  // DoReadComplete is called, and so on.
+  int DoRead();
+  int DoReadComplete(int result);
+  int DoReadError(int result);
+  /////////////////////////////////////////////////////////////////////////////
 
+  // Posts a task to start the read loop.
+  void PostTaskToStartReadLoop();
+  // Stars the read loop if not already started.
+  void StartReadLoop();
   // Processes the contents of header_read_buffer_ and returns true on success.
   bool ProcessHeader();
   // Processes the contents of body_read_buffer_ and returns true on success.
@@ -228,16 +260,7 @@
   bool auth_required_;
   // The IP endpoint of the peer.
   net::IPEndPoint ip_endpoint_;
-  // The last error encountered by the channel.
-  ChannelError error_state_;
-  // The current status of the channel.
-  ReadyState ready_state_;
 
-  // True when there is a write callback pending.
-  bool write_callback_pending_;
-  // True when there is a read callback pending.
-  bool read_callback_pending_;
-
   // IOBuffer for reading the message header.
   scoped_refptr<net::GrowableIOBuffer> header_read_buffer_;
   // IOBuffer for reading the message body.
@@ -252,8 +275,6 @@
   // The NetLog source for this service.
   net::NetLog::Source net_log_source_;
 
-  // Next connection state to transition to.
-  ConnectionState next_state_;
   // Owned ptr to the underlying TCP socket.
   scoped_ptr<net::TCPClientSocket> tcp_socket_;
   // Owned ptr to the underlying SSL socket.
@@ -269,6 +290,17 @@
   // Callback invoked when the socket is connected.
   net::CompletionCallback connect_callback_;
 
+  // Connection flow state machine state.
+  ConnectionState connect_state_;
+  // Write flow state machine state.
+  WriteState write_state_;
+  // Read flow state machine state.
+  ReadState read_state_;
+  // The last error encountered by the channel.
+  ChannelError error_state_;
+  // The current status of the channel.
+  ReadyState ready_state_;
+
   // Message header struct. If fields are added, be sure to update
   // kMessageHeaderSize in the .cc.
   struct MessageHeader {
@@ -301,9 +333,6 @@
   // being written.
   std::queue<WriteRequest> write_queue_;
 
-  // Used to protect against DoConnectLoop() re-entrancy.
-  bool in_connect_loop_;
-
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest, TestCastURLs);
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest, TestRead);
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest, TestReadMany);