Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(203)

Issue 2095523002: Make //crypto factories return std::unique_ptr<>s (Closed)

Created:
4 years, 6 months ago by Ryan Sleevi
Modified:
4 years, 5 months ago
Reviewers:
brettw, davidben
CC:
chromium-reviews, asanka, extensions-reviews_chromium.org, msramek+watch_chromium.org, imcheng+watch_chromium.org, eme-reviews_chromium.org, avayvod+watch_chromium.org, xjz+watch_chromium.org, jam, cbentzel+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, jasonroberts+watch_google.com, chromium-apps-reviews_chromium.org, markusheintz_, isheriff+watch_chromium.org, sync-reviews_chromium.org, miu+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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 Committed: https://crrev.com/ffe5a13751bb5e90c02497f11f07f6ad6b1b7fd1 Cr-Commit-Position: refs/heads/master@{#402368}

Patch Set 1 #

Patch Set 2 : Fix comment #

Total comments: 9

Patch Set 3 : Feedback #

Patch Set 4 : Rebased #

Patch Set 5 : iOS, Mac, & ChromeOS #

Patch Set 6 : CrOS fix #

Patch Set 7 : Rebased #

Patch Set 8 : I'm blind #

Unified diffs Side-by-side diffs Delta from patch set Stats (+285 lines, -315 lines) Patch
M chrome/browser/browsing_data/browsing_data_channel_id_helper_unittest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/browsing_data/browsing_data_remover_unittest.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/chromeos/login/profile_auth_data_unittest.cc View 1 2 3 4 1 chunk +4 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/settings/token_encryptor.h View 1 2 3 4 5 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/settings/token_encryptor.cc View 1 2 3 4 5 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/updater/local_extension_cache_unittest.cc View 1 2 3 4 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/net/quota_policy_channel_id_store_unittest.cc View 1 2 3 4 chunks +6 lines, -9 lines 0 comments Download
M components/crx_file/crx_file.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/os_crypt/os_crypt_mac.mm View 1 2 3 4 1 chunk +4 lines, -6 lines 0 comments Download
M content/browser/download/base_file.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/download/download_item_impl.cc View 1 2 2 chunks +3 lines, -4 lines 0 comments Download
M crypto/ec_private_key.h View 2 chunks +5 lines, -5 lines 0 comments Download
M crypto/ec_private_key.cc View 9 chunks +26 lines, -28 lines 0 comments Download
M crypto/ec_private_key_unittest.cc View 8 chunks +12 lines, -12 lines 0 comments Download
M crypto/ec_signature_creator.h View 3 chunks +3 lines, -2 lines 0 comments Download
M crypto/ec_signature_creator.cc View 1 chunk +5 lines, -3 lines 0 comments Download
M crypto/ec_signature_creator_impl.cc View 1 2 chunks +6 lines, -5 lines 0 comments Download
M crypto/encryptor.cc View 4 chunks +8 lines, -9 lines 0 comments Download
M crypto/hmac.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M crypto/hmac_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M crypto/mock_apple_keychain.h View 1 chunk +1 line, -1 line 0 comments Download
M crypto/nss_util.cc View 13 chunks +21 lines, -21 lines 0 comments Download
M crypto/nss_util_internal.h View 2 chunks +3 lines, -3 lines 0 comments Download
M crypto/secure_hash.h View 3 chunks +4 lines, -2 lines 0 comments Download
M crypto/secure_hash.cc View 3 chunks +6 lines, -5 lines 0 comments Download
M crypto/signature_creator.h View 2 chunks +3 lines, -2 lines 0 comments Download
M crypto/signature_creator.cc View 1 2 6 chunks +15 lines, -18 lines 0 comments Download
M crypto/signature_verifier.cc View 3 chunks +3 lines, -5 lines 0 comments Download
M crypto/symmetric_key.h View 3 chunks +14 lines, -9 lines 0 comments Download
M crypto/symmetric_key.cc View 1 2 6 chunks +22 lines, -18 lines 0 comments Download
M crypto/symmetric_key_unittest.cc View 5 chunks +7 lines, -7 lines 0 comments Download
M crypto/wincrypt_shim.h View 1 chunk +1 line, -1 line 0 comments Download
M extensions/browser/content_verify_job.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M media/cast/common/transport_encryption_handler.cc View 1 chunk +1 line, -2 lines 0 comments Download
M media/cdm/aes_decryptor.cc View 2 chunks +4 lines, -5 lines 0 comments Download
M net/extras/sqlite/sqlite_channel_id_store_unittest.cc View 1 2 5 chunks +6 lines, -9 lines 0 comments Download
M net/spdy/spdy_test_util_common.h View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
M net/spdy/spdy_test_util_common.cc View 1 2 3 4 5 6 2 chunks +4 lines, -5 lines 0 comments Download
M net/ssl/channel_id_service.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M net/ssl/channel_id_service_unittest.cc View 1 2 2 chunks +2 lines, -4 lines 0 comments Download
M net/ssl/channel_id_store.cc View 1 chunk +1 line, -1 line 0 comments Download
M net/ssl/default_channel_id_store.cc View 1 2 3 4 5 6 7 6 chunks +8 lines, -8 lines 0 comments Download
M net/ssl/default_channel_id_store_unittest.cc View 13 chunks +35 lines, -58 lines 0 comments Download
M sync/util/nigori.cc View 1 chunk +20 lines, -19 lines 0 comments Download

Messages

Total messages: 52 (25 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2095523002/1
4 years, 6 months ago (2016-06-23 02:53:34 UTC) #2
Ryan Sleevi
david: Various things bothering me while I was hacking on code, which I was fixing ...
4 years, 6 months ago (2016-06-23 02:54:10 UTC) #3
davidben
lgtm https://codereview.chromium.org/2095523002/diff/20001/chrome/browser/net/quota_policy_channel_id_store_unittest.cc File chrome/browser/net/quota_policy_channel_id_store_unittest.cc (right): https://codereview.chromium.org/2095523002/diff/20001/chrome/browser/net/quota_policy_channel_id_store_unittest.cc#newcode12 chrome/browser/net/quota_policy_channel_id_store_unittest.cc:12: #include "base/memory/ptr_util.h" No longer used, I think. https://codereview.chromium.org/2095523002/diff/20001/content/browser/download/download_item_impl.cc ...
4 years, 6 months ago (2016-06-23 03:09:36 UTC) #4
Ryan Sleevi
https://codereview.chromium.org/2095523002/diff/20001/crypto/symmetric_key.cc File crypto/symmetric_key.cc (right): https://codereview.chromium.org/2095523002/diff/20001/crypto/symmetric_key.cc#newcode108 crypto/symmetric_key.cc:108: SymmetricKey::SymmetricKey() = default; On 2016/06/23 03:09:36, davidben wrote: > ...
4 years, 6 months ago (2016-06-23 21:47:41 UTC) #5
Ryan Sleevi
Brett: Global cleanup stamp requested
4 years, 6 months ago (2016-06-23 21:49:31 UTC) #8
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2095523002/40001
4 years, 6 months ago (2016-06-23 21:50:20 UTC) #10
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/223945)
4 years, 6 months ago (2016-06-23 22:03:05 UTC) #12
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2095523002/80001
4 years, 6 months ago (2016-06-23 22:20:51 UTC) #14
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_compile_dbg_ng/builds/221019) linux_chromium_chromeos_rel_ng on ...
4 years, 6 months ago (2016-06-23 22:39:29 UTC) #16
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2095523002/100001
4 years, 6 months ago (2016-06-23 22:43:22 UTC) #18
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2095523002/100001
4 years, 6 months ago (2016-06-23 23:45:23 UTC) #21
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/234725)
4 years, 6 months ago (2016-06-24 00:18:18 UTC) #23
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2095523002/100001
4 years, 6 months ago (2016-06-24 00:26:49 UTC) #25
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/190932)
4 years, 6 months ago (2016-06-24 00:54:42 UTC) #27
Ryan Sleevi
Rubberstamp ping for brettw
4 years, 6 months ago (2016-06-24 21:13:16 UTC) #28
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2095523002/100001
4 years, 6 months ago (2016-06-24 21:13:51 UTC) #30
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/234266)
4 years, 6 months ago (2016-06-24 22:02:46 UTC) #32
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2095523002/100001
4 years, 6 months ago (2016-06-25 01:09:03 UTC) #34
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/191718)
4 years, 6 months ago (2016-06-25 01:58:26 UTC) #36
brettw
lgtm
4 years, 5 months ago (2016-06-27 21:34:50 UTC) #37
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2095523002/120001
4 years, 5 months ago (2016-06-27 21:39:05 UTC) #39
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/235197)
4 years, 5 months ago (2016-06-27 22:27:52 UTC) #41
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2095523002/140001
4 years, 5 months ago (2016-06-27 23:49:28 UTC) #43
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 5 months ago (2016-06-28 01:45:09 UTC) #45
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2095523002/140001
4 years, 5 months ago (2016-06-28 01:45:55 UTC) #48
commit-bot: I haz the power
Committed patchset #8 (id:140001)
4 years, 5 months ago (2016-06-28 01:52:08 UTC) #50
commit-bot: I haz the power
4 years, 5 months ago (2016-06-28 01:53:44 UTC) #52
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/ffe5a13751bb5e90c02497f11f07f6ad6b1b7fd1
Cr-Commit-Position: refs/heads/master@{#402368}

Powered by Google App Engine
This is Rietveld 408576698