Consolidate shared device constants

Clean up {Hid, Ble}Device by consolidating shared constructs and
rename constants with prefix U2f to have prefix Fido. More specifically,
 1) Consolidate HidDevice::State / BleDevice::State into U2fDevice
    interface.
 2) Rename BLE constants (e.g. service id) to have fido as prefix.
 3) Move U2fDevice::ProtocolVersion to fido_constants.h
 4) Delete u2f_command_type.h file. This contents in this file is
    neither U2F nor correct. U2fCommandType enum currently mixes
    HID command type and BLE command types. Remove the file and
    add separate command type enums in fido_constants.h

Bug: 821215
Change-Id: I45bce9323ac95d83c31e7b90a2548b745d9e00b2
Reviewed-on: https://chromium-review.googlesource.com/961618
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543400}

device/fido/BUILD.gn

@@ -35,6 +35,8 @@ component("fido") {
     "ec_public_key.h",
     "fido_attestation_statement.cc",
     "fido_attestation_statement.h",
+    "fido_ble_uuids.cc",
+    "fido_ble_uuids.h",
     "fido_constants.cc",
     "fido_constants.h",
     "fido_hid_message.cc",
@@ -67,9 +69,6 @@ component("fido") {
     "u2f_ble_frames.h",
     "u2f_ble_transaction.cc",
     "u2f_ble_transaction.h",
-    "u2f_ble_uuids.cc",
-    "u2f_ble_uuids.h",
-    "u2f_command_type.h",
     "u2f_device.cc",
     "u2f_device.h",
     "u2f_discovery.cc",

device/fido/u2f_ble_uuids.cc → device/fido/fido_ble_uuids.cc

@@ -2,17 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "device/fido/u2f_ble_uuids.h"
+#include "device/fido/fido_ble_uuids.h"
 
 namespace device {
 
-const char kU2fServiceUUID[] = "0000fffd-0000-1000-8000-00805f9b34fb";
-const char kU2fControlPointUUID[] = "f1d0fff1-deaa-ecee-b42f-c9ba7ed623bb";
-const char kU2fStatusUUID[] = "f1d0fff2-deaa-ecee-b42f-c9ba7ed623bb";
-const char kU2fControlPointLengthUUID[] =
+const char kFidoServiceUUID[] = "0000fffd-0000-1000-8000-00805f9b34fb";
+const char kFidoControlPointUUID[] = "f1d0fff1-deaa-ecee-b42f-c9ba7ed623bb";
+const char kFidoStatusUUID[] = "f1d0fff2-deaa-ecee-b42f-c9ba7ed623bb";
+const char kFidoControlPointLengthUUID[] =
     "f1d0fff3-deaa-ecee-b42f-c9ba7ed623bb";
-const char kU2fServiceRevisionUUID[] = "00002a28-0000-1000-8000-00805f9b34fb";
-const char kU2fServiceRevisionBitfieldUUID[] =
+const char kFidoServiceRevisionUUID[] = "00002a28-0000-1000-8000-00805f9b34fb";
+const char kFidoServiceRevisionBitfieldUUID[] =
     "f1d0fff4-deaa-ecee-b42f-c9ba7ed623bb";
 
 }  // namespace device

device/fido/u2f_ble_uuids.h → device/fido/fido_ble_uuids.h

@@ -2,27 +2,27 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef DEVICE_FIDO_U2F_BLE_UUIDS_H_
-#define DEVICE_FIDO_U2F_BLE_UUIDS_H_
+#ifndef DEVICE_FIDO_FIDO_BLE_UUIDS_H_
+#define DEVICE_FIDO_FIDO_BLE_UUIDS_H_
 
 #include "base/component_export.h"
 
 namespace device {
 
-// U2F GATT Service's UUIDs as defined by the standard:
-// https://fidoalliance.org/specs/fido-u2f-v1.2-ps-20170411/fido-u2f-bt-protocol-v1.2-ps-20170411.html#h3_u2f-service
+// FIDO GATT Service's UUIDs as defined by the standard:
+// https://fidoalliance.org/specs/fido-v2.0-rd-20161004/fido-client-to-authenticator-protocol-v2.0-rd-20161004.html#gatt-service-description
 //
-// For details on how the short UUIDs for U2F Service (0xFFFD) and U2F Service
+// For details on how the short UUIDs for FIDO Service (0xFFFD) and FIDO Service
 // Revision (0x2A28) were converted to the long canonical ones, see
 // https://www.bluetooth.com/specifications/assigned-numbers/service-discovery
-COMPONENT_EXPORT(DEVICE_FIDO) extern const char kU2fServiceUUID[];
-COMPONENT_EXPORT(DEVICE_FIDO) extern const char kU2fControlPointUUID[];
-COMPONENT_EXPORT(DEVICE_FIDO) extern const char kU2fStatusUUID[];
-COMPONENT_EXPORT(DEVICE_FIDO) extern const char kU2fControlPointLengthUUID[];
-COMPONENT_EXPORT(DEVICE_FIDO) extern const char kU2fServiceRevisionUUID[];
+COMPONENT_EXPORT(DEVICE_FIDO) extern const char kFidoServiceUUID[];
+COMPONENT_EXPORT(DEVICE_FIDO) extern const char kFidoControlPointUUID[];
+COMPONENT_EXPORT(DEVICE_FIDO) extern const char kFidoStatusUUID[];
+COMPONENT_EXPORT(DEVICE_FIDO) extern const char kFidoControlPointLengthUUID[];
+COMPONENT_EXPORT(DEVICE_FIDO) extern const char kFidoServiceRevisionUUID[];
 COMPONENT_EXPORT(DEVICE_FIDO)
-extern const char kU2fServiceRevisionBitfieldUUID[];
+extern const char kFidoServiceRevisionBitfieldUUID[];
 
 }  // namespace device
 
-#endif  // DEVICE_FIDO_U2F_BLE_UUIDS_H_
+#endif  // DEVICE_FIDO_FIDO_BLE_UUIDS_H_

device/fido/fido_constants.cc

@@ -46,4 +46,6 @@ const std::array<uint8_t, 2> kLegacyVersionSuffix = {0x00, 0x00};
 const std::array<uint8_t, 6> kU2fVersionResponse = {'U', '2', 'F',
                                                     '_', 'V', '2'};
 
+const base::TimeDelta kDeviceTimeout = base::TimeDelta::FromSeconds(3);
+
 }  // namespace device

device/fido/fido_constants.h

@@ -11,9 +11,16 @@
 #include <vector>
 
 #include "base/component_export.h"
+#include "base/time/time.h"
 
 namespace device {
 
+enum class ProtocolVersion {
+  kCtap,
+  kU2f,
+  kUnknown,
+};
+
 // CTAP protocol device response code, as specified in
 // https://fidoalliance.org/specs/fido-v2.0-rd-20170927/fido-client-to-authenticator-protocol-v2.0-rd-20170927.html#authenticator-api
 enum class CtapDeviceResponseCode : uint8_t {
@@ -126,30 +133,37 @@ constexpr std::array<CtapDeviceResponseCode, 51> GetCtapResponseCodeList() {
 
 // Commands supported by CTAPHID device as specified in
 // https://fidoalliance.org/specs/fido-v2.0-rd-20170927/fido-client-to-authenticator-protocol-v2.0-rd-20170927.html#ctaphid-commands
-enum class CtapHidDeviceCommand : uint8_t {
-  kCtapHidMsg = 0x03,
-  kCtapHidCBOR = 0x10,
-  kCtapHidInit = 0x06,
-  kCtapHidPing = 0x01,
-  kCtapHidCancel = 0x11,
-  kCtapHidError = 0x3F,
-  kCtapHidKeepAlive = 0x3B,
-  kCtapHidWink = 0x08,
-  kCtapHidLock = 0x04,
+enum class FidoHidDeviceCommand : uint8_t {
+  kMsg = 0x03,
+  kCbor = 0x10,
+  kInit = 0x06,
+  kPing = 0x01,
+  kCancel = 0x11,
+  kError = 0x3F,
+  kKeepAlive = 0x3B,
+  kWink = 0x08,
+  kLock = 0x04,
 };
 
-constexpr std::array<CtapHidDeviceCommand, 9> GetCtapHidDeviceCommandList() {
-  return {CtapHidDeviceCommand::kCtapHidMsg,
-          CtapHidDeviceCommand::kCtapHidCBOR,
-          CtapHidDeviceCommand::kCtapHidInit,
-          CtapHidDeviceCommand::kCtapHidPing,
-          CtapHidDeviceCommand::kCtapHidCancel,
-          CtapHidDeviceCommand::kCtapHidError,
-          CtapHidDeviceCommand::kCtapHidKeepAlive,
-          CtapHidDeviceCommand::kCtapHidWink,
-          CtapHidDeviceCommand::kCtapHidLock};
+constexpr std::array<FidoHidDeviceCommand, 9> GetFidoHidDeviceCommandList() {
+  return {FidoHidDeviceCommand::kMsg,       FidoHidDeviceCommand::kCbor,
+          FidoHidDeviceCommand::kInit,      FidoHidDeviceCommand::kPing,
+          FidoHidDeviceCommand::kCancel,    FidoHidDeviceCommand::kError,
+          FidoHidDeviceCommand::kKeepAlive, FidoHidDeviceCommand::kWink,
+          FidoHidDeviceCommand::kLock};
 }
 
+// BLE device command as specified in
+//  https://fidoalliance.org/specs/fido-v2.0-rd-20170927/fido-client-to-authenticator-protocol-v2.0-rd-20170927.html#command-status-and-error-constants
+// U2F BLE device does not support cancel command.
+enum class FidoBleDeviceCommand : uint8_t {
+  kPing = 0x81,
+  kKeepAlive = 0x82,
+  kMsg = 0x83,
+  kCancel = 0xBE,
+  kError = 0xBF,
+};
+
 // Authenticator API commands supported by CTAP devices, as specified in
 // https://fidoalliance.org/specs/fido-v2.0-rd-20170927/fido-client-to-authenticator-protocol-v2.0-rd-20170927.html#authenticator-api
 enum class CtapRequestCommand : uint8_t {
@@ -230,6 +244,9 @@ extern const std::array<uint8_t, 2> kLegacyVersionSuffix;
 COMPONENT_EXPORT(DEVICE_FIDO)
 extern const std::array<uint8_t, 6> kU2fVersionResponse;
 
+// Maximum wait time before client error outs on device.
+COMPONENT_EXPORT(DEVICE_FIDO) extern const base::TimeDelta kDeviceTimeout;
+
 }  // namespace device
 
 #endif  // DEVICE_FIDO_FIDO_CONSTANTS_H_

device/fido/fido_hid_message.cc

@@ -16,39 +16,39 @@ namespace device {
 // static
 std::unique_ptr<FidoHidMessage> FidoHidMessage::Create(
     uint32_t channel_id,
-    CtapHidDeviceCommand type,
+    FidoHidDeviceCommand type,
     base::span<const uint8_t> data) {
   if (data.size() > kHidMaxMessageSize)
     return nullptr;
 
   switch (type) {
-    case CtapHidDeviceCommand::kCtapHidPing:
+    case FidoHidDeviceCommand::kPing:
       break;
-    case CtapHidDeviceCommand::kCtapHidMsg:
-    case CtapHidDeviceCommand::kCtapHidCBOR: {
+    case FidoHidDeviceCommand::kMsg:
+    case FidoHidDeviceCommand::kCbor: {
       if (data.empty())
         return nullptr;
       break;
     }
 
-    case CtapHidDeviceCommand::kCtapHidCancel:
-    case CtapHidDeviceCommand::kCtapHidWink: {
+    case FidoHidDeviceCommand::kCancel:
+    case FidoHidDeviceCommand::kWink: {
       if (!data.empty())
         return nullptr;
       break;
     }
-    case CtapHidDeviceCommand::kCtapHidLock: {
+    case FidoHidDeviceCommand::kLock: {
       if (data.size() != 1 || data[0] > kHidMaxLockSeconds)
         return nullptr;
       break;
     }
-    case CtapHidDeviceCommand::kCtapHidInit: {
+    case FidoHidDeviceCommand::kInit: {
       if (data.size() != 8)
         return nullptr;
       break;
     }
-    case CtapHidDeviceCommand::kCtapHidKeepAlive:
-    case CtapHidDeviceCommand::kCtapHidError:
+    case FidoHidDeviceCommand::kKeepAlive:
+    case FidoHidDeviceCommand::kError:
       if (data.size() != 1)
         return nullptr;
   }
@@ -124,7 +124,7 @@ size_t FidoHidMessage::NumPackets() const {
 }
 
 FidoHidMessage::FidoHidMessage(uint32_t channel_id,
-                               CtapHidDeviceCommand type,
+                               FidoHidDeviceCommand type,
                                base::span<const uint8_t> data)
     : channel_id_(channel_id) {
   uint8_t sequence = 0;

device/fido/fido_hid_message.h

@@ -28,7 +28,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoHidMessage {
  public:
   // Static functions to create CTAP/U2F HID commands.
   static std::unique_ptr<FidoHidMessage> Create(uint32_t channel_id,
-                                                CtapHidDeviceCommand cmd,
+                                                FidoHidDeviceCommand cmd,
                                                 base::span<const uint8_t> data);
 
   // Reconstruct a message from serialized message data.
@@ -47,7 +47,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoHidMessage {
 
   size_t NumPackets() const;
   uint32_t channel_id() const { return channel_id_; }
-  CtapHidDeviceCommand cmd() const { return cmd_; }
+  FidoHidDeviceCommand cmd() const { return cmd_; }
   const base::circular_deque<std::unique_ptr<FidoHidPacket>>&
   GetPacketsForTesting() const {
     return packets_;
@@ -55,12 +55,12 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoHidMessage {
 
  private:
   FidoHidMessage(uint32_t channel_id,
-                 CtapHidDeviceCommand type,
+                 FidoHidDeviceCommand type,
                  base::span<const uint8_t> data);
   FidoHidMessage(std::unique_ptr<FidoHidInitPacket> init_packet,
                  size_t remaining_size);
   uint32_t channel_id_ = kHidBroadcastChannel;
-  CtapHidDeviceCommand cmd_ = CtapHidDeviceCommand::kCtapHidMsg;
+  FidoHidDeviceCommand cmd_ = FidoHidDeviceCommand::kMsg;
   base::circular_deque<std::unique_ptr<FidoHidPacket>> packets_;
   size_t remaining_size_ = 0;
 

device/fido/fido_hid_message_unittest.cc

@@ -19,7 +19,7 @@ TEST(FidoHidMessageTest, TestPacketSize) {
   std::vector<uint8_t> data;
 
   auto init_packet = std::make_unique<FidoHidInitPacket>(
-      channel_id, CtapHidDeviceCommand::kCtapHidInit, data, data.size());
+      channel_id, FidoHidDeviceCommand::kInit, data, data.size());
   EXPECT_EQ(64u, init_packet->GetSerializedData().size());
 
   auto continuation_packet =
@@ -40,7 +40,7 @@ TEST(FidoHidMessageTest, TestPacketSize) {
 TEST(FidoHidMessageTest, TestPacketData) {
   uint32_t channel_id = 0xF5060708;
   std::vector<uint8_t> data{10, 11};
-  CtapHidDeviceCommand cmd = CtapHidDeviceCommand::kCtapHidWink;
+  FidoHidDeviceCommand cmd = FidoHidDeviceCommand::kWink;
   auto init_packet =
       std::make_unique<FidoHidInitPacket>(channel_id, cmd, data, data.size());
   size_t index = 0;
@@ -63,7 +63,7 @@ TEST(FidoHidMessageTest, TestPacketData) {
 TEST(FidoHidMessageTest, TestPacketConstructors) {
   uint32_t channel_id = 0x05060708;
   std::vector<uint8_t> data{10, 11};
-  CtapHidDeviceCommand cmd = CtapHidDeviceCommand::kCtapHidWink;
+  FidoHidDeviceCommand cmd = FidoHidDeviceCommand::kWink;
   auto orig_packet =
       std::make_unique<FidoHidInitPacket>(channel_id, cmd, data, data.size());
 
@@ -96,8 +96,8 @@ TEST(FidoHidMessageTest, TestMaxLengthPacketConstructors) {
   for (size_t i = 0; i < kHidMaxMessageSize; ++i)
     data.push_back(static_cast<uint8_t>(i % 0xff));
 
-  auto orig_msg = FidoHidMessage::Create(
-      channel_id, CtapHidDeviceCommand::kCtapHidMsg, data);
+  auto orig_msg =
+      FidoHidMessage::Create(channel_id, FidoHidDeviceCommand::kMsg, data);
   ASSERT_TRUE(orig_msg);
 
   const auto& original_msg_packets = orig_msg->GetPacketsForTesting();
@@ -137,20 +137,20 @@ TEST(FidoHidMessageTest, TestMaxLengthPacketConstructors) {
 TEST(FidoHidMessageTest, TestMessagePartitoning) {
   uint32_t channel_id = 0x01010203;
   std::vector<uint8_t> data(kHidInitPacketDataSize + 1);
-  auto two_packet_message = FidoHidMessage::Create(
-      channel_id, CtapHidDeviceCommand::kCtapHidPing, data);
+  auto two_packet_message =
+      FidoHidMessage::Create(channel_id, FidoHidDeviceCommand::kPing, data);
   ASSERT_TRUE(two_packet_message);
   EXPECT_EQ(2U, two_packet_message->NumPackets());
 
   data.resize(kHidInitPacketDataSize);
-  auto one_packet_message = FidoHidMessage::Create(
-      channel_id, CtapHidDeviceCommand::kCtapHidPing, data);
+  auto one_packet_message =
+      FidoHidMessage::Create(channel_id, FidoHidDeviceCommand::kPing, data);
   ASSERT_TRUE(one_packet_message);
   EXPECT_EQ(1U, one_packet_message->NumPackets());
 
   data.resize(kHidInitPacketDataSize + kHidContinuationPacketDataSize + 1);
-  auto three_packet_message = FidoHidMessage::Create(
-      channel_id, CtapHidDeviceCommand::kCtapHidPing, data);
+  auto three_packet_message =
+      FidoHidMessage::Create(channel_id, FidoHidDeviceCommand::kPing, data);
   ASSERT_TRUE(three_packet_message);
   EXPECT_EQ(3U, three_packet_message->NumPackets());
 }
@@ -158,16 +158,16 @@ TEST(FidoHidMessageTest, TestMessagePartitoning) {
 TEST(FidoHidMessageTest, TestMaxSize) {
   uint32_t channel_id = 0x00010203;
   std::vector<uint8_t> data(kHidMaxMessageSize + 1);
-  auto oversize_message = FidoHidMessage::Create(
-      channel_id, CtapHidDeviceCommand::kCtapHidPing, data);
+  auto oversize_message =
+      FidoHidMessage::Create(channel_id, FidoHidDeviceCommand::kPing, data);
   EXPECT_EQ(nullptr, oversize_message);
 }
 
 TEST(FidoHidMessageTest, TestDeconstruct) {
   uint32_t channel_id = 0x0A0B0C0D;
   std::vector<uint8_t> data(kHidMaxMessageSize, 0x7F);
-  auto filled_message = FidoHidMessage::Create(
-      channel_id, CtapHidDeviceCommand::kCtapHidPing, data);
+  auto filled_message =
+      FidoHidMessage::Create(channel_id, FidoHidDeviceCommand::kPing, data);
   ASSERT_TRUE(filled_message);
   EXPECT_THAT(data,
               ::testing::ContainerEq(filled_message->GetMessagePayload()));
@@ -177,8 +177,8 @@ TEST(FidoHidMessageTest, TestDeserialize) {
   uint32_t channel_id = 0x0A0B0C0D;
   std::vector<uint8_t> data(kHidMaxMessageSize);
 
-  auto orig_message = FidoHidMessage::Create(
-      channel_id, CtapHidDeviceCommand::kCtapHidPing, data);
+  auto orig_message =
+      FidoHidMessage::Create(channel_id, FidoHidDeviceCommand::kPing, data);
   ASSERT_TRUE(orig_message);
 
   base::circular_deque<std::vector<uint8_t>> orig_list;

device/fido/fido_hid_packet.cc

@@ -33,8 +33,8 @@ std::unique_ptr<FidoHidInitPacket> FidoHidInitPacket::CreateFromSerializedData(
   channel_id |= (serialized[index++] & 0xff) << 8;
   channel_id |= serialized[index++] & 0xff;
 
-  auto command = static_cast<CtapHidDeviceCommand>(serialized[index++] & 0x7f);
-  if (!base::ContainsValue(GetCtapHidDeviceCommandList(), command))
+  auto command = static_cast<FidoHidDeviceCommand>(serialized[index++] & 0x7f);
+  if (!base::ContainsValue(GetFidoHidDeviceCommandList(), command))
     return nullptr;
 
   uint16_t payload_size = serialized[index++] << 8;
@@ -62,7 +62,7 @@ std::unique_ptr<FidoHidInitPacket> FidoHidInitPacket::CreateFromSerializedData(
 // 6      1       Low order packet payload size
 // 7      (s-7)   Payload data
 FidoHidInitPacket::FidoHidInitPacket(uint32_t channel_id,
-                                     CtapHidDeviceCommand cmd,
+                                     FidoHidDeviceCommand cmd,
                                      std::vector<uint8_t> data,
                                      uint16_t payload_length)
     : FidoHidPacket(std::move(data), channel_id),

device/fido/fido_hid_packet.h

@@ -62,17 +62,17 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoHidInitPacket : public FidoHidPacket {
       size_t* remaining_size);
 
   FidoHidInitPacket(uint32_t channel_id,
-                    CtapHidDeviceCommand cmd,
+                    FidoHidDeviceCommand cmd,
                     std::vector<uint8_t> data,
                     uint16_t payload_length);
   ~FidoHidInitPacket() final;
 
   std::vector<uint8_t> GetSerializedData() const final;
-  CtapHidDeviceCommand command() const { return command_; }
+  FidoHidDeviceCommand command() const { return command_; }
   uint16_t payload_length() const { return payload_length_; }
 
  private:
-  CtapHidDeviceCommand command_;
+  FidoHidDeviceCommand command_;
   uint16_t payload_length_;
 
   DISALLOW_COPY_AND_ASSIGN(FidoHidInitPacket);

device/fido/u2f_ble_connection.cc

@@ -16,7 +16,7 @@
 #include "device/bluetooth/bluetooth_remote_gatt_characteristic.h"
 #include "device/bluetooth/bluetooth_remote_gatt_service.h"
 #include "device/bluetooth/bluetooth_uuid.h"
-#include "device/fido/u2f_ble_uuids.h"
+#include "device/fido/fido_ble_uuids.h"
 
 namespace device {
 
@@ -317,7 +317,7 @@ void U2fBleConnection::ConnectToU2fService() {
       device->GetGattServices();
   auto found =
       std::find_if(services.begin(), services.end(), [](const auto* service) {
-        return service->GetUUID().canonical_value() == kU2fServiceUUID;
+        return service->GetUUID().canonical_value() == kFidoServiceUUID;
       });
 
   if (found == services.end()) {
@@ -334,19 +334,19 @@ void U2fBleConnection::ConnectToU2fService() {
     // NOTE: Since GetUUID() returns a temporary |uuid| can't be a reference,
     // even though canonical_value() returns a const reference.
     const std::string uuid = characteristic->GetUUID().canonical_value();
-    if (uuid == kU2fControlPointLengthUUID) {
+    if (uuid == kFidoControlPointLengthUUID) {
       control_point_length_id_ = characteristic->GetIdentifier();
       DVLOG(2) << "Got U2F Control Point Length: " << *control_point_length_id_;
-    } else if (uuid == kU2fControlPointUUID) {
+    } else if (uuid == kFidoControlPointUUID) {
       control_point_id_ = characteristic->GetIdentifier();
       DVLOG(2) << "Got U2F Control Point: " << *control_point_id_;
-    } else if (uuid == kU2fStatusUUID) {
+    } else if (uuid == kFidoStatusUUID) {
       status_id_ = characteristic->GetIdentifier();
       DVLOG(2) << "Got U2F Status: " << *status_id_;
-    } else if (uuid == kU2fServiceRevisionUUID) {
+    } else if (uuid == kFidoServiceRevisionUUID) {
       service_revision_id_ = characteristic->GetIdentifier();
       DVLOG(2) << "Got U2F Service Revision: " << *service_revision_id_;
-    } else if (uuid == kU2fServiceRevisionBitfieldUUID) {
+    } else if (uuid == kFidoServiceRevisionBitfieldUUID) {
       service_revision_bitfield_id_ = characteristic->GetIdentifier();
       DVLOG(2) << "Got U2F Service Revision Bitfield: "
                << *service_revision_bitfield_id_;

device/fido/u2f_ble_connection_unittest.cc

@@ -19,7 +19,7 @@
 #include "device/bluetooth/test/mock_bluetooth_gatt_connection.h"
 #include "device/bluetooth/test/mock_bluetooth_gatt_notify_session.h"
 #include "device/bluetooth/test/mock_bluetooth_gatt_service.h"
-#include "device/fido/u2f_ble_uuids.h"
+#include "device/fido/fido_ble_uuids.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -332,7 +332,7 @@ class U2fBleConnectionTest : public ::testing::Test {
 
   void AddU2fService() {
     auto u2f_service = std::make_unique<NiceMockBluetoothGattService>(
-        u2f_device_, "u2f_service", BluetoothUUID(kU2fServiceUUID),
+        u2f_device_, "u2f_service", BluetoothUUID(kFidoServiceUUID),
         /* is_primary */ true, /* is_local */ false);
     u2f_service_ = u2f_service.get();
     u2f_device_->AddMockService(std::move(u2f_service));
@@ -353,7 +353,7 @@ class U2fBleConnectionTest : public ::testing::Test {
       auto u2f_control_point =
           std::make_unique<NiceMockBluetoothGattCharacteristic>(
               u2f_service_, "u2f_control_point",
-              BluetoothUUID(kU2fControlPointUUID), is_local,
+              BluetoothUUID(kFidoControlPointUUID), is_local,
               BluetoothGattCharacteristic::PROPERTY_WRITE,
               BluetoothGattCharacteristic::PERMISSION_NONE);
       u2f_control_point_ = u2f_control_point.get();
@@ -362,7 +362,7 @@ class U2fBleConnectionTest : public ::testing::Test {
 
     {
       auto u2f_status = std::make_unique<NiceMockBluetoothGattCharacteristic>(
-          u2f_service_, "u2f_status", BluetoothUUID(kU2fStatusUUID), is_local,
+          u2f_service_, "u2f_status", BluetoothUUID(kFidoStatusUUID), is_local,
           BluetoothGattCharacteristic::PROPERTY_NOTIFY,
           BluetoothGattCharacteristic::PERMISSION_NONE);
       u2f_status_ = u2f_status.get();
@@ -373,7 +373,7 @@ class U2fBleConnectionTest : public ::testing::Test {
       auto u2f_control_point_length =
           std::make_unique<NiceMockBluetoothGattCharacteristic>(
               u2f_service_, "u2f_control_point_length",
-              BluetoothUUID(kU2fControlPointLengthUUID), is_local,
+              BluetoothUUID(kFidoControlPointLengthUUID), is_local,
               BluetoothGattCharacteristic::PROPERTY_READ,
               BluetoothGattCharacteristic::PERMISSION_NONE);
       u2f_control_point_length_ = u2f_control_point_length.get();
@@ -384,7 +384,7 @@ class U2fBleConnectionTest : public ::testing::Test {
       auto u2f_service_revision =
           std::make_unique<NiceMockBluetoothGattCharacteristic>(
               u2f_service_, "u2f_service_revision",
-              BluetoothUUID(kU2fServiceRevisionUUID), is_local,
+              BluetoothUUID(kFidoServiceRevisionUUID), is_local,
               BluetoothGattCharacteristic::PROPERTY_READ,
               BluetoothGattCharacteristic::PERMISSION_NONE);
       u2f_service_revision_ = u2f_service_revision.get();
@@ -395,7 +395,7 @@ class U2fBleConnectionTest : public ::testing::Test {
       auto u2f_service_revision_bitfield =
           std::make_unique<NiceMockBluetoothGattCharacteristic>(
               u2f_service_, "u2f_service_revision_bitfield",
-              BluetoothUUID(kU2fServiceRevisionBitfieldUUID), is_local,
+              BluetoothUUID(kFidoServiceRevisionBitfieldUUID), is_local,
               BluetoothGattCharacteristic::PROPERTY_READ |
                   BluetoothGattCharacteristic::PROPERTY_WRITE,
               BluetoothGattCharacteristic::PERMISSION_NONE);

device/fido/u2f_ble_device.cc

@@ -27,18 +27,18 @@ U2fBleDevice::U2fBleDevice(std::unique_ptr<U2fBleConnection> connection)
 U2fBleDevice::~U2fBleDevice() = default;
 
 void U2fBleDevice::Connect() {
-  if (state_ != State::INIT)
+  if (state_ != State::kInit)
     return;
 
   StartTimeout();
-  state_ = State::BUSY;
+  state_ = State::kBusy;
   connection_->Connect();
 }
 
 void U2fBleDevice::SendPing(std::vector<uint8_t> data,
                             DeviceCallback callback) {
   pending_frames_.emplace(
-      U2fBleFrame(U2fCommandType::CMD_PING, std::move(data)),
+      U2fBleFrame(FidoBleDeviceCommand::kPing, std::move(data)),
       base::BindOnce(
           [](DeviceCallback callback, base::Optional<U2fBleFrame> frame) {
             std::move(callback).Run(frame ? base::make_optional(frame->data())
@@ -76,7 +76,7 @@ U2fBleConnection::ReadCallback U2fBleDevice::GetReadCallbackForTesting() {
 void U2fBleDevice::DeviceTransact(std::vector<uint8_t> command,
                                   DeviceCallback callback) {
   pending_frames_.emplace(
-      U2fBleFrame(U2fCommandType::CMD_MSG, std::move(command)),
+      U2fBleFrame(FidoBleDeviceCommand::kMsg, std::move(command)),
       base::BindOnce(
           [](DeviceCallback callback, base::Optional<U2fBleFrame> frame) {
             std::move(callback).Run(frame ? base::make_optional(frame->data())
@@ -92,16 +92,16 @@ base::WeakPtr<U2fDevice> U2fBleDevice::GetWeakPtr() {
 
 void U2fBleDevice::Transition() {
   switch (state_) {
-    case State::INIT:
+    case State::kInit:
       Connect();
       break;
-    case State::CONNECTED:
+    case State::kConnected:
       StartTimeout();
-      state_ = State::BUSY;
+      state_ = State::kBusy;
       connection_->ReadControlPointLength(base::BindOnce(
           &U2fBleDevice::OnReadControlPointLength, base::Unretained(this)));
       break;
-    case State::READY:
+    case State::kReady:
       if (!pending_frames_.empty()) {
         U2fBleFrame frame;
         FrameCallback callback;
@@ -110,9 +110,9 @@ void U2fBleDevice::Transition() {
         SendRequestFrame(std::move(frame), std::move(callback));
       }
       break;
-    case State::BUSY:
+    case State::kBusy:
       break;
-    case State::DEVICE_ERROR:
+    case State::kDeviceError:
       auto self = GetWeakPtr();
       // Executing callbacks may free |this|. Check |self| first.
       while (self && !pending_frames_.empty()) {
@@ -127,7 +127,7 @@ void U2fBleDevice::Transition() {
 
 void U2fBleDevice::OnConnectionStatus(bool success) {
   StopTimeout();
-  state_ = success ? State::CONNECTED : State::DEVICE_ERROR;
+  state_ = success ? State::kConnected : State::kDeviceError;
   Transition();
 }
 
@@ -135,9 +135,9 @@ void U2fBleDevice::OnReadControlPointLength(base::Optional<uint16_t> length) {
   StopTimeout();
   if (length) {
     transaction_.emplace(connection_.get(), *length);
-    state_ = State::READY;
+    state_ = State::kReady;
   } else {
-    state_ = State::DEVICE_ERROR;
+    state_ = State::kDeviceError;
   }
   Transition();
 }
@@ -148,7 +148,7 @@ void U2fBleDevice::OnStatusMessage(std::vector<uint8_t> data) {
 }
 
 void U2fBleDevice::SendRequestFrame(U2fBleFrame frame, FrameCallback callback) {
-  state_ = State::BUSY;
+  state_ = State::kBusy;
   transaction_->WriteRequestFrame(
       std::move(frame),
       base::BindOnce(&U2fBleDevice::OnResponseFrame, base::Unretained(this),
@@ -157,7 +157,7 @@ void U2fBleDevice::SendRequestFrame(U2fBleFrame frame, FrameCallback callback) {
 
 void U2fBleDevice::OnResponseFrame(FrameCallback callback,
                                    base::Optional<U2fBleFrame> frame) {
-  state_ = frame ? State::READY : State::DEVICE_ERROR;
+  state_ = frame ? State::kReady : State::kDeviceError;
   auto self = GetWeakPtr();
   std::move(callback).Run(std::move(frame));
   // Executing callbacks may free |this|. Check |self| first.
@@ -166,8 +166,7 @@ void U2fBleDevice::OnResponseFrame(FrameCallback callback,
 }
 
 void U2fBleDevice::StartTimeout() {
-  timer_.Start(FROM_HERE, U2fDevice::kDeviceTimeout, this,
-               &U2fBleDevice::OnTimeout);
+  timer_.Start(FROM_HERE, kDeviceTimeout, this, &U2fBleDevice::OnTimeout);
 }
 
 void U2fBleDevice::StopTimeout() {
@@ -175,7 +174,7 @@ void U2fBleDevice::StopTimeout() {
 }
 
 void U2fBleDevice::OnTimeout() {
-  state_ = State::DEVICE_ERROR;
+  state_ = State::kDeviceError;
 }
 
 }  // namespace device

device/fido/u2f_ble_device.h

@@ -17,6 +17,7 @@
 #include "base/optional.h"
 #include "base/strings/string_piece.h"
 #include "base/timer/timer.h"
+#include "device/fido/fido_constants.h"
 #include "device/fido/u2f_ble_connection.h"
 #include "device/fido/u2f_ble_transaction.h"
 #include "device/fido/u2f_device.h"
@@ -51,10 +52,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fBleDevice : public U2fDevice {
   base::WeakPtr<U2fDevice> GetWeakPtr() override;
 
  private:
-  // INIT --> BUSY --> CONNECTED --> BUSY <--> READY.
-  // DEVICE_ERROR persists.
-  enum class State { INIT, CONNECTED, READY, BUSY, DEVICE_ERROR };
-
   void Transition();
 
   void OnConnectionStatus(bool success);
@@ -72,7 +69,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fBleDevice : public U2fDevice {
   void StopTimeout();
   void OnTimeout();
 
-  State state_ = State::INIT;
+  State state_ = State::kInit;
   base::OneShotTimer timer_;
 
   std::unique_ptr<U2fBleConnection> connection_;

device/fido/u2f_ble_device_unittest.cc

@@ -84,7 +84,7 @@ TEST_F(U2fBleDeviceTest, SendPingTest_Failure_Timeout) {
 
   EXPECT_CALL(*connection(), WriteControlPointPtr(_, _))
       .WillOnce(Invoke([this](const auto& data, auto* cb) {
-        scoped_task_environment_.FastForwardBy(U2fDevice::kDeviceTimeout);
+        scoped_task_environment_.FastForwardBy(kDeviceTimeout);
       }));
 
   TestDeviceCallbackReceiver callback_receiver;
@@ -101,7 +101,7 @@ TEST_F(U2fBleDeviceTest, SendPingTest) {
   EXPECT_CALL(*connection(), WriteControlPointPtr(_, _))
       .WillOnce(Invoke([this](const auto& data, auto* cb) {
         auto almost_time_out =
-            U2fDevice::kDeviceTimeout - base::TimeDelta::FromMicroseconds(1);
+            kDeviceTimeout - base::TimeDelta::FromMicroseconds(1);
         scoped_task_environment_.FastForwardBy(almost_time_out);
         connection()->read_callback().Run(data);
         std::move(*cb).Run(true);

device/fido/u2f_ble_discovery.cc

@@ -15,8 +15,8 @@
 #include "device/bluetooth/bluetooth_discovery_filter.h"
 #include "device/bluetooth/bluetooth_discovery_session.h"
 #include "device/bluetooth/bluetooth_uuid.h"
+#include "device/fido/fido_ble_uuids.h"
 #include "device/fido/u2f_ble_device.h"
-#include "device/fido/u2f_ble_uuids.h"
 
 namespace device {
 
@@ -55,7 +55,7 @@ void U2fBleDiscovery::Stop() {
 
 // static
 const BluetoothUUID& U2fBleDiscovery::U2fServiceUUID() {
-  static const BluetoothUUID service_uuid(kU2fServiceUUID);
+  static const BluetoothUUID service_uuid(kFidoServiceUUID);
   return service_uuid;
 }
 

device/fido/u2f_ble_frames.cc

@@ -9,12 +9,14 @@
 
 #include "base/logging.h"
 #include "base/numerics/safe_conversions.h"
+#include "device/fido/fido_constants.h"
 
 namespace device {
 
 U2fBleFrame::U2fBleFrame() = default;
 
-U2fBleFrame::U2fBleFrame(U2fCommandType command, std::vector<uint8_t> data)
+U2fBleFrame::U2fBleFrame(FidoBleDeviceCommand command,
+                         std::vector<uint8_t> data)
     : command_(command), data_(std::move(data)) {}
 
 U2fBleFrame::U2fBleFrame(U2fBleFrame&&) = default;
@@ -24,26 +26,26 @@ U2fBleFrame::~U2fBleFrame() = default;
 
 bool U2fBleFrame::IsValid() const {
   switch (command_) {
-    case U2fCommandType::CMD_PING:
-    case U2fCommandType::CMD_MSG:
+    case FidoBleDeviceCommand::kPing:
+    case FidoBleDeviceCommand::kMsg:
+    case FidoBleDeviceCommand::kCancel:
       return true;
-    case U2fCommandType::CMD_KEEPALIVE:
-    case U2fCommandType::CMD_ERROR:
+    case FidoBleDeviceCommand::kKeepAlive:
+    case FidoBleDeviceCommand::kError:
       return data_.size() == 1;
-    case U2fCommandType::UNDEFINED:
-    default:
-      return false;
   }
+  NOTREACHED();
+  return false;
 }
 
 U2fBleFrame::KeepaliveCode U2fBleFrame::GetKeepaliveCode() const {
-  DCHECK_EQ(command_, U2fCommandType::CMD_KEEPALIVE);
+  DCHECK_EQ(command_, FidoBleDeviceCommand::kKeepAlive);
   DCHECK_EQ(data_.size(), 1u);
   return static_cast<KeepaliveCode>(data_[0]);
 }
 
 U2fBleFrame::ErrorCode U2fBleFrame::GetErrorCode() const {
-  DCHECK_EQ(command_, U2fCommandType::CMD_ERROR);
+  DCHECK_EQ(command_, FidoBleDeviceCommand::kError);
   DCHECK_EQ(data_.size(), 1u);
   return static_cast<ErrorCode>(data_[0]);
 }
@@ -97,7 +99,7 @@ bool U2fBleFrameInitializationFragment::Parse(
   if (data.size() < 3)
     return false;
 
-  const auto command = static_cast<U2fCommandType>(data[0]);
+  const auto command = static_cast<FidoBleDeviceCommand>(data[0]);
   const uint16_t data_length = (static_cast<uint16_t>(data[1]) << 8) + data[2];
   if (static_cast<size_t>(data_length) + 3 < data.size())
     return false;

device/fido/u2f_ble_frames.h

@@ -14,7 +14,7 @@
 #include "base/containers/queue.h"
 #include "base/containers/span.h"
 #include "base/macros.h"
-#include "device/fido/u2f_command_type.h"
+#include "device/fido/fido_constants.h"
 
 namespace device {
 
@@ -56,14 +56,14 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fBleFrame {
   };
 
   U2fBleFrame();
-  U2fBleFrame(U2fCommandType command, std::vector<uint8_t> data);
+  U2fBleFrame(FidoBleDeviceCommand command, std::vector<uint8_t> data);
 
   U2fBleFrame(U2fBleFrame&&);
   U2fBleFrame& operator=(U2fBleFrame&&);
 
   ~U2fBleFrame();
 
-  U2fCommandType command() const { return command_; }
+  FidoBleDeviceCommand command() const { return command_; }
 
   bool IsValid() const;
   KeepaliveCode GetKeepaliveCode() const;
@@ -83,7 +83,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fBleFrame {
   ToFragments(size_t max_fragment_size) const;
 
  private:
-  U2fCommandType command_ = U2fCommandType::UNDEFINED;
+  FidoBleDeviceCommand command_ = FidoBleDeviceCommand::kMsg;
   std::vector<uint8_t> data_;
 
   DISALLOW_COPY_AND_ASSIGN(U2fBleFrame);
@@ -121,20 +121,20 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fBleFrameInitializationFragment
                     U2fBleFrameInitializationFragment* fragment);
 
   U2fBleFrameInitializationFragment() = default;
-  U2fBleFrameInitializationFragment(U2fCommandType command,
+  U2fBleFrameInitializationFragment(FidoBleDeviceCommand command,
                                     uint16_t data_length,
                                     base::span<const uint8_t> fragment)
       : U2fBleFrameFragment(fragment),
         command_(command),
         data_length_(data_length) {}
 
-  U2fCommandType command() const { return command_; }
+  FidoBleDeviceCommand command() const { return command_; }
   uint16_t data_length() const { return data_length_; }
 
   size_t Serialize(std::vector<uint8_t>* buffer) const override;
 
  private:
-  U2fCommandType command_ = U2fCommandType::UNDEFINED;
+  FidoBleDeviceCommand command_ = FidoBleDeviceCommand::kMsg;
   uint16_t data_length_ = 0;
 };
 

device/fido/u2f_ble_frames_fuzzer.cc

@@ -7,8 +7,8 @@
 
 #include <vector>
 
+#include "device/fido/fido_constants.h"
 #include "device/fido/u2f_ble_frames.h"
-#include "device/fido/u2f_command_type.h"
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* raw_data, size_t size) {
   auto data_span = base::make_span(raw_data, size);
@@ -16,7 +16,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* raw_data, size_t size) {
 
   {
     device::U2fBleFrameInitializationFragment fragment(
-        device::U2fCommandType::CMD_MSG, 21123, data_span);
+        device::FidoBleDeviceCommand::kMsg, 21123, data_span);
     std::vector<uint8_t> buffer;
     fragment.Serialize(&buffer);
 
@@ -42,7 +42,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* raw_data, size_t size) {
   }
 
   {
-    device::U2fBleFrame frame(device::U2fCommandType::CMD_PING, data);
+    device::U2fBleFrame frame(device::FidoBleDeviceCommand::kPing, data);
     auto fragments = frame.ToFragments(20);
 
     device::U2fBleFrameAssembler assembler(fragments.first);

device/fido/u2f_ble_frames_unittest.cc

@@ -6,7 +6,6 @@
 
 #include <vector>
 
-#include "device/fido/u2f_command_type.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -27,7 +26,7 @@ TEST(U2fBleFramesTest, InitializationFragment) {
   constexpr uint16_t kDataLength = 21123;
 
   U2fBleFrameInitializationFragment fragment(
-      U2fCommandType::CMD_MSG, kDataLength, base::make_span(data));
+      FidoBleDeviceCommand::kMsg, kDataLength, base::make_span(data));
 
   std::vector<uint8_t> buffer;
   const size_t binary_size = fragment.Serialize(&buffer);
@@ -41,7 +40,7 @@ TEST(U2fBleFramesTest, InitializationFragment) {
 
   EXPECT_EQ(kDataLength, parsed_fragment.data_length());
   EXPECT_EQ(base::make_span(data), parsed_fragment.fragment());
-  EXPECT_EQ(U2fCommandType::CMD_MSG, parsed_fragment.command());
+  EXPECT_EQ(FidoBleDeviceCommand::kMsg, parsed_fragment.command());
 }
 
 TEST(U2fBleFramesTest, ContinuationFragment) {
@@ -68,7 +67,7 @@ TEST(U2fBleFramesTest, SplitAndAssemble) {
                       37, 39, 40, 41, 54, 55, 56, 60, 100, 65535}) {
     SCOPED_TRACE(size);
 
-    U2fBleFrame frame(U2fCommandType::CMD_PING, GetSomeData(size));
+    U2fBleFrame frame(FidoBleDeviceCommand::kPing, GetSomeData(size));
 
     auto fragments = frame.ToFragments(20);
 
@@ -92,7 +91,7 @@ TEST(U2fBleFramesTest, SplitAndAssemble) {
 }
 
 TEST(U2fBleFramesTest, FrameAssemblerError) {
-  U2fBleFrame frame(U2fCommandType::CMD_PING, GetSomeData(30));
+  U2fBleFrame frame(FidoBleDeviceCommand::kPing, GetSomeData(30));
 
   auto fragments = frame.ToFragments(20);
   ASSERT_EQ(1u, fragments.second.size());
@@ -110,17 +109,18 @@ TEST(U2fBleFramesTest, FrameAssemblerError) {
 
 TEST(U2fBleFramesTest, FrameGettersAndValidity) {
   {
-    U2fBleFrame frame(U2fCommandType::CMD_KEEPALIVE, std::vector<uint8_t>(2));
+    U2fBleFrame frame(FidoBleDeviceCommand::kKeepAlive,
+                      std::vector<uint8_t>(2));
     EXPECT_FALSE(frame.IsValid());
   }
   {
-    U2fBleFrame frame(U2fCommandType::CMD_ERROR, {});
+    U2fBleFrame frame(FidoBleDeviceCommand::kError, {});
     EXPECT_FALSE(frame.IsValid());
   }
 
   for (auto code : {U2fBleFrame::KeepaliveCode::TUP_NEEDED,
                     U2fBleFrame::KeepaliveCode::PROCESSING}) {
-    U2fBleFrame frame(U2fCommandType::CMD_KEEPALIVE,
+    U2fBleFrame frame(FidoBleDeviceCommand::kKeepAlive,
                       std::vector<uint8_t>(1, static_cast<uint8_t>(code)));
     EXPECT_TRUE(frame.IsValid());
     EXPECT_EQ(code, frame.GetKeepaliveCode());
@@ -134,7 +134,8 @@ TEST(U2fBleFramesTest, FrameGettersAndValidity) {
            U2fBleFrame::ErrorCode::REQ_TIMEOUT, U2fBleFrame::ErrorCode::NA_1,
            U2fBleFrame::ErrorCode::NA_2, U2fBleFrame::ErrorCode::NA_3,
        }) {
-    U2fBleFrame frame(U2fCommandType::CMD_ERROR, {static_cast<uint8_t>(code)});
+    U2fBleFrame frame(FidoBleDeviceCommand::kError,
+                      {static_cast<uint8_t>(code)});
     EXPECT_TRUE(frame.IsValid());
     EXPECT_EQ(code, frame.GetErrorCode());
   }

device/fido/u2f_ble_transaction.cc

@@ -106,7 +106,7 @@ void U2fBleTransaction::ProcessResponseFrame(U2fBleFrame response_frame) {
     return;
   }
 
-  if (response_frame.command() == U2fCommandType::CMD_KEEPALIVE) {
+  if (response_frame.command() == FidoBleDeviceCommand::kKeepAlive) {
     DVLOG(2) << "CMD_KEEPALIVE: "
              << static_cast<uint8_t>(response_frame.GetKeepaliveCode());
     // Expect another reponse frame soon.
@@ -114,15 +114,14 @@ void U2fBleTransaction::ProcessResponseFrame(U2fBleFrame response_frame) {
     return;
   }
 
-  DCHECK_EQ(response_frame.command(), U2fCommandType::CMD_ERROR);
+  DCHECK_EQ(response_frame.command(), FidoBleDeviceCommand::kError);
   DLOG(ERROR) << "CMD_ERROR: "
               << static_cast<uint8_t>(response_frame.GetErrorCode());
   OnError();
 }
 
 void U2fBleTransaction::StartTimeout() {
-  timer_.Start(FROM_HERE, U2fDevice::kDeviceTimeout, this,
-               &U2fBleTransaction::OnError);
+  timer_.Start(FROM_HERE, kDeviceTimeout, this, &U2fBleTransaction::OnError);
 }
 
 void U2fBleTransaction::StopTimeout() {

device/fido/u2f_command_type.h

-// Copyright 2017 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 DEVICE_FIDO_U2F_COMMAND_TYPE_H_
-#define DEVICE_FIDO_U2F_COMMAND_TYPE_H_
-
-#include <stdint.h>
-
-namespace device {
-
-// The type of a command that can be sent either to or from a U2F authenticator,
-// i.e. a request or a response.
-//
-// Each request sent to a device results in a response of *the same* type sent
-// back, unless there was an error in which case a CMD_ERROR is returned.
-enum class U2fCommandType : uint8_t {
-  UNDEFINED = 0x00,
-
-  // Sends arbitrary data to the device which echoes the same data back.
-  CMD_PING = 0x81,
-
-  // Authenticator sends this in response to requests that it could not process
-  // within a time limit. The client should take action appropriate to the
-  // message reason (e.g., notify the user to perform a test-of-user-presence),
-  // and wait for the next message.
-  CMD_KEEPALIVE = 0x82,
-
-  // Encapsulates a U2F protocol raw message.
-  CMD_MSG = 0x83,
-
-  // Requests a unique channel from a USB/HID device.
-  CMD_INIT = 0x86,
-
-  // Instructs a USB/HID authenticator to show the user that it is active.
-  CMD_WINK = 0x88,
-
-  // Used as a response in case an error occurs during a request.
-  CMD_ERROR = 0xBF,
-};
-
-}  // namespace device
-
-#endif  // DEVICE_FIDO_U2F_COMMAND_TYPE_H_

device/fido/u2f_device.cc

@@ -6,8 +6,6 @@
 
 namespace device {
 
-constexpr base::TimeDelta U2fDevice::kDeviceTimeout;
-
 U2fDevice::U2fDevice() = default;
 U2fDevice::~U2fDevice() = default;
 

device/fido/u2f_device.h

@@ -23,17 +23,12 @@ namespace device {
 // standardized Register, Sign, and GetVersion methods.
 class COMPONENT_EXPORT(DEVICE_FIDO) U2fDevice {
  public:
-  enum class ProtocolVersion {
-    U2F_V2,
-    UNKNOWN,
-  };
-
   using WinkCallback = base::OnceClosure;
-
   using DeviceCallback =
       base::OnceCallback<void(base::Optional<std::vector<uint8_t>> response)>;
 
-  static constexpr auto kDeviceTimeout = base::TimeDelta::FromSeconds(3);
+  // Internal state machine states.
+  enum class State { kInit, kConnected, kBusy, kReady, kDeviceError };
 
   U2fDevice();
   virtual ~U2fDevice();

device/fido/u2f_hid_device.cc

@@ -26,7 +26,7 @@ static constexpr uint8_t kReportId = 0x00;
 U2fHidDevice::U2fHidDevice(device::mojom::HidDeviceInfoPtr device_info,
                            device::mojom::HidManager* hid_manager)
     : U2fDevice(),
-      state_(State::INIT),
+      state_(State::kInit),
       hid_manager_(hid_manager),
       device_info_(std::move(device_info)),
       weak_factory_(this) {}
@@ -45,34 +45,34 @@ void U2fHidDevice::Transition(std::vector<uint8_t> command,
   auto repeating_callback =
       base::AdaptCallbackForRepeating(std::move(callback));
   switch (state_) {
-    case State::INIT:
-      state_ = State::BUSY;
+    case State::kInit:
+      state_ = State::kBusy;
       ArmTimeout(repeating_callback);
       Connect(base::BindOnce(&U2fHidDevice::OnConnect,
                              weak_factory_.GetWeakPtr(), std::move(command),
                              repeating_callback));
       break;
-    case State::CONNECTED:
-      state_ = State::BUSY;
+    case State::kConnected:
+      state_ = State::kBusy;
       ArmTimeout(repeating_callback);
       AllocateChannel(std::move(command), repeating_callback);
       break;
-    case State::IDLE: {
-      state_ = State::BUSY;
+    case State::kReady: {
+      state_ = State::kBusy;
       ArmTimeout(repeating_callback);
       // Write message to the device.
       WriteMessage(
-          FidoHidMessage::Create(channel_id_, CtapHidDeviceCommand::kCtapHidMsg,
+          FidoHidMessage::Create(channel_id_, FidoHidDeviceCommand::kMsg,
                                  std::move(command)),
           true,
           base::BindOnce(&U2fHidDevice::MessageReceived,
                          weak_factory_.GetWeakPtr(), repeating_callback));
       break;
     }
-    case State::BUSY:
+    case State::kBusy:
       pending_transactions_.emplace(std::move(command), repeating_callback);
       break;
-    case State::DEVICE_ERROR:
+    case State::kDeviceError:
     default:
       base::WeakPtr<U2fHidDevice> self = weak_factory_.GetWeakPtr();
       repeating_callback.Run(base::nullopt);
@@ -96,15 +96,15 @@ void U2fHidDevice::Connect(ConnectCallback callback) {
 void U2fHidDevice::OnConnect(std::vector<uint8_t> command,
                              DeviceCallback callback,
                              device::mojom::HidConnectionPtr connection) {
-  if (state_ == State::DEVICE_ERROR)
+  if (state_ == State::kDeviceError)
     return;
   timeout_callback_.Cancel();
 
   if (connection) {
     connection_ = std::move(connection);
-    state_ = State::CONNECTED;
+    state_ = State::kConnected;
   } else {
-    state_ = State::DEVICE_ERROR;
+    state_ = State::kDeviceError;
   }
   Transition(std::move(command), std::move(callback));
 }
@@ -114,12 +114,12 @@ void U2fHidDevice::AllocateChannel(std::vector<uint8_t> command,
   // Send random nonce to device to verify received message.
   std::vector<uint8_t> nonce(8);
   crypto::RandBytes(nonce.data(), nonce.size());
-  WriteMessage(FidoHidMessage::Create(
-                   channel_id_, CtapHidDeviceCommand::kCtapHidInit, nonce),
-               true,
-               base::BindOnce(&U2fHidDevice::OnAllocateChannel,
-                              weak_factory_.GetWeakPtr(), nonce,
-                              std::move(command), std::move(callback)));
+  WriteMessage(
+      FidoHidMessage::Create(channel_id_, FidoHidDeviceCommand::kInit, nonce),
+      true,
+      base::BindOnce(&U2fHidDevice::OnAllocateChannel,
+                     weak_factory_.GetWeakPtr(), nonce, std::move(command),
+                     std::move(callback)));
 }
 
 void U2fHidDevice::OnAllocateChannel(std::vector<uint8_t> nonce,
@@ -127,12 +127,12 @@ void U2fHidDevice::OnAllocateChannel(std::vector<uint8_t> nonce,
                                      DeviceCallback callback,
                                      bool success,
                                      std::unique_ptr<FidoHidMessage> message) {
-  if (state_ == State::DEVICE_ERROR)
+  if (state_ == State::kDeviceError)
     return;
   timeout_callback_.Cancel();
 
   if (!success || !message) {
-    state_ = State::DEVICE_ERROR;
+    state_ = State::kDeviceError;
     Transition(std::vector<uint8_t>(), std::move(callback));
     return;
   }
@@ -147,7 +147,7 @@ void U2fHidDevice::OnAllocateChannel(std::vector<uint8_t> nonce,
   // 16: Capabilities
   std::vector<uint8_t> payload = message->GetMessagePayload();
   if (payload.size() != 17) {
-    state_ = State::DEVICE_ERROR;
+    state_ = State::kDeviceError;
     Transition(std::vector<uint8_t>(), std::move(callback));
     return;
   }
@@ -171,7 +171,7 @@ void U2fHidDevice::OnAllocateChannel(std::vector<uint8_t> nonce,
   channel_id_ |= payload[index++] << 8;
   channel_id_ |= payload[index++];
   capabilities_ = payload[16];
-  state_ = State::IDLE;
+  state_ = State::kReady;
   Transition(std::move(command), std::move(callback));
 }
 
@@ -271,16 +271,16 @@ void U2fHidDevice::OnReadContinuation(
 void U2fHidDevice::MessageReceived(DeviceCallback callback,
                                    bool success,
                                    std::unique_ptr<FidoHidMessage> message) {
-  if (state_ == State::DEVICE_ERROR)
+  if (state_ == State::kDeviceError)
     return;
   timeout_callback_.Cancel();
   if (!success) {
-    state_ = State::DEVICE_ERROR;
+    state_ = State::kDeviceError;
     Transition(std::vector<uint8_t>(), std::move(callback));
     return;
   }
 
-  state_ = State::IDLE;
+  state_ = State::kReady;
   base::WeakPtr<U2fHidDevice> self = weak_factory_.GetWeakPtr();
   std::move(callback).Run(
       (success && message) ? base::make_optional(message->GetMessagePayload())
@@ -298,17 +298,16 @@ void U2fHidDevice::MessageReceived(DeviceCallback callback,
 
 void U2fHidDevice::TryWink(WinkCallback callback) {
   // Only try to wink if device claims support.
-  if (!(capabilities_ & kWinkCapability) || state_ != State::IDLE) {
+  if (!(capabilities_ & kWinkCapability) || state_ != State::kReady) {
     std::move(callback).Run();
     return;
   }
 
-  WriteMessage(
-      FidoHidMessage::Create(channel_id_, CtapHidDeviceCommand::kCtapHidWink,
-                             std::vector<uint8_t>()),
-      true,
-      base::BindOnce(&U2fHidDevice::OnWink, weak_factory_.GetWeakPtr(),
-                     std::move(callback)));
+  WriteMessage(FidoHidMessage::Create(channel_id_, FidoHidDeviceCommand::kWink,
+                                      std::vector<uint8_t>()),
+               true,
+               base::BindOnce(&U2fHidDevice::OnWink, weak_factory_.GetWeakPtr(),
+                              std::move(callback)));
 }
 
 void U2fHidDevice::OnWink(WinkCallback callback,
@@ -328,7 +327,7 @@ void U2fHidDevice::ArmTimeout(DeviceCallback callback) {
 }
 
 void U2fHidDevice::OnTimeout(DeviceCallback callback) {
-  state_ = State::DEVICE_ERROR;
+  state_ = State::kDeviceError;
   Transition(std::vector<uint8_t>(), std::move(callback));
 }
 

device/fido/u2f_hid_device.h

@@ -53,9 +53,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fHidDevice : public U2fDevice {
   static constexpr uint8_t kLockCapability = 0x02;
   static constexpr uint32_t kBroadcastChannel = 0xffffffff;
 
-  // Internal state machine states
-  enum class State { INIT, CONNECTED, BUSY, IDLE, DEVICE_ERROR };
-
   using U2fHidMessageCallback =
       base::OnceCallback<void(bool, std::unique_ptr<FidoHidMessage>)>;
   using ConnectCallback = device::mojom::HidManager::ConnectCallback;
@@ -104,7 +101,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) U2fHidDevice : public U2fDevice {
 
   uint32_t channel_id_ = kBroadcastChannel;
   uint8_t capabilities_ = 0;
-  State state_ = State::INIT;
+  State state_ = State::kInit;
 
   base::CancelableOnceClosure timeout_callback_;
   std::queue<std::pair<std::vector<uint8_t>, DeviceCallback>>

device/fido/u2f_hid_device_unittest.cc

@@ -13,6 +13,7 @@
 #include "components/apdu/apdu_command.h"
 #include "components/apdu/apdu_response.h"
 #include "device/fido/fake_hid_impl_for_testing.h"
+#include "device/fido/fido_constants.h"
 #include "device/fido/test_callback_receiver.h"
 #include "device/fido/u2f_hid_device.h"
 #include "device/fido/u2f_request.h"
@@ -52,9 +53,8 @@ std::vector<uint8_t> MakePacket(base::StringPiece hex) {
 }
 
 // Returns HID_INIT request to send to device with mock connection.
-std::vector<uint8_t> CreateMockHidInitResponse(
-    std::vector<uint8_t> nonce,
-    std::vector<uint8_t> channel_id) {
+std::vector<uint8_t> CreateMockInitResponse(std::vector<uint8_t> nonce,
+                                            std::vector<uint8_t> channel_id) {
   // 4 bytes of broadcast channel identifier(ffffffff), followed by
   // HID_INIT command(86) and 2 byte payload length(11).
   return MakePacket("ffffffff860011" + HexEncode(nonce) +
@@ -156,7 +156,7 @@ TEST_F(U2fHidDeviceTest, TestConnectionFailure) {
   ASSERT_EQ(static_cast<size_t>(1), u2f_devices.size());
   auto& device = u2f_devices.front();
   // Put device in IDLE state.
-  device->state_ = U2fHidDevice::State::IDLE;
+  device->state_ = U2fDevice::State::kReady;
 
   // Manually delete connection.
   device->connection_ = nullptr;
@@ -172,7 +172,7 @@ TEST_F(U2fHidDeviceTest, TestConnectionFailure) {
   device->DeviceTransact(U2fRequest::GetU2fVersionApduCommand(),
                          receiver_3.callback());
 
-  EXPECT_EQ(U2fHidDevice::State::DEVICE_ERROR, device->state_);
+  EXPECT_EQ(U2fDevice::State::kDeviceError, device->state_);
 
   EXPECT_FALSE(std::get<0>(*receiver_1.result()));
   EXPECT_FALSE(std::get<0>(*receiver_2.result()));
@@ -196,13 +196,13 @@ TEST_F(U2fHidDeviceTest, TestDeviceError) {
 
   // Mock connection where writes always fail.
   FakeHidConnection::mock_connection_error_ = true;
-  device->state_ = U2fHidDevice::State::IDLE;
+  device->state_ = U2fDevice::State::kReady;
 
   TestDeviceCallbackReceiver receiver_0;
   device->DeviceTransact(U2fRequest::GetU2fVersionApduCommand(),
                          receiver_0.callback());
   EXPECT_FALSE(std::get<0>(*receiver_0.result()));
-  EXPECT_EQ(U2fHidDevice::State::DEVICE_ERROR, device->state_);
+  EXPECT_EQ(U2fDevice::State::kDeviceError, device->state_);
 
   // Add pending transactions manually and ensure they are processed.
   // Add pending transactions manually and ensure they are processed.
@@ -217,7 +217,7 @@ TEST_F(U2fHidDeviceTest, TestDeviceError) {
                          receiver_3.callback());
   FakeHidConnection::mock_connection_error_ = false;
 
-  EXPECT_EQ(U2fHidDevice::State::DEVICE_ERROR, device->state_);
+  EXPECT_EQ(U2fDevice::State::kDeviceError, device->state_);
   EXPECT_FALSE(std::get<0>(*receiver_1.result()));
   EXPECT_FALSE(std::get<0>(*receiver_2.result()));
   EXPECT_FALSE(std::get<0>(*receiver_3.result()));
@@ -260,14 +260,14 @@ TEST_F(U2fHidDeviceTest, TestRetryChannelAllocation) {
                      device::mojom::HidConnection::ReadCallback* cb) {
             std::move(*cb).Run(
                 true, 0,
-                CreateMockHidInitResponse(
+                CreateMockInitResponse(
                     kIncorrectNonce, mock_connection.connection_channel_id()));
           })))
       // Second response to HID_INIT request with correct nonce.
       .WillOnce(WithArg<0>(Invoke(
           [&mock_connection](device::mojom::HidConnection::ReadCallback* cb) {
             std::move(*cb).Run(true, 0,
-                               CreateMockHidInitResponse(
+                               CreateMockInitResponse(
                                    mock_connection.nonce(),
                                    mock_connection.connection_channel_id()));
           })))

device/fido/u2f_request.cc

@@ -159,7 +159,7 @@ void U2fRequest::OnDeviceVersionRequest(
       apdu_response->status() == apdu::ApduResponse::Status::SW_NO_ERROR &&
       std::equal(apdu_response->data().cbegin(), apdu_response->data().cend(),
                  kU2fVersionResponse.cbegin(), kU2fVersionResponse.cend())) {
-    std::move(callback).Run(U2fDevice::ProtocolVersion::U2F_V2);
+    std::move(callback).Run(ProtocolVersion::kU2f);
   } else if (!legacy) {
     // Standard GetVersion failed, attempt legacy GetVersion command.
     device->DeviceTransact(
@@ -168,7 +168,7 @@ void U2fRequest::OnDeviceVersionRequest(
                        weak_factory_.GetWeakPtr(), std::move(callback), device,
                        true /* legacy */));
   } else {
-    std::move(callback).Run(U2fDevice::ProtocolVersion::UNKNOWN);
+    std::move(callback).Run(ProtocolVersion::kUnknown);
   }
 }
 

device/fido/u2f_request.h

@@ -30,8 +30,7 @@ namespace device {
 
 class COMPONENT_EXPORT(DEVICE_FIDO) U2fRequest : public U2fDiscovery::Observer {
  public:
-  using VersionCallback =
-      base::OnceCallback<void(U2fDevice::ProtocolVersion version)>;
+  using VersionCallback = base::OnceCallback<void(ProtocolVersion version)>;
 
   // U2fRequest will create a discovery instance and register itself as an
   // observer for each passed in transport protocol.

device/fido/u2f_request_unittest.cc

@@ -36,7 +36,7 @@ class FakeU2fRequest : public U2fRequest {
 };
 
 using TestVersionCallback =
-    ::device::test::TestCallbackReceiver<U2fDevice::ProtocolVersion>;
+    ::device::test::TestCallbackReceiver<ProtocolVersion>;
 
 }  // namespace
 
@@ -371,7 +371,7 @@ TEST_F(U2fRequestTest, TestLegacyVersionRequest) {
                                  device_ptr->GetWeakPtr(), false /* legacy */,
                                  std::vector<uint8_t>());
 
-  EXPECT_EQ(U2fDevice::ProtocolVersion::U2F_V2,
+  EXPECT_EQ(ProtocolVersion::kU2f,
             std::get<0>(*version_callback_receiver().result()));
 }