Remove unused server designated connection ID from QUIC No behavior change - deleting dead client quic code PiperOrigin-RevId: 317798951 Change-Id: I72e9f09f6072af89db6b0de34de8c00121b6fd92
diff --git a/quic/core/crypto/quic_crypto_client_config.cc b/quic/core/crypto/quic_crypto_client_config.cc index 09c259e..28193ee 100644 --- a/quic/core/crypto/quic_crypto_client_config.cc +++ b/quic/core/crypto/quic_crypto_client_config.cc
@@ -134,16 +134,6 @@ return scfg_.get(); } -void QuicCryptoClientConfig::CachedState::add_server_designated_connection_id( - QuicConnectionId connection_id) { - server_designated_connection_ids_.push(connection_id); -} - -bool QuicCryptoClientConfig::CachedState::has_server_designated_connection_id() - const { - return !server_designated_connection_ids_.empty(); -} - void QuicCryptoClientConfig::CachedState::add_server_nonce( const std::string& server_nonce) { server_nonces_.push(server_nonce); @@ -206,9 +196,6 @@ server_config_.clear(); scfg_.reset(); SetProofInvalid(); - QuicQueue<QuicConnectionId> empty_queue; - using std::swap; - swap(server_designated_connection_ids_, empty_queue); } void QuicCryptoClientConfig::CachedState::SetProof( @@ -251,9 +238,6 @@ proof_verify_details_.reset(); scfg_.reset(); ++generation_counter_; - QuicQueue<QuicConnectionId> empty_queue; - using std::swap; - swap(server_designated_connection_ids_, empty_queue); } void QuicCryptoClientConfig::CachedState::ClearProof() { @@ -372,7 +356,6 @@ chlo_hash_ = other.chlo_hash_; server_config_sig_ = other.server_config_sig_; server_config_valid_ = other.server_config_valid_; - server_designated_connection_ids_ = other.server_designated_connection_ids_; expiration_time_ = other.expiration_time_; if (other.proof_verify_details_ != nullptr) { proof_verify_details_.reset(other.proof_verify_details_->Clone()); @@ -380,18 +363,6 @@ ++generation_counter_; } -QuicConnectionId -QuicCryptoClientConfig::CachedState::GetNextServerDesignatedConnectionId() { - if (server_designated_connection_ids_.empty()) { - QUIC_BUG - << "Attempting to consume a connection id that was never designated."; - return EmptyQuicConnectionId(); - } - const QuicConnectionId next_id = server_designated_connection_ids_.front(); - server_designated_connection_ids_.pop(); - return next_id; -} - std::string QuicCryptoClientConfig::CachedState::GetNextServerNonce() { if (server_nonces_.empty()) { QUIC_BUG
diff --git a/quic/core/crypto/quic_crypto_client_config.h b/quic/core/crypto/quic_crypto_client_config.h index 6f057c2..e90cc7e 100644 --- a/quic/core/crypto/quic_crypto_client_config.h +++ b/quic/core/crypto/quic_crypto_client_config.h
@@ -175,20 +175,6 @@ void set_cert_sct(quiche::QuicheStringPiece cert_sct); - // Adds the connection ID to the queue of server-designated connection-ids. - void add_server_designated_connection_id(QuicConnectionId connection_id); - - // If true, the crypto config contains at least one connection ID specified - // by the server, and the client should use one of these IDs when initiating - // the next connection. - bool has_server_designated_connection_id() const; - - // This function should only be called when - // has_server_designated_connection_id is true. Returns the next - // connection_id specified by the server and removes it from the - // queue of ids. - QuicConnectionId GetNextServerDesignatedConnectionId(); - // Adds the servernonce to the queue of server nonces. void add_server_nonce(const std::string& server_nonce); @@ -242,10 +228,6 @@ // scfg contains the cached, parsed value of |server_config|. mutable std::unique_ptr<CryptoHandshakeMessage> scfg_; - // TODO(jokulik): Consider using a hash-set as extra book-keeping to ensure - // that no connection-id is added twice. Also, consider keeping the server - // nonces and connection_ids together in one queue. - QuicQueue<QuicConnectionId> server_designated_connection_ids_; QuicQueue<std::string> server_nonces_; };
diff --git a/quic/core/crypto/quic_crypto_client_config_test.cc b/quic/core/crypto/quic_crypto_client_config_test.cc index 5eaa31f..c995e8a 100644 --- a/quic/core/crypto/quic_crypto_client_config_test.cc +++ b/quic/core/crypto/quic_crypto_client_config_test.cc
@@ -81,46 +81,6 @@ EXPECT_EQ(details, state.proof_verify_details()); } -TEST_F(QuicCryptoClientConfigTest, CachedState_ServerDesignatedConnectionId) { - QuicCryptoClientConfig::CachedState state; - EXPECT_FALSE(state.has_server_designated_connection_id()); - - uint64_t conn_id = 1234; - QuicConnectionId connection_id = TestConnectionId(conn_id); - state.add_server_designated_connection_id(connection_id); - EXPECT_TRUE(state.has_server_designated_connection_id()); - EXPECT_EQ(connection_id, state.GetNextServerDesignatedConnectionId()); - EXPECT_FALSE(state.has_server_designated_connection_id()); - - // Allow the ID to be set multiple times. It's unusual that this would - // happen, but not impossible. - connection_id = TestConnectionId(++conn_id); - state.add_server_designated_connection_id(connection_id); - EXPECT_TRUE(state.has_server_designated_connection_id()); - EXPECT_EQ(connection_id, state.GetNextServerDesignatedConnectionId()); - connection_id = TestConnectionId(++conn_id); - state.add_server_designated_connection_id(connection_id); - EXPECT_EQ(connection_id, state.GetNextServerDesignatedConnectionId()); - EXPECT_FALSE(state.has_server_designated_connection_id()); - - // Test FIFO behavior. - const QuicConnectionId first_cid = TestConnectionId(0xdeadbeef); - const QuicConnectionId second_cid = TestConnectionId(0xfeedbead); - state.add_server_designated_connection_id(first_cid); - state.add_server_designated_connection_id(second_cid); - EXPECT_TRUE(state.has_server_designated_connection_id()); - EXPECT_EQ(first_cid, state.GetNextServerDesignatedConnectionId()); - EXPECT_EQ(second_cid, state.GetNextServerDesignatedConnectionId()); -} - -TEST_F(QuicCryptoClientConfigTest, CachedState_ServerIdConsumedBeforeSet) { - QuicCryptoClientConfig::CachedState state; - EXPECT_FALSE(state.has_server_designated_connection_id()); - EXPECT_QUIC_BUG(state.GetNextServerDesignatedConnectionId(), - "Attempting to consume a connection id " - "that was never designated."); -} - TEST_F(QuicCryptoClientConfigTest, CachedState_ServerNonce) { QuicCryptoClientConfig::CachedState state; EXPECT_FALSE(state.has_server_nonce()); @@ -170,7 +130,6 @@ EXPECT_EQ(state.source_address_token(), other.source_address_token()); EXPECT_EQ(state.certs(), other.certs()); EXPECT_EQ(1u, other.generation_counter()); - EXPECT_FALSE(state.has_server_designated_connection_id()); EXPECT_FALSE(state.has_server_nonce()); } @@ -536,7 +495,6 @@ AllSupportedTransportVersions().front(), "", &cached, out_params, &error), IsQuicNoError()); - EXPECT_FALSE(cached.has_server_designated_connection_id()); EXPECT_FALSE(cached.has_server_nonce()); }
diff --git a/quic/tools/quic_client_base.cc b/quic/tools/quic_client_base.cc index bc09d8c..82b549f 100644 --- a/quic/tools/quic_client_base.cc +++ b/quic/tools/quic_client_base.cc
@@ -303,21 +303,7 @@ } QuicConnectionId QuicClientBase::GetNextConnectionId() { - QuicConnectionId server_designated_id = GetNextServerDesignatedConnectionId(); - return !server_designated_id.IsEmpty() ? server_designated_id - : GenerateNewConnectionId(); -} - -QuicConnectionId QuicClientBase::GetNextServerDesignatedConnectionId() { - QuicCryptoClientConfig::CachedState* cached = - crypto_config_.LookupOrCreate(server_id_); - // If the cached state indicates that we should use a server-designated - // connection ID, then return that connection ID. - CHECK(cached != nullptr) << "QuicClientCryptoConfig::LookupOrCreate returned " - << "unexpected nullptr."; - return cached->has_server_designated_connection_id() - ? cached->GetNextServerDesignatedConnectionId() - : EmptyQuicConnectionId(); + return GenerateNewConnectionId(); } QuicConnectionId QuicClientBase::GenerateNewConnectionId() {
diff --git a/quic/tools/quic_client_base.h b/quic/tools/quic_client_base.h index 74fc2af..842afc6 100644 --- a/quic/tools/quic_client_base.h +++ b/quic/tools/quic_client_base.h
@@ -276,10 +276,6 @@ // returned. Otherwise, the next random ID will be returned. QuicConnectionId GetNextConnectionId(); - // Returns the next server-designated ConnectionId from the cached config for - // |server_id_|, if it exists. Otherwise, returns 0. - QuicConnectionId GetNextServerDesignatedConnectionId(); - // Generates a new, random connection ID (as opposed to a server-designated // connection ID). virtual QuicConnectionId GenerateNewConnectionId();