Use unique_ptrs to manage callback lifetimes in some QUIC code

net/quic/core/crypto/crypto_server_test.cc

 // Copyright (c) 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 <algorithm>
 #include <cstdint>
 #include <memory>
 #include <ostream>
 #include <vector>
 
@@ skipping 183 matching lines @@
                      bool should_succeed,
                      const char* error_substr,
                      bool* called)
         : test_(test),
           should_succeed_(should_succeed),
           error_substr_(error_substr),
           called_(called) {
       *called_ = false;
     }
 
-    void RunImpl(std::unique_ptr<Result> result,
-                 std::unique_ptr<ProofSource::Details> /* details */) override {
+    void Run(std::unique_ptr<Result> result,
+             std::unique_ptr<ProofSource::Details> /* details */) override {
       {
         // Ensure that the strike register client lock is not held.
         QuicCryptoServerConfigPeer peer(&test_->config_);
         base::Lock* m = peer.GetStrikeRegisterClientLock();
         // In Chromium, we will dead lock if the lock is held by the current
         // thread. Chromium doesn't have AssertNotHeld API call.
         // m->AssertNotHeld();
         base::AutoLock lock(*m);
       }
       ASSERT_FALSE(*called_);
@@ skipping 24 matching lines @@
     EXPECT_EQ(client_address_.address(), decoder.ip());
     EXPECT_EQ(client_address_.port(), decoder.port());
   }
 
   void ShouldSucceed(const CryptoHandshakeMessage& message) {
     bool called = false;
     IPAddress server_ip;
     config_.ValidateClientHello(message, client_address_.address(), server_ip,
                                 supported_versions_.front(), &clock_,
                                 &crypto_proof_,
-                                new ValidateCallback(this, true, "", &called));
+        std::unique_ptr<ValidateCallback>(
+            new ValidateCallback(this, true, "", &called)));
     EXPECT_TRUE(called);
   }
 
   void ShouldFailMentioning(const char* error_substr,
                             const CryptoHandshakeMessage& message) {
     bool called = false;
     ShouldFailMentioning(error_substr, message, &called);
     EXPECT_TRUE(called);
   }
 
   void ShouldFailMentioning(const char* error_substr,
                             const CryptoHandshakeMessage& message,
                             bool* called) {
     IPAddress server_ip;
     config_.ValidateClientHello(
         message, client_address_.address(), server_ip,
         supported_versions_.front(), &clock_, &crypto_proof_,
-        new ValidateCallback(this, false, error_substr, called));
+        std::unique_ptr<ValidateCallback>(
+            new ValidateCallback(this, false, error_substr, called)));
   }
 
   void ProcessValidationResult(const ValidateCallback::Result& result,
                                bool should_succeed,
                                const char* error_substr) {
     IPAddress server_ip;
     DiversificationNonce diversification_nonce;
     string error_details;
     QuicConnectionId server_designated_connection_id =
         rand_for_id_generation_.RandUint64();
@@ skipping 867 matching lines @@
     strike_register_client_ = new DelayedVerifyStrikeRegisterClient(
         10000,  // strike_register_max_entries
         static_cast<uint32_t>(clock_.WallNow().ToUNIXSeconds()),
         60,  // strike_register_window_secs
         reinterpret_cast<const uint8_t*>(kOrbit.c_str()),
         StrikeRegister::NO_STARTUP_PERIOD_NEEDED);
     config_.SetStrikeRegisterClient(strike_register_client_);
     ASSERT_NO_FATAL_FAILURE(CryptoServerTest::SetUp());
     strike_register_client_->StartDelayingVerification();
   }
 

[Zhongyi Shi, 2016/09/13 21:13:21]

This code seems out of sync with internal. ShouldQueue() is missing here. 

The internal cl: 116790643 specifies do not merge, but then no follow-up.

[Ryan Hamilton, 2016/09/13 21:26:04]

Yeah, I noticed that too! No follow-up==bad!

Thankfully, ShouldQueue is unused internally and I'm removing it in my "merge
changes from chrome" CL. Sorry for not giving you the heads up.

   DelayedVerifyStrikeRegisterClient* strike_register_client_;
 };
 
 TEST_P(AsyncStrikeServerVerificationTest, AsyncReplayProtection) {
   // This tests async validation with a strike register works.
   // clang-format off
   CryptoHandshakeMessage msg = CryptoTestUtils::Message(
       "CHLO",
       "PDMD", "X509",
       "AEAD", "AESG",
       "KEXS", "C255",
       "SCID", scid_hex_.c_str(),
       "#004b5453", srct_hex_.c_str(),
       "PUBS", pub_hex_.c_str(),
       "NONC", nonce_hex_.c_str(),
       "VER\0", client_version_string_.c_str(),
       "$padding", static_cast<int>(kClientHelloMinimumSize),
       nullptr);
   // clang-format on
 
   // Clear the message tag.
   out_.set_tag(0);
 
   bool called = false;
   IPAddress server_ip;
   config_.ValidateClientHello(msg, client_address_.address(), server_ip,
                               client_version_, &clock_, &crypto_proof_,
-                              new ValidateCallback(this, true, "", &called));
+                              std::unique_ptr<ValidateCallback>(
+                          new ValidateCallback(this, true, "", &called)));
   // The verification request was queued.
   ASSERT_FALSE(called);
   EXPECT_EQ(0u, out_.tag());
   EXPECT_EQ(1, strike_register_client_->PendingVerifications());
 
   // Continue processing the verification request.
   strike_register_client_->RunPendingVerifications();
   ASSERT_TRUE(called);
   EXPECT_EQ(0, strike_register_client_->PendingVerifications());
   // The message should be accepted now.
   EXPECT_EQ(kSHLO, out_.tag());
 
   // Rejected if replayed.
   config_.ValidateClientHello(msg, client_address_.address(), server_ip,
                               client_version_, &clock_, &crypto_proof_,
-                              new ValidateCallback(this, true, "", &called));
+                              std::unique_ptr<ValidateCallback>(
+                                  new ValidateCallback(this, true, "", &called)));
   // The verification request was queued.
   ASSERT_FALSE(called);
   EXPECT_EQ(1, strike_register_client_->PendingVerifications());
 
   strike_register_client_->RunPendingVerifications();
   ASSERT_TRUE(called);
   EXPECT_EQ(0, strike_register_client_->PendingVerifications());
   // The message should be rejected now.
   CheckRejectTag();
 }
@@ skipping 30 matching lines @@
     EXPECT_EQ(0, strike_register_client_->PendingVerifications());
   } else {
     // version 33.
     ASSERT_EQ(kSHLO, out_.tag());
     CheckServerHello(out_);
   }
 }
 
 }  // namespace test
 }  // namespace net