De-pointerize QuicServerConfigProtobuf variables
This CL converts a bunch of unique_ptr<QuicServerConfigProtobuf> objects to plain QuicServerConfigProtobuf. The extra indirection is not helping us here, and just introduces a bunch of "but what if it's null?" confusion. Especially since protobufs are moveable now, there's almost no performance implication to this change, and none of this code is in the serving path anyhow.
gfe-relnote: Use plain objects instead of unique_ptrs. No functional change intended, not flag-protected.
PiperOrigin-RevId: 239653446
Change-Id: I9007e89b33dfe201354d89fdbaa4d12c5d86fbed
diff --git a/quic/core/crypto/crypto_server_test.cc b/quic/core/crypto/crypto_server_test.cc
index f784bdb..4d080c7 100644
--- a/quic/core/crypto/crypto_server_test.cc
+++ b/quic/core/crypto/crypto_server_test.cc
@@ -135,9 +135,9 @@
old_config_options.id = kOldConfigId;
config_.AddDefaultConfig(rand_, &clock_, old_config_options);
clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1000));
- std::unique_ptr<QuicServerConfigProtobuf> primary_config(
- config_.GenerateConfig(rand_, &clock_, config_options_));
- primary_config->set_primary_time(clock_.WallNow().ToUNIXSeconds());
+ QuicServerConfigProtobuf primary_config =
+ config_.GenerateConfig(rand_, &clock_, config_options_);
+ primary_config.set_primary_time(clock_.WallNow().ToUNIXSeconds());
std::unique_ptr<CryptoHandshakeMessage> msg(
config_.AddConfig(std::move(primary_config), clock_.WallNow()));
diff --git a/quic/core/crypto/quic_crypto_server_config.cc b/quic/core/crypto/quic_crypto_server_config.cc
index 1c9fd92..253fb0e 100644
--- a/quic/core/crypto/quic_crypto_server_config.cc
+++ b/quic/core/crypto/quic_crypto_server_config.cc
@@ -267,10 +267,10 @@
QuicCryptoServerConfig::~QuicCryptoServerConfig() {}
// static
-std::unique_ptr<QuicServerConfigProtobuf>
-QuicCryptoServerConfig::GenerateConfig(QuicRandom* rand,
- const QuicClock* clock,
- const ConfigOptions& options) {
+QuicServerConfigProtobuf QuicCryptoServerConfig::GenerateConfig(
+ QuicRandom* rand,
+ const QuicClock* clock,
+ const ConfigOptions& options) {
CryptoHandshakeMessage msg;
const std::string curve25519_private_key =
@@ -368,14 +368,14 @@
std::unique_ptr<QuicData> serialized =
CryptoFramer::ConstructHandshakeMessage(msg);
- auto config = QuicMakeUnique<QuicServerConfigProtobuf>();
- config->set_config(std::string(serialized->AsStringPiece()));
- QuicServerConfigProtobuf::PrivateKey* curve25519_key = config->add_key();
+ QuicServerConfigProtobuf config;
+ config.set_config(std::string(serialized->AsStringPiece()));
+ QuicServerConfigProtobuf::PrivateKey* curve25519_key = config.add_key();
curve25519_key->set_tag(kC255);
curve25519_key->set_private_key(curve25519_private_key);
if (options.p256) {
- QuicServerConfigProtobuf::PrivateKey* p256_key = config->add_key();
+ QuicServerConfigProtobuf::PrivateKey* p256_key = config.add_key();
p256_key->set_tag(kP256);
p256_key->set_private_key(p256_private_key);
}
@@ -384,10 +384,10 @@
}
std::unique_ptr<CryptoHandshakeMessage> QuicCryptoServerConfig::AddConfig(
- std::unique_ptr<QuicServerConfigProtobuf> protobuf,
+ const QuicServerConfigProtobuf& protobuf,
const QuicWallTime now) {
std::unique_ptr<CryptoHandshakeMessage> msg =
- CryptoFramer::ParseMessage(protobuf->config());
+ CryptoFramer::ParseMessage(protobuf.config());
if (!msg.get()) {
QUIC_LOG(WARNING) << "Failed to parse server config message";
@@ -427,7 +427,7 @@
}
bool QuicCryptoServerConfig::SetConfigs(
- const std::vector<std::unique_ptr<QuicServerConfigProtobuf>>& protobufs,
+ const std::vector<QuicServerConfigProtobuf>& protobufs,
const QuicWallTime now) {
std::vector<QuicReferenceCountedPointer<Config>> parsed_configs;
for (auto& protobuf : protobufs) {
@@ -1492,9 +1492,9 @@
QuicReferenceCountedPointer<QuicCryptoServerConfig::Config>
QuicCryptoServerConfig::ParseConfigProtobuf(
- const std::unique_ptr<QuicServerConfigProtobuf>& protobuf) {
+ const QuicServerConfigProtobuf& protobuf) {
std::unique_ptr<CryptoHandshakeMessage> msg =
- CryptoFramer::ParseMessage(protobuf->config());
+ CryptoFramer::ParseMessage(protobuf.config());
if (msg->tag() != kSCFG) {
QUIC_LOG(WARNING) << "Server config message has tag " << msg->tag()
@@ -1503,15 +1503,15 @@
}
QuicReferenceCountedPointer<Config> config(new Config);
- config->serialized = protobuf->config();
+ config->serialized = protobuf.config();
config->source_address_token_boxer = &source_address_token_boxer_;
- if (protobuf->has_primary_time()) {
+ if (protobuf.has_primary_time()) {
config->primary_time =
- QuicWallTime::FromUNIXSeconds(protobuf->primary_time());
+ QuicWallTime::FromUNIXSeconds(protobuf.primary_time());
}
- config->priority = protobuf->priority();
+ config->priority = protobuf.priority();
QuicStringPiece scid;
if (!msg->GetStringPiece(kSCID, &scid)) {
@@ -1554,10 +1554,10 @@
static_assert(sizeof(config->orbit) == kOrbitSize, "incorrect orbit size");
memcpy(config->orbit, orbit.data(), sizeof(config->orbit));
- if (kexs_tags.size() != static_cast<size_t>(protobuf->key_size())) {
+ if (kexs_tags.size() != static_cast<size_t>(protobuf.key_size())) {
QUIC_LOG(WARNING) << "Server config has " << kexs_tags.size()
<< " key exchange methods configured, but "
- << protobuf->key_size() << " private keys";
+ << protobuf.key_size() << " private keys";
return nullptr;
}
@@ -1577,8 +1577,8 @@
config->kexs.push_back(tag);
- for (int j = 0; j < protobuf->key_size(); j++) {
- const QuicServerConfigProtobuf::PrivateKey& key = protobuf->key(i);
+ for (int j = 0; j < protobuf.key_size(); j++) {
+ const QuicServerConfigProtobuf::PrivateKey& key = protobuf.key(i);
if (key.tag() == tag) {
private_key = key.private_key();
break;
diff --git a/quic/core/crypto/quic_crypto_server_config.h b/quic/core/crypto/quic_crypto_server_config.h
index b664757..651195b 100644
--- a/quic/core/crypto/quic_crypto_server_config.h
+++ b/quic/core/crypto/quic_crypto_server_config.h
@@ -230,17 +230,16 @@
// Generates a QuicServerConfigProtobuf protobuf suitable for
// AddConfig and SetConfigs.
- static std::unique_ptr<QuicServerConfigProtobuf> GenerateConfig(
- QuicRandom* rand,
- const QuicClock* clock,
- const ConfigOptions& options);
+ static QuicServerConfigProtobuf GenerateConfig(QuicRandom* rand,
+ const QuicClock* clock,
+ const ConfigOptions& options);
// AddConfig adds a QuicServerConfigProtobuf to the available configurations.
// It returns the SCFG message from the config if successful. |now| is used in
// conjunction with |protobuf->primary_time()| to determine whether the
// config should be made primary.
std::unique_ptr<CryptoHandshakeMessage> AddConfig(
- std::unique_ptr<QuicServerConfigProtobuf> protobuf,
+ const QuicServerConfigProtobuf& protobuf,
QuicWallTime now);
// AddDefaultConfig calls DefaultConfig to create a config and then calls
@@ -258,9 +257,8 @@
// known, but are missing from the protobufs are deleted, unless they are
// currently the primary config. SetConfigs returns false if any errors were
// encountered and no changes to the QuicCryptoServerConfig will occur.
- bool SetConfigs(
- const std::vector<std::unique_ptr<QuicServerConfigProtobuf>>& protobufs,
- QuicWallTime now);
+ bool SetConfigs(const std::vector<QuicServerConfigProtobuf>& protobufs,
+ QuicWallTime now);
// SetSourceAddressTokenKeys sets the keys to be tried, in order, when
// decrypting a source address token. Note that these keys are used *without*
@@ -726,7 +724,7 @@
// QuicReferenceCountedPointer<Config> if successful. The caller adopts the
// reference to the Config. On error, ParseConfigProtobuf returns nullptr.
QuicReferenceCountedPointer<Config> ParseConfigProtobuf(
- const std::unique_ptr<QuicServerConfigProtobuf>& protobuf);
+ const QuicServerConfigProtobuf& protobuf);
// NewSourceAddressToken returns a fresh source address token for the given
// IP address. |cached_network_params| is optional, and can be nullptr.
diff --git a/quic/core/crypto/quic_crypto_server_config_test.cc b/quic/core/crypto/quic_crypto_server_config_test.cc
index f8e4861..61f8006 100644
--- a/quic/core/crypto/quic_crypto_server_config_test.cc
+++ b/quic/core/crypto/quic_crypto_server_config_test.cc
@@ -334,7 +334,7 @@
bool has_invalid = false;
- std::vector<std::unique_ptr<QuicServerConfigProtobuf>> protobufs;
+ std::vector<QuicServerConfigProtobuf> protobufs;
for (const auto& config : configs) {
const ServerConfigID& server_config_id = config.server_config_id;
const int primary_time = config.primary_time;
@@ -343,12 +343,12 @@
QuicCryptoServerConfig::ConfigOptions options;
options.id = server_config_id;
options.orbit = kOrbit;
- std::unique_ptr<QuicServerConfigProtobuf> protobuf =
+ QuicServerConfigProtobuf protobuf =
QuicCryptoServerConfig::GenerateConfig(rand_, &clock_, options);
- protobuf->set_primary_time(primary_time);
- protobuf->set_priority(priority);
+ protobuf.set_primary_time(primary_time);
+ protobuf.set_priority(priority);
if (std::string(server_config_id).find("INVALID") == 0) {
- protobuf->clear_key();
+ protobuf.clear_key();
has_invalid = true;
}
protobufs.push_back(std::move(protobuf));
diff --git a/quic/core/quic_crypto_client_handshaker_test.cc b/quic/core/quic_crypto_client_handshaker_test.cc
index 329f3c3..992df8f 100644
--- a/quic/core/quic_crypto_client_handshaker_test.cc
+++ b/quic/core/quic_crypto_client_handshaker_test.cc
@@ -153,11 +153,10 @@
void InitializeServerParametersToEnableFullHello() {
QuicCryptoServerConfig::ConfigOptions options;
- std::unique_ptr<QuicServerConfigProtobuf> config =
- QuicCryptoServerConfig::GenerateConfig(helper_.GetRandomGenerator(),
- helper_.GetClock(), options);
+ QuicServerConfigProtobuf config = QuicCryptoServerConfig::GenerateConfig(
+ helper_.GetRandomGenerator(), helper_.GetClock(), options);
state_.Initialize(
- config->config(), "sourcetoken", std::vector<std::string>{"Dummy cert"},
+ config.config(), "sourcetoken", std::vector<std::string>{"Dummy cert"},
"", "chlo_hash", "signature", helper_.GetClock()->WallNow(),
helper_.GetClock()->WallNow().Add(QuicTime::Delta::FromSeconds(30)));
diff --git a/quic/test_tools/crypto_test_utils.cc b/quic/test_tools/crypto_test_utils.cc
index 94ca8e1..66692bd 100644
--- a/quic/test_tools/crypto_test_utils.cc
+++ b/quic/test_tools/crypto_test_utils.cc
@@ -1008,10 +1008,9 @@
old_config_options.id = "old-config-id";
crypto_config->AddDefaultConfig(QuicRandom::GetInstance(), clock,
old_config_options);
- std::unique_ptr<QuicServerConfigProtobuf> primary_config =
- crypto_config->GenerateConfig(QuicRandom::GetInstance(), clock,
- new_config_options);
- primary_config->set_primary_time(clock->WallNow().ToUNIXSeconds());
+ QuicServerConfigProtobuf primary_config = crypto_config->GenerateConfig(
+ QuicRandom::GetInstance(), clock, new_config_options);
+ primary_config.set_primary_time(clock->WallNow().ToUNIXSeconds());
std::unique_ptr<CryptoHandshakeMessage> msg =
crypto_config->AddConfig(std::move(primary_config), clock->WallNow());
QuicStringPiece orbit;
diff --git a/quic/test_tools/crypto_test_utils_test.cc b/quic/test_tools/crypto_test_utils_test.cc
index ff288d9..a62cb96 100644
--- a/quic/test_tools/crypto_test_utils_test.cc
+++ b/quic/test_tools/crypto_test_utils_test.cc
@@ -127,10 +127,9 @@
crypto_config.AddDefaultConfig(QuicRandom::GetInstance(), &clock,
old_config_options);
QuicCryptoServerConfig::ConfigOptions new_config_options;
- std::unique_ptr<QuicServerConfigProtobuf> primary_config =
- crypto_config.GenerateConfig(QuicRandom::GetInstance(), &clock,
- new_config_options);
- primary_config->set_primary_time(clock.WallNow().ToUNIXSeconds());
+ QuicServerConfigProtobuf primary_config = crypto_config.GenerateConfig(
+ QuicRandom::GetInstance(), &clock, new_config_options);
+ primary_config.set_primary_time(clock.WallNow().ToUNIXSeconds());
std::unique_ptr<CryptoHandshakeMessage> msg =
crypto_config.AddConfig(std::move(primary_config), clock.WallNow());
QuicStringPiece orbit;
