Make //crypto factories return std::unique_ptr<>s

Rather than make callers use base::WrapUnique or .reset(),
have //crypto functions that create new instances return them
in std::unique_ptr<>s

Also fixup NULL vs nullptr where it matters most, and remove
superflous .get() tests from the unique_ptr<>s

BUG=none
R=davidben@chromium.org

Review-Url: https://codereview.chromium.org/2095523002
Cr-Commit-Position: refs/heads/master@{#402368}

chrome/browser/browsing_data/browsing_data_channel_id_helper_unittest.cc

@@ -44,11 +44,11 @@ class BrowsingDataChannelIDHelperTest
     channel_id_store->SetChannelID(
         base::WrapUnique(new net::ChannelIDStore::ChannelID(
             "https://www.google.com:443", base::Time(),
-            base::WrapUnique(crypto::ECPrivateKey::Create()))));
+            crypto::ECPrivateKey::Create())));
     channel_id_store->SetChannelID(
         base::WrapUnique(new net::ChannelIDStore::ChannelID(
             "https://www.youtube.com:443", base::Time(),
-            base::WrapUnique(crypto::ECPrivateKey::Create()))));
+            crypto::ECPrivateKey::Create())));
   }
 
   void FetchCallback(
@@ -142,7 +142,7 @@ TEST_F(BrowsingDataChannelIDHelperTest, CannedEmpty) {
 
   ASSERT_TRUE(helper->empty());
   helper->AddChannelID(net::ChannelIDStore::ChannelID(
-      origin, base::Time(), base::WrapUnique(crypto::ECPrivateKey::Create())));
+      origin, base::Time(), crypto::ECPrivateKey::Create()));
   ASSERT_FALSE(helper->empty());
   helper->Reset();
   ASSERT_TRUE(helper->empty());

chrome/browser/browsing_data/browsing_data_remover_unittest.cc

@@ -493,8 +493,7 @@ class RemoveChannelIDTester : public net::SSLConfigService::Observer {
                              base::Time creation_time) {
     GetChannelIDStore()->SetChannelID(
         base::WrapUnique(new net::ChannelIDStore::ChannelID(
-            server_identifier, creation_time,
-            base::WrapUnique(crypto::ECPrivateKey::Create()))));
+            server_identifier, creation_time, crypto::ECPrivateKey::Create())));
   }
 
   // Add a server bound cert for |server|, with the current time as the

chrome/browser/chromeos/login/profile_auth_data_unittest.cc

@@ -110,17 +110,15 @@ class ProfileAuthDataTest : public testing::Test {
 };
 
 void ProfileAuthDataTest::SetUp() {
-  channel_id_key1_.reset(crypto::ECPrivateKey::Create());
-  channel_id_key2_.reset(crypto::ECPrivateKey::Create());
+  channel_id_key1_ = crypto::ECPrivateKey::Create();
+  channel_id_key2_ = crypto::ECPrivateKey::Create();
   PopulateBrowserContext(&login_browser_context_, kProxyAuthPassword1,
-                         kCookieValue1,
-                         base::WrapUnique(channel_id_key1_->Copy()));
+                         kCookieValue1, channel_id_key1_->Copy());
 }
 
 void ProfileAuthDataTest::PopulateUserBrowserContext() {
   PopulateBrowserContext(&user_browser_context_, kProxyAuthPassword2,
-                         kCookieValue2,
-                         base::WrapUnique(channel_id_key2_->Copy()));
+                         kCookieValue2, channel_id_key2_->Copy());
 }
 
 void ProfileAuthDataTest::Transfer(

chrome/browser/chromeos/settings/token_encryptor.cc

@@ -31,7 +31,7 @@ CryptohomeTokenEncryptor::CryptohomeTokenEncryptor(
   DCHECK(!system_salt.empty());
   // TODO(davidroche): should this use the system salt for both the password
   // and the salt value, or should this use a separate salt value?
-  system_salt_key_.reset(PassphraseToKey(system_salt_, system_salt_));
+  system_salt_key_ = PassphraseToKey(system_salt_, system_salt_);
 }
 
 CryptohomeTokenEncryptor::~CryptohomeTokenEncryptor() {
@@ -67,7 +67,7 @@ std::string CryptohomeTokenEncryptor::DecryptWithSystemSalt(
                              encrypted_token_hex);
 }
 
-crypto::SymmetricKey* CryptohomeTokenEncryptor::PassphraseToKey(
+std::unique_ptr<crypto::SymmetricKey> CryptohomeTokenEncryptor::PassphraseToKey(
     const std::string& passphrase,
     const std::string& salt) {
   return crypto::SymmetricKey::DeriveKeyFromPassword(

chrome/browser/chromeos/settings/token_encryptor.h

@@ -47,8 +47,9 @@ class CryptohomeTokenEncryptor : public TokenEncryptor {
 
  private:
   // Converts |passphrase| to a SymmetricKey using the given |salt|.
-  crypto::SymmetricKey* PassphraseToKey(const std::string& passphrase,
-                                        const std::string& salt);
+  std::unique_ptr<crypto::SymmetricKey> PassphraseToKey(
+      const std::string& passphrase,
+      const std::string& salt);
 
   // Encrypts (AES) the token given |key| and |salt|.
   std::string EncryptTokenWithKey(crypto::SymmetricKey* key,

chrome/browser/extensions/updater/local_extension_cache_unittest.cc

@@ -98,14 +98,13 @@ class LocalExtensionCacheTest : public testing::Test {
                                         base::FilePath* filename) {
     std::string data(size, 0);
 
-    crypto::SecureHash* hash =
+    std::unique_ptr<crypto::SecureHash> hash =
         crypto::SecureHash::Create(crypto::SecureHash::SHA256);
     hash->Update(data.c_str(), size);
     uint8_t output[crypto::kSHA256Length];
     hash->Finish(output, sizeof(output));
     const std::string hex_hash =
         base::ToLowerASCII(base::HexEncode(output, sizeof(output)));
-    delete hash;
 
     const base::FilePath file =
         GetExtensionFileName(dir, id, version, hex_hash);

chrome/browser/net/quota_policy_channel_id_store_unittest.cc

@@ -9,7 +9,6 @@
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
@@ -84,11 +83,9 @@ TEST_F(QuotaPolicyChannelIDStoreTest, TestPersistence) {
       crypto::ECPrivateKey::Create());
   std::unique_ptr<crypto::ECPrivateKey> foo_key(crypto::ECPrivateKey::Create());
   store_->AddChannelID(net::DefaultChannelIDStore::ChannelID(
-      "google.com", base::Time::FromInternalValue(1),
-      base::WrapUnique(goog_key->Copy())));
+      "google.com", base::Time::FromInternalValue(1), goog_key->Copy()));
   store_->AddChannelID(net::DefaultChannelIDStore::ChannelID(
-      "foo.com", base::Time::FromInternalValue(3),
-      base::WrapUnique(foo_key->Copy())));
+      "foo.com", base::Time::FromInternalValue(3), foo_key->Copy()));
 
   std::vector<std::unique_ptr<net::DefaultChannelIDStore::ChannelID>>
       channel_ids;
@@ -143,10 +140,10 @@ TEST_F(QuotaPolicyChannelIDStoreTest, TestPersistence) {
 TEST_F(QuotaPolicyChannelIDStoreTest, TestPolicy) {
   store_->AddChannelID(net::DefaultChannelIDStore::ChannelID(
       "google.com", base::Time::FromInternalValue(1),
-      base::WrapUnique(crypto::ECPrivateKey::Create())));
+      crypto::ECPrivateKey::Create()));
   store_->AddChannelID(net::DefaultChannelIDStore::ChannelID(
       "nonpersistent.com", base::Time::FromInternalValue(3),
-      base::WrapUnique(crypto::ECPrivateKey::Create())));
+      crypto::ECPrivateKey::Create()));
 
   std::vector<std::unique_ptr<net::DefaultChannelIDStore::ChannelID>>
       channel_ids;
@@ -176,10 +173,10 @@ TEST_F(QuotaPolicyChannelIDStoreTest, TestPolicy) {
   // being committed to disk.
   store_->AddChannelID(net::DefaultChannelIDStore::ChannelID(
       "nonpersistent.com", base::Time::FromInternalValue(5),
-      base::WrapUnique(crypto::ECPrivateKey::Create())));
+      crypto::ECPrivateKey::Create()));
   store_->AddChannelID(net::DefaultChannelIDStore::ChannelID(
       "persistent.com", base::Time::FromInternalValue(7),
-      base::WrapUnique(crypto::ECPrivateKey::Create())));
+      crypto::ECPrivateKey::Create()));
 
   // Now close the store, and the nonpersistent.com channel IDs should be
   // deleted according to policy.

components/crx_file/crx_file.cc

@@ -111,7 +111,7 @@ CrxFile::ValidateError CrxFile::ValidateSignature(
   base::ScopedFILE file(base::OpenFile(crx_path, "rb"));
   std::unique_ptr<crypto::SecureHash> hash;
   if (!expected_hash.empty())
-    hash.reset(crypto::SecureHash::Create(crypto::SecureHash::SHA256));
+    hash = crypto::SecureHash::Create(crypto::SecureHash::SHA256);
 
   if (!file.get())
     return ValidateError::CRX_FILE_NOT_READABLE;

components/os_crypt/os_crypt_mac.mm

@@ -77,12 +77,10 @@
 
   // Create an encryption key from our password and salt. The key is
   // intentionally leaked.
-  cached_encryption_key =
-      crypto::SymmetricKey::DeriveKeyFromPassword(crypto::SymmetricKey::AES,
-                                                  password,
-                                                  salt,
-                                                  kEncryptionIterations,
-                                                  kDerivedKeySizeInBits);
+  cached_encryption_key = crypto::SymmetricKey::DeriveKeyFromPassword(
+                              crypto::SymmetricKey::AES, password, salt,
+                              kEncryptionIterations, kDerivedKeySizeInBits)
+                              .release();
   ANNOTATE_LEAKING_OBJECT_PTR(cached_encryption_key);
   DCHECK(cached_encryption_key);
   return cached_encryption_key;

content/browser/download/base_file.cc

@@ -208,7 +208,7 @@ std::string BaseFile::DebugString() const {
 
 DownloadInterruptReason BaseFile::CalculatePartialHash(
     const std::string& hash_to_expect) {
-  secure_hash_.reset(crypto::SecureHash::Create(crypto::SecureHash::SHA256));
+  secure_hash_ = crypto::SecureHash::Create(crypto::SecureHash::SHA256);
 
   if (bytes_so_far_ == 0)
     return DOWNLOAD_INTERRUPT_REASON_NONE;

content/browser/download/download_item_impl.cc

@@ -31,7 +31,6 @@
 #include "base/format_macros.h"
 #include "base/guid.h"
 #include "base/logging.h"
-#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
@@ -1175,9 +1174,9 @@ void DownloadItemImpl::Start(
 
     int64_t offset = new_create_info.save_info->offset;
     std::unique_ptr<crypto::SecureHash> hash_state =
-        base::WrapUnique(new_create_info.save_info->hash_state
-                             ? new_create_info.save_info->hash_state->Clone()
-                             : nullptr);
+        new_create_info.save_info->hash_state
+            ? new_create_info.save_info->hash_state->Clone()
+            : nullptr;
 
     // Interrupted downloads also need a target path.
     if (target_path_.empty()) {

crypto/ec_private_key.cc

@@ -13,8 +13,6 @@
 #include <stddef.h>
 #include <stdint.h>
 
-#include <memory>
-
 #include "base/logging.h"
 #include "crypto/auto_cbb.h"
 #include "crypto/openssl_util.h"
@@ -43,13 +41,13 @@ bool ExportKeyWithBio(const void* key,
     return false;
 
   ScopedBIO bio(BIO_new(BIO_s_mem()));
-  if (!bio.get())
+  if (!bio)
     return false;
 
   if (!export_fn(bio.get(), key))
     return false;
 
-  char* data = NULL;
+  char* data = nullptr;
   long len = BIO_get_mem_data(bio.get(), &data);
   if (!data || len < 0)
     return false;
@@ -65,28 +63,21 @@ ECPrivateKey::~ECPrivateKey() {
     EVP_PKEY_free(key_);
 }
 
-ECPrivateKey* ECPrivateKey::Copy() const {
-  std::unique_ptr<ECPrivateKey> copy(new ECPrivateKey);
-  if (key_)
-    copy->key_ = EVP_PKEY_up_ref(key_);
-  return copy.release();
-}
-
 // static
-ECPrivateKey* ECPrivateKey::Create() {
+std::unique_ptr<ECPrivateKey> ECPrivateKey::Create() {
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
 
   ScopedEC_KEY ec_key(EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
-  if (!ec_key.get() || !EC_KEY_generate_key(ec_key.get()))
-    return NULL;
+  if (!ec_key || !EC_KEY_generate_key(ec_key.get()))
+    return nullptr;
 
   std::unique_ptr<ECPrivateKey> result(new ECPrivateKey());
   result->key_ = EVP_PKEY_new();
   if (!result->key_ || !EVP_PKEY_set1_EC_KEY(result->key_, ec_key.get()))
-    return NULL;
+    return nullptr;
 
   CHECK_EQ(EVP_PKEY_EC, EVP_PKEY_id(result->key_));
-  return result.release();
+  return result;
 }
 
 // static
@@ -100,30 +91,30 @@ std::unique_ptr<ECPrivateKey> ECPrivateKey::CreateFromPrivateKeyInfo(
   if (!pkey || CBS_len(&cbs) != 0 || EVP_PKEY_id(pkey.get()) != EVP_PKEY_EC)
     return nullptr;
 
-  std::unique_ptr<ECPrivateKey> result(new ECPrivateKey);
+  std::unique_ptr<ECPrivateKey> result(new ECPrivateKey());
   result->key_ = pkey.release();
   return result;
 }
 
 // static
-ECPrivateKey* ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
+std::unique_ptr<ECPrivateKey> ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
     const std::string& password,
     const std::vector<uint8_t>& encrypted_private_key_info,
     const std::vector<uint8_t>& subject_public_key_info) {
   // NOTE: The |subject_public_key_info| can be ignored here, it is only
   // useful for the NSS implementation (which uses the public key's SHA1
   // as a lookup key when storing the private one in its store).
   if (encrypted_private_key_info.empty())
-    return NULL;
+    return nullptr;
 
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
 
   const uint8_t* data = &encrypted_private_key_info[0];
   const uint8_t* ptr = data;
   ScopedX509_SIG p8_encrypted(
-      d2i_X509_SIG(NULL, &ptr, encrypted_private_key_info.size()));
+      d2i_X509_SIG(nullptr, &ptr, encrypted_private_key_info.size()));
   if (!p8_encrypted || ptr != data + encrypted_private_key_info.size())
-    return NULL;
+    return nullptr;
 
   ScopedPKCS8_PRIV_KEY_INFO p8_decrypted;
   if (password.empty()) {
@@ -142,15 +133,22 @@ ECPrivateKey* ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
   }
 
   if (!p8_decrypted)
-    return NULL;
+    return nullptr;
 
   // Create a new EVP_PKEY for it.
-  std::unique_ptr<ECPrivateKey> result(new ECPrivateKey);
+  std::unique_ptr<ECPrivateKey> result(new ECPrivateKey());
   result->key_ = EVP_PKCS82PKEY(p8_decrypted.get());
   if (!result->key_ || EVP_PKEY_id(result->key_) != EVP_PKEY_EC)
-    return NULL;
+    return nullptr;
 
-  return result.release();
+  return result;
+}
+
+std::unique_ptr<ECPrivateKey> ECPrivateKey::Copy() const {
+  std::unique_ptr<ECPrivateKey> copy(new ECPrivateKey());
+  if (key_)
+    copy->key_ = EVP_PKEY_up_ref(key_);
+  return copy;
 }
 
 bool ECPrivateKey::ExportPrivateKey(std::vector<uint8_t>* output) const {
@@ -174,7 +172,7 @@ bool ECPrivateKey::ExportEncryptedPrivateKey(
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
   // Convert into a PKCS#8 object.
   ScopedPKCS8_PRIV_KEY_INFO pkcs8(EVP_PKEY2PKCS8(key_));
-  if (!pkcs8.get())
+  if (!pkcs8)
     return false;
 
   // Encrypt the object.
@@ -190,7 +188,7 @@ bool ECPrivateKey::ExportEncryptedPrivateKey(
       0,
       iterations,
       pkcs8.get()));
-  if (!encrypted.get())
+  if (!encrypted)
     return false;
 
   // Write it into |*output|
@@ -236,6 +234,6 @@ bool ECPrivateKey::ExportRawPublicKey(std::string* output) const {
   return true;
 }
 
-ECPrivateKey::ECPrivateKey() : key_(NULL) {}
+ECPrivateKey::ECPrivateKey() : key_(nullptr) {}
 
 }  // namespace crypto

crypto/ec_private_key.h

@@ -30,10 +30,10 @@ class CRYPTO_EXPORT ECPrivateKey {
  public:
   ~ECPrivateKey();
 
-  // Creates a new random instance. Can return NULL if initialization fails.
+  // Creates a new random instance. Can return nullptr if initialization fails.
   // The created key will use the NIST P-256 curve.
   // TODO(mattm): Add a curve parameter.
-  static ECPrivateKey* Create();
+  static std::unique_ptr<ECPrivateKey> Create();
 
   // Create a new instance by importing an existing private key. The format is
   // an ASN.1-encoded PrivateKeyInfo block from PKCS #8. This can return
@@ -44,17 +44,17 @@ class CRYPTO_EXPORT ECPrivateKey {
   // Creates a new instance by importing an existing key pair.
   // The key pair is given as an ASN.1-encoded PKCS #8 EncryptedPrivateKeyInfo
   // block and an X.509 SubjectPublicKeyInfo block.
-  // Returns NULL if initialization fails.
+  // Returns nullptr if initialization fails.
   //
   // This function is deprecated. Use CreateFromPrivateKeyInfo for new code.
   // See https://crbug.com/603319.
-  static ECPrivateKey* CreateFromEncryptedPrivateKeyInfo(
+  static std::unique_ptr<ECPrivateKey> CreateFromEncryptedPrivateKeyInfo(
       const std::string& password,
       const std::vector<uint8_t>& encrypted_private_key_info,
       const std::vector<uint8_t>& subject_public_key_info);
 
   // Returns a copy of the object.
-  ECPrivateKey* Copy() const;
+  std::unique_ptr<ECPrivateKey> Copy() const;
 
   EVP_PKEY* key() { return key_; }