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

Issue 2257103002: Re-write many calls to WrapUnique() with MakeUnique() (Closed)

Created:
4 years, 4 months ago by Adam Rice
Modified:
4 years, 4 months ago
CC:
chromium-reviews, yusukes+watch_chromium.org, tzik, hidehiko+watch_chromium.org, rginda+watch_chromium.org, lhchavez+watch_chromium.org, dmazzoni+watch_chromium.org, stevenjb+watch_chromium.org, extensions-reviews_chromium.org, cbentzel+watch_chromium.org, aboxhall+watch_chromium.org, dzhioev+watch_chromium.org, achuith+watch_chromium.org, je_julie, chromium-apps-reviews_chromium.org, derat+watch_chromium.org, nhiroki, yuzo+watch_chromium.org, oshima+watch_chromium.org, elijahtaylor+arcwatch_chromium.org, nektar+watch_chromium.org, dtseng+watch_chromium.org, kinuko+fileapi, davemoore+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Re-write many calls to WrapUnique() with MakeUnique() A mostly-automated change to convert instances of WrapUnique(new Foo(...)) to MakeUnique<Foo>(...). See the thread at https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/iQgMedVA8-k for background. To avoid requiring too many manual fixups, the change skips some cases that are frequently problematic. In particular, in methods named Foo::Method() it will not try to change WrapUnique(new Foo()) to MakeUnique<Foo>(). This is because Foo::Method() may be accessing an internal constructor of Foo. Cases where MakeUnique<NestedClass>(...) is called within a method of OuterClass are common but hard to detect automatically, so have been fixed-up manually. The only types of manual fix ups applied are: 1) Revert MakeUnique back to WrapUnique 2) Change NULL to nullptr in argument list (MakeUnique cannot forward NULL correctly) 3) Add base:: namespace qualifier where missing. WrapUnique(new Foo) has not been converted to MakeUnique<Foo>() as this might change behaviour if Foo does not have a user-defined constructor. For example, WrapUnique(new int) creates an unitialised integer, but MakeUnique<int>() creates an integer initialised to 0. git cl format has been been run over the CL. Spot-checking has uncovered no cases of mis-formatting. BUG=637812 Committed: https://crrev.com/46ad5f4fc0f8d35f71c890431e8e4518d228f2e3 Cr-Commit-Position: refs/heads/master@{#413954}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+367 lines, -382 lines) Patch
M chrome/browser/chromeos/app_mode/kiosk_app_manager.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/arc/arc_auth_service_browsertest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/arc/arc_policy_bridge.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/arc/arc_policy_bridge_unittest.cc View 10 chunks +27 lines, -29 lines 0 comments Download
M chrome/browser/chromeos/certificate_provider/certificate_provider_service.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/certificate_provider/thread_safe_certificate_map.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/chrome_browser_main_chromeos.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/display/display_preferences_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_manager/event_router.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/chromeos/extensions/quick_unlock_private/quick_unlock_private_api_unittest.cc View 4 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/fileapi/provider_async_file_util_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/operations/operation.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/provided_file_system.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/provided_file_system_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/file_system_provider/service.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/file_system_provider/throttled_file_system_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/fileapi/file_system_backend.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/launcher_search_provider/launcher_search_provider_service.cc View 3 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc View 2 chunks +4 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/login/profile_auth_data_unittest.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/login/saml/saml_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/login/session/user_session_manager.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.cc View 3 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc View 3 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/net/proxy_config_handler.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/platform_keys/key_permissions.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/platform_keys/platform_keys_nss.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/platform_keys/platform_keys_service.cc View 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/chromeos/platform_keys/platform_keys_service_factory.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/cloud_external_data_manager_base.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/policy/cloud_external_data_manager_base_test_util.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/cloud_external_data_manager_base_unittest.cc View 3 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/policy/configuration_policy_handler_chromeos_unittest.cc View 8 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/chromeos/policy/device_cloud_policy_initializer.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/policy/device_local_account_browsertest.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/policy/device_local_account_external_data_manager.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/device_local_account_policy_provider.cc View 1 chunk +5 lines, -5 lines 0 comments Download
M chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc View 3 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/chromeos/policy/device_local_account_policy_store.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc View 35 chunks +133 lines, -134 lines 0 comments Download
M chrome/browser/chromeos/policy/device_status_collector.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/device_status_collector_browsertest.cc View 7 chunks +12 lines, -12 lines 0 comments Download
M chrome/browser/chromeos/policy/extension_cache_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc View 7 chunks +11 lines, -11 lines 0 comments Download
M chrome/browser/chromeos/policy/remote_commands/device_command_screenshot_job.cc View 7 chunks +13 lines, -14 lines 0 comments Download
M chrome/browser/chromeos/policy/remote_commands/device_command_screenshot_job_unittest.cc View 4 chunks +6 lines, -7 lines 0 comments Download
M chrome/browser/chromeos/policy/remote_commands/device_commands_factory_chromeos.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/policy/status_uploader_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/system_log_uploader.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/policy/system_log_uploader_unittest.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/policy/upload_job_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/user_cloud_external_data_manager.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc View 1 chunk +11 lines, -14 lines 0 comments Download
M chrome/browser/chromeos/policy/user_cloud_policy_store_chromeos_unittest.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/policy/variations_service_policy_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/power/extension_event_observer.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/settings/scoped_cros_settings_test_helper.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/system/tray_accessibility_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 16 (8 generated)
Adam Rice
4 years, 4 months ago (2016-08-19 07:40:37 UTC) #6
achuithb
I don't think Zel will have time to look at this, Xiaoyin - do you ...
4 years, 4 months ago (2016-08-20 00:38:19 UTC) #8
xiaoyinh(OOO Sep 11-29)
LGTM. You still need owner's approval though. Thanks!
4 years, 4 months ago (2016-08-22 17:26:07 UTC) #9
xiaoyinh(OOO Sep 11-29)
On 2016/08/20 00:38:19, achuithb wrote: > I don't think Zel will have time to look ...
4 years, 4 months ago (2016-08-22 17:26:28 UTC) #10
achuithb
lgtm
4 years, 4 months ago (2016-08-22 17:45:40 UTC) #11
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/2257103002/1
4 years, 4 months ago (2016-08-24 01:46:11 UTC) #13
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 4 months ago (2016-08-24 02:45:15 UTC) #14
commit-bot: I haz the power
4 years, 4 months ago (2016-08-24 02:46:41 UTC) #16
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/46ad5f4fc0f8d35f71c890431e8e4518d228f2e3
Cr-Commit-Position: refs/heads/master@{#413954}

Powered by Google App Engine
This is Rietveld 408576698