Revert of Various cleanups in crypto code discovered while syncing recent changes. (https://codereview.chromium.org/217653003/)

Reason for revert:
Seems to be related to a tree-closing failure.

http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%2BLSan%20Tests%20%281%29/builds/984

See bug for more details.

BUG=358919

Original issue's description:
> Various cleanups in crypto code discovered while syncing recent changes.
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260979

TBR=rtenneti@chromium.org,rch@chromium.org
NOTREECHECKS=true
NOTRY=true

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@261017 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 40 insertions, 92 deletions

diff --git a/net/quic/crypto/proof_verifier.h b/net/quic/crypto/proof_verifier.h
index 779d63f..0cbdb6a 100644
--- a/net/quic/crypto/proof_verifier.h
+++ b/net/quic/crypto/proof_verifier.h
@@ -8,7 +8,7 @@
 #include <string>
 #include <vector>
 
-#include "base/memory/scoped_ptr.h"
+#include "net/base/completion_callback.h"
 #include "net/base/net_export.h"
 
 namespace net {
diff --git a/net/quic/crypto/proof_verifier_chromium.cc b/net/quic/crypto/proof_verifier_chromium.cc
index cbb4436..fdb6a0d 100644
--- a/net/quic/crypto/proof_verifier_chromium.cc
+++ b/net/quic/crypto/proof_verifier_chromium.cc
@@ -110,6 +110,7 @@ ProofVerifierChromium::Status ProofVerifierChromium::Job::VerifyProof(
   DCHECK(verify_details);
   DCHECK(callback);
 
+  callback_.reset(callback);
   error_details->clear();
 
   if (STATE_NONE != next_state_) {
@@ -160,7 +161,6 @@ ProofVerifierChromium::Status ProofVerifierChromium::Job::VerifyProof(
       verify_details->reset(verify_details_.release());
       return SUCCESS;
     case ERR_IO_PENDING:
-      callback_.reset(callback);
       return PENDING;
     default:
       *error_details = error_details_;
diff --git a/net/quic/crypto/quic_crypto_client_config.cc b/net/quic/crypto/quic_crypto_client_config.cc
index a6b0e39..e58e4d1 100644
--- a/net/quic/crypto/quic_crypto_client_config.cc
+++ b/net/quic/crypto/quic_crypto_client_config.cc
@@ -711,7 +711,7 @@ void QuicCryptoClientConfig::PopulateFromCanonicalConfig(
     const QuicServerId& server_id,
     CachedState* server_state) {
   DCHECK(server_state->IsEmpty());
-  size_t i = 0;
+  unsigned i = 0;
   for (; i < canoncial_suffixes_.size(); ++i) {
     if (EndsWith(server_id.host(), canoncial_suffixes_[i], false)) {
       break;
diff --git a/net/quic/crypto/quic_crypto_client_config_test.cc b/net/quic/crypto/quic_crypto_client_config_test.cc
index cb2e48f..a085d16 100644
--- a/net/quic/crypto/quic_crypto_client_config_test.cc
+++ b/net/quic/crypto/quic_crypto_client_config_test.cc
@@ -66,13 +66,35 @@ TEST(QuicCryptoClientConfigTest, InchoateChlo) {
   EXPECT_EQ(QuicVersionToQuicTag(QuicVersionMax()), cver);
 }
 
-TEST(QuicCryptoClientConfigTest, PreferAesGcm) {
+TEST(QuicCryptoClientConfigTest, FillClientHello) {
+  QuicCryptoClientConfig::CachedState state;
   QuicCryptoClientConfig config;
-  config.SetDefaults();
-  if (config.aead.size() > 1)
-    EXPECT_NE(kAESG, config.aead[0]);
-  config.PreferAesGcm();
-  EXPECT_EQ(kAESG, config.aead[0]);
+  QuicCryptoNegotiatedParameters params;
+  QuicConnectionId kConnectionId = 1234;
+  uint32 kInitialFlowControlWindow = 5678;
+  string error_details;
+  MockRandom rand;
+  CryptoHandshakeMessage chlo;
+  QuicServerId server_id("www.google.com", 80, false, PRIVACY_MODE_DISABLED);
+  config.FillClientHello(server_id,
+                         kConnectionId,
+                         QuicVersionMax(),
+                         kInitialFlowControlWindow,
+                         &state,
+                         QuicWallTime::Zero(),
+                         &rand,
+                         &params,
+                         &chlo,
+                         &error_details);
+
+  // Verify that certain QuicTags have been set correctly in the CHLO.
+  QuicTag cver;
+  EXPECT_EQ(QUIC_NO_ERROR, chlo.GetUint32(kVER, &cver));
+  EXPECT_EQ(QuicVersionToQuicTag(QuicVersionMax()), cver);
+
+  QuicTag ifcw;
+  EXPECT_EQ(QUIC_NO_ERROR, chlo.GetUint32(kIFCW, &ifcw));
+  EXPECT_EQ(kInitialFlowControlWindow, ifcw);
 }
 
 TEST(QuicCryptoClientConfigTest, InchoateChloSecure) {
@@ -104,37 +126,6 @@ TEST(QuicCryptoClientConfigTest, InchoateChloSecureNoEcdsa) {
   EXPECT_EQ(kX59R, pdmd);
 }
 
-TEST(QuicCryptoClientConfigTest, FillClientHello) {
-  QuicCryptoClientConfig::CachedState state;
-  QuicCryptoClientConfig config;
-  QuicCryptoNegotiatedParameters params;
-  QuicConnectionId kConnectionId = 1234;
-  uint32 kInitialFlowControlWindow = 5678;
-  string error_details;
-  MockRandom rand;
-  CryptoHandshakeMessage chlo;
-  QuicServerId server_id("www.google.com", 80, false, PRIVACY_MODE_DISABLED);
-  config.FillClientHello(server_id,
-                         kConnectionId,
-                         QuicVersionMax(),
-                         kInitialFlowControlWindow,
-                         &state,
-                         QuicWallTime::Zero(),
-                         &rand,
-                         &params,
-                         &chlo,
-                         &error_details);
-
-  // Verify that certain QuicTags have been set correctly in the CHLO.
-  QuicTag cver;
-  EXPECT_EQ(QUIC_NO_ERROR, chlo.GetUint32(kVER, &cver));
-  EXPECT_EQ(QuicVersionToQuicTag(QuicVersionMax()), cver);
-
-  QuicTag ifcw;
-  EXPECT_EQ(QUIC_NO_ERROR, chlo.GetUint32(kIFCW, &ifcw));
-  EXPECT_EQ(kInitialFlowControlWindow, ifcw);
-}
-
 TEST(QuicCryptoClientConfigTest, ProcessServerDowngradeAttack) {
   QuicVersionVector supported_versions = QuicSupportedVersions();
   if (supported_versions.size() == 1) {
@@ -182,49 +173,5 @@ TEST(QuicCryptoClientConfigTest, InitializeFrom) {
   EXPECT_EQ(1u, other->generation_counter());
 }
 
-TEST(QuicCryptoClientConfigTest, Canonical) {
-  QuicCryptoClientConfig config;
-  config.AddCanonicalSuffix(".google.com");
-  QuicServerId canonical_id1("www.google.com", 80, false,
-                              PRIVACY_MODE_DISABLED);
-  QuicServerId canonical_id2("mail.google.com", 80, false,
-                              PRIVACY_MODE_DISABLED);
-  QuicCryptoClientConfig::CachedState* state =
-      config.LookupOrCreate(canonical_id1);
-  // TODO(rch): Populate other fields of |state|.
-  state->set_source_address_token("TOKEN");
-  state->SetProofValid();
-
-  QuicCryptoClientConfig::CachedState* other =
-      config.LookupOrCreate(canonical_id2);
-
-  EXPECT_TRUE(state->IsEmpty());
-  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());
-
-  QuicServerId different_id("mail.google.org", 80, false,
-                             PRIVACY_MODE_DISABLED);
-  EXPECT_TRUE(config.LookupOrCreate(different_id)->IsEmpty());
-}
-
-TEST(QuicCryptoClientConfigTest, CanonicalNotUsedIfNotValid) {
-  QuicCryptoClientConfig config;
-  config.AddCanonicalSuffix(".google.com");
-  QuicServerId canonical_id1("www.google.com", 80, false,
-                              PRIVACY_MODE_DISABLED);
-  QuicServerId canonical_id2("mail.google.com", 80, false,
-                              PRIVACY_MODE_DISABLED);
-  QuicCryptoClientConfig::CachedState* state =
-      config.LookupOrCreate(canonical_id1);
-  // 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());
-}
-
 }  // namespace test
 }  // namespace net
diff --git a/net/quic/quic_crypto_client_stream.cc b/net/quic/quic_crypto_client_stream.cc
index 856696d..0ae7247 100644
--- a/net/quic/quic_crypto_client_stream.cc
+++ b/net/quic/quic_crypto_client_stream.cc
@@ -10,7 +10,6 @@
 #include "net/quic/crypto/proof_verifier.h"
 #include "net/quic/quic_client_session_base.h"
 #include "net/quic/quic_protocol.h"
-#include "net/quic/quic_session.h"
 
 namespace net {
 
@@ -128,7 +127,9 @@ void QuicCryptoClientStream::DoHandshakeLoop(
           crypto_config_->FillInchoateClientHello(
               server_id_,
               session()->connection()->supported_versions().front(),
-              cached, &crypto_negotiated_params_, &out);
+              cached,
+              &crypto_negotiated_params_,
+              &out);
           // Pad the inchoate client hello to fill up a packet.
           const size_t kFramingOverhead = 50;  // A rough estimate.
           const size_t max_packet_size =
@@ -252,10 +253,8 @@ void QuicCryptoClientStream::DoHandshakeLoop(
             DVLOG(1) << "Doing VerifyProof";
             return;
           case ProofVerifier::FAILURE:
-            delete proof_verify_callback;
             break;
           case ProofVerifier::SUCCESS:
-            delete proof_verify_callback;
             verify_ok_ = true;
             break;
         }
diff --git a/net/quic/quic_crypto_server_stream_test.cc b/net/quic/quic_crypto_server_stream_test.cc
index 3d1127b..bbc0525 100644
--- a/net/quic/quic_crypto_server_stream_test.cc
+++ b/net/quic/quic_crypto_server_stream_test.cc
@@ -156,7 +156,8 @@ TEST_P(QuicCryptoServerStreamTest, ZeroRTT) {
   CHECK(client->CryptoConnect());
   CHECK_EQ(1u, client_conn->packets_.size());
 
-  scoped_ptr<TestSession> server_session(new TestSession(server_conn, config_));
+  scoped_ptr<TestClientSession> server_session(
+      new TestClientSession(server_conn, config_));
   scoped_ptr<QuicCryptoServerStream> server(
       new QuicCryptoServerStream(crypto_config_, server_session.get()));
   server_session->SetCryptoStream(server.get());
@@ -179,9 +180,9 @@ TEST_P(QuicCryptoServerStreamTest, ZeroRTT) {
   // strike-register from rejecting the repeated nonce.
   reinterpret_cast<MockRandom*>(client_conn->random_generator())->ChangeValue();
   client_session.reset(new TestClientSession(client_conn, client_config));
-  server_session.reset(new TestSession(server_conn, config_));
-  client.reset(new QuicCryptoClientStream(server_id, client_session.get(),
-                                          NULL, &client_crypto_config));
+  server_session.reset(new TestClientSession(server_conn, config_));
+  client.reset(new QuicCryptoClientStream(
+      server_id, client_session.get(), NULL, &client_crypto_config));
   client_session->SetCryptoStream(client.get());
 
   server.reset(new QuicCryptoServerStream(crypto_config_,
@@ -254,6 +255,7 @@ TEST_P(QuicCryptoServerStreamTest, WithoutCertificates) {
 
 TEST_P(QuicCryptoServerStreamTest, ChannelID) {
   client_options_.channel_id_enabled = true;
+  // TODO(rtenneti): Enable testing of ProofVerifier.
   // CompleteCryptoHandshake verifies
   // stream_.crypto_negotiated_params().channel_id is correct.
   EXPECT_EQ(2, CompleteCryptoHandshake());