This CL replaces user_manager::UserID with AccountId.

This CL replaces user_manager::UserID with AccountId.

This CL replaces API of user_manager::UserManager, thus modifying API of all
child objects, and all calls to UserManager.

This is part of transition to AccountId.

BUG=468875
TEST=manual

Committed: https://crrev.com/cd06408625de9e0d5cf4c7c1ede412cb5c82542b
Cr-Commit-Position: refs/heads/master@{#357119}

Committed: https://crrev.com/3ffbde6fa1c63df6ff55ab5f4fb3151bd881e539
Cr-Commit-Position: refs/heads/master@{#357512}

[Alexander Alekseev, 2015-10-16 23:41:54]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-16 23:43:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/1

[commit-bot: I haz the power, 2015-10-16 23:46:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-16 23:46:56]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Alexander Alekseev, 2015-10-17 00:44:23]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-17 00:45:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/20001

[commit-bot: I haz the power, 2015-10-17 00:51:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-17 00:51:52]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Alexander Alekseev, 2015-10-17 01:52:29]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-17 01:53:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/40001

[commit-bot: I haz the power, 2015-10-17 02:05:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-17 02:05:48]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Alexander Alekseev, 2015-10-17 03:27:15]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-17 03:28:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/60001

[commit-bot: I haz the power, 2015-10-17 03:38:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-17 03:38:44]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Alexander Alekseev, 2015-10-17 06:30:21]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-17 06:30:55]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/60001

[commit-bot: I haz the power, 2015-10-17 06:42:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-17 06:42:12]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Alexander Alekseev, 2015-10-17 08:11:19]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-17 08:12:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/60001

[commit-bot: I haz the power, 2015-10-17 08:22:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-17 08:22:36]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Alexander Alekseev, 2015-10-20 03:05:34]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-20 03:06:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/80001

[commit-bot: I haz the power, 2015-10-20 03:18:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-20 03:18:01]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Alexander Alekseev, 2015-10-20 19:56:26]

alemate@chromium.org changed reviewers:
+ achuith@chromium.org, bshe@chromium.org, rogerta@chromium.org,
stevenjb@chromium.org

[Alexander Alekseev, 2015-10-20 19:56:27]

Please review:

rogerta@ :  */DEPS (as dependency owner) and c/b/s/* :
    ash/DEPS
    ash/system/DEPS
    components/user_manager/DEPS
    components/wallpaper/DEPS
    chrome/browser/signin/*

bshe@ :
    components/wallpaper*

achuith@:
    c/b/c/*
    chromeos/login/*
    chrome/browser/ui/webui/chromeos/*
    components/user_manager/*

stevenjb@:
    ash/system/*
    chrome/browser/extensions/*
    chrome/browser/notifications/*
    chrome/browser/ui/{ash,webui}/*

[Alexander Alekseev, 2015-10-20 20:47:38]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-20 20:48:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/100001

[commit-bot: I haz the power, 2015-10-20 21:04:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-20 21:04:38]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Alexander Alekseev, 2015-10-22 05:49:05]

Friendly ping.

[bshe, 2015-10-22 13:20:31]

On 2015/10/22 05:49:05, Alexander Alekseev wrote:
> Friendly ping.

wallpaper/* lgtm

[stevenjb, 2015-10-22 17:52:36]

In general:
* I think we would be better off converting more classes to use AccountId
instead of email first.
* I'd really prefer using 'AccoungId()' and 'account_id' instead of UserID() and
user_id as we convert code to reduce confusion.

https://codereview.chromium.org/1412813003/diff/100001/ash/system/user/tray_u...
File ash/system/user/tray_user.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/ash/system/user/tray_u...
ash/system/user/tray_user.cc:123: user_id);
Can we actually change GetUserAccountsDelegate to take a const AccountId&
instead? (and change user_id to a const AccountId&)

https://codereview.chromium.org/1412813003/diff/100001/ash/system/user/user_v...
File ash/system/user/user_view.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/ash/system/user/user_v...
ash/system/user/user_view.cc:82:
delegate->GetUserInfo(user_index)->GetUserID().GetUserEmail());
This would be good to switch to taking AccountId& to, but this one might be more
of a pain to switch so I am fine with postponing that.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/extensi...
File chrome/browser/extensions/crx_installer_browsertest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/crx_installer_browsertest.cc:538: const AccountId
user_id(AccountId::FromUserEmail("example@example.com"));
nit: account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/extensi...
File chrome/browser/extensions/extension_assets_manager_chromeos.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/extension_assets_manager_chromeos.cc:283: const
std::string& user_id = profile->GetProfileUserName();
It would be nice if Profile had a GetAccountId() method instead.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc:816:
->AddUser(account_id.GetUserEmail());
This would also be better taking a const AccountId&, but might be better to  do
in a follow-up.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/multi_user/multi_user_context_menu_chromeos.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/multi_user/multi_user_context_menu_chromeos.cc:126:
base::Bind(&OnAcceptTeleportWarning, user_id, window);
I would change OnAcceptTeleportWarning to take an AccountId since ideally we
will eventually change SetWindowForUser to take AccountId instead of a string
anyway.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/session_state_delegate_chromeos.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/session_state_delegate_chromeos.cc:273:
user_manager::UserManager::Get()->SwitchActiveUser(
I'd like to see SwitchActiveUser() take an AccountId but again, that might be a
TODO item.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/session_state_delegate_chromeos_unittest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/session_state_delegate_chromeos_unittest.cc:177: const
AccountId user_id(AccountId::FromUserEmail(kUser));
nit: account_id here and below

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:442: if
(user_manager && !user_manager->IsKnownUser(authenticated_id)) {
nit: invert and early exit

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:443: AccountId
old_account_id(EmptyAccountId());
It looks like this ship has sailed, but it is a bit jarring to me to discover
that EmptyAccountId is in the global namespace. (I spent some time looking for
the local definition).

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:445:
&old_account_id)) {
nit: invert and early exit (same below, maybe combine)

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc:133: NULL);
nullptr

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/chromeos/shared_options_browsertest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/chromeos/shared_options_browsertest.cc:280:
RegisterUser(test_non_owner_id_.GetUserEmail());
Would it be worth adding versions of LoginUser / AddUser / RegisterUser that
take AccountId to ease the transition?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/chromeos/user_image_source.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/chromeos/user_image_source.cc:40: const
std::string& email,
It looks like we only call this once, it should take const AccointId&

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user_info.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_info.h:40: virtual AccountId GetUserID() const = 0;
I know it's a PITA, but I really wish we would rename this GetAccountId. This
change is already potentially confusing; having GetUserID return "AccountId" is
not going to help.

[achuithb, 2015-10-23 00:08:53]

I think a more consistent naming would help, and shouldn't be too difficult to
do.

user_id, username, email, etc should all be renamed to account_id.

I still have to look at a number of test files.

https://codereview.chromium.org/1412813003/diff/100001/ash/test/test_session_...
File ash/test/test_session_state_delegate.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/ash/test/test_session_...
ash/test/test_session_state_delegate.cc:221: const AccountId
user_id(GetUserIDFromEmail(user_id_str));
account_id?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc:266:
const AccountId test_account_id_;
AccountId test_account_id_ = AccountId::FromUserEmail(kTestUserName);

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc:601:
const AccountId user_id = AccountId::FromUserEmail(GetParam());
account_id?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc:631:
const AccountId user_id = AccountId::FromUserEmail(GetParam());
account_id?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/accessibility/magnification_manager_browsertest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/accessibility/magnification_manager_browsertest.cc:168:
const AccountId test_account_id_;
const AccountId test_account_id_ = AccountId::FromUserEmail(kTestUserName);

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/app_mode/kiosk_app_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:644: const AccountId
active_user_id = active_user->GetUserID();
active_account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/app_mode/kiosk_profile_loader.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_profile_loader.cc:161: if
(context.GetUserID() == chromeos::login::GuestAccountId())
drop chromeos::

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc:40: const
AccountId test_account1_;
Use in-class member initialization

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc:208: void
SwitchActiveUser(const AccountId& active_user_id);
active_account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc:238: const
AccountId& active_user_id,
active_account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc:252: const
AccountId& active_user_id) {
active_account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/input_method/input_method_persistence_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/input_method/input_method_persistence_unittest.cc:46:
const char kTestUserName[] = "test-user@example.com";
Maybe just get rid of this variable

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller.cc:151:
last_login_attempt_account_(EmptyAccountId()),
use in-class data member initialization

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller.cc:528: } else if
(last_login_attempt_account_ == chromeos::login::GuestAccountId()) {
drop chromeos::

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File
chrome/browser/chromeos/login/existing_user_controller_auto_login_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_auto_login_unittest.cc:45:
kAutoLoginAccountId,
Does this need to be changed as well?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_auto_login_unittest.cc:75:
kAutoLoginAccountId);
and here?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_auto_login_unittest.cc:98:
void SetAutoLoginSettings(const std::string& account_id, int delay) {
here too?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_browsertest.cc:109: :
mock_login_display_(NULL),
in-class member initialization

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_browsertest.cc:201: const
AccountId auto_login_account_id() const {
Return AccountId not const AccountId

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_browsertest.cc:385:
account->set_account_id(kPublicSessionAccountId);
This is still correct, right? We don't store the admin email in AccountId,
right?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/kiosk_browsertest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/kiosk_browsertest.cc:454:
test_owner_account_id_(AccountId::FromUserEmail(kTestOwnerEmail)),
in-class member initialization

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc:158: UserContext
user_context(chromeos::login::StubAccountId());
drop chromeos::

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc:215: UserContext
user_context(chromeos::login::StubAccountId());
drop chromeos::

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/reauth_stats.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/reauth_stats.cc:12: void RecordReauthReason(const
AccountId& user_id, ReauthReason reason) {
account_id here and below

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/reauth_stats.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/reauth_stats.h:56: void RecordReauthReason(const
AccountId& user_id, ReauthReason reason);
account_id here and below

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/saml/saml_offline_signin_limiter.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/saml/saml_offline_signin_limiter.cc:48: const
AccountId user_id = user->GetUserID();
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/saml/saml_offline_signin_limiter_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/saml/saml_offline_signin_limiter_unittest.cc:50:
const AccountId test_account_id_;
in-class member initialization

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/screens/user_selection_screen.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:426: const
AccountId& user_id,
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/screens/user_selection_screen.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/screens/user_selection_screen.h:125: void
OnUserStatusChecked(const AccountId& user_id,
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.cc:888: bool
is_demo_session = DemoAppLauncher::IsDemoAppSession(
const bool

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.cc:971: AccountInfo
info = account_tracker->FindAccountInfoByEmail(
const

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.cc:997: const
AccountId& user_id) {
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.cc:1741: void
UserSessionManager::OnTokenHandleObtained(const AccountId& id,
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/session/user_session_manager.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.h:304: const
AccountId& user_id);
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.h:372: void
OnTokenHandleObtained(const AccountId& id, bool success);
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/device_id_browsertest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/device_id_browsertest.cc:60: std::string
GetDeviceId(const AccountId& user_id) {
account_id, here and below

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/oauth2_login_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/oauth2_login_manager.cc:134: const
std::string& account_id) {
rename to user_email?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/token_handle_fetcher.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_fetcher.cc:45: const
AccountId& user_id)
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_fetcher.cc:49:
token_service_(nullptr),
You could move this initialization to the header file while you are here

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/token_handle_fetcher.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_fetcher.h:32:
TokenHandleFetcher(TokenHandleUtil* util, const AccountId& user_id);
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/token_handle_util.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_util.cc:37: bool
TokenHandleUtil::HasToken(const AccountId& user_id) {
account_id here and below

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/token_handle_util.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_util.h:43: bool HasToken(const
AccountId& user_id);
account_id here and everywhere below

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_util.h:80: AccountId user_id_;
account_id_

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_util.h:89: void
OnObtainTokenComplete(const AccountId& id);
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File
chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc:108:
test_account_id_(AccountId::FromUserEmail(kTestUser1)),
in class member initialization

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc:195:
void ExpectNewUserImageInfo(const std::string& username,
Why not change this and related methods to take const AccountId& instead?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:209: const
AccountId& user_id) {
account_id here and below

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:320: const
AccountId& user_id,
account_id?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:541: for
(std::vector<AccountId>::const_iterator it = public_sessions.begin();
for (const auto& account_id : public_sessions) {

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1095:
RemoveNonOwnerUserInternal(account_id, NULL);
nullptr

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.h:73:
UserImageManager* GetUserImageManager(const AccountId& user_id) override;
account_id here and elsewhere

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc:22:
owner_id_(EmptyAccountId()),
in class member initialization

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc:30: const
AccountId& account) {
account_id? here and below

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc:92: const
AccountId& /* user_id */) {
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/fake_chrome_user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.h:32: void
AddKioskAppUser(const AccountId& kiosk_app_user_id);
kiosk_app_account_id?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.h:35: const
user_manager::User* AddPublicAccountUser(const AccountId& user_id);
account_id?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.h:86: AccountId
owner_id_;
owner_account_id_?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.h:91: typedef
std::map<AccountId, UserFlow*> FlowMap;
using FlowMap = std::map<AccountId, UserFlow*>;

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/mock_user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/mock_user_manager.h:154: // TODO (alemate):
remove temporary_owner_id_ as soon as User::GetUserID will
Please reference a bug.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/user_manager_interface.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/user_manager_interface.h:30: virtual
UserImageManager* GetUserImageManager(const AccountId& user_id) = 0;
account_id, here and below

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/user_manager_unittest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/user_manager_unittest.cc:52: :
owner_at_invalid_domain_(
in class member initialization

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/user_manager_unittest.cc:111: const
AccountId GetUserManagerOwnerId() const {
Don't need const return type

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/user_manager_unittest.cc:115: void
SetUserManagerOwnerId(const AccountId& owner_id) {
owner_account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/settings/device_settings_test_helper.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/settings/device_settings_test_helper.cc:249: void
DeviceSettingsTestBase::InitOwner(const AccountId& user_id,
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/settings/device_settings_test_helper.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/settings/device_settings_test_helper.h:173: void
InitOwner(const AccountId& user_id, bool tpm_is_ready);
account_id

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/signin/...
File chrome/browser/signin/easy_unlock_service.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/signin/...
chrome/browser/signin/easy_unlock_service.cc:60: #include
"components/signin/core/account_id/account_id.h"
why twice?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/supervi...
File chrome/browser/supervised_user/supervised_user_service.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/supervi...
chrome/browser/supervised_user/supervised_user_service.cc:42: #include
"components/signin/core/account_id/account_id.h"
Why is this necessary?

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:472: UserContext
user_context(account_id);
Just inline invocation to GetAccountId(email, gaia_id) here

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:594: UserContext
user_context(account_id);
inline GetAccountId

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc:74:
bool is_owner =
const

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc:93:
std::string display_email =
const

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/ex...
File chromeos/login/auth/extended_authenticator_impl.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/ex...
chromeos/login/auth/extended_authenticator_impl.cc:191:
gaia::CanonicalizeEmail(user_context.GetUserID().GetUserEmail());
isn't it already canonicalized? also const.

same for below

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/lo...
File chromeos/login/auth/login_performer.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/lo...
chromeos/login/auth/login_performer.cc:132: std::string email =
const, isn't this already canonicalized?

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/us...
File chromeos/login/auth/user_context.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/us...
chromeos/login/auth/user_context.cc:11: : user_id_(EmptyAccountId()),
in class member initialization, also remove initializers below

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/us...
File chromeos/login/auth/user_context.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/us...
chromeos/login/auth/user_context.h:48: const AccountId& GetUserID() const;
I think this should be GetAccountID(). I know it's a big change.

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/us...
chromeos/login/auth/user_context.h:84: AccountId user_id_;
account_id_

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/fake_user_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.cc:34: primary_user_(NULL),
in class member initialization

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.cc:56: // TODO (alemate): Chenge this
to GetUserID(), once a real AccountId is
s/Chenge/Change

Please reference a bug.

same below

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/fake_user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:27: virtual const
user_manager::User* AddUser(const AccountId& user_id);
account_id, here and below

user_id, email, account, username, all to account_id

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:36: void LoginUser(const AccountId&
email);
account_id

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:44: void UserLoggedIn(const
AccountId& account,
account_id

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:60: const AccountId& GetOwnerId()
const override;
GetOwnerAccountID()?

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:127: AccountId active_user_id_;
active_account_id_

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:134: AccountId owner_id_;
owner_account_id_

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.cc:45: explicit RegularUser(const AccountId&
user_id);
account_id

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.cc:206:
oauth_token_status_(OAUTH_TOKEN_STATUS_UNKNOWN),
in class member initialization

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.cc:291: SupervisedUser::SupervisedUser(const
AccountId& user_id) : User(user_id) {
account_id

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.h:86: const std::string& email() const;
Add a TODO to rename this to GetUserEmail()

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.h:99: AccountId GetUserID() const override;
GetAccountID()

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.h:185: static User* CreateRegularUser(const
AccountId& user_id);
account_id, here and below

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.h:187: static User* CreateKioskAppUser(const
AccountId& kiosk_app_username);
kiosk_app_account_id

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.h:191: explicit User(const AccountId& email);
account_id

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user_info.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_info.h:40: virtual AccountId GetUserID() const = 0;
On 2015/10/22 17:52:36, stevenjb wrote:
> I know it's a PITA, but I really wish we would rename this GetAccountId. This
> change is already potentially confusing; having GetUserID return "AccountId"
is
> not going to help.

+1

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager.h:174: virtual void RemoveUser(const
AccountId& user_id,
account_id, here and below

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager.h:368: // TODO(antrim): Update this once
AccountId contains GAIA ID.
TODO(alemate)?

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager.h:369: virtual void UpdateGaiaID(const
AccountId& user_id,
account_id

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user_manager_base.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager_base.cc:158: : active_user_(NULL),
in class member initialization

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager_base.cc:207: const AccountId&
UserManagerBase::GetOwnerId() const {
GetOwnerAccountID?

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager_base.cc:1027: AccountId last_active_user =
const AccountId last_active_account_id =

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user_manager_base.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager_base.h:406: AccountId owner_id_;
owner_account_id_

[Alexander Alekseev, 2015-10-23 09:02:24]

On 2015/10/22 17:52:36, stevenjb wrote:
> In general:
> * I think we would be better off converting more classes to use AccountId
> instead of email first.

I'm going to replace std::string with AccountId in all the chromium code.

> * I'd really prefer using 'AccoungId()' and 'account_id' instead of UserID()
and
> user_id as we convert code to reduce confusion.

Done.

[Alexander Alekseev, 2015-10-23 09:03:06]

On 2015/10/23 00:08:53, achuithb wrote:
> I think a more consistent naming would help, and shouldn't be too difficult to
> do.
> 
> user_id, username, email, etc should all be renamed to account_id.

Done.

[Alexander Alekseev, 2015-10-23 09:11:25]

I implemented most of reviewers suggestions.
There are only a two notes on using in-class initialization:

1) in-class initialization actually implies "default value" for members, which
is dangerous for copyable objects.

AccountId has no default initialization by purpose, so that it will not be left
empty anywhere.

So I used member initialization only in tests and non-copyable objects. Notable
exception is UserContext, where default initialization seems to be dangerous.

2) Default initialization for non-trivial members requires either
copy-constructor, or brace syntax. Brace initialization is banned by code style,
so all non-copyable objects still cannot be in-class initialized. So in tests
not all members could be in-class initialized.

All the other comments should be resolved.

https://codereview.chromium.org/1412813003/diff/100001/ash/system/user/tray_u...
File ash/system/user/tray_user.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/ash/system/user/tray_u...
ash/system/user/tray_user.cc:123: user_id);
On 2015/10/22 17:52:36, stevenjb wrote:
> Can we actually change GetUserAccountsDelegate to take a const
> AccountId& instead? (and change user_id to a const AccountId&)

Let's leave this for another CL. This one replaces only interfaces in
UserManager namespace.

Also it is not yet possible to use reference here, because GetUserID() returns a
copy for historic reasons. I'll update this to use reference as soon as UserInfo
would store real ID.

https://codereview.chromium.org/1412813003/diff/100001/ash/system/user/user_v...
File ash/system/user/user_view.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/ash/system/user/user_v...
ash/system/user/user_view.cc:82:
delegate->GetUserInfo(user_index)->GetUserID().GetUserEmail());
On 2015/10/22 17:52:36, stevenjb wrote:
> This would be good to switch to taking AccountId& to, but this one might be
more
> of a pain to switch so I am fine with postponing that.

Yes, let's postpone this.

https://codereview.chromium.org/1412813003/diff/100001/ash/test/test_session_...
File ash/test/test_session_state_delegate.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/ash/test/test_session_...
ash/test/test_session_state_delegate.cc:221: const AccountId
user_id(GetUserIDFromEmail(user_id_str));
On 2015/10/23 00:08:49, achuithb wrote:
> account_id?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc:266:
const AccountId test_account_id_;
On 2015/10/23 00:08:49, achuithb wrote:
> AccountId test_account_id_ = AccountId::FromUserEmail(kTestUserName);

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc:601:
const AccountId user_id = AccountId::FromUserEmail(GetParam());
On 2015/10/23 00:08:49, achuithb wrote:
> account_id?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc:631:
const AccountId user_id = AccountId::FromUserEmail(GetParam());
On 2015/10/23 00:08:49, achuithb wrote:
> account_id?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/accessibility/magnification_manager_browsertest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/accessibility/magnification_manager_browsertest.cc:168:
const AccountId test_account_id_;
On 2015/10/23 00:08:49, achuithb wrote:
> const AccountId test_account_id_ = AccountId::FromUserEmail(kTestUserName);

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/app_mode/kiosk_app_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:644: const AccountId
active_user_id = active_user->GetUserID();
On 2015/10/23 00:08:49, achuithb wrote:
> active_account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/app_mode/kiosk_profile_loader.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_profile_loader.cc:161: if
(context.GetUserID() == chromeos::login::GuestAccountId())
On 2015/10/23 00:08:49, achuithb wrote:
> drop chromeos::

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc:40: const
AccountId test_account1_;
On 2015/10/23 00:08:49, achuithb wrote:
> Use in-class member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc:208: void
SwitchActiveUser(const AccountId& active_user_id);
On 2015/10/23 00:08:49, achuithb wrote:
> active_account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc:238: const
AccountId& active_user_id,
On 2015/10/23 00:08:49, achuithb wrote:
> active_account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc:252: const
AccountId& active_user_id) {
On 2015/10/23 00:08:49, achuithb wrote:
> active_account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/input_method/input_method_persistence_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/input_method/input_method_persistence_unittest.cc:46:
const char kTestUserName[] = "test-user@example.com";
On 2015/10/23 00:08:49, achuithb wrote:
> Maybe just get rid of this variable

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller.cc:151:
last_login_attempt_account_(EmptyAccountId()),
On 2015/10/23 00:08:49, achuithb wrote:
> use in-class data member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller.cc:528: } else if
(last_login_attempt_account_ == chromeos::login::GuestAccountId()) {
On 2015/10/23 00:08:49, achuithb wrote:
> drop chromeos::

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File
chrome/browser/chromeos/login/existing_user_controller_auto_login_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_auto_login_unittest.cc:45:
kAutoLoginAccountId,
On 2015/10/23 00:08:49, achuithb wrote:
> Does this need to be changed as well?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_auto_login_unittest.cc:75:
kAutoLoginAccountId);
On 2015/10/23 00:08:49, achuithb wrote:
> and here?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_auto_login_unittest.cc:98:
void SetAutoLoginSettings(const std::string& account_id, int delay) {
On 2015/10/23 00:08:49, achuithb wrote:
> here too?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_browsertest.cc:109: :
mock_login_display_(NULL),
On 2015/10/23 00:08:49, achuithb wrote:
> in-class member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_browsertest.cc:201: const
AccountId auto_login_account_id() const {
On 2015/10/23 00:08:49, achuithb wrote:
> Return AccountId not const AccountId

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/existing_user_controller_browsertest.cc:385:
account->set_account_id(kPublicSessionAccountId);
On 2015/10/23 00:08:49, achuithb wrote:
> This is still correct, right? We don't store the admin email in AccountId,
> right?

AFAIK, this is not admin email. This is local account id (i.e. ID of
kPublicSessionAccountId).

chrome_device_policy.proto says we need to store some unique identifier for each
local account. So this is still valid until we switch to opaque Id there.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/kiosk_browsertest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/kiosk_browsertest.cc:454:
test_owner_account_id_(AccountId::FromUserEmail(kTestOwnerEmail)),
On 2015/10/23 00:08:49, achuithb wrote:
> in-class member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc:158: UserContext
user_context(chromeos::login::StubAccountId());
On 2015/10/23 00:08:50, achuithb wrote:
> drop chromeos::

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/lock/screen_locker_browsertest.cc:215: UserContext
user_context(chromeos::login::StubAccountId());
On 2015/10/23 00:08:50, achuithb wrote:
> drop chromeos::

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/reauth_stats.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/reauth_stats.cc:12: void RecordReauthReason(const
AccountId& user_id, ReauthReason reason) {
On 2015/10/23 00:08:50, achuithb wrote:
> account_id here and below

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/reauth_stats.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/reauth_stats.h:56: void RecordReauthReason(const
AccountId& user_id, ReauthReason reason);
On 2015/10/23 00:08:50, achuithb wrote:
> account_id here and below

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/saml/saml_offline_signin_limiter.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/saml/saml_offline_signin_limiter.cc:48: const
AccountId user_id = user->GetUserID();
On 2015/10/23 00:08:50, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/saml/saml_offline_signin_limiter_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/saml/saml_offline_signin_limiter_unittest.cc:50:
const AccountId test_account_id_;
On 2015/10/23 00:08:50, achuithb wrote:
> in-class member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/screens/user_selection_screen.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:426: const
AccountId& user_id,
On 2015/10/23 00:08:50, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/screens/user_selection_screen.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/screens/user_selection_screen.h:125: void
OnUserStatusChecked(const AccountId& user_id,
On 2015/10/23 00:08:50, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.cc:888: bool
is_demo_session = DemoAppLauncher::IsDemoAppSession(
On 2015/10/23 00:08:50, achuithb wrote:
> const bool

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.cc:971: AccountInfo
info = account_tracker->FindAccountInfoByEmail(
On 2015/10/23 00:08:50, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.cc:997: const
AccountId& user_id) {
On 2015/10/23 00:08:50, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.cc:1741: void
UserSessionManager::OnTokenHandleObtained(const AccountId& id,
On 2015/10/23 00:08:50, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/session/user_session_manager.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.h:304: const
AccountId& user_id);
On 2015/10/23 00:08:50, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.h:372: void
OnTokenHandleObtained(const AccountId& id, bool success);
On 2015/10/23 00:08:50, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/device_id_browsertest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/device_id_browsertest.cc:60: std::string
GetDeviceId(const AccountId& user_id) {
On 2015/10/23 00:08:50, achuithb wrote:
> account_id, here and below

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/oauth2_login_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/oauth2_login_manager.cc:134: const
std::string& account_id) {
On 2015/10/23 00:08:50, achuithb wrote:
> rename to user_email?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/token_handle_fetcher.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_fetcher.cc:45: const
AccountId& user_id)
On 2015/10/23 00:08:50, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_fetcher.cc:49:
token_service_(nullptr),
On 2015/10/23 00:08:50, achuithb wrote:
> You could move this initialization to the header file while you are here

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/token_handle_fetcher.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_fetcher.h:32:
TokenHandleFetcher(TokenHandleUtil* util, const AccountId& user_id);
On 2015/10/23 00:08:50, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/token_handle_util.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_util.cc:37: bool
TokenHandleUtil::HasToken(const AccountId& user_id) {
On 2015/10/23 00:08:50, achuithb wrote:
> account_id here and below

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/signin/token_handle_util.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_util.h:43: bool HasToken(const
AccountId& user_id);
On 2015/10/23 00:08:50, achuithb wrote:
> account_id here and everywhere below

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_util.h:80: AccountId user_id_;
On 2015/10/23 00:08:50, achuithb wrote:
> account_id_

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/signin/token_handle_util.h:89: void
OnObtainTokenComplete(const AccountId& id);
On 2015/10/23 00:08:50, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File
chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc:108:
test_account_id_(AccountId::FromUserEmail(kTestUser1)),
On 2015/10/23 00:08:50, achuithb wrote:
> in class member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc:195:
void ExpectNewUserImageInfo(const std::string& username,
On 2015/10/23 00:08:50, achuithb wrote:
> Why not change this and related methods to take const AccountId& instead?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:209: const
AccountId& user_id) {
On 2015/10/23 00:08:50, achuithb wrote:
> account_id here and below

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:320: const
AccountId& user_id,
On 2015/10/23 00:08:51, achuithb wrote:
> account_id? 

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:541: for
(std::vector<AccountId>::const_iterator it = public_sessions.begin();
On 2015/10/23 00:08:51, achuithb wrote:
> for (const auto& account_id : public_sessions) {

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1095:
RemoveNonOwnerUserInternal(account_id, NULL);
On 2015/10/23 00:08:51, achuithb wrote:
> nullptr

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.h:73:
UserImageManager* GetUserImageManager(const AccountId& user_id) override;
On 2015/10/23 00:08:51, achuithb wrote:
> account_id here and elsewhere

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc:22:
owner_id_(EmptyAccountId()),
On 2015/10/23 00:08:51, achuithb wrote:
> in class member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc:30: const
AccountId& account) {
On 2015/10/23 00:08:51, achuithb wrote:
> account_id? here and below

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc:92: const
AccountId& /* user_id */) {
On 2015/10/23 00:08:51, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/fake_chrome_user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.h:32: void
AddKioskAppUser(const AccountId& kiosk_app_user_id);
On 2015/10/23 00:08:51, achuithb wrote:
> kiosk_app_account_id?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.h:35: const
user_manager::User* AddPublicAccountUser(const AccountId& user_id);
On 2015/10/23 00:08:51, achuithb wrote:
> account_id?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.h:86: AccountId
owner_id_;
On 2015/10/23 00:08:51, achuithb wrote:
> owner_account_id_?

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.h:91: typedef
std::map<AccountId, UserFlow*> FlowMap;
On 2015/10/23 00:08:51, achuithb wrote:
> using FlowMap = std::map<AccountId, UserFlow*>;

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/mock_user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/mock_user_manager.h:154: // TODO (alemate):
remove temporary_owner_id_ as soon as User::GetUserID will
On 2015/10/23 00:08:51, achuithb wrote:
> Please reference a bug.

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/user_manager_interface.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/user_manager_interface.h:30: virtual
UserImageManager* GetUserImageManager(const AccountId& user_id) = 0;
On 2015/10/23 00:08:51, achuithb wrote:
> account_id, here and below

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/user_manager_unittest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/user_manager_unittest.cc:52: :
owner_at_invalid_domain_(
On 2015/10/23 00:08:51, achuithb wrote:
> in class member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/user_manager_unittest.cc:111: const
AccountId GetUserManagerOwnerId() const {
On 2015/10/23 00:08:51, achuithb wrote:
> Don't need const return type

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/user_manager_unittest.cc:115: void
SetUserManagerOwnerId(const AccountId& owner_id) {
On 2015/10/23 00:08:51, achuithb wrote:
> owner_account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/settings/device_settings_test_helper.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/settings/device_settings_test_helper.cc:249: void
DeviceSettingsTestBase::InitOwner(const AccountId& user_id,
On 2015/10/23 00:08:51, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/settings/device_settings_test_helper.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/settings/device_settings_test_helper.h:173: void
InitOwner(const AccountId& user_id, bool tpm_is_ready);
On 2015/10/23 00:08:51, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/extensi...
File chrome/browser/extensions/crx_installer_browsertest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/crx_installer_browsertest.cc:538: const AccountId
user_id(AccountId::FromUserEmail("example@example.com"));
On 2015/10/22 17:52:36, stevenjb wrote:
> nit: account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/extensi...
File chrome/browser/extensions/extension_assets_manager_chromeos.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/extension_assets_manager_chromeos.cc:283: const
std::string& user_id = profile->GetProfileUserName();
On 2015/10/22 17:52:36, stevenjb wrote:
> It would be nice if Profile had a GetAccountId() method instead.

It would have it later ;-)

This CL is limited to user_manager::* API.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/signin/...
File chrome/browser/signin/easy_unlock_service.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/signin/...
chrome/browser/signin/easy_unlock_service.cc:60: #include
"components/signin/core/account_id/account_id.h"
On 2015/10/23 00:08:51, achuithb wrote:
> why twice?

Actually only this one is needed.
Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/supervi...
File chrome/browser/supervised_user/supervised_user_service.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/supervi...
chrome/browser/supervised_user/supervised_user_service.cc:42: #include
"components/signin/core/account_id/account_id.h"
On 2015/10/23 00:08:51, achuithb wrote:
> Why is this necessary?

This include was temporary. I removed it.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc:816:
->AddUser(account_id.GetUserEmail());
On 2015/10/22 17:52:36, stevenjb wrote:
> This would also be better taking a const AccountId&, but might be better to 
do
> in a follow-up.

Yes. I'm going to update SessionStateDelegate later.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/multi_user/multi_user_context_menu_chromeos.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/multi_user/multi_user_context_menu_chromeos.cc:126:
base::Bind(&OnAcceptTeleportWarning, user_id, window);
On 2015/10/22 17:52:36, stevenjb wrote:
> I would change OnAcceptTeleportWarning to take an AccountId since ideally we
> will eventually change SetWindowForUser to take AccountId instead of a string
> anyway.

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/session_state_delegate_chromeos.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/session_state_delegate_chromeos.cc:273:
user_manager::UserManager::Get()->SwitchActiveUser(
On 2015/10/22 17:52:36, stevenjb wrote:
> I'd like to see SwitchActiveUser() take an AccountId but again, that might be
a
> TODO item.

Yes, this is a TODO.

As "concept" CL https://codereview.chromium.org/1165323004/ shows, ASH would not
need anything except pointer to AccountId once migration is done. So all
references to AccountId code from ASH are temporary (except from ASH tests).

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/session_state_delegate_chromeos_unittest.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/session_state_delegate_chromeos_unittest.cc:177: const
AccountId user_id(AccountId::FromUserEmail(kUser));
On 2015/10/22 17:52:36, stevenjb wrote:
> nit: account_id here and below

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:442: if
(user_manager && !user_manager->IsKnownUser(authenticated_id)) {
On 2015/10/22 17:52:36, stevenjb wrote:
> nit: invert and early exit

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:443: AccountId
old_account_id(EmptyAccountId());
On 2015/10/22 17:52:36, stevenjb wrote:
> It looks like this ship has sailed, but it is a bit jarring to me to discover
> that EmptyAccountId is in the global namespace. (I spent some time looking for
> the local definition).

The reason for this is to make EmptyAccountId() look like other reserved 
account ids (like GuestAccountId(), etc...). At least for now.

Probably it would be better to allow AccountId to be default-constructible, but
it is dangerous while we are in transition. Once the transition is finished,
probably it would become just AccountId() .

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:445:
&old_account_id)) {
On 2015/10/22 17:52:36, stevenjb wrote:
> nit: invert and early exit (same below, maybe combine)

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:472: UserContext
user_context(account_id);
On 2015/10/23 00:08:51, achuithb wrote:
> Just inline invocation to GetAccountId(email, gaia_id) here

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:594: UserContext
user_context(account_id);
On 2015/10/23 00:08:51, achuithb wrote:
> inline GetAccountId

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc:133: NULL);
On 2015/10/22 17:52:36, stevenjb wrote:
> nullptr

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc:74:
bool is_owner =
On 2015/10/23 00:08:51, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc:93:
std::string display_email =
On 2015/10/23 00:08:51, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/chromeos/shared_options_browsertest.cc
(right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/chromeos/shared_options_browsertest.cc:280:
RegisterUser(test_non_owner_id_.GetUserEmail());
On 2015/10/22 17:52:36, stevenjb wrote:
> Would it be worth adding versions of LoginUser / AddUser / RegisterUser that
> take AccountId to ease the transition?

There are total 239 lines in 79 files referencing LoginUser / AddUser /
RegisterUser .

You're right, I'll add them in another CL.

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/chromeos/user_image_source.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/chromeos/user_image_source.cc:40: const
std::string& email,
On 2015/10/22 17:52:36, stevenjb wrote:
> It looks like we only call this once, it should take const AccointId&

Done.

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/ex...
File chromeos/login/auth/extended_authenticator_impl.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/ex...
chromeos/login/auth/extended_authenticator_impl.cc:191:
gaia::CanonicalizeEmail(user_context.GetUserID().GetUserEmail());
On 2015/10/23 00:08:51, achuithb wrote:
> isn't it already canonicalized? also const.
> 
> same for below

Not yet ;-(

The problem comes from UserContext (which doesn't have display_email) and
UserInfo (which donesn't store canonicalized Id, regenerating it on each
request). At some point, UserContext becomes UserInfo (actually
user_manager::User), so we cannot be sure it is canonicalized until migration is
done.

This will eventually go away, once AccountId will always point to a
canonicalized email.

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/lo...
File chromeos/login/auth/login_performer.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/lo...
chromeos/login/auth/login_performer.cc:132: std::string email =
On 2015/10/23 00:08:51, achuithb wrote:
> const, isn't this already canonicalized?

No ;-(

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/us...
File chromeos/login/auth/user_context.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/us...
chromeos/login/auth/user_context.cc:11: : user_id_(EmptyAccountId()),
On 2015/10/23 00:08:51, achuithb wrote:
> in class member initialization, also remove initializers below

This is dangerous, as EmptyAccountId fits only default constructor.
This structure has many field members, and constructors must initialize
account_id_ specifically to match constructor context.

That is why we do not have default initialization in AccountId.
So let's force constructors to explicitly initialize this field.

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/us...
File chromeos/login/auth/user_context.h (right):

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/us...
chromeos/login/auth/user_context.h:48: const AccountId& GetUserID() const;
On 2015/10/23 00:08:52, achuithb wrote:
> I think this should be GetAccountID(). I know it's a big change.

Done.

https://codereview.chromium.org/1412813003/diff/100001/chromeos/login/auth/us...
chromeos/login/auth/user_context.h:84: AccountId user_id_;
On 2015/10/23 00:08:51, achuithb wrote:
> account_id_

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/fake_user_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.cc:34: primary_user_(NULL),
On 2015/10/23 00:08:52, achuithb wrote:
> in class member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.cc:56: // TODO (alemate): Chenge this
to GetUserID(), once a real AccountId is
On 2015/10/23 00:08:52, achuithb wrote:
> s/Chenge/Change
> 
> Please reference a bug.
> 
> same below

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/fake_user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:27: virtual const
user_manager::User* AddUser(const AccountId& user_id);
On 2015/10/23 00:08:52, achuithb wrote:
> account_id, here and below
> 
> user_id, email, account, username, all to account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:36: void LoginUser(const AccountId&
email);
On 2015/10/23 00:08:52, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:44: void UserLoggedIn(const
AccountId& account,
On 2015/10/23 00:08:52, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:60: const AccountId& GetOwnerId()
const override;
On 2015/10/23 00:08:52, achuithb wrote:
> GetOwnerAccountID()?

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:127: AccountId active_user_id_;
On 2015/10/23 00:08:52, achuithb wrote:
> active_account_id_

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/fake_user_manager.h:134: AccountId owner_id_;
On 2015/10/23 00:08:52, achuithb wrote:
> owner_account_id_

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.cc:45: explicit RegularUser(const AccountId&
user_id);
On 2015/10/23 00:08:52, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.cc:206:
oauth_token_status_(OAUTH_TOKEN_STATUS_UNKNOWN),
On 2015/10/23 00:08:52, achuithb wrote:
> in class member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.cc:291: SupervisedUser::SupervisedUser(const
AccountId& user_id) : User(user_id) {
On 2015/10/23 00:08:52, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.h:86: const std::string& email() const;
On 2015/10/23 00:08:52, achuithb wrote:
> Add a TODO to rename this to GetUserEmail()

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.h:99: AccountId GetUserID() const override;
On 2015/10/23 00:08:52, achuithb wrote:
> GetAccountID()

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.h:185: static User* CreateRegularUser(const
AccountId& user_id);
On 2015/10/23 00:08:52, achuithb wrote:
> account_id, here and below

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.h:187: static User* CreateKioskAppUser(const
AccountId& kiosk_app_username);
On 2015/10/23 00:08:52, achuithb wrote:
> kiosk_app_account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user.h:191: explicit User(const AccountId& email);
On 2015/10/23 00:08:52, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user_info.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_info.h:40: virtual AccountId GetUserID() const = 0;
On 2015/10/22 17:52:36, stevenjb wrote:
> I know it's a PITA, but I really wish we would rename this GetAccountId. This
> change is already potentially confusing; having GetUserID return "AccountId"
is
> not going to help.

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_info.h:40: virtual AccountId GetUserID() const = 0;
On 2015/10/23 00:08:52, achuithb wrote:
> On 2015/10/22 17:52:36, stevenjb wrote:
> > I know it's a PITA, but I really wish we would rename this GetAccountId.
This
> > change is already potentially confusing; having GetUserID return "AccountId"
> is
> > not going to help.
> 
> +1

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager.h:174: virtual void RemoveUser(const
AccountId& user_id,
On 2015/10/23 00:08:52, achuithb wrote:
> account_id, here and below

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager.h:368: // TODO(antrim): Update this once
AccountId contains GAIA ID.
On 2015/10/23 00:08:52, achuithb wrote:
> TODO(alemate)?

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager.h:369: virtual void UpdateGaiaID(const
AccountId& user_id,
On 2015/10/23 00:08:52, achuithb wrote:
> account_id

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user_manager_base.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager_base.cc:158: : active_user_(NULL),
On 2015/10/23 00:08:52, achuithb wrote:
> in class member initialization

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager_base.cc:207: const AccountId&
UserManagerBase::GetOwnerId() const {
On 2015/10/23 00:08:52, achuithb wrote:
> GetOwnerAccountID?

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager_base.cc:1027: AccountId last_active_user =
On 2015/10/23 00:08:52, achuithb wrote:
> const AccountId last_active_account_id =

Done.

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
File components/user_manager/user_manager_base.h (right):

https://codereview.chromium.org/1412813003/diff/100001/components/user_manage...
components/user_manager/user_manager_base.h:406: AccountId owner_id_;
On 2015/10/23 00:08:52, achuithb wrote:
> owner_account_id_

Done.

[stevenjb, 2015-10-23 17:17:03]

This is definitely better, although now I think that we should introduce
GetAccountId() but leave GetUserId() temporarily, then update the core classes
(User*, Profile*, Session*, etc) to take AccountId in waves, and then eliminate
GetUserId.

That said, I am not going to block this if other reviewers are OK with this
approach. I just have a few more comments this round.

Also, someone familiar with login should look at
chrome/browser/ui/webui/chromeos/login/.

https://codereview.chromium.org/1412813003/diff/100001/ash/system/user/tray_u...
File ash/system/user/tray_user.cc (right):

https://codereview.chromium.org/1412813003/diff/100001/ash/system/user/tray_u...
ash/system/user/tray_user.cc:123: user_id);
On 2015/10/23 09:11:20, Alexander Alekseev wrote:
> On 2015/10/22 17:52:36, stevenjb wrote:
> > Can we actually change GetUserAccountsDelegate to take a const
> > AccountId& instead? (and change user_id to a const AccountId&)
> 
> Let's leave this for another CL. This one replaces only interfaces in
> UserManager namespace.
> 
> Also it is not yet possible to use reference here, because GetUserID() returns
a
> copy for historic reasons. I'll update this to use reference as soon as
UserInfo
> would store real ID.
> 

I suppose that is OK, although it does seem like we would be better off changing
UserInfo to store a copy and return a const& first, otherwise we have a version
that is potentially doing a lot of extra copying.

https://codereview.chromium.org/1412813003/diff/120001/ash/test/test_session_...
File ash/test/test_session_state_delegate.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/ash/test/test_session_...
ash/test/test_session_state_delegate.cc:36: : display_email_(display_email),
Don't store display_email_ separately, there is the possibility of it getting
out of sunc, just have GetEmail() return the email from account_id_. (I know
it's just a test delegate, but still...)

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/app_mode/kiosk_app_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:646: if
(it.second->user_id() == active_account_id.GetUserEmail()) {
Is GetUserEmail() just an accessor? If so, we should rename it user_mail() (in a
separate CL). If not, we should store it locally here instead of calling a
function for every app.

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/app_launch_controller.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/login/app_launch_controller.cc:325: return
!user_manager::UserManager::Get()->GetOwnerAccountId().empty();
This is weird, AccountId is no longer a string, right?

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc:824:
profile_manager()->CreateTestingProfile(account_id.GetUserEmail());
We should definitely switch Profile classes to accept AccountId sooner than
later (this is OK for now, but let's make sure there is an issue tracking that).

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc:828:
created_profiles_[profile] = account_id.GetUserEmail();
This should store AccountId (and probably be better named). That said, using a
pointer as a key in a map is kind of terrible, so maybe just leave it for a
cleanup pass in a followup once Profile and SessionDelegate are updated.

[Alexander Alekseev, 2015-10-23 23:21:23]

Steven, I am replacing std::string with AccountId all around Chromium code. So I
expect quite a few CL like this.
And I still hope to finish this transition in M38.

So calls to GetUserEmail / FromUserEmail should either disappear, or be changed
to serialize/deserialize .
(And this is still planned for M38).

Their names are specifically chosen to be distinctive, so that simple grep in
code should reveal all places that are still to be updated.

On 2015/10/23 17:17:03, stevenjb wrote:
> This is definitely better, although now I think that we should introduce
> GetAccountId() but leave GetUserId() temporarily, then update the core classes
> (User*, Profile*, Session*, etc) to take AccountId in waves, and then
eliminate
> GetUserId.

GetUserID is completely removed in this CL. I introduced a lot of
GetUserEmail/FromUserEmail to be removed in later CL.

I don't think it is a good idea to change actual code logic in all this CLs. Or
transition to AccountId will never end.


> That said, I am not going to block this if other reviewers are OK with this
> approach. I just have a few more comments this round.
> 
> Also, someone familiar with login should look at
> chrome/browser/ui/webui/chromeos/login/.
>

https://codereview.chromium.org/1412813003/diff/120001/ash/test/test_session_...
File ash/test/test_session_state_delegate.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/ash/test/test_session_...
ash/test/test_session_state_delegate.cc:36: : display_email_(display_email),
On 2015/10/23 17:17:02, stevenjb wrote:
> Don't store display_email_ separately, there is the possibility of it getting
> out of sunc, just have GetEmail() return the email from account_id_. (I know
> it's just a test delegate, but still...)

Display Email must be different from account id, this is what it all is for ;)

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/app_mode/kiosk_app_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:646: if
(it.second->user_id() == active_account_id.GetUserEmail()) {
On 2015/10/23 17:17:02, stevenjb wrote:
> Is GetUserEmail() just an accessor? If so, we should rename it user_mail() (in
a
> separate CL). If not, we should store it locally here instead of calling a
> function for every app.

user_id must return AccountId (and it will shortly). So this is just a reminder,
that we need to drop GetUserEmail() here.

GetUserEmail() may stay as an accessor, or may become something complex. It has
specifically chosen name so that we could grep sources for it and find how many
files are still to be updated.

Actually, I expect GetUserEmail() to disappear at some moment.

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/app_launch_controller.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/login/app_launch_controller.cc:325: return
!user_manager::UserManager::Get()->GetOwnerAccountId().empty();
On 2015/10/23 17:17:02, stevenjb wrote:
> This is weird, AccountId is no longer a string, right?

Why cannot it be empty? We have a lot of code that relies on empty id.

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc:824:
profile_manager()->CreateTestingProfile(account_id.GetUserEmail());
On 2015/10/23 17:17:03, stevenjb wrote:
> We should definitely switch Profile classes to accept AccountId sooner than
> later (this is OK for now, but let's make sure there is an issue tracking
that).
> 

This is the same issue. I am replacing std::string all around Chromium code.
Right now ;)

[stevenjb, 2015-10-26 18:28:00]

I would like to see AccountId.empty() either clearly documented or (preferably)
replaced with is_valid() or renamed something more clear (e.g. is_set()). The
transition between a string identifier and AccountId is confusing enough without
using string methods on AccountId :)

Otherwise, this lgtm.

https://codereview.chromium.org/1412813003/diff/120001/ash/test/test_session_...
File ash/test/test_session_state_delegate.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/ash/test/test_session_...
ash/test/test_session_state_delegate.cc:36: : display_email_(display_email),
On 2015/10/23 23:21:22, Alexander Alekseev wrote:
> On 2015/10/23 17:17:02, stevenjb wrote:
> > Don't store display_email_ separately, there is the possibility of it
getting
> > out of sunc, just have GetEmail() return the email from account_id_. (I know
> > it's just a test delegate, but still...)
> 
> Display Email must be different from account id, this is what it all is for ;)

Acknowledged.

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/app_mode/kiosk_app_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:646: if
(it.second->user_id() == active_account_id.GetUserEmail()) {
On 2015/10/23 23:21:22, Alexander Alekseev wrote:
> On 2015/10/23 17:17:02, stevenjb wrote:
> > Is GetUserEmail() just an accessor? If so, we should rename it user_mail()
(in
> a
> > separate CL). If not, we should store it locally here instead of calling a
> > function for every app.
> 
> user_id must return AccountId (and it will shortly). So this is just a
reminder,
> that we need to drop GetUserEmail() here.
> 
> GetUserEmail() may stay as an accessor, or may become something complex. It
has
> specifically chosen name so that we could grep sources for it and find how
many
> files are still to be updated.
> 
> Actually, I expect GetUserEmail() to disappear at some moment.

So then, in case GetUserEmail doesn't get removed and ends up being something
non trivial, we should store it locally, i.e. 'std::string account_email =
active_account_id.GetUserEmail()'. But if we're definitely going to eliminate it
soon, I guess that is optional.

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/app_launch_controller.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/login/app_launch_controller.cc:325: return
!user_manager::UserManager::Get()->GetOwnerAccountId().empty();
On 2015/10/23 23:21:22, Alexander Alekseev wrote:
> On 2015/10/23 17:17:02, stevenjb wrote:
> > This is weird, AccountId is no longer a string, right?
> 
> Why cannot it be empty? We have a lot of code that relies on empty id.

I see now that AccountId has an empty() method. I personally find this very
confusing on something that is not a string. It is not clear what "empty" means
for AccountId. We should either document that method or (preferably IMHO)
eliminate it. Logically, shouldn't this be is_valid() anyway?

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc:824:
profile_manager()->CreateTestingProfile(account_id.GetUserEmail());
On 2015/10/23 23:21:23, Alexander Alekseev wrote:
> On 2015/10/23 17:17:03, stevenjb wrote:
> > We should definitely switch Profile classes to accept AccountId sooner than
> > later (this is OK for now, but let's make sure there is an issue tracking
> that).
> > 
> 
> This is the same issue. I am replacing std::string all around Chromium code.
> Right now ;)

Acknowledged.

[Alexander Alekseev, 2015-10-27 20:34:22]

Dear reviewers,

I should point out, that this CL is _only_ about migration of UserManager API.
So the actual code changes are grouped in components/user_manager/* and
*/chromeos/login/* .

All the other changes should be absolutely minimal.
That is why, most of other changes look like this:

- session_delegate()->SwitchActiveUser(account_id);
+ session_delegate()->SwitchActiveUser(account_id.GetUserEmail());
GetFakeUserManager()->SwitchActiveUser(account_id);

- as you see, because only Usermanager API has changed, we are using temporary
GetUserEmail() call, which should be removed in future CLs.

Also, I don't think it is a good idea to replace other APIs, including AccountId
API which should mimic as much as possible to string API for now. This
simplifies merge of other CLs, and also reverting of CLs if this is required.

All logical changes should be done later in separate CLs.

Otherwise, this CL would grow to my initial "proof of concept CL", which was a
way larger and didn't include tests.



On 2015/10/26 18:28:00, stevenjb wrote:
> I would like to see AccountId.empty() either clearly documented or
(preferably)
> replaced with is_valid() or renamed something more clear (e.g. is_set()). The
> transition between a string identifier and AccountId is confusing enough
without
> using string methods on AccountId :)
> 
> Otherwise, this lgtm.

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/app_launch_controller.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/login/app_launch_controller.cc:325: return
!user_manager::UserManager::Get()->GetOwnerAccountId().empty();
On 2015/10/26 18:28:00, stevenjb wrote:
> On 2015/10/23 23:21:22, Alexander Alekseev wrote:
> > On 2015/10/23 17:17:02, stevenjb wrote:
> > > This is weird, AccountId is no longer a string, right?
> > 
> > Why cannot it be empty? We have a lot of code that relies on empty id.
> 
> I see now that AccountId has an empty() method. I personally find this very
> confusing on something that is not a string. It is not clear what "empty"
means
> for AccountId. We should either document that method or (preferably IMHO)
> eliminate it. Logically, shouldn't this be is_valid() anyway?

Ok, I've changed this.

But actually .empty() method was added specifically to mimic string behavior. I
think that replacing .empty() with negated .is_valid() is actually a logical
change, that should be probably done outside of migration process. By now,
AccountId should mimic string behavior as close as possible.

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
(right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc:828:
created_profiles_[profile] = account_id.GetUserEmail();
On 2015/10/23 17:17:03, stevenjb wrote:
> This should store AccountId (and probably be better named). That said, using a
> pointer as a key in a map is kind of terrible, so maybe just leave it for a
> cleanup pass in a followup once Profile and SessionDelegate are updated.
> 

It will. What makes you think that this line would continue using std::string as
user identifier when we are migrating to AccountId ?

[stevenjb, 2015-10-27 21:00:27]

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/app_launch_controller.cc (right):

https://codereview.chromium.org/1412813003/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/login/app_launch_controller.cc:325: return
!user_manager::UserManager::Get()->GetOwnerAccountId().empty();
On 2015/10/27 20:34:21, Alexander Alekseev wrote:
> On 2015/10/26 18:28:00, stevenjb wrote:
> > On 2015/10/23 23:21:22, Alexander Alekseev wrote:
> > > On 2015/10/23 17:17:02, stevenjb wrote:
> > > > This is weird, AccountId is no longer a string, right?
> > > 
> > > Why cannot it be empty? We have a lot of code that relies on empty id.
> > 
> > I see now that AccountId has an empty() method. I personally find this very
> > confusing on something that is not a string. It is not clear what "empty"
> means
> > for AccountId. We should either document that method or (preferably IMHO)
> > eliminate it. Logically, shouldn't this be is_valid() anyway?
> 
> Ok, I've changed this.
> 
> But actually .empty() method was added specifically to mimic string behavior.
I
> think that replacing .empty() with negated .is_valid() is actually a logical
> change, that should be probably done outside of migration process. By now,
> AccountId should mimic string behavior as close as possible.

I guess I'm saying that adding empty() "specifically to mimic string behavior"
was maybe not the right call. I think it just adds confusion. For example, you
mention that using is_valid() is a logical change, which it is, but it is not
especially clear whether or not it is more or less correct (looking at the code
it seems like is_valid() is in fact more correct).

[achuithb, 2015-10-27 22:49:09]

The CQ bit was checked by achuith@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-27 22:50:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/50185
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/50185

[commit-bot: I haz the power, 2015-10-27 22:56:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-27 22:57:00]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Alexander Alekseev, 2015-10-27 23:11:33]

On 2015/10/27 22:57:00, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
>   ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
>   mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
>   mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
>   mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

This won't apply now, as it conflicts with my other CL that has landed.
I am waiting for more comments, ans then I'll rebase it.

[achuithb, 2015-10-27 23:32:49]

On 2015/10/27 23:11:33, Alexander Alekseev wrote:
> On 2015/10/27 22:57:00, commit-bot: I haz the power wrote:
> > Dry run: Try jobs failed on following builders:
> >   ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
> >   ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
> >   mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
> >   mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
> >   mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
> 
> This won't apply now, as it conflicts with my other CL that has landed.
> I am waiting for more comments, ans then I'll rebase it.

Ok

[achuithb, 2015-10-28 23:11:47]

Sorry about the delay

https://codereview.chromium.org/1412813003/diff/50185/ash/test/test_session_s...
File ash/test/test_session_state_delegate.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/ash/test/test_session_s...
ash/test/test_session_state_delegate.cc:24: // The the "canonicalized" user ID
from a given |email| address.
Update comment

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File
chrome/browser/chromeos/login/easy_unlock/easy_unlock_create_keys_operation.cc
(right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/easy_unlock/easy_unlock_create_keys_operation.cc:349:
std::string canonicalized =
const

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/easy_unlock/easy_unlock_get_keys_operation.cc
(right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/easy_unlock/easy_unlock_get_keys_operation.cc:36:
std::string canonicalized =
const

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/easy_unlock/easy_unlock_key_manager.cc
(right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/easy_unlock/easy_unlock_key_manager.cc:86:
std::string tpm_public_key = tpm_key_manager->GetPublicTpmKey(
const

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File
chrome/browser/chromeos/login/easy_unlock/easy_unlock_remove_keys_operation.cc
(right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/easy_unlock/easy_unlock_remove_keys_operation.cc:50:
std::string canonicalized =
const

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/existing_user_controller.cc:534: bool
is_known_user = user_manager::UserManager::Get()->IsKnownUser(
const

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/existing_user_controller.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/existing_user_controller.h:248: std::string
public_session_auto_login_username_;
AccountId?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/existing_user_controller.h:257: // Username of the
last login attempt.
AccountId

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/lock/screen_locker.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/lock/screen_locker.cc:290: const std::string&
user_id) {
AccountId?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/lock/screen_locker.cc:504: bool
ScreenLocker::IsUserLoggedIn(const std::string& username) {
AccountId instead?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/screens/user_selection_screen.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:176: &gaia_id)) {
can drop {} I believe?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/session/user_session_manager.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/session/user_session_manager.h:355: void
OnEasyUnlockKeyOpsFinished(const std::string& user_id,
AccountId?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/oauth2_login_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/oauth2_login_manager.cc:163: const
std::string& OAuth2LoginManager::GetPrimaryAccountId() {
should this return AccountId?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/token_handle_fetcher.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/token_handle_fetcher.cc:63: std::string
user_email = signin_manager->GetAuthenticatedAccountId();
const

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/token_handle_fetcher.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/token_handle_fetcher.h:35: typedef
base::Callback<void(const AccountId&, bool success)>
using

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/token_handle_util.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/token_handle_util.h:39: typedef
base::Callback<void(const AccountId&, TokenHandleStatus)>
using

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:337: if
(account_id == AccountId::FromUserEmail(owner)) {
This is changing to GaiaId, right?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:361: if
(!IsUserNonCryptohomeDataEphemeral(account_id))
I think use {} here because of the line break?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:417:
image_manager->UserProfileCreated();
Maybe just:
GetUserImageManager(user->GetAccountId())->UserProfileCreated();

Might fit on one line.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:670: const
AccountId owner_id = GetActiveUser()->GetAccountId();
owner_account_id?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1082:
!UserSessionManager::GetInstance()->UserSessionsRestoreInProgress()) {
can probably drop {}

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.h:163: typedef
base::hash_map<AccountId, linked_ptr<UserImageManager> >
using since you're here

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.h:241: typedef
std::map<AccountId, UserFlow*> FlowMap;
using

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc:52:
user->set_username_hash(ProfileHelper::GetUserIdHashByUserIdForTesting(
This will eventually take AccountId, right?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/mock_user_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/mock_user_manager.cc:139:
user_manager::UserList::iterator user;
move this into for loop initializer

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/mock_user_manager.cc:147: if (user->email()
== user_id) {
drop {}

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/mock_user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/mock_user_manager.h:100: void(const
std::string& user_id,
Why not AccountId conversion here?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/mock_user_manager.h:124: bool
ShouldReportUser(const std::string& user_id) const override;
Why not AccountId conversion here?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/supervised_user_manager_impl.cc
(right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/supervised_user_manager_impl.cc:152:
user_exists = (NULL != owner_->FindUser(AccountId::FromUserEmail(id)));
nullptr

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/supervised_user_manager_impl.cc:167: const
std::string& manager_id) const {
Is this going to be replaced with AccountId?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/supervised_user_manager_impl.cc:447:
owner_->RemoveNonOwnerUserInternal(AccountId::FromUserEmail(user_id), NULL);
nullptr

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/user_manager_interface.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/user_manager_interface.h:22: // Chrome
specific add-ons interface for the UserManager.
ChromeOS specific?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc:484: bool
is_persistent =
const

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/signin/c...
File chrome/browser/signin/chrome_signin_client.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/signin/c...
chrome/browser/signin/chrome_signin_client.cc:67: const AccountId user_id =
user->GetAccountId();
account_id please

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/signin/e...
File chrome/browser/signin/easy_unlock_service.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/signin/e...
chrome/browser/signin/easy_unlock_service.cc:617:
base::Bind(&EasyUnlockService::OnCryptohomeKeysFetchedForChecking,
This method will take AccountId in the future, right?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:434: const
std::string sanitized_email = gaia::SanitizeEmail(authenticated_email);
I think you can get rid of this temporary and inline the call below?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:438: const
AccountId authenticated_id(
authenticated_account_id?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:450: if
(!user_manager->GetKnownUserAccountId(authenticated_id, &old_account_id)) {
Don't need {}

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:455: LOG(WARNING)
<< "Existing user '" << old_account_id.GetUserEmail()
Wonder if we should make this VLOG(1) instead

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:456: << "'
authenticated by alias '" << sanitized_email << "'.";
canonicalized_email?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:460: return
authenticated_id;
This is a change of behavior right? Because now you're returning
canonicalized_email effectively?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/chromeos/user_image_source.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/chromeos/user_image_source.h:41: // no user with
such id, returns the first default image.
s/id/an id/

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/cry...
File chromeos/login/auth/cryptohome_authenticator.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/cry...
chromeos/login/auth/cryptohome_authenticator.cc:152:
attempt->user_context.GetAccountId().GetUserEmail()),
does GetUserEmail() return canonicalized email?

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/ext...
File chromeos/login/auth/extended_authenticator_impl.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/ext...
chromeos/login/auth/extended_authenticator_impl.cc:191:
gaia::CanonicalizeEmail(user_context.GetAccountId().GetUserEmail());
Do we need CanonicalizeEmail or does GetUserEmail return a canonicalized email?

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/ext...
chromeos/login/auth/extended_authenticator_impl.cc:215:
cryptohome::Identification id(canonicalized);
we want to change this, right?

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/fak...
File chromeos/login/auth/fake_extended_authenticator.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/fak...
chromeos/login/auth/fake_extended_authenticator.cc:42:
reported_user_context.GetAccountId().GetUserEmail() + "-hash";
Is the plan to move away from using user email for hash?

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/stu...
File chromeos/login/auth/stub_authenticator.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/stu...
chromeos/login/auth/stub_authenticator.cc:77: void
StubAuthenticator::LoginAsKioskAccount(const std::string& app_user_id,
/* app_user_id */ since you are here

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
File chromeos/login/auth/user_context.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.cc:11: : account_id_(EmptyAccountId()),
I think you can still use in-class member initialization, and only set the
values that are different from the default in the ctors?

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.cc:137: void UserContext::SetUserID(const
std::string& user_id) {
Why not SetAccountId instead?

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
File chromeos/login/auth/user_context.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.h:67: void SetUserID(const std::string&
user_id);
Does this not need to change?

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.h:92: bool is_using_oauth_;
= true;

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.h:93: AuthFlow auth_flow_;
= AUTH_FLOW_OFFLINE;

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.h:94: user_manager::UserType user_type_;
= user_manager::USER_TYPE_REGULAR;

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/fake_user_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/fake_user_manager.cc:34: primary_user_(NULL) {}
switch to nullptr for this file as well, since there are a lot of changes
already.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/fake_user_manager.cc:102: if (users_.size()) {
if (!users_.empty()) since you are here

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/user.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user.cc:54: bool is_child_;
= false

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/user.h (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user.h:86: // TODO: rename this to GetUserEmail()
TODO(alemate), please reference bug

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user.h:260: bool using_saml_;
missed this one

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user.h:303: typedef std::vector<User*> UserList;
since you're here, let's switch to
using UserList = std::vector<User*>;

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager.h:182: // Returns true if a user with the
given user id is found in the persistent
s/user id/|account_id|/, here and all comments below

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager.h:296: // Returns true if data stored or
cached for the user with the given user id
|account_id|

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager.h:366: // Returns it in |out_id|.
out_account_id

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager.h:371: // TODO(alemate): Update this once
AccountId contains GAIA ID.
reference bug?

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/user_manager_base.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager_base.cc:1001: NULL);
nullptr, here and everywhere else in this file, since there are a lot of changes
already.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/user_manager_base.h (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager_base.h:147: const int reauth_reason)
override;
don't need const here

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager_base.h:150: virtual void
SetIsCurrentUserNew(bool is_new);
Could you add a comment for this?

[Alexander Alekseev, 2015-10-29 02:00:43]

https://codereview.chromium.org/1412813003/diff/50185/ash/test/test_session_s...
File ash/test/test_session_state_delegate.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/ash/test/test_session_s...
ash/test/test_session_state_delegate.cc:24: // The the "canonicalized" user ID
from a given |email| address.
On 2015/10/28 23:11:44, achuithb wrote:
> Update comment

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File
chrome/browser/chromeos/login/easy_unlock/easy_unlock_create_keys_operation.cc
(right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/easy_unlock/easy_unlock_create_keys_operation.cc:349:
std::string canonicalized =
On 2015/10/28 23:11:44, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/easy_unlock/easy_unlock_get_keys_operation.cc
(right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/easy_unlock/easy_unlock_get_keys_operation.cc:36:
std::string canonicalized =
On 2015/10/28 23:11:44, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/easy_unlock/easy_unlock_key_manager.cc
(right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/easy_unlock/easy_unlock_key_manager.cc:86:
std::string tpm_public_key = tpm_key_manager->GetPublicTpmKey(
On 2015/10/28 23:11:44, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File
chrome/browser/chromeos/login/easy_unlock/easy_unlock_remove_keys_operation.cc
(right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/easy_unlock/easy_unlock_remove_keys_operation.cc:50:
std::string canonicalized =
On 2015/10/28 23:11:44, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/existing_user_controller.cc:534: bool
is_known_user = user_manager::UserManager::Get()->IsKnownUser(
On 2015/10/28 23:11:44, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/existing_user_controller.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/existing_user_controller.h:248: std::string
public_session_auto_login_username_;
On 2015/10/28 23:11:45, achuithb wrote:
> AccountId?

This will be cleared in another CL.

My code update algorithm should be like this:
1) Update a set of files, replacing std::string with AccountId.
2) Make temporary conversions in all adjacent files to make the code compile.
This introduces a set of GetUserEmail / FromUserEmail calls.
3) Land CL.

Then, start a new CL with "replace FromUserEmail/GetUserEmail with AccountId
objects further".

This CL is about replacing UserID in user_manager namespace. If I start
replacing strings with AccountIds further, it would grow enormously. I'll do it
in a next couple of CLs.

This member is translated to AccountId:

chrome/browser/chromeos/login/existing_user_controller.cc:850:     
AccountId::FromUserEmail(public_session_auto_login_username_));

This means that it is pending for transformation due to call to FromUserEmail.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/existing_user_controller.h:257: // Username of the
last login attempt.
On 2015/10/28 23:11:45, achuithb wrote:
> AccountId

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/lock/screen_locker.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/lock/screen_locker.cc:290: const std::string&
user_id) {
On 2015/10/28 23:11:45, achuithb wrote:
> AccountId?

Sure. In another CL.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/lock/screen_locker.cc:504: bool
ScreenLocker::IsUserLoggedIn(const std::string& username) {
On 2015/10/28 23:11:45, achuithb wrote:
> AccountId instead?

In another CL.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/screens/user_selection_screen.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:176: &gaia_id)) {
On 2015/10/28 23:11:45, achuithb wrote:
> can drop {} I believe?

The condition is multiline here. This doesn't seem as a good candidate to drop
{}.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/session/user_session_manager.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/session/user_session_manager.h:355: void
OnEasyUnlockKeyOpsFinished(const std::string& user_id,
On 2015/10/28 23:11:45, achuithb wrote:
> AccountId?

This is not a part of user_manager::* API, so this change is waiting for next
CLs.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/oauth2_login_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/oauth2_login_manager.cc:163: const
std::string& OAuth2LoginManager::GetPrimaryAccountId() {
On 2015/10/28 23:11:45, achuithb wrote:
> should this return AccountId?

Yes. As soon as OAuth2LoginManager will be migrated to AccountId.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/token_handle_fetcher.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/token_handle_fetcher.cc:63: std::string
user_email = signin_manager->GetAuthenticatedAccountId();
On 2015/10/28 23:11:45, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/token_handle_fetcher.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/token_handle_fetcher.h:35: typedef
base::Callback<void(const AccountId&, bool success)>
On 2015/10/28 23:11:45, achuithb wrote:
> using

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/token_handle_util.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/token_handle_util.h:39: typedef
base::Callback<void(const AccountId&, TokenHandleStatus)>
On 2015/10/28 23:11:45, achuithb wrote:
> using

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:337: if
(account_id == AccountId::FromUserEmail(owner)) {
On 2015/10/28 23:11:45, achuithb wrote:
> This is changing to GaiaId, right?

Yes.
This is one of several serialization parts of code, where we would need to
replace it with gaiaid.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:361: if
(!IsUserNonCryptohomeDataEphemeral(account_id))
On 2015/10/28 23:11:45, achuithb wrote:
> I think use {} here because of the line break?

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:417:
image_manager->UserProfileCreated();
On 2015/10/28 23:11:45, achuithb wrote:
> Maybe just:
> GetUserImageManager(user->GetAccountId())->UserProfileCreated();
> 
> Might fit on one line.

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:670: const
AccountId owner_id = GetActiveUser()->GetAccountId();
On 2015/10/28 23:11:45, achuithb wrote:
> owner_account_id?

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1082:
!UserSessionManager::GetInstance()->UserSessionsRestoreInProgress()) {
On 2015/10/28 23:11:45, achuithb wrote:
> can probably drop {}

Here is a multiline condition. I don't think this will look good.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.h:163: typedef
base::hash_map<AccountId, linked_ptr<UserImageManager> >
On 2015/10/28 23:11:45, achuithb wrote:
> using since you're here

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.h:241: typedef
std::map<AccountId, UserFlow*> FlowMap;
On 2015/10/28 23:11:45, achuithb wrote:
> using

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc:52:
user->set_username_hash(ProfileHelper::GetUserIdHashByUserIdForTesting(
On 2015/10/28 23:11:45, achuithb wrote:
> This will eventually take AccountId, right?

Yes.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/mock_user_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/mock_user_manager.cc:139:
user_manager::UserList::iterator user;
On 2015/10/28 23:11:45, achuithb wrote:
> move this into for loop initializer

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/mock_user_manager.cc:147: if (user->email()
== user_id) {
On 2015/10/28 23:11:45, achuithb wrote:
> drop {}

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/mock_user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/mock_user_manager.h:100: void(const
std::string& user_id,
On 2015/10/28 23:11:45, achuithb wrote:
> Why not AccountId conversion here?

Because this is actually a part of ChromeUserManager, not User manager API. It
is incorrectly attributed here.

(I moved it down)

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/mock_user_manager.h:124: bool
ShouldReportUser(const std::string& user_id) const override;
On 2015/10/28 23:11:45, achuithb wrote:
> Why not AccountId conversion here?

Because this is a part of ChromeUserManager API, not Usermanager.
This is waiting for ChromeUserManager migration.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/supervised_user_manager_impl.cc
(right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/supervised_user_manager_impl.cc:152:
user_exists = (NULL != owner_->FindUser(AccountId::FromUserEmail(id)));
On 2015/10/28 23:11:45, achuithb wrote:
> nullptr

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/supervised_user_manager_impl.cc:167: const
std::string& manager_id) const {
On 2015/10/28 23:11:45, achuithb wrote:
> Is this going to be replaced with AccountId?

Yes. As long as SupervisedUserManager is going to be migrated.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/supervised_user_manager_impl.cc:447:
owner_->RemoveNonOwnerUserInternal(AccountId::FromUserEmail(user_id), NULL);
On 2015/10/28 23:11:45, achuithb wrote:
> nullptr

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/user_manager_interface.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/user_manager_interface.h:22: // Chrome
specific add-ons interface for the UserManager.
On 2015/10/28 23:11:45, achuithb wrote:
> ChromeOS specific?

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc:484: bool
is_persistent =
On 2015/10/28 23:11:46, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/signin/c...
File chrome/browser/signin/chrome_signin_client.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/signin/c...
chrome/browser/signin/chrome_signin_client.cc:67: const AccountId user_id =
user->GetAccountId();
On 2015/10/28 23:11:46, achuithb wrote:
> account_id please

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/signin/e...
File chrome/browser/signin/easy_unlock_service.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/signin/e...
chrome/browser/signin/easy_unlock_service.cc:617:
base::Bind(&EasyUnlockService::OnCryptohomeKeysFetchedForChecking,
On 2015/10/28 23:11:46, achuithb wrote:
> This method will take AccountId in the future, right?

Yes.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:434: const
std::string sanitized_email = gaia::SanitizeEmail(authenticated_email);
On 2015/10/28 23:11:46, achuithb wrote:
> I think you can get rid of this temporary and inline the call below?

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:438: const
AccountId authenticated_id(
On 2015/10/28 23:11:46, achuithb wrote:
> authenticated_account_id?

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:450: if
(!user_manager->GetKnownUserAccountId(authenticated_id, &old_account_id)) {
On 2015/10/28 23:11:46, achuithb wrote:
> Don't need {}

Now it has multiline condition.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:455: LOG(WARNING)
<< "Existing user '" << old_account_id.GetUserEmail()
On 2015/10/28 23:11:46, achuithb wrote:
> Wonder if we should make this VLOG(1) instead

This is only 1 log line per user login. Do you think it is too much?

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:456: << "'
authenticated by alias '" << sanitized_email << "'.";
On 2015/10/28 23:11:46, achuithb wrote:
> canonicalized_email?

Done.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:460: return
authenticated_id;
On 2015/10/28 23:11:46, achuithb wrote:
> This is a change of behavior right? Because now you're returning
> canonicalized_email effectively?

Yes!

This is a first step to have AccountId associated with canonicalized email.

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/chromeos/user_image_source.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/chromeos/user_image_source.h:41: // no user with
such id, returns the first default image.
On 2015/10/28 23:11:46, achuithb wrote:
> s/id/an id/

Done.

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/cry...
File chromeos/login/auth/cryptohome_authenticator.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/cry...
chromeos/login/auth/cryptohome_authenticator.cc:152:
attempt->user_context.GetAccountId().GetUserEmail()),
On 2015/10/28 23:11:46, achuithb wrote:
> does GetUserEmail() return canonicalized email?

Not. And this is bad.
But this all works, because GAIA returns us canonicalized e-mail, which then
gets into AccountId.

I am not sure what happens for local accounts, but for Gaia we rely on Gaia
return value.

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/ext...
File chromeos/login/auth/extended_authenticator_impl.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/ext...
chromeos/login/auth/extended_authenticator_impl.cc:191:
gaia::CanonicalizeEmail(user_context.GetAccountId().GetUserEmail());
On 2015/10/28 23:11:46, achuithb wrote:
> Do we need CanonicalizeEmail or does GetUserEmail return a canonicalized
email?

I would better not touch this right now. It will disapped after AccountId will
always contain canonical email.

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/ext...
chromeos/login/auth/extended_authenticator_impl.cc:215:
cryptohome::Identification id(canonicalized);
On 2015/10/28 23:11:46, achuithb wrote:
> we want to change this, right?

Yes.

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/fak...
File chromeos/login/auth/fake_extended_authenticator.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/fak...
chromeos/login/auth/fake_extended_authenticator.cc:42:
reported_user_context.GetAccountId().GetUserEmail() + "-hash";
On 2015/10/28 23:11:46, achuithb wrote:
> Is the plan to move away from using user email for hash?

Yes.

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/stu...
File chromeos/login/auth/stub_authenticator.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/stu...
chromeos/login/auth/stub_authenticator.cc:77: void
StubAuthenticator::LoginAsKioskAccount(const std::string& app_user_id,
On 2015/10/28 23:11:46, achuithb wrote:
> /* app_user_id */ since you are here

Done.

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
File chromeos/login/auth/user_context.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.cc:11: : account_id_(EmptyAccountId()),
On 2015/10/28 23:11:46, achuithb wrote:
> I think you can still use in-class member initialization, and only set the
> values that are different from the default in the ctors?

I mean that adding a new constructor here without explicit initialization of
account_id is probably an error here.

I hope we would be able to make it constant member at some point.

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.cc:137: void UserContext::SetUserID(const
std::string& user_id) {
On 2015/10/28 23:11:46, achuithb wrote:
> Why not SetAccountId instead?

Because this CL is limited to UserManager API.

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
File chromeos/login/auth/user_context.h (right):

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.h:67: void SetUserID(const std::string&
user_id);
On 2015/10/28 23:11:46, achuithb wrote:
> Does this not need to change?

This CL is limited to UserManager API ;)

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.h:92: bool is_using_oauth_;
On 2015/10/28 23:11:46, achuithb wrote:
> = true;

Done.

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.h:93: AuthFlow auth_flow_;
On 2015/10/28 23:11:46, achuithb wrote:
> = AUTH_FLOW_OFFLINE;

Done.

https://codereview.chromium.org/1412813003/diff/50185/chromeos/login/auth/use...
chromeos/login/auth/user_context.h:94: user_manager::UserType user_type_;
On 2015/10/28 23:11:46, achuithb wrote:
> = user_manager::USER_TYPE_REGULAR;

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/fake_user_manager.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/fake_user_manager.cc:34: primary_user_(NULL) {}
On 2015/10/28 23:11:46, achuithb wrote:
> switch to nullptr for this file as well, since there are a lot of changes
> already.

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/fake_user_manager.cc:102: if (users_.size()) {
On 2015/10/28 23:11:46, achuithb wrote:
> if (!users_.empty()) since you are here

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/user.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user.cc:54: bool is_child_;
On 2015/10/28 23:11:46, achuithb wrote:
> = false

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/user.h (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user.h:86: // TODO: rename this to GetUserEmail()
On 2015/10/28 23:11:46, achuithb wrote:
> TODO(alemate), please reference bug

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user.h:260: bool using_saml_;
On 2015/10/28 23:11:46, achuithb wrote:
> missed this one

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user.h:303: typedef std::vector<User*> UserList;
On 2015/10/28 23:11:46, achuithb wrote:
> since you're here, let's switch to
> using UserList = std::vector<User*>;

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/user_manager.h (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager.h:182: // Returns true if a user with the
given user id is found in the persistent
On 2015/10/28 23:11:47, achuithb wrote:
> s/user id/|account_id|/, here and all comments below

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager.h:296: // Returns true if data stored or
cached for the user with the given user id
On 2015/10/28 23:11:47, achuithb wrote:
> |account_id|

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager.h:366: // Returns it in |out_id|.
On 2015/10/28 23:11:47, achuithb wrote:
> out_account_id

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager.h:371: // TODO(alemate): Update this once
AccountId contains GAIA ID.
On 2015/10/28 23:11:47, achuithb wrote:
> reference bug?

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/user_manager_base.cc (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager_base.cc:1001: NULL);
On 2015/10/28 23:11:47, achuithb wrote:
> nullptr, here and everywhere else in this file, since there are a lot of
changes
> already.

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
File components/user_manager/user_manager_base.h (right):

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager_base.h:147: const int reauth_reason)
override;
On 2015/10/28 23:11:47, achuithb wrote:
> don't need const here

Done.

https://codereview.chromium.org/1412813003/diff/50185/components/user_manager...
components/user_manager/user_manager_base.h:150: virtual void
SetIsCurrentUserNew(bool is_new);
On 2015/10/28 23:11:47, achuithb wrote:
> Could you add a comment for this?

Done.

[Alexander Alekseev, 2015-10-29 03:32:07]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-29 03:33:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/170001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/170001

[commit-bot: I haz the power, 2015-10-29 03:43:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-29 03:43:47]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Alexander Alekseev, 2015-10-29 07:33:35]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-29 07:34:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/190001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/190001

[commit-bot: I haz the power, 2015-10-29 07:45:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-29 07:45:30]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[achuithb, 2015-10-29 16:49:56]

lgtm. Thanks for your patience.

[Alexander Alekseev, 2015-10-29 22:42:14]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-29 22:44:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/190001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/190001

[Alexander Alekseev, 2015-10-29 23:15:10]

alemate@chromium.org changed reviewers:
+ jochen@chromium.org, sky@chromium.org

[Alexander Alekseev, 2015-10-29 23:15:12]

+ sky@
+ jochen@

(rogerta@, friendly ping)

Please review (the list of files I am asking for review is below).


A note to reviewers:

We are moving from std::string user identifier to class AccountId. This will
help us make serialization/deserialization of AccountId in one place, making it
possible to migrate from stored "e-mails as identifiers" completely.

I am updating current chromium code in this way:
1) Update a set of files, replacing std::string with AccountId.
2) Make temporary conversions in all adjacent files to make the code compile.
(Adding temporary dependencies!)
This introduces a set of GetUserEmail / FromUserEmail calls.
3) Land CL.

Then, start a new CL with "replace FromUserEmail/GetUserEmail with AccountId
objects further".

This CL is about replacing UserID in user_manager namespace. All the other
changes should be absolutely minimal. So all the changes outside of
components/user_manager/* mostly look like this:

That is why, most of other changes look like this:

- session_delegate()->SwitchActiveUser(account_id);
+ session_delegate()->SwitchActiveUser(account_id.GetUserEmail());
  GetFakeUserManager()->SwitchActiveUser(account_id);

- as you see, because only Usermanager API has changed, we are using temporary
GetUserEmail() call, which should be removed in future CLs.

Also, ChromeUserManager is dirrerent from UserManager, so it is not updated
here.



sky@, please review:

ash/BUILD.gn
ash/DEPS     (A temporary dependency is added here. It will be removed as we
fully migrate to AccountId.)
ash/ash.gyp
ash/test/test_session_state_delegate.cc
c/b/browsing_data/browsing_data_remover_unittest.cc
c/b/download/notification/download_notification_browsertest.cc
c/b/extensions/api/bluetooth_low_energy/bluetooth_low_energy_apitest_chromeos.cc
c/b/extensions/api/braille_display_private/braille_display_private_apitest.cc
c/b/extensions/api/file_system/file_system_api_unittest.cc
c/b/extensions/api/file_system/file_system_apitest_chromeos.cc
c/b/extensions/crx_installer_browsertest.cc
c/b/extensions/extension_assets_manager_chromeos.cc
c/b/extensions/extension_garbage_collector_chromeos_unittest.cc
c/b/metrics/chromeos_metrics_provider_unittest.cc
c/b/net/nss_context_chromeos_browsertest.cc
c/b/profiles/profile_impl.cc
c/b/profiles/profile_manager_unittest.cc
c/b/renderer_context_menu/render_view_context_menu_browsertest.cc
c/b/supervised_user/chromeos/manager_password_service.cc
c/b/supervised_user/supervised_user_service.cc
c/b/sync/sync_ui_util.cc

jochen@, please review:

components/user_manager.gypi 

rogerta@ : */DEPS (as deps owner)

[commit-bot: I haz the power, 2015-10-29 23:36:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-29 23:36:12]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sky, 2015-10-29 23:41:55]

LGTM

https://codereview.chromium.org/1412813003/diff/190001/ash/test/test_session_...
File ash/test/test_session_state_delegate.cc (right):

https://codereview.chromium.org/1412813003/diff/190001/ash/test/test_session_...
ash/test/test_session_state_delegate.cc:62: AccountId account_id_;
nit: const

[jochen (gone - plz use gerrit), 2015-10-30 12:00:17]

lgtm

[Roger Tawa OOO till Jul 10th, 2015-10-30 12:15:39]

chrome/browser/signin lgtm

[Alexander Alekseev, 2015-10-30 14:58:11]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2015-10-30 14:58:12]

The patchset sent to the CQ was uploaded after l-g-t-m from bshe@chromium.org,
stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/1412813003/#ps190001
(title: "Fixed tests.")

[Alexander Alekseev, 2015-10-30 14:58:25]

The CQ bit was unchecked by alemate@chromium.org

[Alexander Alekseev, 2015-10-30 15:07:54]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2015-10-30 15:07:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, jochen@chromium.org, sky@chromium.org,
rogerta@chromium.org, achuith@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/1412813003/#ps210001
(title: "Update after review.")

[commit-bot: I haz the power, 2015-10-30 15:08:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/210001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/210001

[commit-bot: I haz the power, 2015-10-30 16:22:51]

Committed patchset #12 (id:210001)

[commit-bot: I haz the power, 2015-10-30 16:23:21]

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

[rjkroege, 2015-10-30 23:20:47]

A revert of this CL (patchset #12 id:210001) has been created in
https://codereview.chromium.org/1425093004/ by rjkroege@chromium.org.

The reason for reverting is: breaks the GN build on windows:

FAILED: E:/b/depot_tools/python276_bin/python.exe gyp-win-tool link-wrapper
environment.x86 False link.exe /nologo /OUT:ash_unittests.exe
/PDB:ash_unittests.exe.pdb @ash_unittests.exe.rsp
test_session_state_delegate.obj : error LNK2019: unresolved external symbol
"public: __thiscall AccountId::AccountId(class AccountId const &)"
(??0AccountId@@QAE@ABV0@@Z) referenced in function "public: virtual class
AccountId __thiscall ash::test::MockUserInfo::GetAccountId(void)const "
(?GetAccountId@MockUserInfo@test@ash@@UBE?AVAccountId@@XZ)

test_session_state_delegate.obj : error LNK2019: unresolved external symbol
"public: bool __thiscall AccountId::operator==(class AccountId const &)const "
(??8AccountId@@QBE_NABV0@@Z) referenced in function "public: virtual void
__thiscall ash::test::TestSessionStateDelegate::SwitchActiveUser(class
std::basic_string<char,struct std::char_traits<char>,class std::allocator<char>
> const &)"
(?SwitchActiveUser@TestSessionStateDelegate@test@ash@@UAEXABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z)

test_session_state_delegate.obj : error LNK2019: unresolved external symbol
"public: class std::basic_string<char,struct std::char_traits<char>,class
std::allocator<char> > const & __thiscall AccountId::GetUserEmail(void)const "
(?GetUserEmail@AccountId@@QBEABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@XZ)
referenced in function "public: virtual void __thiscall
ash::test::TestSessionStateDelegate::SwitchActiveUser(class
std::basic_string<char,struct std::char_traits<char>,class std::allocator<char>
> const &)"
(?SwitchActiveUser@TestSessionStateDelegate@test@ash@@UAEXABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z)

test_session_state_delegate.obj : error LNK2019: unresolved external symbol
"public: static class AccountId __cdecl AccountId::FromUserEmail(class
std::basic_string<char,struct std::char_traits<char>,class std::allocator<char>
> const &)"
(?FromUserEmail@AccountId@@SA?AV1@ABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z)
referenced in function "class AccountId __cdecl ash::test::`anonymous
namespace'::GetAccountIdFromEmail(class std::basic_string<char,struct
std::char_traits<char>,class std::allocator<char> > const &)"
(?GetAccountIdFromEmail@?A0x8b8d8028@test@ash@@YA?AVAccountId@@ABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z)

ash_unittests.exe : fatal error LNK1120: 4 unresolved externals

ninja: build stopped: subcommand failed..

[Alexander Alekseev, 2015-11-02 21:15:21]

Description was changed from

==========
This CL replaces user_manager::UserID with AccountId.

This CL replaces API of user_manager::UserManager, thus modifying API of all
child objects, and all calls to UserManager.

This is part of transition to AccountId.

BUG=468875
TEST=manual

Committed: https://crrev.com/cd06408625de9e0d5cf4c7c1ede412cb5c82542b
Cr-Commit-Position: refs/heads/master@{#357119}
==========

to

==========
This CL replaces user_manager::UserID with AccountId.

This CL replaces API of user_manager::UserManager, thus modifying API of all
child objects, and all calls to UserManager.

This is part of transition to AccountId.

BUG=468875
TEST=manual

Committed: https://crrev.com/cd06408625de9e0d5cf4c7c1ede412cb5c82542b
Cr-Commit-Position: refs/heads/master@{#357119}
==========

[Alexander Alekseev, 2015-11-03 00:12:10]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-03 00:14:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/230001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/230001

[commit-bot: I haz the power, 2015-11-03 01:42:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-03 01:42:14]

Dry run: This issue passed the CQ dry run.

[Alexander Alekseev, 2015-11-03 01:52:25]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2015-11-03 01:52:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, jochen@chromium.org, sky@chromium.org,
rogerta@chromium.org, achuith@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/1412813003/#ps230001
(title: "Fix Win GN build.")

[commit-bot: I haz the power, 2015-11-03 01:53:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412813003/230001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412813003/230001

[commit-bot: I haz the power, 2015-11-03 02:03:10]

Committed patchset #13 (id:230001)

[commit-bot: I haz the power, 2015-11-03 02:04:02]

Patchset 13 (id:??) landed as
https://crrev.com/3ffbde6fa1c63df6ff55ab5f4fb3151bd881e539
Cr-Commit-Position: refs/heads/master@{#357512}