This CL replaces e-mail with AccountId in easy signin code.

chrome/browser/signin/easy_unlock_service_signin_chromeos.cc

Index: chrome/browser/signin/easy_unlock_service_signin_chromeos.cc
diff --git a/chrome/browser/signin/easy_unlock_service_signin_chromeos.cc b/chrome/browser/signin/easy_unlock_service_signin_chromeos.cc
index 938b3c9fa4ed3321062372e50e3359c89acb4700..d39692ad6d42df3d364f785a25b803e358c0c403 100644
--- a/chrome/browser/signin/easy_unlock_service_signin_chromeos.cc
+++ b/chrome/browser/signin/easy_unlock_service_signin_chromeos.cc
@@ -45,7 +45,7 @@ uint32 GetNextBackoffInterval(uint32 backoff) {
 }
 
 void LoadDataForUser(
-    const std::string& user_id,
+    const AccountId& account_id,
     uint32 backoff_ms,
     const chromeos::EasyUnlockKeyManager::GetDeviceDataListCallback& callback);
 
@@ -56,7 +56,7 @@ void LoadDataForUser(
 // |LoadDataForUser| call with some backoff. If no further retires are allowed,
 // it invokes |callback| with the |LoadDataForUser| results.
 void RetryDataLoadOnError(
-    const std::string& user_id,
+    const AccountId& account_id,
     uint32 backoff_ms,
     const chromeos::EasyUnlockKeyManager::GetDeviceDataListCallback& callback,
     bool success,
@@ -74,13 +74,13 @@ void RetryDataLoadOnError(
 
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&LoadDataForUser, user_id, next_backoff_ms, callback),
+      base::Bind(&LoadDataForUser, account_id, next_backoff_ms, callback),
       base::TimeDelta::FromMilliseconds(next_backoff_ms));
 }
 
 // Loads device data list associated with the user's Easy unlock keys.
 void LoadDataForUser(
-    const std::string& user_id,
+    const AccountId& account_id,
     uint32 backoff_ms,
     const chromeos::EasyUnlockKeyManager::GetDeviceDataListCallback& callback) {
   chromeos::EasyUnlockKeyManager* key_manager =
@@ -88,8 +88,8 @@ void LoadDataForUser(
   DCHECK(key_manager);
 
   key_manager->GetDeviceDataList(
-      chromeos::UserContext(AccountId::FromUserEmail(user_id)),
-      base::Bind(&RetryDataLoadOnError, user_id, backoff_ms, callback));
+      chromeos::UserContext(account_id),
+      base::Bind(&RetryDataLoadOnError, account_id, backoff_ms, callback));
 }
 
 }  // namespace
@@ -102,28 +102,28 @@ EasyUnlockServiceSignin::UserData::~UserData() {}
 
 EasyUnlockServiceSignin::EasyUnlockServiceSignin(Profile* profile)
     : EasyUnlockService(profile),
+      account_id_(EmptyAccountId()),

[achuithb, 2015/12/04 10:12:53]

Doesn't the default ctor do the right thing?

[Alexander Alekseev, 2015/12/04 12:44:07]

No.

       allow_cryptohome_backoff_(true),
       service_active_(false),
       user_pod_last_focused_timestamp_(base::TimeTicks::Now()),
-      weak_ptr_factory_(this) {
-}
+      weak_ptr_factory_(this) {}
 
 EasyUnlockServiceSignin::~EasyUnlockServiceSignin() {
 }
 
-void EasyUnlockServiceSignin::SetCurrentUser(const std::string& user_id) {
-  OnFocusedUserChanged(user_id);
+void EasyUnlockServiceSignin::SetCurrentUser(const AccountId& account_id) {
+  OnFocusedUserChanged(account_id);
 }
 
 void EasyUnlockServiceSignin::WrapChallengeForUserAndDevice(
-    const std::string& user_id,
+    const AccountId& account_id,
     const std::string& device_public_key,
     const std::string& channel_binding_data,
     base::Callback<void(const std::string& wraped_challenge)> callback) {
-  std::map<std::string, UserData*>::const_iterator it =
-      user_data_.find(user_id);
+  std::map<AccountId, UserData*>::const_iterator it =

[achuithb, 2015/12/04 10:12:53]

auto?

[Alexander Alekseev, 2015/12/04 12:44:07]

We know the exact type here, so we can use const_iterator instead of iterator.
So let's leave as it is.

Or, if it looks too long, we can make a shorter typedef for this map.

[achuithb, 2015/12/04 20:16:56]

If you look at the style guide, I think the idea is to use auto for precisely
this case:
https://google.github.io/styleguide/cppguide.html#auto

It is to improve readability when you have a long type name that doesn't add any
value.

[Alexander Alekseev, 2015/12/05 05:20:07]

Done.

+      user_data_.find(account_id);
   if (it == user_data_.end() || it->second->state != USER_DATA_STATE_LOADED) {
-    PA_LOG(ERROR) << "TPM data not loaded for " << user_id;
+    PA_LOG(ERROR) << "TPM data not loaded for " << account_id.Serialize();
     callback.Run(std::string());
     return;
   }
@@ -134,16 +134,18 @@ void EasyUnlockServiceSignin::WrapChallengeForUserAndDevice(
                         &device_public_key_base64);
   for (const auto& device_data : it->second->devices) {
     if (device_data.public_key == device_public_key_base64) {
-      PA_LOG(INFO) << "Wrapping challenge for " << user_id << "...";
+      PA_LOG(INFO) << "Wrapping challenge for " << account_id.Serialize()
+                   << "...";
       challenge_wrapper_.reset(new chromeos::EasyUnlockChallengeWrapper(
-          device_data.challenge, channel_binding_data, user_id,
+          device_data.challenge, channel_binding_data, account_id,
           EasyUnlockTpmKeyManagerFactory::GetInstance()->Get(profile())));
       challenge_wrapper_->WrapChallenge(callback);
       return;
     }
   }
 
-  PA_LOG(ERROR) << "Unable to find device record for " << user_id;
+  PA_LOG(ERROR) << "Unable to find device record for "
+                << account_id.Serialize();
   callback.Run(std::string());
 }
 
@@ -151,8 +153,8 @@ EasyUnlockService::Type EasyUnlockServiceSignin::GetType() const {
   return EasyUnlockService::TYPE_SIGNIN;
 }
 
-std::string EasyUnlockServiceSignin::GetUserEmail() const {
-  return user_id_;
+const AccountId EasyUnlockServiceSignin::GetAccountId() const {
+  return account_id_;
 }
 
 void EasyUnlockServiceSignin::LaunchSetup() {
@@ -221,9 +223,11 @@ std::string EasyUnlockServiceSignin::GetWrappedSecret() const {
 }
 
 void EasyUnlockServiceSignin::RecordEasySignInOutcome(
-    const std::string& user_id,
+    const AccountId& account_id,
     bool success) const {
-  DCHECK_EQ(GetUserEmail(), user_id);
+  DCHECK(GetAccountId() == account_id)
+      << "GetAccountId()=" << GetAccountId().Serialize()
+      << " != account_id=" << account_id.Serialize();
 
   RecordEasyUnlockSigninEvent(
       success ? EASY_UNLOCK_SUCCESS : EASY_UNLOCK_FAILURE);
@@ -235,10 +239,10 @@ void EasyUnlockServiceSignin::RecordEasySignInOutcome(
 }
 
 void EasyUnlockServiceSignin::RecordPasswordLoginEvent(
-    const std::string& user_id) const {
+    const AccountId& account_id) const {
   // This happens during tests, where a user could log in without the user pod
   // being focused.
-  if (GetUserEmail() != user_id)
+  if (GetAccountId() != account_id)
     return;
 
   if (!IsEnabled())
@@ -270,8 +274,8 @@ void EasyUnlockServiceSignin::InitializeInternal() {
   proximity_auth::ScreenlockBridge* screenlock_bridge =
       proximity_auth::ScreenlockBridge::Get();
   screenlock_bridge->AddObserver(this);
-  if (!screenlock_bridge->focused_user_id().empty())
-    OnFocusedUserChanged(screenlock_bridge->focused_user_id());
+  if (screenlock_bridge->focused_account_id().is_valid())
+    OnFocusedUserChanged(screenlock_bridge->focused_account_id());
 }
 
 void EasyUnlockServiceSignin::ShutdownInternal() {
@@ -287,8 +291,7 @@ void EasyUnlockServiceSignin::ShutdownInternal() {
 }
 
 bool EasyUnlockServiceSignin::IsAllowedInternal() const {
-  return service_active_ &&
-         !user_id_.empty() &&
+  return service_active_ && account_id_.is_valid() &&
          !chromeos::LoginState::Get()->IsUserLoggedIn();
 }
 
@@ -328,15 +331,18 @@ void EasyUnlockServiceSignin::OnScreenDidUnlock(
   Shutdown();
 }
 
-void EasyUnlockServiceSignin::OnFocusedUserChanged(const std::string& user_id) {
-  if (user_id_ == user_id)
+void EasyUnlockServiceSignin::OnFocusedUserChanged(
+    const AccountId& account_id) {
+  if (account_id_ == account_id)
     return;
 
-  // Setting or clearing the user_id may changed |IsAllowed| value, so in these
+  // Setting or clearing the account_id may changed |IsAllowed| value, so in
+  // these
   // cases update the app state. Otherwise, it's enough to notify the app the
   // user data has been updated.
-  bool should_update_app_state = user_id_.empty() != user_id.empty();
-  user_id_ = user_id;
+  bool should_update_app_state =

[achuithb, 2015/12/04 10:12:53]

const

[Alexander Alekseev, 2015/12/04 12:44:07]

Done.

+      account_id_.is_valid() != account_id.is_valid();
+  account_id_ = account_id;
   user_pod_last_focused_timestamp_ = base::TimeTicks::Now();
 
   ResetScreenlockState();
@@ -371,39 +377,38 @@ void EasyUnlockServiceSignin::LoadCurrentUserDataIfNeeded() {
   if (!base::SysInfo::IsRunningOnChromeOS())
     return;
 
-  if (user_id_.empty() || !service_active_)
+  if (account_id_.is_valid() || !service_active_)
     return;
 
-  std::map<std::string, UserData*>::iterator it = user_data_.find(user_id_);
+  std::map<AccountId, UserData*>::iterator it = user_data_.find(account_id_);

[achuithb, 2015/12/04 10:12:53]

auto?

[Alexander Alekseev, 2015/12/04 12:44:07]

Let's add const here ;)

[achuithb, 2015/12/04 20:16:56]

I really believe auto is recommended in this case.

[Alexander Alekseev, 2015/12/05 05:20:07]

Done.

   if (it == user_data_.end())
-    user_data_.insert(std::make_pair(user_id_, new UserData()));
+    user_data_.insert(std::make_pair(account_id_, new UserData()));
 
-  UserData* data = user_data_[user_id_];
+  UserData* data = user_data_[account_id_];
 
   if (data->state != USER_DATA_STATE_INITIAL)
     return;
   data->state = USER_DATA_STATE_LOADING;
 
   LoadDataForUser(
-      user_id_,
+      account_id_,
       allow_cryptohome_backoff_ ? 0u : kMaxCryptohomeBackoffIntervalMs,
       base::Bind(&EasyUnlockServiceSignin::OnUserDataLoaded,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 user_id_));
+                 weak_ptr_factory_.GetWeakPtr(), account_id_));
 }
 
 void EasyUnlockServiceSignin::OnUserDataLoaded(
-    const std::string& user_id,
+    const AccountId& account_id,
     bool success,
     const chromeos::EasyUnlockDeviceKeyDataList& devices) {
   allow_cryptohome_backoff_ = false;
 
-  UserData* data = user_data_[user_id];
+  UserData* data = user_data_[account_id];
   data->state = USER_DATA_STATE_LOADED;
   if (success) {
     data->devices = devices;
     chromeos::EasyUnlockKeyManager::DeviceDataListToRemoteDeviceList(
-        user_id, devices, &data->remote_devices_value);
+        account_id, devices, &data->remote_devices_value);
 
     // User could have a NO_HARDLOCK state but has no remote devices if
     // previous user session shuts down before
@@ -413,17 +418,17 @@ void EasyUnlockServiceSignin::OnUserDataLoaded(
     if (devices.empty() &&
         GetPersistedHardlockState(&hardlock_state) &&
         hardlock_state == EasyUnlockScreenlockStateHandler::NO_HARDLOCK) {
-      SetHardlockStateForUser(user_id,
+      SetHardlockStateForUser(account_id,
                               EasyUnlockScreenlockStateHandler::NO_PAIRING);
     }
   }
 
   // If the fetched data belongs to the currently focused user, notify the app
   // that it has to refresh it's user data.
-  if (user_id == user_id_)
+  if (account_id == account_id_)
     NotifyUserUpdated();
 
-  if (user_id != user_id || devices.empty())
+  if (account_id != account_id || devices.empty())
     return;
 
   // TODO(tengs): Currently, ProximityAuthSystem only supports one device. Once
@@ -449,8 +454,9 @@ void EasyUnlockServiceSignin::OnUserDataLoaded(
           : proximity_auth::RemoteDevice::BLUETOOTH_CLASSIC;
 
   proximity_auth::RemoteDevice remote_device(
-      user_id, std::string(), decoded_public_key, bluetooth_type,
-      devices[0].bluetooth_address, decoded_psk, decoded_challenge);
+      account_id.GetUserEmail(), std::string(), decoded_public_key,
+      bluetooth_type, devices[0].bluetooth_address, decoded_psk,
+      decoded_challenge);
   PA_LOG(INFO) << "Loaded Remote Device:\n"
                << "  user id: " << remote_device.user_id << "\n"
                << "  name: " << remote_device.name << "\n"
@@ -461,11 +467,11 @@ void EasyUnlockServiceSignin::OnUserDataLoaded(
 
 const EasyUnlockServiceSignin::UserData*
     EasyUnlockServiceSignin::FindLoadedDataForCurrentUser() const {
-  if (user_id_.empty())
+  if (account_id_.is_valid())
     return NULL;

[achuithb, 2015/12/04 10:12:53]

nullptr

[Alexander Alekseev, 2015/12/04 12:44:07]

Done.

 
-  std::map<std::string, UserData*>::const_iterator it =
-      user_data_.find(user_id_);
+  std::map<AccountId, UserData*>::const_iterator it =

[achuithb, 2015/12/04 10:12:53]

auto?

[Alexander Alekseev, 2015/12/05 05:20:07]

Done.

+      user_data_.find(account_id_);
   if (it == user_data_.end())
     return NULL;

[achuithb, 2015/12/04 10:12:53]

nullptr. Could you please replace all instances in this file?

[Alexander Alekseev, 2015/12/04 12:44:07]

Done.

   if (it->second->state != USER_DATA_STATE_LOADED)