Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(150)

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

Created:
4 years, 3 months ago by Adam Rice
Modified:
4 years, 3 months ago
Reviewers:
sky
CC:
chromium-reviews, asanka, dominickn+watch_chromium.org, skanuj+watch_chromium.org, qsr+mojo_chromium.org, zea+watch_chromium.org, viettrungluu+watch_chromium.org, dhollowa+watch_chromium.org, dougw+watch_chromium.org, browser-components-watch_chromium.org, msramek+watch_chromium.org, yzshen+watch_chromium.org, loading-reviews+metrics_chromium.org, markusheintz_, miu+watch_chromium.org, cbentzel+watch_chromium.org, melevin+watch_chromium.org, awdf+watch_chromium.org, mlamouri+watch-geolocation_chromium.org, tommycli, tzik, raymes+watch_chromium.org, abarth-chromium, rlp+watch_chromium.org, sync-reviews_chromium.org, devtools-reviews_chromium.org, groby+spellwatch_chromium.org, johnme+watch_chromium.org, vabr+watchlistpasswordmanager_chromium.org, Peter Beverloo, mlamouri+watch-notifications_chromium.org, nhiroki, feature-media-reviews_chromium.org, jfweitz+watch_chromium.org, pam+watch_chromium.org, rouslan+spell_chromium.org, mcasas+watch+vc_chromium.org, Michael van Ouwerkerk, mlamouri+watch-permissions_chromium.org, gcasto+watchlist_chromium.org, Jered, media-router+watch_chromium.org, Lei Zhang, tfarina, donnd+watch_chromium.org, csharrison+watch_chromium.org, asvitkine+watch_chromium.org, Aaron Boodman, David Black, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, darin (slow to review), James Su, kinuko+fileapi, pfeldman
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. Additional changes: Four C-style casts have been changed to static_cast. One instance of using "" to construct an empty string has been changed to std::string(). BUG=637812 Committed: https://crrev.com/86fa1dd92331ce969c55ade017aab9950e0c5156 Cr-Commit-Position: refs/heads/master@{#418180}

Patch Set 1 #

Total comments: 4

Patch Set 2 : Changes from review by sky #

Unified diffs Side-by-side diffs Delta from patch set Stats (+532 lines, -564 lines) Patch
M chrome/browser/android/bottombar/overlay_panel_content.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/android/compositor/tab_content_manager.cc View 1 2 chunks +7 lines, -6 lines 0 comments Download
M chrome/browser/android/data_usage/data_use_matcher.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/android/history_report/delta_file_backend_leveldb.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/omnibox/autocomplete_controller_android.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/tab_android.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/tab_web_contents_delegate_android.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/usb/web_usb_chooser_service_android.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/apps/drive/drive_app_provider_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/autocomplete/autocomplete_classifier_factory.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/autocomplete/shortcuts_backend_factory.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/banners/app_banner_settings_helper.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/bitmap_fetcher/bitmap_fetcher_service_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/bookmarks/bookmark_model_factory.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/bookmarks/managed_bookmark_service_factory.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/browser_process_impl.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/browsing_data/browsing_data_channel_id_helper_unittest.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/browsing_data/browsing_data_remover.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/browsing_data/browsing_data_remover_unittest.cc View 3 chunks +21 lines, -24 lines 0 comments Download
M chrome/browser/browsing_data/hosted_apps_counter.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/browsing_data/media_licenses_counter.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/browsing_data/media_licenses_counter_browsertest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chrome_browser_main.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chrome_content_browser_client.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/content_settings/local_shared_objects_container.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/devtools/device/adb/mock_adb_server.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/devtools/device/usb/android_usb_browsertest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/devtools/device/usb/android_usb_device.cc View 2 chunks +9 lines, -9 lines 0 comments Download
M chrome/browser/devtools/remote_debugging_server.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/download/download_dir_policy_handler.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/download/download_dir_policy_handler_unittest.cc View 4 chunks +9 lines, -10 lines 0 comments Download
M chrome/browser/download/download_ui_controller_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/engagement/site_engagement_score.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/favicon/favicon_service_factory.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/file_select_helper.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/geolocation/geolocation_permission_context_unittest.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/history/chrome_history_client.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/history/history_service_factory.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/io_thread.cc View 1 chunk +6 lines, -6 lines 0 comments Download
M chrome/browser/media/android/router/media_router_android.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/router/mojo/media_router_mojo_impl_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/media/webrtc/media_stream_capture_indicator.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media/webrtc/webrtc_log_uploader_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media_galleries/fileapi/picasa_data_provider.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/media_galleries/fileapi/safe_media_metadata_parser.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/media_galleries/linux/mtp_read_file_worker.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/metrics/chrome_metrics_services_manager_client.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/metrics/subprocess_metrics_provider.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M chrome/browser/metrics/subprocess_metrics_provider_unittest.cc View 1 2 chunks +5 lines, -4 lines 0 comments Download
M chrome/browser/net/disk_cache_dir_policy_handler.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/net/disk_cache_dir_policy_handler_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/net/errorpage_browsertest.cc View 1 chunk +6 lines, -6 lines 0 comments Download
M chrome/browser/net/predictor_browsertest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_io_data.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_unittest.cc View 2 chunks +2 lines, -3 lines 0 comments Download
M chrome/browser/net/utility_process_mojo_proxy_resolver_factory.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/notifications/message_center_notification_manager.cc View 3 chunks +9 lines, -8 lines 0 comments Download
M chrome/browser/page_load_metrics/metrics_web_contents_observer.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/page_load_metrics/page_load_metrics_initialize.cc View 2 chunks +14 lines, -15 lines 0 comments Download
M chrome/browser/password_manager/password_store_win_unittest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/password_manager/password_store_x_unittest.cc View 1 chunk +5 lines, -5 lines 0 comments Download
M chrome/browser/password_manager/update_password_infobar_delegate_android.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/permissions/chooser_context_base.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/permissions/delegation_tracker.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/permissions/permission_decision_auto_blocker.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/permissions/permission_manager.cc View 1 chunk +14 lines, -14 lines 0 comments Download
M chrome/browser/policy/cloud/cloud_policy_browsertest.cc View 4 chunks +15 lines, -15 lines 0 comments Download
M chrome/browser/policy/cloud/policy_header_service_factory.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/policy/configuration_policy_handler_list_factory.cc View 2 chunks +68 lines, -73 lines 0 comments Download
M chrome/browser/policy/file_selection_dialogs_policy_handler_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/policy/javascript_policy_handler_unittest.cc View 2 chunks +7 lines, -8 lines 0 comments Download
M chrome/browser/policy/managed_bookmarks_policy_handler_unittest.cc View 1 chunk +9 lines, -10 lines 0 comments Download
M chrome/browser/policy/policy_browsertest.cc View 50 chunks +84 lines, -96 lines 0 comments Download
M chrome/browser/policy/policy_network_browsertest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/policy/policy_startup_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/policy/profile_policy_connector_unittest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/prefs/chrome_pref_service_factory.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/prefs/command_line_pref_store.cc View 5 chunks +9 lines, -9 lines 0 comments Download
M chrome/browser/prefs/profile_pref_store_manager_unittest.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/prefs/proxy_policy_unittest.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/prefs/synced_pref_change_registrar_browsertest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/printing/cloud_print/privet_device_lister_unittest.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/printing/cloud_print/privet_notifications_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/profiles/incognito_mode_policy_handler_unittest.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/profiles/off_the_record_profile_impl.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/profiles/profile_impl.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/profiles/profile_io_data.cc View 6 chunks +15 lines, -15 lines 0 comments Download
M chrome/browser/profiles/profile_statistics_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/search/contextual_search_policy_handler_android_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/search/hotword_installer_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/search/hotword_service_unittest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/search_engines/chrome_template_url_service_client.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/search_engines/template_url_service_factory.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/search_engines/template_url_service_sync_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/services/gcm/gcm_profile_service_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/sessions/persistent_tab_restore_service_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/sessions/tab_loader.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/shell_integration_win.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/signin/chrome_proximity_auth_client.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/signin/easy_unlock_app_manager_unittest.cc View 2 chunks +4 lines, -6 lines 0 comments Download
M chrome/browser/signin/easy_unlock_service_regular.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/signin/fake_gaia_cookie_manager_service_builder.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/signin/test_signin_client_builder.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc View 2 chunks +2 lines, -3 lines 0 comments Download
M chrome/browser/ssl/chrome_expect_ct_reporter.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ssl/ssl_client_auth_requestor_mock.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/supervised_user/child_accounts/child_account_service.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/supervised_user/experimental/safe_search_url_reporter.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/supervised_user/supervised_user_url_filter.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/chrome_sync_client.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_factory.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/sync/profile_sync_test_util.cc View 4 chunks +8 lines, -9 lines 0 comments Download
M chrome/browser/sync/sync_error_notifier_ash_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/sync_global_error_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/test/integration/sync_test.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/sync_file_system/drive_backend/leveldb_wrapper.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync_file_system/drive_backend/metadata_database_index_on_disk_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync_file_system/drive_backend/register_app_task_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync_file_system/drive_backend/sync_engine.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync_file_system/local/local_file_sync_context.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/usb/usb_chooser_context.cc View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 16 (10 generated)
Adam Rice
+sky This CL is very large because there is no natural way to split it. ...
4 years, 3 months ago (2016-09-12 09:50:28 UTC) #6
sky
LGTM https://codereview.chromium.org/2334613003/diff/1/chrome/browser/android/compositor/tab_content_manager.cc File chrome/browser/android/compositor/tab_content_manager.cc (right): https://codereview.chromium.org/2334613003/diff/1/chrome/browser/android/compositor/tab_content_manager.cc#newcode116 chrome/browser/android/compositor/tab_content_manager.cc:116: (size_t)default_cache_size, (size_t)approximation_cache_size, Fix these too static_cast? https://codereview.chromium.org/2334613003/diff/1/chrome/browser/metrics/subprocess_metrics_provider_unittest.cc File ...
4 years, 3 months ago (2016-09-12 17:13:37 UTC) #7
Adam Rice
https://codereview.chromium.org/2334613003/diff/1/chrome/browser/android/compositor/tab_content_manager.cc File chrome/browser/android/compositor/tab_content_manager.cc (right): https://codereview.chromium.org/2334613003/diff/1/chrome/browser/android/compositor/tab_content_manager.cc#newcode116 chrome/browser/android/compositor/tab_content_manager.cc:116: (size_t)default_cache_size, (size_t)approximation_cache_size, On 2016/09/12 17:13:36, sky wrote: > Fix ...
4 years, 3 months ago (2016-09-13 03:02:49 UTC) #9
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/2334613003/20001
4 years, 3 months ago (2016-09-13 05:09:27 UTC) #12
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 3 months ago (2016-09-13 06:00:05 UTC) #14
commit-bot: I haz the power
4 years, 3 months ago (2016-09-13 06:02:16 UTC) #16
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/86fa1dd92331ce969c55ade017aab9950e0c5156
Cr-Commit-Position: refs/heads/master@{#418180}

Powered by Google App Engine
This is Rietveld 408576698