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

Issue 1926663003: Refactor of ProfileInfoCache: hide methods which return ProfileInfoCache (Closed)

Created:
4 years, 7 months ago by lwchkg
Modified:
4 years, 6 months ago
Reviewers:
WC Leung, anthonyvd, sky
CC:
chromium-reviews, WC Leung, Mike Lerman, rginda+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Refactor of ProfileInfoCache: hide methods which return ProfileInfoCache Currently almost all code using ProfileInfoCache is already migrated to the new interface. This CL hides methods that provide the old interface (i.e. ProfileManager::GetProfileInfoCache() and TestingProfileManager::profile_info_cache() from the public (except for existing code), so new callers cannot use the old interface by mistake. BUG=305720 Committed: https://crrev.com/070dc6ff1861a3ce286e9b6d6de66a2c6d4f1adb Cr-Commit-Position: refs/heads/master@{#396755}

Patch Set 1 #

Total comments: 4

Patch Set 2 : Respond to anthonyvd@'s comment #

Total comments: 4

Patch Set 3 : Use friend class in TestingProfileManager instead of a complicated but more fine-grained mechanism #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -7 lines) Patch
M chrome/browser/profiles/profile_manager.h View 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/test/base/testing_profile_manager.h View 1 2 3 chunks +7 lines, -2 lines 0 comments Download

Messages

Total messages: 37 (15 generated)
lwchkg
Dear Anthony and Mike, PTAL and also start a CQ dry run (I need to ...
4 years, 7 months ago (2016-04-27 18:04:54 UTC) #3
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1926663003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1926663003/1
4 years, 7 months ago (2016-04-28 13:33:54 UTC) #5
Mike Lerman
On 2016/04/27 18:04:54, lwchkg wrote: > Dear Anthony and Mike, > > PTAL and also ...
4 years, 7 months ago (2016-04-28 13:34:34 UTC) #6
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 7 months ago (2016-04-28 14:23:57 UTC) #8
anthonyvd
https://codereview.chromium.org/1926663003/diff/1/chrome/test/base/testing_profile_manager.h File chrome/test/base/testing_profile_manager.h (right): https://codereview.chromium.org/1926663003/diff/1/chrome/test/base/testing_profile_manager.h#newcode32 chrome/test/base/testing_profile_manager.h:32: class TestingProfileInfoCacheHelper { Why not modify the tests right ...
4 years, 7 months ago (2016-04-28 15:31:50 UTC) #9
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1926663003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1926663003/20001
4 years, 7 months ago (2016-04-28 17:10:04 UTC) #11
commit-bot: I haz the power
Dry run: No L-G-T-M from a valid reviewer yet. CQ run can only be started ...
4 years, 7 months ago (2016-04-28 17:10:06 UTC) #13
lwchkg
New patch uploaded. PTAL. Mike: Just tested and I still don't have trybot access. Anyway, ...
4 years, 7 months ago (2016-04-28 17:31:53 UTC) #14
anthonyvd
lgtm. You should have trybot access now (and should have gotten an email about it ...
4 years, 7 months ago (2016-05-17 18:34:01 UTC) #15
lwchkg
Dear Scott, PTAL as owner of chrome/test/. Thanks. Regards, WC Leung.
4 years, 7 months ago (2016-05-17 22:24:46 UTC) #17
sky
https://codereview.chromium.org/1926663003/diff/20001/chrome/test/base/testing_profile_manager.h File chrome/test/base/testing_profile_manager.h (right): https://codereview.chromium.org/1926663003/diff/20001/chrome/test/base/testing_profile_manager.h#newcode31 chrome/test/base/testing_profile_manager.h:31: class TestingProfileInfoCacheHelper { Why do you need a new ...
4 years, 7 months ago (2016-05-18 00:05:42 UTC) #18
lwchkg
https://codereview.chromium.org/1926663003/diff/20001/chrome/test/base/testing_profile_manager.h File chrome/test/base/testing_profile_manager.h (right): https://codereview.chromium.org/1926663003/diff/20001/chrome/test/base/testing_profile_manager.h#newcode31 chrome/test/base/testing_profile_manager.h:31: class TestingProfileInfoCacheHelper { On 2016/05/18 00:05:42, sky wrote: > ...
4 years, 7 months ago (2016-05-18 10:35:11 UTC) #19
sky
https://codereview.chromium.org/1926663003/diff/20001/chrome/test/base/testing_profile_manager.h File chrome/test/base/testing_profile_manager.h (right): https://codereview.chromium.org/1926663003/diff/20001/chrome/test/base/testing_profile_manager.h#newcode31 chrome/test/base/testing_profile_manager.h:31: class TestingProfileInfoCacheHelper { On 2016/05/18 10:35:11, lwchkg wrote: > ...
4 years, 7 months ago (2016-05-18 16:27:46 UTC) #20
lwchkg
Dear sky@ and anthonyvd@, New patch uploaded. PTAL. Regards, WC Leung. (anthonyvd@: I need your ...
4 years, 7 months ago (2016-05-19 20:50:01 UTC) #22
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1926663003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1926663003/60001
4 years, 7 months ago (2016-05-19 20:52:33 UTC) #24
sky
LGTM
4 years, 7 months ago (2016-05-19 21:00:52 UTC) #25
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 7 months ago (2016-05-19 23:16:39 UTC) #27
WC Leung
ping Anthony: need your final review before committing (despite your L-G-T-M in an old patch).
4 years, 6 months ago (2016-05-30 17:34:20 UTC) #30
anthonyvd
lgtm
4 years, 6 months ago (2016-05-30 18:11:37 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1926663003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1926663003/60001
4 years, 6 months ago (2016-05-30 18:30:30 UTC) #33
commit-bot: I haz the power
Committed patchset #3 (id:60001)
4 years, 6 months ago (2016-05-30 22:11:55 UTC) #35
commit-bot: I haz the power
4 years, 6 months ago (2016-05-30 22:13:56 UTC) #37
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/070dc6ff1861a3ce286e9b6d6de66a2c6d4f1adb
Cr-Commit-Position: refs/heads/master@{#396755}

Powered by Google App Engine
This is Rietveld 408576698