Create dedicated class for handling wire message formatting and parsing.

extensions/browser/api/cast_channel/cast_socket.h

 // Copyright 2014 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 EXTENSIONS_BROWSER_API_CAST_CHANNEL_CAST_SOCKET_H_
 #define EXTENSIONS_BROWSER_API_CAST_CHANNEL_CAST_SOCKET_H_
 
 #include <queue>
 #include <string>
 
@@ skipping 20 matching lines @@
 class TransportSecurityState;
 }
 
 namespace extensions {
 namespace core_api {
 namespace cast_channel {
 
 class CastMessage;
 class Logger;
 struct LastErrors;
+class MessageFramer;
 
 // This class implements a channel between Chrome and a Cast device using a TCP
 // socket with SSL.  The channel may authenticate that the receiver is a genuine
 // Cast device.  All CastSocket objects must be used only on the IO thread.
 //
 // NOTE: Not called "CastChannel" to reduce confusion with the generated API
 // code.
 class CastSocket : public ApiResource {
  public:
   // Object to be informed of incoming messages and errors.  The CastSocket that
@@ skipping 97 matching lines @@
 
   // Internal read states.
   enum ReadState {
     READ_STATE_NONE,
     READ_STATE_READ,
     READ_STATE_READ_COMPLETE,
     READ_STATE_DO_CALLBACK,
     READ_STATE_ERROR,
   };
 
- protected:
-  // Message header struct. If fields are added, be sure to update
-  // header_size().  Protected to allow use of *_size() methods in unit tests.
-  struct MessageHeader {
-    MessageHeader();
-    // Sets the message size.
-    void SetMessageSize(size_t message_size);
-    // Prepends this header to |str|.
-    void PrependToString(std::string* str);
-    // Reads |header| from the beginning of |buffer|.
-    static void ReadFromIOBuffer(net::GrowableIOBuffer* buffer,
-                                 MessageHeader* header);
-    // Size (in bytes) of the message header.
-    static uint32 header_size() { return sizeof(uint32); }
-
-    // Maximum size (in bytes) of a message payload on the wire (does not
-    // include header).
-    static uint32 max_message_size() { return 65536; }
-
-    std::string ToString();
-    // The size of the following protocol message in bytes, in host byte order.
-    uint32 message_size;
-  };
-
  private:
   friend class ApiResourceManager<CastSocket>;
   friend class CastSocketTest;
   friend class TestCastSocket;
 
   static const char* service_name() { return "CastSocketManager"; }
 
   // Creates an instance of TCPClientSocket.
   virtual scoped_ptr<net::TCPClientSocket> CreateTcpSocket();
   // Creates an instance of SSLClientSocket with the given underlying |socket|.
@@ skipping 67 matching lines @@
   /////////////////////////////////////////////////////////////////////////////
 
   // Runs the external connection callback and resets it.
   void DoConnectCallback(int result);
   // Adds |message| to the write queue and starts the write loop if needed.
   void SendCastMessageInternal(const CastMessage& message,
                                const net::CompletionCallback& callback);
   void PostTaskToStartConnectLoop(int result);
   void PostTaskToStartReadLoop();
   void StartReadLoop();
-  // Parses the contents of header_read_buffer_ and sets current_message_size_
-  // to the size of the body of the message.
-  bool ProcessHeader();
-  // Parses the contents of body_read_buffer_ and sets current_message_ to
-  // the message received.
-  bool ProcessBody();
   // Closes socket, signaling the delegate that |error| has occurred.
   void CloseWithError();
   // Frees resources and cancels pending callbacks.  |ready_state_| will be set
   // READY_STATE_CLOSED on completion.  A no-op if |ready_state_| is already
   // READY_STATE_CLOSED.
   void CloseInternal();
   // Runs pending callbacks that are passed into us to notify API clients that
   // pending operations will fail because the socket has been closed.
   void RunPendingCallbacksOnClose();
   // Serializes the content of message_proto (with a header) to |message_data|.
@@ skipping 16 matching lines @@
   int channel_id_;
 
   // The IP endpoint that the the channel is connected to.
   net::IPEndPoint ip_endpoint_;
   // Receiver authentication requested for the channel.
   ChannelAuthType channel_auth_;
   // Delegate to inform of incoming messages and errors.
   Delegate* delegate_;
 
   // IOBuffer for reading the message header.
-  scoped_refptr<net::GrowableIOBuffer> header_read_buffer_;
-  // IOBuffer for reading the message body.
-  scoped_refptr<net::GrowableIOBuffer> body_read_buffer_;
-  // IOBuffer to currently read into.
-  scoped_refptr<net::GrowableIOBuffer> current_read_buffer_;
-  // The number of bytes in the current message body.
-  uint32 current_message_size_;
-  // Last message received on the socket.
-  scoped_ptr<CastMessage> current_message_;
+  scoped_refptr<net::GrowableIOBuffer> read_buffer_;
+  scoped_ptr<MessageFramer> framer_;
 
   // The NetLog for this service.
   net::NetLog* net_log_;
   // The NetLog source for this service.
   net::NetLog::Source net_log_source_;
 
   // Logger used to track multiple CastSockets. Does NOT own this object.
   scoped_refptr<Logger> logger_;
 
   // CertVerifier is owned by us but should be deleted AFTER SSLClientSocket
@@ skipping 18 matching lines @@
   // Callback invoked by |connect_timeout_timer_| to cancel the connection.
   base::CancelableClosure connect_timeout_callback_;
   // Duration to wait before timing out.
   base::TimeDelta connect_timeout_;
   // Timer invoked when the connection has timed out.
   scoped_ptr<base::Timer> connect_timeout_timer_;
   // Set when a timeout is triggered and the connection process has
   // canceled.
   bool is_canceled_;
 
+  scoped_ptr<CastMessage> current_message_;
+
   // 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_;
@@ skipping 26 matching lines @@
   std::queue<WriteRequest> write_queue_;
 
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest, TestFullSecureConnectionFlowAsync);
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest, TestRead);
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest, TestReadHeaderParseError);
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest, TestReadMany);
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest, TestWriteErrorLargeMessage);
   DISALLOW_COPY_AND_ASSIGN(CastSocket);
 };
 
+// Class for constructing and parsing CastMessage packet data.
+class MessageFramer {
+ public:
+  explicit MessageFramer(scoped_refptr<net::GrowableIOBuffer> input_buffer);

[mark a. foltz, 2014/08/28 21:45:22]

Document |input_buffer|.

[Kevin M, 2014/08/28 23:33:32]

Done.

+  ~MessageFramer();
+
+  // The number of bytes requested from the incoming data stream.

[mark a. foltz, 2014/08/28 21:45:22]

The number of requested from |input_buffer| to complete the CastMessage being
read.

[Kevin M, 2014/08/28 23:33:32]

An improvement, thanks.

+  size_t BytesRequested();
+
+  // Constructs a packet with the serialized form of |message_proto| in

[mark a. foltz, 2014/08/28 21:45:22]

Shorten first sentence to "Serializes |message_proto| into |message_data|."
 (The packetization is handled by the socket, not the framer.)

[Kevin M, 2014/08/28 23:33:32]

Done.

+  // |message_data|.
+  // Returns true if the message was serialized successfully, false otherwise.
+  static bool Serialize(const CastMessage& message_proto,
+                        std::string* message_data);
+
+  // Reads bytes from an incoming data buffer and returns CastMessage

[mark a. foltz, 2014/08/28 21:45:22]

Reads bytes from |input_buffer| and returns a new CastMessage if one is fully 
read.

[Kevin M, 2014/08/28 23:33:32]

Done.

+  // objects once they are fully read.
+  //
+  // num_bytes: the number of bytes received by a read operation.
+  //            Value must be <= BytesRequested().
+  // message_length: size of the deserialized message object, in bytes. For

[mark a. foltz, 2014/08/28 21:45:22]

What is *message_length set to if no message was parsed by Ingest()?

[mark a. foltz, 2014/08/28 21:45:22]

The usual format I have seen is to use pipes to delimit parameter names.

Also the first word of the parameter description should be capitalized.

[mark a. foltz, 2014/08/28 21:45:22]

The size of the parsed CastMessage (if any) ...

I would drop the "For logging purposes."

[Kevin M, 2014/08/28 23:33:32]

Done.

[Kevin M, 2014/08/28 23:33:32]

Done.

[Kevin M, 2014/08/28 23:33:32]

Zero, now. :)

+  //                 logging purposes.
+  // error: the status of the ingest operation.

[mark a. foltz, 2014/08/28 21:45:22]

What is this set to if no error occurred?

[Kevin M, 2014/08/28 23:33:32]

Done.

+  // return value: a pointer to a parsed CastMessage if a message was received
+  //               in its entirety, NULL otherwise.
+  scoped_ptr<CastMessage> Ingest(size_t num_bytes,
+                                 size_t* message_length,
+                                 ChannelError* error);
+
+  // Message header struct. If fields are added, be sure to update
+  // header_size().  Protected to allow use of *_size() methods in unit tests.
+  //
+  // MessageHeader is publicly visible for testing purposes.
+  struct MessageHeader {
+    MessageHeader();
+    // Sets the message size.
+    void SetMessageSize(size_t message_size);
+    // Prepends this header to |str|.
+    void PrependToString(std::string* str);
+    // Reads |header| from the bytes specified by |data|.
+    static void Deserialize(char* data, MessageHeader* header);
+    // Size (in bytes) of the message header.
+    static size_t header_size();
+    // Maximum size (in bytes) of a message payload on the wire (does not
+    // include header).
+    static size_t max_message_size();
+    std::string ToString();
+    // The size of the following protocol message in bytes, in host byte order.
+    size_t message_size;
+  };
+
+ private:
+  enum MessageElement { HEADER, BODY };
+
+  // Prepares the framer for ingesting a new message.
+  void Reset();
+
+  // The element of the message that will be read on the next call to Ingest().
+  MessageElement current_element_;
+
+  // Total size of the message, in bytes (head + body).
+  size_t packet_bytes_read_;

[mark a. foltz, 2014/08/28 21:45:22]

message_size_?

[Kevin M, 2014/08/28 23:33:32]

This is a count of the partial bytes received so far, not the size of the entire
message.

How about message_bytes_received_ ?

+
+  // Size of the body alone, in bytes.
+  size_t body_size_;
+
+  // Input buffer which carries message data read from the socket.

[mark a. foltz, 2014/08/28 21:45:22]

You might mention that the caller of this class should write message data into
this buffer. 

[Kevin M, 2014/08/28 23:33:32]

Done.

+  scoped_refptr<net::GrowableIOBuffer> input_buffer_;
+
+  DISALLOW_COPY_AND_ASSIGN(MessageFramer);
+};
 }  // namespace cast_channel
 }  // namespace core_api
 }  // namespace extensions
 
 #endif  // EXTENSIONS_BROWSER_API_CAST_CHANNEL_CAST_SOCKET_H_