Currently there are many ways to serialize and deserialize the classes existing in //device/fido. Important conversions include to and from CBOR, as well as to and from byte vectors. However, we currently don't have a uniform interface for this, and most classes use slightly different names for the serialization. Taking conversion to CBOR as example, we currently have the following methods:

cbor::CBORValue AuthenticatorSupportedOptions::ConvertToCBOR();
cbor::CBORValue PublicKeyCredentialDescriptor::ConvertToCBOR();
cbor::CBORValue PublicKeyCredentialParams::ConvertToCBOR();
cbor::CBORValue PublicKeyCredentialRpEntity::ConvertToCBOR();
cbor::CBORValue PublicKeyCredentialUserEntity::ConvertToCBOR();
cbor::CBORValue::MapValue AttestationStatement::GetAsCBORMap();
std::vector<uint8_t> AttestationObject::SerializeToCBOREncodedBytes();
std::vector<uint8_t> CtapGetAssertionRequest::EncodeAsCBOR();
std::vector<uint8_t> CtapMakeCredentialRequest::EncodeAsCBOR();
std::vector<uint8_t> PublicKey::EncodeAsCOSEKey();

These methods differ in their return type, as well as in their names. Ideally we would have consistency here. To this end (and to follow the advice in [1] and [2]) I propose introducing namespace-scoped

  cbor::CBORValue AsCBOR(...);

overloads, performing the same logic. These overloads could live in a neutral location such as  //device/fido/serialization.h. This might require introducing a few new public getters, but this shouldn't effect encapsulation, as most relevant members are already passed via the constructor.

Similarly, we should clean up the following to byte conversions:

std::vector<uint8_t> CtapEmptyAuthenticatorRequest::Serialize();
size_t FidoBleFrameFragment::Serialize(std::vector<int>*);
std::vector<uint8_t> AttestedCredentialData::SerializeAsBytes();
std::vector<uint8_t> AuthenticatorData::SerializeToByteArray();
std::vector<uint8_t> FidoHidMessage::GetMessagePayload();
std::vector<uint8_t> FidoHidPacket::GetSerializedData();

For this we could introduce

  std::vector<uint8_t> AsBytes(...);

overloads, also living in the serialization header.

Lastly, we should consider refactoring the following deserialization functions as well. The vast majority of them could be implemented as non-member non-friend functions as well, since the corresponding constructors are public. However, since we can't overload on return values, these functions would require long names to disambiguate them. Maybe it is better to keep them as factory functions, unify their naming, and making the corresponding constructor private, if appropriate.

base::Optional<AuthentiatorData> DecodeAuthenticatorData(base::span<const uint8_t>);
base::Optional<AuthenticatorMakeCredentialResponse>
CreateFromU2fRegisterResponse(const std::vector<uint8_t>&, base::span<const uint8_t>);
base::Optional<AuthenticatorGetAssertionResponse>
CreateFromU2fSignResponse(
	const std::vector<uint8_t>&,
	const std::vector<uint8_t>&,
	const std::vector<uint8_t>&);
base::Optional<AttestedCredentialData> DecodeFromCtapResponse(base::span<const uint8_t>);
base::Optional<AttestedCredentialData> CreateFromU2fRegisterResonse(
base::span<const uint8_t>, std::unique_ptr<PublicKey>);
std::unique_ptr<FidoAttestationStatement>
CreateFromU2fRegisterResponse(base::span<const uint8_t>);
bool FidoBleFrameFragment::Parse(base::span<const uint8_t>, FidoBleFrameFragment*);
base::Optional<FidoHidMessage> CreateFromSerializedData();
base::Optional<PublicKeyCredentialDescriptor> CreateFromCBORValue();
base::Optional<PublicKeyCredentialUserEntity> CreateFromCBORValue();


[1] http://www.drdobbs.com/cpp/how-non-member-functions-improve-encapsu/184401197
[2] http://www.gotw.ca/gotw/084.htm