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

Issue 2767893002: Remove ScopedVector from chrome/browser/. (Closed)

Created:
3 years, 9 months ago by leonhsl(Using Gerrit)
Modified:
3 years, 8 months ago
CC:
Aaron Boodman, abarth-chromium, avayvod+watch_chromium.org, cbentzel+watch_chromium.org, chfremer+watch_chromium.org, chromium-reviews, darin (slow to review), darin-cc_chromium.org, dbeam+watch-settings_chromium.org, devtools-reviews_chromium.org, elijahtaylor+arcwatch_chromium.org, feature-media-reviews_chromium.org, gavinp+prer_chromium.org, gcasto+watchlist_chromium.org, groby+spellwatch_chromium.org, grt+watch_chromium.org, hidehiko+watch_chromium.org, jam, kinuko+fileapi, lhchavez+watch_chromium.org, mac-reviews_chromium.org, media-router+watch_chromium.org, Matt Giuca, michaelpg+watch-md-settings_chromium.org, mlamouri+watch-media_chromium.org, net-reviews_chromium.org, nhiroki, pam+watch_chromium.org, pfeldman, qsr+mojo_chromium.org, rlp+watch_chromium.org, rouslan+spell_chromium.org, stevenjb+watch-md-settings_chromium.org, sync-reviews_chromium.org, tapted, tburkard+watch_chromium.org, tfarina, Lei Zhang, timvolodine, tommycli, tzik, vabr+watchlistpasswordmanager_chromium.org, vakh+watch_chromium.org, victorhsieh+watch_chromium.org, viettrungluu+watch_chromium.org, yusukes+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove ScopedVector from chrome/browser/. base::ScopedVector is deprecated, see bug. BUG=554289 Review-Url: https://codereview.chromium.org/2767893002 Cr-Commit-Position: refs/heads/master@{#460000} Committed: https://chromium.googlesource.com/chromium/src/+/4ea301f06b3c94bc2cf9a8be6ca151e3c01987ba

Patch Set 1 #

Total comments: 29

Patch Set 2 : Address comments #

Patch Set 3 : Address comments from zea@ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+376 lines, -347 lines) Patch
M chrome/browser/android/tab_state.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/apps/app_shim/app_shim_host_mac_unittest.cc View 4 chunks +7 lines, -6 lines 0 comments Download
M chrome/browser/apps/drive/drive_app_converter.h View 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/apps/drive/drive_app_converter.cc View 3 chunks +9 lines, -4 lines 0 comments Download
M chrome/browser/apps/drive/drive_app_provider.h View 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/apps/drive/drive_app_provider.cc View 4 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/background/background_mode_manager.h View 3 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/background/background_mode_manager.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/bitmap_fetcher/bitmap_fetcher_service.h View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc View 4 chunks +4 lines, -6 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/chrome_content_browser_client_parts.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/devtools/device/usb/android_usb_device.h View 1 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/devtools/device/usb/android_usb_device.cc View 2 chunks +4 lines, -7 lines 0 comments Download
M chrome/browser/feedback/system_logs/about_system_logs_fetcher.cc View 2 chunks +9 lines, -8 lines 0 comments Download
M chrome/browser/feedback/system_logs/scrubbed_system_logs_fetcher.cc View 2 chunks +10 lines, -9 lines 0 comments Download
M chrome/browser/feedback/system_logs/system_logs_fetcher_base.h View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/media/android/remote/remote_media_player_manager.h View 3 chunks +3 lines, -4 lines 0 comments Download
M chrome/browser/media/android/remote/remote_media_player_manager.cc View 7 chunks +17 lines, -17 lines 0 comments Download
M chrome/browser/media/android/router/media_router_android.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/media/router/media_router.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/media/router/mojo/media_router_mojo_impl.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/media/webrtc/media_capture_devices_dispatcher.h View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/media/webrtc/media_capture_devices_dispatcher.cc View 7 chunks +23 lines, -15 lines 0 comments Download
M chrome/browser/media_galleries/media_file_system_registry_unittest.cc View 5 chunks +10 lines, -5 lines 0 comments Download
M chrome/browser/net/quota_policy_channel_id_store.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/net/quota_policy_channel_id_store_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/policy/policy_prefs_browsertest.cc View 1 15 chunks +44 lines, -60 lines 0 comments Download
M chrome/browser/predictors/resource_prefetcher.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/prerender/prerender_browsertest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/safe_browsing/incident_reporting/module_load_analyzer_win_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/safe_search_api/safe_search_url_checker.h View 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/safe_search_api/safe_search_url_checker.cc View 3 chunks +6 lines, -4 lines 0 comments Download
M chrome/browser/search_engines/template_url_service_sync_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/sessions/session_restore.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h View 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc View 3 chunks +10 lines, -4 lines 0 comments Download
M chrome/browser/site_details_browsertest.cc View 5 chunks +6 lines, -4 lines 0 comments Download
M chrome/browser/speech/chrome_speech_recognition_manager_delegate.cc View 4 chunks +12 lines, -10 lines 0 comments Download
M chrome/browser/spellchecker/spellcheck_message_filter_platform_mac_browsertest.cc View 3 chunks +6 lines, -4 lines 0 comments Download
M chrome/browser/spellchecker/spellcheck_message_filter_unittest.cc View 5 chunks +7 lines, -4 lines 0 comments Download
M chrome/browser/status_icons/status_tray.h View 1 2 chunks +5 lines, -3 lines 0 comments Download
M chrome/browser/status_icons/status_tray.cc View 2 chunks +12 lines, -14 lines 0 comments Download
M chrome/browser/status_icons/status_tray_unittest.cc View 1 2 chunks +5 lines, -3 lines 0 comments Download
M chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.h View 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc View 5 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/supervised_user/supervised_user_service.h View 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/supervised_user/supervised_user_service.cc View 4 chunks +13 lines, -11 lines 0 comments Download
M chrome/browser/sync/test/integration/apps_helper.h View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/sync/test/integration/apps_helper.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/sync/test/integration/migration_test.cc View 4 chunks +8 lines, -5 lines 0 comments Download
M chrome/browser/sync/test/integration/passwords_helper.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/sync/test/integration/performance/passwords_sync_perf_test.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/sync/test/integration/profile_sync_service_harness.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/test/integration/profile_sync_service_harness.cc View 1 2 2 chunks +4 lines, -5 lines 0 comments Download
M chrome/browser/sync/test/integration/quiesce_status_change_checker.h View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/sync/test/integration/quiesce_status_change_checker.cc View 3 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/sync/test/integration/sync_test.h View 1 3 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/sync/test/integration/sync_test.cc View 1 5 chunks +16 lines, -6 lines 0 comments Download
M chrome/browser/sync_file_system/drive_backend/metadata_database.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/sync_file_system/sync_file_system_service.h View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/sync_file_system/sync_file_system_service.cc View 3 chunks +9 lines, -12 lines 0 comments Download
M chrome/browser/task_manager/sampling/task_manager_impl.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/app_list/arc/arc_app_item.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/status_icons/status_tray_mac.h View 1 chunk +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/cocoa/status_icons/status_tray_mac.mm View 1 2 chunks +6 lines, -3 lines 0 comments Download
M chrome/browser/ui/passwords/password_manager_presenter.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/views/status_icons/status_icon_linux_wrapper.h View 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/status_icons/status_icon_linux_wrapper.cc View 1 2 chunks +11 lines, -9 lines 0 comments Download
M chrome/browser/ui/views/status_icons/status_tray_linux.h View 1 chunk +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/status_icons/status_tray_linux.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/status_icons/status_tray_win.h View 1 chunk +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/status_icons/status_tray_win.cc View 4 chunks +8 lines, -7 lines 0 comments Download
M chrome/browser/ui/webui/settings/certificates_handler.cc View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/media/android/browser_media_player_manager.h View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/media/android/browser_media_player_manager.cc View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 47 (27 generated)
leonhsl(Using Gerrit)
PTAL, Thanks! avi@: Overall and content/ OWNER review thestig@: Overall and chrome/browser/ OWNER review
3 years, 9 months ago (2017-03-23 02:02:12 UTC) #10
Avi (use Gerrit)
https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc File chrome/browser/sync/test/integration/sync_test.cc (right): https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc#newcode595 chrome/browser/sync/test/integration/sync_test.cc:595: clients_[index].release(); Is this right? I know that this preserves ...
3 years, 9 months ago (2017-03-23 02:44:48 UTC) #11
Lei Zhang
Mostly nits. In the future, maybe split large CLs into smaller pieces. e.g. c/b/{sync,ui} could ...
3 years, 9 months ago (2017-03-23 02:59:26 UTC) #13
leonhsl(Using Gerrit)
Thank you very much for kindly review! Uploaded ps#2 to address comments, PTAnL. https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/devtools/device/usb/android_usb_device.h File ...
3 years, 9 months ago (2017-03-23 15:03:17 UTC) #16
Avi (use Gerrit)
Nicolas, Can you take a look at https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc#newcode595 ? It looks like sync code is ...
3 years, 9 months ago (2017-03-23 18:32:56 UTC) #22
skym
https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc File chrome/browser/sync/test/integration/sync_test.cc (right): https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc#newcode595 chrome/browser/sync/test/integration/sync_test.cc:595: clients_[index].release(); On 2017/03/23 15:03:17, leonhsl wrote: > On 2017/03/23 ...
3 years, 9 months ago (2017-03-23 19:28:05 UTC) #24
Avi (use Gerrit)
https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc File chrome/browser/sync/test/integration/sync_test.cc (right): https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc#newcode595 chrome/browser/sync/test/integration/sync_test.cc:595: clients_[index].release(); On 2017/03/23 19:28:05, skym wrote: > I don't ...
3 years, 9 months ago (2017-03-23 19:36:33 UTC) #25
skym
On 2017/03/23 19:36:33, Avi wrote: > https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc > File chrome/browser/sync/test/integration/sync_test.cc (right): > > https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc#newcode595 > ...
3 years, 9 months ago (2017-03-23 20:05:50 UTC) #26
Nicolas Zea
https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc File chrome/browser/sync/test/integration/sync_test.cc (right): https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc#newcode595 chrome/browser/sync/test/integration/sync_test.cc:595: clients_[index].release(); On 2017/03/23 19:36:33, Avi wrote: > On 2017/03/23 ...
3 years, 9 months ago (2017-03-23 20:06:47 UTC) #27
Avi (use Gerrit)
On 2017/03/23 20:06:47, Nicolas Zea wrote: > https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc > File chrome/browser/sync/test/integration/sync_test.cc (right): > > https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/sync_test.cc#newcode595 ...
3 years, 9 months ago (2017-03-23 20:12:16 UTC) #28
Lei Zhang
lgtm, but I didn't check c/b/sync very carefully since there are others looking at it.
3 years, 9 months ago (2017-03-23 20:26:27 UTC) #29
mark a. foltz
Non-reviewer LGTM for */media/*
3 years, 9 months ago (2017-03-23 21:34:54 UTC) #31
Nicolas Zea
https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/profile_sync_service_harness.cc File chrome/browser/sync/test/integration/profile_sync_service_harness.cc (right): https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/profile_sync_service_harness.cc#newcode302 chrome/browser/sync/test/integration/profile_sync_service_harness.cc:302: std::vector<ProfileSyncServiceHarness*> clients) { On 2017/03/23 15:03:17, leonhsl wrote: > ...
3 years, 9 months ago (2017-03-23 23:10:07 UTC) #32
leonhsl(Using Gerrit)
Thanks all for the discussions and review! https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/profile_sync_service_harness.cc File chrome/browser/sync/test/integration/profile_sync_service_harness.cc (right): https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/profile_sync_service_harness.cc#newcode302 chrome/browser/sync/test/integration/profile_sync_service_harness.cc:302: std::vector<ProfileSyncServiceHarness*> clients) ...
3 years, 9 months ago (2017-03-24 02:23:12 UTC) #33
Avi (use Gerrit)
BTW with the sync leak fix, LGTM.
3 years, 9 months ago (2017-03-24 05:50:01 UTC) #34
Nicolas Zea
https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/profile_sync_service_harness.cc File chrome/browser/sync/test/integration/profile_sync_service_harness.cc (right): https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/profile_sync_service_harness.cc#newcode302 chrome/browser/sync/test/integration/profile_sync_service_harness.cc:302: std::vector<ProfileSyncServiceHarness*> clients) { On 2017/03/24 02:23:12, leonhsl wrote: > ...
3 years, 9 months ago (2017-03-24 16:48:03 UTC) #35
leonhsl(Using Gerrit)
Hi, Nicolas, uploaded ps#3, PTAnL, Thanks. https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/profile_sync_service_harness.cc File chrome/browser/sync/test/integration/profile_sync_service_harness.cc (right): https://codereview.chromium.org/2767893002/diff/20001/chrome/browser/sync/test/integration/profile_sync_service_harness.cc#newcode302 chrome/browser/sync/test/integration/profile_sync_service_harness.cc:302: std::vector<ProfileSyncServiceHarness*> clients) { ...
3 years, 9 months ago (2017-03-25 14:41:13 UTC) #38
Nicolas Zea
sync LGTM thanks!
3 years, 9 months ago (2017-03-27 17:28:49 UTC) #41
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/2767893002/60001
3 years, 9 months ago (2017-03-28 02:04:44 UTC) #44
commit-bot: I haz the power
3 years, 8 months ago (2017-03-28 03:37:54 UTC) #47
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/4ea301f06b3c94bc2cf9a8be6ca1...

Powered by Google App Engine
This is Rietveld 408576698