Adding the message definitions for the remote_security_key STDIO communication classes.

remoting/host/security_key/security_key_message.h

+// Copyright 2016 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 REMOTING_HOST_SECURITY_KEY_SECURITY_KEY_MESSAGE_H_
+#define REMOTING_HOST_SECURITY_KEY_SECURITY_KEY_MESSAGE_H_
+
+#include <stdint.h>
+
+#include <string>
+
+#include "base/callback.h"

[Sergey Ulanov, 2016/03/24 21:51:17]

callback_forward.h please

[joedow, 2016/03/28 19:44:26]

Done.

+
+namespace remoting {
+
+// -----------------------------------------------------------------------------
+// Introduction
+// -----------------------------------------------------------------------------
+//
+// This file defines the message format and messages used to interact with the
+// remote_security_key process which is used to forward security key requests
+// to a remote client and return security key responses from the remote client.
+
+// -----------------------------------------------------------------------------
+// Message Format
+// -----------------------------------------------------------------------------
+//
+// {Header: uint32_t}{Control Code: uint8_t}{Payload: optional, variable length}
+//
+// Header: Defines the length of the message (Control Code + Payload).
+//         The header endianness is determined by the platform.
+// Control Code: Contains a value representing the message type.
+// Payload: An optional field of variable length.  Data being represented in
+//          this field is dependent on the message type.
+//          The endianess of the payload is dependent on the message type.
+//          Multi-byte payloads which are part of the security key request and
+//          response messages are expected to be big endian.  The bytes for
+//          these messages will be transmitted across the network as-is.
+//          The format for all other payloads is the endianness of the platform.
+
+// -----------------------------------------------------------------------------
+// Messages
+// -----------------------------------------------------------------------------
+enum class RemoteSecurityKeyMessageType : uint8_t {
+  kUnitialized = 0,
+
+  // Sent to the remote_security_key process to ask it to establish a
+  // connection to the Chromoting host if a security key enabled session
+  // exists for the current user.
+  // Payload length: 0
+  kConnectToSecurityKeyEnabledSessionMsg = 1,

[Sergey Ulanov, 2016/03/24 21:51:17]

Please use MACRO_STYLE for these names. Chromium style guide says the following:

  Though the Google C++ Style Guide now says to use kConstantNaming for
  enums, Chromium was written using MACRO_STYLE naming. In enums that are
  actually enumerations (i.e. have multiple values), continue to use this
  style for consistency. Use kConstantNaming when using the "enum hack"
  to define a single constant, as you would for a const int or the like.

const-style names are particularly confusing when you have enum class instead of
just regular enum.

Also, given that this is a class enum, and so the names are scoped, they don't
need to be that long. Particularly you don't need "SecurityKey" in any of these
names. E.g. they can be named as follows:

INVALID
CONNECT
CONNECT_RESPONSE
CONNECT_ERROR
REQUEST
REQUEST_RESPONSE
REQUEST_ERROR
UNKNOWN_COMMAND
UNKNOWN_ERROR

Note that the style guide doesn't allow to truncate Response to Rsp and Error to
Err: https://google.github.io/styleguide/cppguide.html#General_Naming_Rules

[joedow, 2016/03/28 19:44:26]

Done.

+
+  // Sent is sent by the remote_security_key process in response to a
+  // |kConnectToSecurityKeyEnabledSessionMsg| request.
+  // Payload length: 1 byte bool indicating connection state.
+  // True(1): A connection with the Chromoting Host is established and ready
+  //          to receive security key requests to forward.
+  // False(0): No security key session was found.
+  kConnectToSecurityKeyEnabledSessionRsp = 2,
+
+  // Sent by the remote_security_key proces when an error occurs while
+  // attempting to detect the existence of a Chromoting session or when
+  // establishing a connection to it.
+  // Payload length: variable.  If > 0 bytes, the bytes represent an error
+  // string which is ascii encoded and does not include a null terminator.
+  kConnectToSecurityKeyEnabledSessionErr = 3,
+
+  // Sent to the remote_security_key process to ask it to  forward the
+  // security key request payload bytes to the remote machine.
+  // Payload length: > 0 bytes consisting of the security key message to
+  // forward to the remote machine using Length-Value format.
+  kRemoteSecurityKeyRequestMsg = 4,
+
+  // Sent by the remote_security_key once a response has been received from
+  // the remote machine.
+  // Payload length: > 0 bytes consisting of the security key response from
+  // the remote machine using Length-Value format.
+  kRemoteSecurityKeyRequestRsp = 5,
+
+  // Sent by the remote_security_key if an error occurs either in sending the
+  // request data to the remote host or when receiving the response.
+  // Payload length: variable.  If > 0 bytes, the bytes represent an error
+  // string which is ascii encoded and does not include a null terminator.
+  kRemoteSecurityKeyRequestErr = 6,
+
+  // Sent by the remote_security_key if it receives an unknown command.
+  // Payload length: 0 bytes.
+  kRemoteSecurityKeyUnknownMessageErrorMsg = 254,
+
+  // Sent by the remote_security_key if an error occurs which does not conform
+  // to any existing category.  No response to this message is expected.
+  // Payload length: variable.  If > 0 bytes, the bytes represent an error
+  // string which is ascii encoded and does not include a null terminator.
+  kRemoteSecurityKeyUnknownErrorMsg = 255,
+};
+
+class SecurityKeyMessage final {
+ public:
+  SecurityKeyMessage();
+  ~SecurityKeyMessage();
+  // Used to pass remote security key message data between classes.
+  typedef base::Callback<void(scoped_ptr<SecurityKeyMessage> message)> Callback;

[Sergey Ulanov, 2016/03/24 21:51:17]

Move this above the constructor:
https://google.github.io/styleguide/cppguide.html#Declaration_Order

[Sergey Ulanov, 2016/03/24 21:51:18]

I don't think this callback should be part of this class. Maybe move it to the
reader, or just outside of this class?

[joedow, 2016/03/28 19:44:26]

Acknowledged.

[joedow, 2016/03/28 19:44:26]

The callback is used by the reader and handler (also channel but that may be
going away).  I don't mind moving it out of the class.

+
+  // The number of bytes used to represent the header.
+  static const int kHeaderSizeBytes;

[Sergey Ulanov, 2016/03/24 21:51:17]

move this above constructor. Also add "= 4" here - it doesn't need to be in the
.cc file

[joedow, 2016/03/28 19:44:26]

Done.

+
+  // When given a header value (uint32_t), this method will return whether the
+  // length is within the allowable size range.
+  static bool IsValidMessageSize(uint32_t message_size);
+
+  // Parses |message_data| and initializes the internal members.  Returns true
+  // if |message_data| was parsed and the instance was initialized successfully.
+  bool ParseMessage(const std::string& message_data);
+
+  RemoteSecurityKeyMessageType type() { return type_; }
+
+  const std::string& payload() { return payload_; }
+
+ private:
+  RemoteSecurityKeyMessageType type_ =
+      RemoteSecurityKeyMessageType::kUnitialized;
+  std::string payload_;
+
+  DISALLOW_COPY_AND_ASSIGN(SecurityKeyMessage);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_SECURITY_KEY_SECURITY_KEY_MESSAGE_H_