Skip to content

Commit

Permalink
Pass QuicCryptoServerConfig by pointer instead of reference.
Browse files Browse the repository at this point in the history
QuicCryptoServerStream was storing const reference to the config, which
makes the interface bug-prone because it's not clear for the caller
that the config must outlive the stream.

Review URL: https://codereview.chromium.org/1023273003

Cr-Commit-Position: refs/heads/master@{#321683}
  • Loading branch information
SergeyUlanov authored and Commit bot committed Mar 21, 2015
1 parent 6c5716b commit 3a405f3
Show file tree
Hide file tree
Showing 11 changed files with 25 additions and 23 deletions.
8 changes: 4 additions & 4 deletions net/quic/quic_crypto_server_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ void ServerHelloNotifier::OnAckNotification(
}

QuicCryptoServerStream::QuicCryptoServerStream(
const QuicCryptoServerConfig& crypto_config,
const QuicCryptoServerConfig* crypto_config,
QuicSession* session)
: QuicCryptoStream(session),
crypto_config_(crypto_config),
Expand Down Expand Up @@ -72,7 +72,7 @@ void QuicCryptoServerStream::OnHandshakeMessage(
}

validate_client_hello_cb_ = new ValidateCallback(this);
return crypto_config_.ValidateClientHello(
return crypto_config_->ValidateClientHello(
message,
session()->connection()->peer_address(),
session()->connection()->clock(),
Expand Down Expand Up @@ -153,7 +153,7 @@ void QuicCryptoServerStream::SendServerConfigUpdate(
}

CryptoHandshakeMessage server_config_update_message;
if (!crypto_config_.BuildServerConfigUpdateMessage(
if (!crypto_config_->BuildServerConfigUpdateMessage(
previous_source_address_tokens_,
session()->connection()->self_address(),
session()->connection()->peer_address(),
Expand Down Expand Up @@ -225,7 +225,7 @@ QuicErrorCode QuicCryptoServerStream::ProcessClientHello(
}
previous_source_address_tokens_ = result.info.source_address_tokens;

return crypto_config_.ProcessClientHello(
return crypto_config_->ProcessClientHello(
result, session()->connection()->connection_id(),
session()->connection()->self_address(),
session()->connection()->peer_address(),
Expand Down
5 changes: 3 additions & 2 deletions net/quic/quic_crypto_server_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ class NET_EXPORT_PRIVATE ServerHelloNotifier : public

class NET_EXPORT_PRIVATE QuicCryptoServerStream : public QuicCryptoStream {
public:
QuicCryptoServerStream(const QuicCryptoServerConfig& crypto_config,
// |crypto_config| must outlive the stream.
QuicCryptoServerStream(const QuicCryptoServerConfig* crypto_config,
QuicSession* session);
~QuicCryptoServerStream() override;

Expand Down Expand Up @@ -121,7 +122,7 @@ class NET_EXPORT_PRIVATE QuicCryptoServerStream : public QuicCryptoStream {
const ValidateClientHelloResultCallback::Result& result);

// crypto_config_ contains crypto parameters for the handshake.
const QuicCryptoServerConfig& crypto_config_;
const QuicCryptoServerConfig* crypto_config_;

// Pointer to the active callback that will receive the result of
// the client hello validation request and forward it to
Expand Down
6 changes: 3 additions & 3 deletions net/quic/quic_crypto_server_stream_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class QuicCryptoServerStreamTest : public ::testing::TestWithParam<bool> {
session_(connection_, DefaultQuicConfig()),
crypto_config_(QuicCryptoServerConfig::TESTING,
QuicRandom::GetInstance()),
stream_(crypto_config_, &session_),
stream_(&crypto_config_, &session_),
strike_register_client_(nullptr) {
session_.SetCryptoStream(&stream_);
// We advance the clock initially because the default time is zero and the
Expand Down Expand Up @@ -158,7 +158,7 @@ TEST_P(QuicCryptoServerStreamTest, ZeroRTT) {

scoped_ptr<TestSession> server_session(new TestSession(server_conn, config_));
scoped_ptr<QuicCryptoServerStream> server(
new QuicCryptoServerStream(crypto_config_, server_session.get()));
new QuicCryptoServerStream(&crypto_config_, server_session.get()));
server_session->SetCryptoStream(server.get());

CryptoTestUtils::CommunicateHandshakeMessages(
Expand All @@ -184,7 +184,7 @@ TEST_P(QuicCryptoServerStreamTest, ZeroRTT) {
nullptr, &client_crypto_config));
client_session->SetCryptoStream(client.get());

server.reset(new QuicCryptoServerStream(crypto_config_,
server.reset(new QuicCryptoServerStream(&crypto_config_,
server_session.get()));
server_session->SetCryptoStream(server.get());

Expand Down
2 changes: 1 addition & 1 deletion net/quic/quic_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ QuicSession* QuicDispatcher::CreateQuicSession(
crypto_config_.HasProofSource(), supported_versions_);

QuicServerSession* session = new QuicServerSession(config_, connection, this);
session->InitializeSession(crypto_config_);
session->InitializeSession(&crypto_config_);
return session;
}

Expand Down
4 changes: 2 additions & 2 deletions net/quic/quic_server_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ QuicServerSession::QuicServerSession(const QuicConfig& config,
QuicServerSession::~QuicServerSession() {}

void QuicServerSession::InitializeSession(
const QuicCryptoServerConfig& crypto_config) {
const QuicCryptoServerConfig* crypto_config) {
QuicSession::InitializeSession();
crypto_stream_.reset(CreateQuicCryptoServerStream(crypto_config));
}

QuicCryptoServerStream* QuicServerSession::CreateQuicCryptoServerStream(
const QuicCryptoServerConfig& crypto_config) {
const QuicCryptoServerConfig* crypto_config) {
return new QuicCryptoServerStream(crypto_config, this);
}

Expand Down
4 changes: 2 additions & 2 deletions net/quic/quic_server_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class QuicServerSession : public QuicSession {

~QuicServerSession() override;

virtual void InitializeSession(const QuicCryptoServerConfig& crypto_config);
virtual void InitializeSession(const QuicCryptoServerConfig* crypto_config);

const QuicCryptoServerStream* crypto_stream() const {
return crypto_stream_.get();
Expand All @@ -90,7 +90,7 @@ class QuicServerSession : public QuicSession {
virtual bool ShouldCreateIncomingDataStream(QuicStreamId id);

virtual QuicCryptoServerStream* CreateQuicCryptoServerStream(
const QuicCryptoServerConfig& crypto_config);
const QuicCryptoServerConfig* crypto_config);

private:
friend class test::QuicServerSessionPeer;
Expand Down
2 changes: 1 addition & 1 deletion net/quic/test_tools/crypto_test_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ int CryptoTestUtils::HandshakeWithFakeServer(
server_session.connection()->random_generator(),
server_session.config(), &crypto_config);

QuicCryptoServerStream server(crypto_config, &server_session);
QuicCryptoServerStream server(&crypto_config, &server_session);
server_session.SetCryptoStream(&server);

// The client's handshake must have been started already.
Expand Down
2 changes: 1 addition & 1 deletion net/tools/quic/quic_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ QuicSession* QuicDispatcher::CreateQuicSession(
crypto_config_.HasProofSource(), supported_versions_);

QuicServerSession* session = new QuicServerSession(config_, connection, this);
session->InitializeSession(crypto_config_);
session->InitializeSession(&crypto_config_);
return session;
}

Expand Down
4 changes: 2 additions & 2 deletions net/tools/quic/quic_server_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ QuicServerSession::QuicServerSession(const QuicConfig& config,
QuicServerSession::~QuicServerSession() {}

void QuicServerSession::InitializeSession(
const QuicCryptoServerConfig& crypto_config) {
const QuicCryptoServerConfig* crypto_config) {
QuicSession::InitializeSession();
crypto_stream_.reset(CreateQuicCryptoServerStream(crypto_config));
}

QuicCryptoServerStream* QuicServerSession::CreateQuicCryptoServerStream(
const QuicCryptoServerConfig& crypto_config) {
const QuicCryptoServerConfig* crypto_config) {
return new QuicCryptoServerStream(crypto_config, this);
}

Expand Down
5 changes: 3 additions & 2 deletions net/tools/quic/quic_server_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ class QuicServerSession : public QuicSession {

~QuicServerSession() override;

virtual void InitializeSession(const QuicCryptoServerConfig& crypto_config);
// |crypto_config| must outlive the session.
virtual void InitializeSession(const QuicCryptoServerConfig* crypto_config);

const QuicCryptoServerStream* crypto_stream() const {
return crypto_stream_.get();
Expand All @@ -91,7 +92,7 @@ class QuicServerSession : public QuicSession {
virtual bool ShouldCreateIncomingDataStream(QuicStreamId id);

virtual QuicCryptoServerStream* CreateQuicCryptoServerStream(
const QuicCryptoServerConfig& crypto_config);
const QuicCryptoServerConfig* crypto_config);

private:
friend class test::QuicServerSessionPeer;
Expand Down
6 changes: 3 additions & 3 deletions net/tools/quic/quic_server_session_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class QuicServerSessionTest : public ::testing::TestWithParam<QuicVersion> {
handshake_message_.reset(crypto_config_.AddDefaultConfig(
QuicRandom::GetInstance(), &clock,
QuicCryptoServerConfig::ConfigOptions()));
session_->InitializeSession(crypto_config_);
session_->InitializeSession(&crypto_config_);
visitor_ = QuicConnectionPeer::GetVisitor(connection_);
}

Expand Down Expand Up @@ -281,7 +281,7 @@ TEST_P(QuicServerSessionTest, SetFecProtectionFromConfig) {
class MockQuicCryptoServerStream : public QuicCryptoServerStream {
public:
explicit MockQuicCryptoServerStream(
const QuicCryptoServerConfig& crypto_config, QuicSession* session)
const QuicCryptoServerConfig* crypto_config, QuicSession* session)
: QuicCryptoServerStream(crypto_config, session) {}
~MockQuicCryptoServerStream() override {}

Expand All @@ -304,7 +304,7 @@ TEST_P(QuicServerSessionTest, BandwidthEstimates) {
session_->set_serving_region(serving_region);

MockQuicCryptoServerStream* crypto_stream =
new MockQuicCryptoServerStream(crypto_config_, session_.get());
new MockQuicCryptoServerStream(&crypto_config_, session_.get());
QuicServerSessionPeer::SetCryptoStream(session_.get(), crypto_stream);

// Set some initial bandwidth values.
Expand Down

0 comments on commit 3a405f3

Please sign in to comment.