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

chrome/browser/signin/easy_unlock_service_signin_chromeos.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/signin/easy_unlock_service_signin_chromeos.h"
 
 #include "base/base64url.h"
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 27 matching lines @@
 
 // Calculates the backoff interval that should be used next.
 // |backoff| The last backoff interval used.
 uint32 GetNextBackoffInterval(uint32 backoff) {
   if (backoff == 0u)
     return kInitialCryptohomeBackoffIntervalMs;
   return backoff * 2;
 }
 
 void LoadDataForUser(
-    const std::string& user_id,
+    const AccountId& account_id,
     uint32 backoff_ms,
     const chromeos::EasyUnlockKeyManager::GetDeviceDataListCallback& callback);
 
 // Callback passed to |LoadDataForUser()|.
 // If |LoadDataForUser| function succeeded, it invokes |callback| with the
 // results.
 // If |LoadDataForUser| failed and further retries are allowed, schedules new
 // |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,
     const chromeos::EasyUnlockDeviceKeyDataList& data_list) {
   if (success) {
     callback.Run(success, data_list);
     return;
   }
 
   uint32 next_backoff_ms = GetNextBackoffInterval(backoff_ms);
   if (next_backoff_ms > kMaxCryptohomeBackoffIntervalMs) {
     callback.Run(false, data_list);
     return;
   }
 
   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 =
       chromeos::UserSessionManager::GetInstance()->GetEasyUnlockKeyManager();
   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
 
 EasyUnlockServiceSignin::UserData::UserData()
     : state(EasyUnlockServiceSignin::USER_DATA_STATE_INITIAL) {
 }
 
 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;
   }
 
   std::string device_public_key_base64;
   base::Base64UrlEncode(device_public_key,
                         base::Base64UrlEncodePolicy::INCLUDE_PADDING,
                         &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());
 }
 
 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() {
   NOTREACHED();
 }
 
 const base::DictionaryValue* EasyUnlockServiceSignin::GetPermitAccess() const {
   return NULL;
 }
 
@@ skipping 48 matching lines @@
 std::string EasyUnlockServiceSignin::GetWrappedSecret() const {
   const UserData* data = FindLoadedDataForCurrentUser();
   // TODO(xiyuan): Use correct remote device instead of hard coded first one.
   uint32 device_index = 0;
   if (!data || data->devices.size() <= device_index)
     return std::string();
   return data->devices[device_index].wrapped_secret;
 }
 
 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);
   if (success) {
     RecordEasyUnlockSigninDuration(
         base::TimeTicks::Now() - user_pod_last_focused_timestamp_);
   }
   DVLOG(1) << "Easy sign-in " << (success ? "success" : "failure");
 }
 
 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())
     return;
 
   EasyUnlockAuthEvent event = GetPasswordAuthEvent();
   RecordEasyUnlockSigninEvent(event);
   DVLOG(1) << "Easy Sign-in password login event, event=" << event;
 }
 
@@ skipping 11 matching lines @@
 void EasyUnlockServiceSignin::InitializeInternal() {
   if (chromeos::LoginState::Get()->IsUserLoggedIn())
     return;
 
   service_active_ = true;
 
   chromeos::LoginState::Get()->AddObserver(this);
   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() {
   if (!service_active_)
     return;
   service_active_ = false;
 
   weak_ptr_factory_.InvalidateWeakPtrs();
   proximity_auth::ScreenlockBridge::Get()->RemoveObserver(this);
   chromeos::LoginState::Get()->RemoveObserver(this);
   STLDeleteContainerPairSecondPointers(user_data_.begin(), user_data_.end());
   user_data_.clear();
 }
 
 bool EasyUnlockServiceSignin::IsAllowedInternal() const {
-  return service_active_ &&
-         !user_id_.empty() &&
+  return service_active_ && account_id_.is_valid() &&
          !chromeos::LoginState::Get()->IsUserLoggedIn();
 }
 
 void EasyUnlockServiceSignin::OnWillFinalizeUnlock(bool success) {
   // This code path should only be exercised for the lock screen, not for the
   // sign-in screen.
   NOTREACHED();
 }
 
 void EasyUnlockServiceSignin::OnSuspendDoneInternal() {
@@ skipping 19 matching lines @@
   // in tests, the screen type might be different.
   if (screen_type !=
           proximity_auth::ScreenlockBridge::LockHandler::SIGNIN_SCREEN)
     return;
 
   DisableAppWithoutResettingScreenlockState();
 
   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();
   ShowInitialUserState();
 
   if (should_update_app_state) {
     UpdateAppState();
   } else {
     NotifyUserUpdated();
   }
@@ skipping 14 matching lines @@
   if (!chromeos::LoginState::Get()->IsUserLoggedIn())
     return;
   DisableAppWithoutResettingScreenlockState();
 }
 
 void EasyUnlockServiceSignin::LoadCurrentUserDataIfNeeded() {
   // TODO(xiyuan): Revisit this when adding tests.
   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
     // CheckCryptohomeKeysAndMaybeHardlock finishes. Set NO_PAIRING state
     // and update UI to remove the confusing spinner in this case.
     EasyUnlockScreenlockStateHandler::HardlockState hardlock_state;
     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
   // multiple devices are supported, we need to load all devices.
   std::string decoded_public_key, decoded_psk, decoded_challenge;
   if (!base::Base64UrlDecode(devices[0].public_key,
                              base::Base64UrlDecodePolicy::REQUIRE_PADDING,
                              &decoded_public_key) ||
       !base::Base64UrlDecode(devices[0].psk,
                              base::Base64UrlDecodePolicy::REQUIRE_PADDING,
                              &decoded_psk) ||
       !base::Base64UrlDecode(devices[0].challenge,
                              base::Base64UrlDecodePolicy::REQUIRE_PADDING,
                              &decoded_challenge)) {
     PA_LOG(ERROR) << "Unable to base64url decode the input data.";
     return;
   }
 
   proximity_auth::RemoteDevice::BluetoothType bluetooth_type =
       devices[0].bluetooth_type ==
               chromeos::EasyUnlockDeviceKeyData::BLUETOOTH_LE
           ? proximity_auth::RemoteDevice::BLUETOOTH_LE
           : 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"
                << "  public key" << devices[0].public_key << "\n"
                << "  bt_addr:" << remote_device.bluetooth_address;
   OnRemoteDeviceChanged(&remote_device);
 }
 
 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)
     return NULL;
   return it->second;
 }