Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(866)

Unified Diff: extensions/browser/api/cast_channel/cast_socket.h

Issue 505453002: Create dedicated class for handling wire message formatting and parsing. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Code review feedback Created 6 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: extensions/browser/api/cast_channel/cast_socket.h
diff --git a/extensions/browser/api/cast_channel/cast_socket.h b/extensions/browser/api/cast_channel/cast_socket.h
index 1de7ac3717430e5d9f0f6767f1e0da1abe14dc17..c3ee1979e0a377af6d39e8a8a1a3df7c3ee4cbb3 100644
--- a/extensions/browser/api/cast_channel/cast_socket.h
+++ b/extensions/browser/api/cast_channel/cast_socket.h
@@ -38,6 +38,7 @@ 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
@@ -155,30 +156,6 @@ class CastSocket : public ApiResource {
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;
@@ -266,12 +243,6 @@ class CastSocket : public ApiResource {
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
@@ -308,15 +279,8 @@ class CastSocket : public ApiResource {
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_;
@@ -355,6 +319,8 @@ class CastSocket : public ApiResource {
// canceled.
bool is_canceled_;
+ scoped_ptr<CastMessage> current_message_;
+
// Connection flow state machine state.
ConnectionState connect_state_;
// Write flow state machine state.
@@ -401,6 +367,77 @@ class CastSocket : public ApiResource {
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 com
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_pro
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
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 p
mark a. foltz 2014/08/28 21:45:22 The usual format I have seen is to use pipes to de
mark a. foltz 2014/08/28 21:45:22 The size of the parsed CastMessage (if any) ... I
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 f
+
+ // 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 sh
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
« no previous file with comments | « no previous file | extensions/browser/api/cast_channel/cast_socket.cc » ('j') | extensions/browser/api/cast_channel/cast_socket.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698