Make sure avatar menu does not use account_id for display purposes.

Make sure avatar menu does not use account_id for display purposes.

BUG=341408
Committed: https://crrev.com/fb6cea801d50d40e9e275bae619022f7e60de2ed
Cr-Commit-Position: refs/heads/master@{#292178}

[Roger Tawa OOO till Jul 10th, 2014-08-21 01:41:12]

Hi Hui,

Please review.  Thanks.

[guohui, 2014-08-21 19:01:53]

it seems all calls to GetAuthenticatedUserName in profile_chooser* should (or at
least could) be changed to GetAuthenticatedAccountId.

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:945:
browser_->profile())->GetAuthenticatedUsername();
here it should be changed to GetAuthenticatedAccountId.

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1820:
account_id:primaryAccount
primaryAccount is filled with the authenticated username at line 1792, so it
cant be used as account id here.

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1902:
browser_->profile())->GetAuthenticatedUsername();
here it should be getauthenticatedaccountid.

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1929:
base::UTF8ToUTF16(accountIdToRemove_), base::string16(), &offsets);
need to convert id to display email.

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1434:
browser_->profile())->GetAuthenticatedUsername();
you changed account_id_to_remove_ to use the account id above, but here the
primary account is still the authenticated username, you need to change it to
account id as well and translate it into a display email below at line 1448.

[Roger Tawa OOO till Jul 10th, 2014-08-21 20:39:25]

Thanks Hui.  Changes uploaded.

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:945:
browser_->profile())->GetAuthenticatedUsername();
On 2014/08/21 19:01:53, guohui wrote:
> here it should be changed to GetAuthenticatedAccountId.

I was planning to do a cleanup of all places in a separate CL, but agree it
makes sense to clean up this file while I'm here.

Done.

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1820:
account_id:primaryAccount
On 2014/08/21 19:01:52, guohui wrote:
> primaryAccount is filled with the authenticated username at line 1792, so it
> cant be used as account id here.

Done.

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1902:
browser_->profile())->GetAuthenticatedUsername();
On 2014/08/21 19:01:52, guohui wrote:
> here it should be getauthenticatedaccountid.

Done.

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1929:
base::UTF8ToUTF16(accountIdToRemove_), base::string16(), &offsets);
On 2014/08/21 19:01:52, guohui wrote:
> need to convert id to display email.

Good catch.  Done.

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1434:
browser_->profile())->GetAuthenticatedUsername();
On 2014/08/21 19:01:53, guohui wrote:
> you changed account_id_to_remove_ to use the account id above, but here the
> primary account is still the authenticated username, you need to change it to
> account id as well and translate it into a display email below at line 1448.

Done.

[guohui, 2014-08-21 20:42:41]

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1434:
browser_->profile())->GetAuthenticatedUsername();
you need to convert account id to display email at line 1449?

[Roger Tawa OOO till Jul 10th, 2014-08-21 20:59:34]

Hopefully this is the last one :-)

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/471333003/diff/120001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1434:
browser_->profile())->GetAuthenticatedUsername();
On 2014/08/21 20:42:41, guohui wrote:
> you need to convert account id to display email at line 1449?

Done.

[guohui, 2014-08-22 21:14:58]

lgtm

[Roger Tawa OOO till Jul 10th, 2014-08-25 20:47:10]

The CQ bit was checked by rogerta@chromium.org

[Roger Tawa OOO till Jul 10th, 2014-08-25 20:47:42]

The CQ bit was unchecked by rogerta@chromium.org

[Roger Tawa OOO till Jul 10th, 2014-08-26 00:22:21]

Patchset #6 (id:180001) has been deleted

[Roger Tawa OOO till Jul 10th, 2014-08-26 00:22:36]

Patchset #6 (id:200001) has been deleted

[Roger Tawa OOO till Jul 10th, 2014-08-26 00:22:49]

Patchset #6 (id:220001) has been deleted

[Roger Tawa OOO till Jul 10th, 2014-08-26 00:23:02]

Patchset #6 (id:240001) has been deleted

[Roger Tawa OOO till Jul 10th, 2014-08-26 00:24:17]

Patchset #6 (id:260001) has been deleted

[Roger Tawa OOO till Jul 10th, 2014-08-26 00:52:22]

Hi Alexei, Scott,

Alexei: Please take a look at profile_chooser_controller.mm
Scott: Please take a look at profile_chooser_view.h|cc

Thanks.

[Roger Tawa OOO till Jul 10th, 2014-08-26 00:53:42]

rogerta@chromium.org changed reviewers:
+ asvitkine@chromium.org, sky@chromium.org

[Roger Tawa OOO till Jul 10th, 2014-08-26 00:53:42]

Hi Alexei, Scott,

(Actually adding you guys as reviewers this time.)

Alexei: Please take a look at profile_chooser_controller.mm
Scott: Please take a look at profile_chooser_view.h|cc

Thanks.

[Alexei Svitkine (slow), 2014-08-26 01:04:45]

LGTM % comment - please address before landing

https://codereview.chromium.org/471333003/diff/280001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/471333003/diff/280001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:864:
account_id:(const std::string&)account_id
Nit: accountId change on both sides of :.

[sky, 2014-08-26 16:41:23]

LGTM

[Roger Tawa OOO till Jul 10th, 2014-08-26 19:25:12]

Thanks Alexei, Scott.

Alexei: please take another look.

https://codereview.chromium.org/471333003/diff/280001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/471333003/diff/280001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:864:
account_id:(const std::string&)account_id
On 2014/08/26 01:04:44, Alexei Svitkine wrote:
> Nit: accountId change on both sides of :.

Done.

[Alexei Svitkine (slow), 2014-08-26 20:35:41]

lgtm

[Roger Tawa OOO till Jul 10th, 2014-08-26 22:20:38]

Patchset #9 (id:340001) has been deleted

[Roger Tawa OOO till Jul 10th, 2014-08-26 23:45:16]

Patchset #9 (id:360001) has been deleted

[noms (inactive), 2014-08-27 15:06:28]

noms@chromium.org changed reviewers:
+ noms@chromium.org

[noms (inactive), 2014-08-27 15:06:28]

fake tracker service lgtm % nit

https://codereview.chromium.org/471333003/diff/500001/chrome/browser/signin/f...
File chrome/browser/signin/fake_account_tracker_service.h (right):

https://codereview.chromium.org/471333003/diff/500001/chrome/browser/signin/f...
chrome/browser/signin/fake_account_tracker_service.h:16: //
AccountTrackerService is a KeyedService that retrieves and caches GAIA
Should this comment maybe say something about the FakeAccountTrackerService?

[Roger Tawa OOO till Jul 10th, 2014-08-27 15:11:40]

Thanks Monica.

https://codereview.chromium.org/471333003/diff/500001/chrome/browser/signin/f...
File chrome/browser/signin/fake_account_tracker_service.h (right):

https://codereview.chromium.org/471333003/diff/500001/chrome/browser/signin/f...
chrome/browser/signin/fake_account_tracker_service.h:16: //
AccountTrackerService is a KeyedService that retrieves and caches GAIA
On 2014/08/27 15:06:27, Monica Dinculescu wrote:
> Should this comment maybe say something about the FakeAccountTrackerService?

Added more description.

[Roger Tawa OOO till Jul 10th, 2014-08-27 15:11:56]

The CQ bit was checked by rogerta@chromium.org

[commit-bot: I haz the power, 2014-08-27 15:13:08]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rogerta@chromium.org/471333003/520001

[commit-bot: I haz the power, 2014-08-27 16:42:30]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[commit-bot: I haz the power, 2014-08-27 17:42:16]

Committed patchset #16 (id:520001) as 131250eb22a1f61c816ee2cb08b31b7d5036b100

[commit-bot: I haz the power, 2014-09-10 02:52:34]

Patchset 16 (id:??) landed as
https://crrev.com/fb6cea801d50d40e9e275bae619022f7e60de2ed
Cr-Commit-Position: refs/heads/master@{#292178}