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

Issue 2782553004: Move TestingPrefService to use unique_ptr<Value> (Closed)

Created:
3 years, 8 months ago by vabr (Chromium)
Modified:
3 years, 8 months ago
CC:
asanka, browser-components-watch_chromium.org, certificate-transparency-chrome_googlegroups.com, chromium-apps-reviews_chromium.org, chromium-reviews, davemoore+watch_chromium.org, elijahtaylor+arcwatch_chromium.org, Eran Messeri, extensions-reviews_chromium.org, gcasto+watchlist_chromium.org, hansberry+watch-tether_chromium.org, hidehiko+watch_chromium.org, jam, jhawkins+watch-tether_chromium.org, jlklein+watch-tether_chromium.org, kalyank, khorimoto+watch-tether_chromium.org, lhchavez+watch_chromium.org, markusheintz_, martijn+crwatch_martijnc.be, Matt Giuca, msramek+watch_chromium.org, oshima+watch_chromium.org, raymes+watch_chromium.org, rginda+watch_chromium.org, rsleevi+watch_chromium.org, sadrul, sync-reviews_chromium.org, tbansal+watch-data-reduction-proxy_chromium.org, tengs+watch-tether_chromium.org, tfarina, vabr+watchlistpasswordmanager_chromium.org, victorhsieh+watch_chromium.org, yusukes+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Move TestingPrefService to use unique_ptr<Value> Various Set*Pref methods of TestingPrefService pass Value ownership with a raw pointer. This CL changes that to passing unique_ptr, because that is a safer and clearer option. It is also a pre-requisite for migrating Value API off raw pointer ownership passing. BUG=697817 Review-Url: https://codereview.chromium.org/2782553004 Cr-Commit-Position: refs/heads/master@{#460363} Committed: https://chromium.googlesource.com/chromium/src/+/8684c9acc69aa307f5fabfef0182b61c7fe8ce00

Patch Set 1 #

Patch Set 2 : Minor fixes #

Patch Set 3 : Rebased #

Patch Set 4 : Mac and iOS #

Patch Set 5 : Android #

Total comments: 4

Patch Set 6 : comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+386 lines, -286 lines) Patch
M chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/bookmarks/managed_bookmark_service_unittest.cc View 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/arc/arc_play_store_enabled_preference_handler_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/arc/arc_session_manager_unittest.cc View 4 chunks +8 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/arc/arc_util_unittest.cc View 4 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/auto_enrollment_client_unittest.cc View 3 chunks +7 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/printing/printers_manager_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/proxy_config_service_impl_unittest.cc View 4 chunks +6 lines, -5 lines 0 comments Download
M chrome/browser/chromeos/session_length_limiter_unittest.cc View 5 chunks +9 lines, -7 lines 0 comments Download
M chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/chromeos/system/automatic_reboot_manager_unittest.cc View 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/component_updater/chrome_component_updater_configurator_unittest.cc View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/content_settings/content_settings_policy_provider_unittest.cc View 8 chunks +12 lines, -13 lines 0 comments Download
M chrome/browser/content_settings/host_content_settings_map_unittest.cc View 7 chunks +8 lines, -7 lines 0 comments Download
M chrome/browser/download/download_target_determiner_unittest.cc View 1 chunk +2 lines, -4 lines 0 comments Download
M chrome/browser/extensions/api/developer_private/developer_private_api_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_management_test_util.h View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/extensions/extension_management_unittest.cc View 16 chunks +39 lines, -26 lines 0 comments Download
M chrome/browser/extensions/external_policy_loader_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/password_manager/chrome_password_manager_client_unittest.cc View 6 chunks +7 lines, -6 lines 0 comments Download
M chrome/browser/plugins/chrome_plugin_service_filter_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/plugins/plugin_info_message_filter_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/prefs/chrome_pref_service_unittest.cc View 2 chunks +7 lines, -4 lines 0 comments Download
M chrome/browser/prefs/incognito_mode_prefs_unittest.cc View 3 chunks +10 lines, -6 lines 0 comments Download
M chrome/browser/prefs/session_startup_pref_unittest.cc View 3 chunks +9 lines, -6 lines 0 comments Download
M chrome/browser/printing/cloud_print/cloud_print_proxy_service_unittest.cc View 12 chunks +38 lines, -23 lines 0 comments Download
M chrome/browser/profiles/profile_manager_unittest.cc View 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/search_engines/template_url_service_test_util.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/search_engines/template_url_service_unittest.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/signin/easy_unlock_service_unittest_chromeos.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/app_list/arc/arc_app_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc View 5 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/ui/bookmarks/bookmark_context_menu_controller_unittest.cc View 1 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/chrome_select_file_policy_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller_unittest.mm View 1 2 3 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M components/bookmarks/managed/managed_bookmarks_tracker_unittest.cc View 1 4 chunks +13 lines, -14 lines 0 comments Download
M components/browser_sync/profile_sync_service_unittest.cc View 5 chunks +10 lines, -5 lines 0 comments Download
M components/certificate_transparency/ct_policy_manager_unittest.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M components/cryptauth/cryptauth_device_manager_unittest.cc View 1 chunk +7 lines, -5 lines 0 comments Download
M components/cryptauth/cryptauth_enrollment_manager_unittest.cc View 2 chunks +6 lines, -5 lines 0 comments Download
M components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_test_utils.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/policy/core/browser/url_blacklist_manager_unittest.cc View 1 chunk +6 lines, -4 lines 0 comments Download
M components/prefs/pref_change_registrar_unittest.cc View 1 2 3 4 5 3 chunks +13 lines, -6 lines 0 comments Download
M components/prefs/pref_service_unittest.cc View 6 chunks +8 lines, -5 lines 0 comments Download
M components/prefs/testing_pref_service.h View 9 chunks +23 lines, -16 lines 0 comments Download
M components/proxy_config/pref_proxy_config_tracker_impl_unittest.cc View 7 chunks +21 lines, -13 lines 0 comments Download
M components/search_engines/default_search_manager_unittest.cc View 4 chunks +6 lines, -4 lines 0 comments Download
M components/search_engines/search_engines_test_util.cc View 2 chunks +3 lines, -1 line 0 comments Download
M components/search_engines/template_url_prepopulate_data_unittest.cc View 1 7 chunks +10 lines, -10 lines 0 comments Download
M components/ssl_config/ssl_config_service_manager_pref_unittest.cc View 9 chunks +13 lines, -10 lines 0 comments Download
M components/sync_preferences/pref_service_syncable_unittest.cc View 7 chunks +7 lines, -7 lines 0 comments Download
M components/translate/core/browser/translate_manager_unittest.cc View 2 chunks +4 lines, -1 line 0 comments Download
M components/translate/core/browser/translate_prefs_unittest.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M ios/chrome/browser/ui/settings/utils/pref_backed_boolean_unittest.mm View 1 2 3 4 5 2 chunks +5 lines, -2 lines 0 comments Download

Messages

Total messages: 41 (32 generated)
vabr (Chromium)
Hi all! bauerb@, please review components/prefs/testing_pref_service.h, which is the key change. jdoerrie@, please spot check ...
3 years, 8 months ago (2017-03-28 16:37:17 UTC) #25
jochen (gone - plz use gerrit)
rubberstamp lgtm
3 years, 8 months ago (2017-03-29 06:27:02 UTC) #26
jdoerrie
Spot checked, LGTM! https://codereview.chromium.org/2782553004/diff/80001/components/prefs/pref_change_registrar_unittest.cc File components/prefs/pref_change_registrar_unittest.cc (right): https://codereview.chromium.org/2782553004/diff/80001/components/prefs/pref_change_registrar_unittest.cc#newcode5 components/prefs/pref_change_registrar_unittest.cc:5: #include "components/prefs/pref_change_registrar.h" Nit: Missing New Line ...
3 years, 8 months ago (2017-03-29 09:28:00 UTC) #27
vabr (Chromium)
Thanks! Once bauerb@ is happy with components/prefs/testing_pref_service.h I will send to CQ. Cheers, Vaclav https://codereview.chromium.org/2782553004/diff/80001/components/prefs/pref_change_registrar_unittest.cc ...
3 years, 8 months ago (2017-03-29 09:43:56 UTC) #30
vabr (Chromium)
> Thanks for catching this! Turns out clang-format moved this header up (to > conform ...
3 years, 8 months ago (2017-03-29 09:54:23 UTC) #31
sdefresne
ios/chrome/browser/ui/settings/utils/pref_backed_boolean_unittest.mm lgtm
3 years, 8 months ago (2017-03-29 11:04:55 UTC) #34
Bernhard Bauer
TestingPrefService LGTM
3 years, 8 months ago (2017-03-29 11:51:50 UTC) #35
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/2782553004/100001
3 years, 8 months ago (2017-03-29 13:08:09 UTC) #38
commit-bot: I haz the power
3 years, 8 months ago (2017-03-29 13:15:56 UTC) #41
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/8684c9acc69aa307f5fabfef0182...

Powered by Google App Engine
This is Rietveld 408576698