QUIC - Minor clean up of QUIC Server Info memory cache.

net/quic/crypto/quic_crypto_client_config_test.cc

 // Copyright 2013 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.
 
 #include "net/quic/crypto/quic_crypto_client_config.h"
 
 #include "net/quic/crypto/proof_verifier.h"
 #include "net/quic/quic_server_id.h"
 #include "net/quic/test_tools/mock_random.h"
 #include "net/quic/test_tools/quic_test_utils.h"
@@ skipping 145 matching lines @@
   string error;
   QuicCryptoClientConfig config;
   EXPECT_EQ(QUIC_VERSION_NEGOTIATION_MISMATCH,
             config.ProcessServerHello(msg, 0, supported_versions,
                                       &cached, &out_params, &error));
   EXPECT_EQ("Downgrade attack detected", error);
 }
 
 TEST(QuicCryptoClientConfigTest, InitializeFrom) {
   QuicCryptoClientConfig config;
-  QuicServerId canonical_server_id("www.google.com", 80, false,

[wtc, 2014/04/25 18:17:28]

Please undo the changes to this unit test. In this unit test,
canonical_server_id is used as the canonical server id. other_server_id contains
a different hostname (mail.google.com vs. www.google.com).

[ramant (doing other things), 2014/04/25 18:55:30]

Done.

-                                   PRIVACY_MODE_DISABLED);
-  QuicCryptoClientConfig::CachedState* state =
-      config.LookupOrCreate(canonical_server_id);
+  QuicServerId server_id("www.google.com", 80, false, PRIVACY_MODE_DISABLED);
+  QuicCryptoClientConfig::CachedState* state = config.LookupOrCreate(server_id);
   // TODO(rch): Populate other fields of |state|.
   state->set_source_address_token("TOKEN");
   state->SetProofValid();
 
   QuicServerId other_server_id("mail.google.com", 80, false,
                                PRIVACY_MODE_DISABLED);
-  config.InitializeFrom(other_server_id, canonical_server_id, &config);
+  config.InitializeFrom(other_server_id, server_id, &config);
   QuicCryptoClientConfig::CachedState* other =
       config.LookupOrCreate(other_server_id);
 
   EXPECT_EQ(state->server_config(), other->server_config());
   EXPECT_EQ(state->source_address_token(), other->source_address_token());
   EXPECT_EQ(state->certs(), other->certs());
   EXPECT_EQ(1u, other->generation_counter());
 }
 
 TEST(QuicCryptoClientConfigTest, Canonical) {
@@ skipping 35 matching lines @@
   // TODO(rch): Populate other fields of |state|.
   state->set_source_address_token("TOKEN");
 
   // Do not set the proof as valid, and check that it is not used
   // as a canonical entry.
   EXPECT_TRUE(config.LookupOrCreate(canonical_id2)->IsEmpty());
 }
 
 TEST(QuicCryptoClientConfigTest, ClearCachedStates) {
   QuicCryptoClientConfig config;
-  QuicServerId canonical_server_id("www.google.com", 80, false,
-                                   PRIVACY_MODE_DISABLED);
-  QuicCryptoClientConfig::CachedState* state =
-      config.LookupOrCreate(canonical_server_id);
+  QuicServerId server_id("www.google.com", 80, false, PRIVACY_MODE_DISABLED);
+  QuicCryptoClientConfig::CachedState* state = config.LookupOrCreate(server_id);
   // TODO(rch): Populate other fields of |state|.
   vector<string> certs(1);
   certs[0] = "Hello Cert";
   state->SetProof(certs, "signature");
   state->set_source_address_token("TOKEN");
   state->SetProofValid();
 
   // Verify LookupOrCreate returns the same data.
-  QuicServerId other_server_id("www.google.com", 80, false,
-                               PRIVACY_MODE_DISABLED);
+  QuicCryptoClientConfig::CachedState* other = config.LookupOrCreate(server_id);
 
-  QuicCryptoClientConfig::CachedState* other =
-      config.LookupOrCreate(other_server_id);
-
+  EXPECT_EQ(state, other);
   EXPECT_TRUE(other->proof_valid());
   EXPECT_EQ(state->server_config(), other->server_config());
   EXPECT_EQ(state->signature(), other->signature());
   EXPECT_EQ(state->certs(), other->certs());
   EXPECT_EQ(state->source_address_token(), other->source_address_token());
   EXPECT_EQ(1u, other->generation_counter());

[wtc, 2014/04/25 18:17:28]

Once we verify |other| and |state| point to the same object, lines 243-247
should all be deleted. Line 248 can be kept because it is related to the
EXPECT_EQ on line 262.

You can add
  EXPECT_EQ(1u, state->generation_counter());
after line 237.

[ramant (doing other things), 2014/04/25 18:55:30]

Done.

 
-  // Clear the cached state.
+  // Clear the cached states.
   config.ClearCachedStates();
 
   // Verify LookupOrCreate doesn't have any data.
   QuicCryptoClientConfig::CachedState* cleared_cache =
-      config.LookupOrCreate(other_server_id);
+      config.LookupOrCreate(server_id);
 
+  EXPECT_EQ(state, cleared_cache);
   EXPECT_FALSE(cleared_cache->proof_valid());
   EXPECT_TRUE(cleared_cache->server_config().empty());
   EXPECT_TRUE(cleared_cache->certs().empty());
   EXPECT_TRUE(cleared_cache->signature().empty());
-  EXPECT_LT(1u, cleared_cache->generation_counter());
+  EXPECT_EQ(2u, cleared_cache->generation_counter());
 }
 
 }  // namespace test
 }  // namespace net