We should use UserID object to identify users instead of username.

We should use UserID object to identify users instead of username.

In order to support deletion of Gmail accounts, we need to identify all
parts of code, where username / user_email / etc. are used to store
user information to permanent storage.

To achieve this, we need to stop using plain std::string to store user
identifier. So that we could easily identify all places, where it is actually
converted to string.

This CL actually replaces std::string with a structure in most of Chromium code.
It compiles and Chrome for ChromeOS runs successfully. I consider it as a
"prof of concept" to look as the size of the work to be done.

BUG=468875
TEST=none

[Alexander Alekseev, 2015-06-10 06:16:23]

alemate@chromium.org changed reviewers:
+ antrim@chromium.org, atwilson@chromium.org, bshe@chromium.org,
jam@chromium.org, nkostylev@chromium.org, oshima@chromium.org,
rogerta@chromium.org, sacomoto@chromium.org, sky@chromium.org,
thestig@chromium.org, zelidrag@chromium.org

[Alexander Alekseev, 2015-06-10 06:16:26]

The concept of the CL is here: https://crbug.com/468875#c15 .

This CL intentionally left unformatted so the differences
would be minimal to simplify comparison.

Chrome compiles and runs successfully.

tests are not supported in this CL to keep it minimal.

This CL should also be split into several smaller, so all suggestions are
welcome.


The list of directories below is so large, that it is probably not worth
mentioning. But here it is if you need it.

Please review:

nkostylev@ :
chrome/browser/chromeos
chrome/browser/chromeos/accessibility
chrome/browser/chromeos/app_mode
chrome/browser/chromeos/attestation
chrome/browser/chromeos/background
chrome/browser/chromeos/drive
chrome/browser/chromeos/extensions
chrome/browser/chromeos/extensions/file_manager
chrome/browser/chromeos/net
chrome/browser/chromeos/ownership
chrome/browser/chromeos/platform_keys
chrome/browser/chromeos/policy
chrome/browser/chromeos/profiles
chrome/browser/chromeos/settings
chrome/browser/ui/webui
chrome/browser/ui/webui/chromeos
chrome/browser/ui/webui/chromeos/login
chrome/browser/ui/webui/help
chrome/browser/ui/webui/options
chrome/browser/ui/webui/options/chromeos
chromeos/login
chromeos/login/auth
components
components/user_manager

jam@ :
ash/session
ash/system/tray
ash/system/user
ash/test
chrome/browser/browsing_data
chrome/browser/chromeos
chrome/browser/chromeos/accessibility
chrome/browser/chromeos/app_mode
chrome/browser/chromeos/attestation
chrome/browser/chromeos/background
chrome/browser/chromeos/drive
chrome/browser/chromeos/extensions
chrome/browser/chromeos/extensions/file_manager
chrome/browser/chromeos/net
chrome/browser/chromeos/ownership
chrome/browser/chromeos/platform_keys
chrome/browser/chromeos/policy
chrome/browser/chromeos/profiles
chrome/browser/chromeos/settings
chrome/browser/drive
chrome/browser/extensions
chrome/browser/extensions/api/autotest_private
chrome/browser/extensions/api/easy_unlock_private
chrome/browser/extensions/api/enterprise_platform_keys_private
chrome/browser/extensions/api/screenlock_private
chrome/browser/feedback
chrome/browser/notifications
chrome/browser/policy
chrome/browser/profiles
chrome/browser/signin
chrome/browser/supervised_user
chrome/browser/supervised_user/child_accounts
chrome/browser/supervised_user/chromeos
chrome/browser/supervised_user/legacy
chrome/browser/sync
chrome/browser/sync_file_system/drive_backend
chrome/browser/ui
chrome/browser/ui/app_list
chrome/browser/ui/ash
chrome/browser/ui/ash/launcher
chrome/browser/ui/ash/multi_user
chrome/browser/ui/startup
chrome/browser/ui/views/frame
chrome/browser/ui/webui
chrome/browser/ui/webui/chromeos
chrome/browser/ui/webui/chromeos/login
chrome/browser/ui/webui/help
chrome/browser/ui/webui/options
chrome/browser/ui/webui/options/chromeos
chromeos
chromeos/attestation
chromeos/cryptohome
chromeos/dbus
chromeos/login
chromeos/login/auth
chromeos/tpm
components
components/autofill
components/autofill/core/browser
components/policy/core
components/policy/core/browser
components/policy/core/common/cloud
components/proximity_auth
components/proximity_auth/ble
components/proximity_auth/cryptauth
components/user_manager
components/wallpaper

oshima@ :
ash/session
ash/system/tray
ash/system/user
ash/test
chrome/browser/chromeos
chrome/browser/chromeos/accessibility
chrome/browser/chromeos/app_mode
chrome/browser/chromeos/attestation
chrome/browser/chromeos/background
chrome/browser/chromeos/drive
chrome/browser/chromeos/extensions
chrome/browser/chromeos/extensions/file_manager
chrome/browser/chromeos/net
chrome/browser/chromeos/ownership
chrome/browser/chromeos/platform_keys
chrome/browser/chromeos/policy
chrome/browser/chromeos/profiles
chrome/browser/chromeos/settings
chrome/browser/ui/ash
chrome/browser/ui/ash/launcher
chrome/browser/ui/ash/multi_user
chromeos
chromeos/attestation
chromeos/cryptohome
chromeos/dbus
chromeos/login
chromeos/login/auth
chromeos/tpm

zelidrag@ :

chrome/browser/chromeos
chrome/browser/chromeos/accessibility
chrome/browser/chromeos/app_mode
chrome/browser/chromeos/attestation
chrome/browser/chromeos/background
chrome/browser/chromeos/drive
chrome/browser/chromeos/extensions
chrome/browser/chromeos/extensions/file_manager
chrome/browser/chromeos/net
chrome/browser/chromeos/ownership
chrome/browser/chromeos/platform_keys
chrome/browser/chromeos/policy
chrome/browser/chromeos/profiles
chrome/browser/chromeos/settings
chrome/browser/extensions/api/autotest_private
chrome/browser/ui/webui/chromeos
chrome/browser/ui/webui/chromeos/login
chromeos
chromeos/attestation
chromeos/cryptohome
chromeos/dbus
chromeos/login
chromeos/login/auth
chromeos/tpm

sky@:
ash/session
ash/system/tray
ash/system/user
ash/test
chrome/browser/browsing_data
chrome/browser/chromeos
chrome/browser/chromeos/accessibility
chrome/browser/chromeos/app_mode
chrome/browser/chromeos/attestation
chrome/browser/chromeos/background
chrome/browser/chromeos/drive
chrome/browser/chromeos/extensions
chrome/browser/chromeos/extensions/file_manager
chrome/browser/chromeos/net
chrome/browser/chromeos/ownership
chrome/browser/chromeos/platform_keys
chrome/browser/chromeos/policy
chrome/browser/chromeos/profiles
chrome/browser/chromeos/settings
chrome/browser/drive
chrome/browser/extensions
chrome/browser/extensions/api/autotest_private
chrome/browser/extensions/api/easy_unlock_private
chrome/browser/extensions/api/enterprise_platform_keys_private
chrome/browser/extensions/api/screenlock_private
chrome/browser/feedback
chrome/browser/notifications
chrome/browser/policy
chrome/browser/profiles
chrome/browser/signin
chrome/browser/supervised_user
chrome/browser/supervised_user/child_accounts
chrome/browser/supervised_user/chromeos
chrome/browser/supervised_user/legacy
chrome/browser/sync
chrome/browser/sync_file_system/drive_backend
chrome/browser/ui
chrome/browser/ui/app_list
chrome/browser/ui/ash
chrome/browser/ui/ash/launcher
chrome/browser/ui/ash/multi_user
chrome/browser/ui/startup
chrome/browser/ui/views/frame
chrome/browser/ui/webui
chrome/browser/ui/webui/chromeos
chrome/browser/ui/webui/chromeos/login
chrome/browser/ui/webui/help
chrome/browser/ui/webui/options
chrome/browser/ui/webui/options/chromeos

atwilson@ :
chrome/browser/chromeos/policy
chrome/browser/policy
chrome/browser/signin
chrome/browser/sync
components/policy/core
components/policy/core/browser
components/policy/core/common/cloud

rogerta@ :
chrome/browser/extensions/signin
chrome/browser/rlz
chrome/browser/signin
chrome/browser/ui/sync
chrome/browser/ui/webui/options
chrome/browser/ui/webui/signin
components
components/signin
google_apis
google_apis/gaia
rlz

[Nikita (slow), 2015-06-10 13:27:03]

nkostylev@chromium.org changed reviewers:
+ dpolukhin@chromium.org
- nkostylev@chromium.org

[Nikita (slow), 2015-06-10 13:27:28]

On 2015/06/10 06:16:26, Alexander Alekseev wrote:
> The concept of the CL is here: https://crbug.com/468875#c15 .
> 
> Please review:
> 
> nkostylev@ :
> chrome/browser/chromeos
> chrome/browser/chromeos/accessibility
> chrome/browser/chromeos/app_mode
> chrome/browser/chromeos/attestation
> chrome/browser/chromeos/background
> chrome/browser/chromeos/drive
> chrome/browser/chromeos/extensions
> chrome/browser/chromeos/extensions/file_manager
> chrome/browser/chromeos/net
> chrome/browser/chromeos/ownership
> chrome/browser/chromeos/platform_keys
> chrome/browser/chromeos/policy
> chrome/browser/chromeos/profiles
> chrome/browser/chromeos/settings
> chrome/browser/ui/webui
> chrome/browser/ui/webui/chromeos
> chrome/browser/ui/webui/chromeos/login
> chrome/browser/ui/webui/help
> chrome/browser/ui/webui/options
> chrome/browser/ui/webui/options/chromeos
> chromeos/login
> chromeos/login/auth
> components
> components/user_manager

Now that I've switched to another project, I'll let Dmitry review that.

[Denis Kuznetsov (DE-MUC), 2015-06-10 16:50:47]

Well, I've looked through about 50% of CL, and I have to say it is completely
unmanageble - chrome crashes every 15-20 comments.
I will post all drafts I've made so far, and will post suggestions in next
comment.

https://codereview.chromium.org/1165323004/diff/1/ash/test/test_session_state...
File ash/test/test_session_state_delegate.cc (right):

https://codereview.chromium.org/1165323004/diff/1/ash/test/test_session_state...
ash/test/test_session_state_delegate.cc:236: EXPECT_EQ(user_id.GetUserEmail(),
GetUserIDFromEmail(user_id.GetUserEmail()));
No need for this check here. This check should be done at UserID creation time.

https://codereview.chromium.org/1165323004/diff/1/ash/test/test_session_state...
ash/test/test_session_state_delegate.cc:250:
SwitchActiveUser(user_manager::UserID(std::string(), "someone@tray"));
FromUserEmail()

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/app...
File chrome/browser/chromeos/app_mode/kiosk_app_manager.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/app...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:102: const
user_manager::UserID user_id(std::string() /* gaia_id */, it.key());
FromUserEmail()

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/app...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:179:
KioskAppManager::App::App() : user_id(std::string(), std::string()),
EmptyUserId() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/app...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:614: user_manager::UserID
active_user_id = active_user->GetUserID();
auto?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/chr...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/chr...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:453: const
user_manager::UserID user_id(std::string() /* gaia_id */,
login::CanonicalizeUserID(
FromUserEmail()

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/chr...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:473: VLOG(1) <<
"Relaunching browser for user: " << user_id.Serialize()
Not sure that Serialize() method is suitable for logging.
Add some ToString() / value() method.

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/ext...
File chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc
(right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:340: :
user_manager::UserID(std::string(), std::string());
EmptyUserID()?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/ext...
File chrome/browser/chromeos/extensions/wallpaper_api.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/wallpaper_api.cc:96:
WallpaperSetWallpaperFunction::WallpaperSetWallpaperFunction() :
user_id_(std::string(), std::string()) {
EmptyUserID() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/ext...
File chrome/browser/chromeos/extensions/wallpaper_private_api.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/wallpaper_private_api.cc:344:
WallpaperPrivateSetWallpaperIfExistsFunction() : user_id_(std::string(),
std::string()) {}
EmptyUserID() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/wallpaper_private_api.cc:454:
WallpaperPrivateSetWallpaperFunction::WallpaperPrivateSetWallpaperFunction() :
user_id_(std::string(), std::string()) {
EmptyUserID() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/wallpaper_private_api.cc:591:
WallpaperPrivateSetCustomWallpaperFunction() : user_id_(std::string(),
std::string()) {}
EmptyUserID() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/app_launch_signin_screen.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/app_launch_signin_screen.cc:53: if
(user->GetUserID() == owner_id) {
We should think about == contract for UserID, like, if one userid is (GaiaID,
someone@gmail.com) and second one is ("", someone@gmail.com) should be they
treated as same or not?
Maybe we should keep == for field-by-field equality, and have separate method
bool matches(UserID other) for "equal, even though one of elements can be
incomplete", ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File
chrome/browser/chromeos/login/easy_unlock/easy_unlock_remove_keys_operation.cc
(right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/easy_unlock/easy_unlock_remove_keys_operation.cc:53:
cryptohome::Identification id(user_manager::UserID(std::string() /* gaia_id */,
canonicalized));
FromUserEmail()

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File
chrome/browser/chromeos/login/easy_unlock/easy_unlock_tpm_key_manager_factory.cc
(right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/easy_unlock/easy_unlock_tpm_key_manager_factory.cc:65:
user ? user->GetUserID() : user_manager::UserID(std::string(), std::string()),
EmptyUserID()

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/existing_user_controller.cc:158:
last_login_attempt_user_id_(std::string(), std::string()),
EmptyUserID()
here and above?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/existing_user_controller.cc:165:
online_succeeded_for_(std::string(), std::string()),
EmptyUserID()?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/existing_user_controller.cc:390: if
(gaia::ExtractDomainName(user_context.GetUserID().GetUserEmail()) ==
Maybe add chromeos::login::IsSupervisedUser(UserID) method?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/existing_user_controller.cc:541: } else if
(last_login_attempt_user_id_ == chromeos::login::GetGuestUserID()) {
Maybe introduce chromeos::login::IsGuestUser(UserID) method?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/existing_user_controller.cc:877:
host_->StartAppLaunch(user_id.GetUserEmail(), diagnostic_mode, auto_start);
Please add ToDo that app id can be at some point be separated to another field
(appId rather than e-mail).

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/existing_user_controller.cc:1047: if
(last_login_attempt_user_id_ != user_context.GetUserID()) {
Same question about (un)equality as above.

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/screens/user_selection_screen.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:325: const
user_manager::UserID& user_id = (*it)->GetUserID();
auto

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/session/chrome_session_manager.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/chrome_session_manager.cc:47: const
user_manager::UserID login_user_id =
auto

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/user_session_manager.cc:864:
user_manager::UserID(std::string(), std::string()));
EmptyGaiaID() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/user_session_manager.cc:1350: const
user_manager::UserID& active_user_id =
user_manager->GetActiveUser()->GetUserID();
auto

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/signin/auth_sync_observer.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/signin/auth_sync_observer.cc:65:
user_manager::UserID user_id = user->GetUserID();
auto

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File
chrome/browser/chromeos/login/supervised/supervised_user_creation_controller_new.cc
(right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/supervised/supervised_user_creation_controller_new.cc:71:
UserCreationContext() : manager_id(std::string(), std::string()),
local_user_id(std::string(), std::string()) {}
EmptyGaiaID() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:414:
SetPendingUserSwitchID(user_manager::UserID(std::string(), std::string()));
EmptyGaiaID() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:521: const
user_manager::UserID user_id(user_manager::UserID::FromUserEmail(*it));
auto user_id = ...?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:578:
SetOwnerID(user_manager::UserID(std::string(), std::string()));
EmptyGaiaID() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc:114: void
FakeChromeUserManager::SwitchActiveUser(const UserID& user_id /*email*/) {
drop commented part

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/users/fake_supervised_user_manager.cc
(right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/fake_supervised_user_manager.cc:30:
returnuser_manager::UserID(std::string() /* gaia_id */, std::string() /*
user_email */);
Typo (space is missing), does it compile???

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/users/multi_profile_user_controller.cc
(right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/multi_profile_user_controller.cc:135:
user_manager::UserID primary_user_id = user_manager::UserID(std::string(),
std::string());
EmptyGaiaID() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/multi_profile_user_controller.cc:223: const
user_manager::UserID user_id(std::string() /* gaia_id */, user_email);
FromUserEmail()

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/users/supervised_user_manager_impl.cc
(right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/users/supervised_user_manager_impl.cc:147:
user_manager::UserID user_id = user_manager::UserID(std::string(),
std::string());
EmptyGaiaID() ?

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/own...
File chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc
(right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/chromeos/own...
chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc:199:
user_id_(std::string(), std::string()),
EmptyUserId()

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc
(right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc:157:
return user_manager::UserID(std::string(), std::string());
EmptyUserID

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc:338:
user_manager::UserID(std::string(), std::string()), // Not used.
EmptyUserID

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc:357:
user_manager::UserID(std::string(), std::string()), // Not used.
EmptyUserId

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/feedback/sho...
File chrome/browser/feedback/show_feedback_page.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/feedback/sho...
chrome/browser/feedback/show_feedback_page.cc:73: :
user_manager::UserID(std::string(), std::string());
EmptyUserID

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/profiles/pro...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/profiles/pro...
chrome/browser/profiles/profile_manager.cc:698:
user_manager::UserID(std::string(), std::string()));
EmptyUserID

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/profiles/pro...
chrome/browser/profiles/profile_manager.cc:721:
user_manager::UserID(std::string(), std::string()));
EmtpyUserID

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/profiles/pro...
chrome/browser/profiles/profile_manager.cc:799: user_manager::UserID
supervised_user_id = user_manager::UserID(std::string(), std::string());
EmptyUserID

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/profiles/pro...
File chrome/browser/profiles/profile_window.cc (right):

https://codereview.chromium.org/1165323004/diff/1/chrome/browser/profiles/pro...
chrome/browser/profiles/profile_window.cc:291:
user_manager::UserID(std::string(), std::string()));
EmptyUserID

https://codereview.chromium.org/1165323004/diff/1/components/user_manager/use...
File components/user_manager/user_id.cc (right):

https://codereview.chromium.org/1165323004/diff/1/components/user_manager/use...
components/user_manager/user_id.cc:30: return (!gaia_id_.empty() && gaia_id_ ==
other.gaia_id_) || (!user_email_.empty() && user_email_ == other.user_email_);
what if our gaia_id_ is not empty, but other.gaia_id_ is?
Same for user email?

https://codereview.chromium.org/1165323004/diff/1/components/user_manager/use...
components/user_manager/user_id.cc:38: return user_email_ < right.user_email_;
What if e-mails are both empty, but GaiaIDs are different?
We should be extremely careful with <, == and Hash contracts, as they are used
in data structures.

https://codereview.chromium.org/1165323004/diff/1/components/user_manager/use...
File components/user_manager/user_id.h (right):

https://codereview.chromium.org/1165323004/diff/1/components/user_manager/use...
components/user_manager/user_id.h:18: UserID(const std::string& gaia_id, const
std::string& user_email);
I would make this one private and force using FromEMail / FromGaiaID / FromBoth.

[Denis Kuznetsov (DE-MUC), 2015-06-10 17:11:27]

> The concept of the CL is here: https://crbug.com/468875#c15 .
> This CL should also be split into several smaller, so all suggestions are
> welcome.

This CL is really unmanageable - it took me several hours just to read through.
I can't even imagine the pain of merging of this CL or rolling it back.

This is still a work to be done, but I strongly suggest to do it in much smaller
steps:
1) Without actually changing "typedef UserID std::string" change type of
parameters/return types/fields from std:string to UserID. Use auto for local
variables, rename fields/parameters/getters. Do that in several small CLs,
around 10-15 files per CL.
2) Introduce methods like UserIDFromEmail(std::string) / EmptyUserId() /
GetEMail(UserId), and use them in relevant places. This can also be done several
small CLs.
3) Switch UserID from std::string to struct. Here is the CL where
equality/order/hashing for is introduced and tested. With changes above it
should not affect other code.
4) [can be done in parallel] : Transform UserManager way of storing per-user
values to KnownUserPrefs. This should include both changes in UserManager as
well as writing migration script.
5) At that point we could slowly switch to using GaiaID, but you should keep in
mind that we have to support at least accounts without GaiaID (supervised users,
guest user, users who have not logged in in last N month) for pretty long time. 
It is also possible that accounts without email (but with GaiaID) would appear
at some point in future.

[achuithb, 2015-07-07 19:32:52]

On 2015/06/10 17:11:27, Denis Kuznetsov wrote:
> > The concept of the CL is here: https://crbug.com/468875#c15 .
> > This CL should also be split into several smaller, so all suggestions are
> > welcome.
> 
> This CL is really unmanageable - it took me several hours just to read
through.
> I can't even imagine the pain of merging of this CL or rolling it back.
> 
> This is still a work to be done, but I strongly suggest to do it in much
smaller
> steps:
> 1) Without actually changing "typedef UserID std::string" change type of
> parameters/return types/fields from std:string to UserID. Use auto for local
> variables, rename fields/parameters/getters. Do that in several small CLs,
> around 10-15 files per CL.
> 2) Introduce methods like UserIDFromEmail(std::string) / EmptyUserId() /
> GetEMail(UserId), and use them in relevant places. This can also be done
several
> small CLs.
> 3) Switch UserID from std::string to struct. Here is the CL where
> equality/order/hashing for is introduced and tested. With changes above it
> should not affect other code.
> 4) [can be done in parallel] : Transform UserManager way of storing per-user
> values to KnownUserPrefs. This should include both changes in UserManager as
> well as writing migration script.
> 5) At that point we could slowly switch to using GaiaID, but you should keep
in
> mind that we have to support at least accounts without GaiaID (supervised
users,
> guest user, users who have not logged in in last N month) for pretty long
time. 
> It is also possible that accounts without email (but with GaiaID) would appear
> at some point in future.

+1

[achuithb, 2015-07-07 19:33:16]

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

[Matt Giuca, 2015-07-08 02:29:18]

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

[Matt Giuca, 2015-07-08 02:29:20]

Note the original CL description was: "I consider it as a
"prof of concept" to look as the size of the work to be done." I don't think
Alexander intended to land it as-is.

[Matt Giuca, 2015-07-08 02:29:41]

mgiuca@chromium.org changed reviewers:
- mgiuca@chromium.org

[jam, 2015-07-09 16:37:07]

I'm not really familiar with this code so please find a better reviewer than me.

It seems there are still outstanding questions in the bug between Roger on
whether we need to do this at all.

[Alexander Alekseev, 2015-07-09 21:38:43]

On 2015/07/09 16:37:07, jam wrote:
> I'm not really familiar with this code so please find a better reviewer than
me.
> 
> It seems there are still outstanding questions in the bug between Roger on
> whether we need to do this at all.

Yes, it is actually being discussed now.

[Alexander Alekseev, 2015-10-22 03:28:17]

alemate@chromium.org changed reviewers:
- achuith@chromium.org, antrim@chromium.org, atwilson@chromium.org,
bshe@chromium.org, dpolukhin@chromium.org, jam@chromium.org,
oshima@chromium.org, rogerta@chromium.org, sacomoto@chromium.org,
sky@chromium.org, thestig@chromium.org, zelidrag@chromium.org

[Alexander Alekseev, 2015-10-22 03:28:18]

Removing all reviewers as this is mostly a "proof of concept" CL.