Add public accounts to UserManager

chrome/browser/chromeos/login/user_manager_impl.cc

 // Copyright (c) 2012 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/chromeos/login/user_manager_impl.h"
 
+#include <cstddef>
+
 #include "ash/shell.h"
 #include "base/bind.h"
 #include "base/chromeos/chromeos_version.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/rand_util.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/cros/cert_library.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/input_method/input_method_manager.h"
 #include "chrome/browser/chromeos/login/login_display.h"
 #include "chrome/browser/chromeos/login/remove_user_delegate.h"
 #include "chrome/browser/chromeos/login/user_image_manager_impl.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
-#include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/cryptohome/async_method_caller.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 
 using content::BrowserThread;
 
 namespace chromeos {
 
 namespace {
 
-// A vector pref of the users who have logged into the device.
-const char kLoggedInUsers[] = "LoggedInUsers";
+// A vector pref of the regular users who have logged into this device.
+const char kRegularUsers[] = "LoggedInUsers";
+
+// A vector pref of the device-local users defined on this device.
+const char kLocalUsers[] = "LocalUsers";
+
+// A string pref that gets set when a device-local user is to be removed but is
+// currently logged in, requiring the user's data to be removed after logout.
+const char kUserPendingDataRemoval[] = "UserPendingDataRemoval";
 
 // A dictionary that maps usernames to the displayed name.
 const char kUserDisplayName[] = "UserDisplayName";
 
 // A dictionary that maps usernames to the displayed (non-canonical) emails.
 const char kUserDisplayEmail[] = "UserDisplayEmail";
 
 // A dictionary that maps usernames to OAuth token presence flag.
 const char kUserOAuthTokenStatus[] = "OAuthTokenStatus";
 
@@ skipping 35 matching lines @@
       user_email, base::Bind(&OnRemoveUserComplete, user_email));
 
   if (delegate)
     delegate->OnUserRemoved(user_email);
 }
 
 }  // namespace
 
 // static
 void UserManager::RegisterPrefs(PrefService* local_state) {
-  local_state->RegisterListPref(kLoggedInUsers, PrefService::UNSYNCABLE_PREF);
+  local_state->RegisterListPref(kRegularUsers, PrefService::UNSYNCABLE_PREF);
+  local_state->RegisterListPref(kLocalUsers, PrefService::UNSYNCABLE_PREF);
+  local_state->RegisterStringPref(kUserPendingDataRemoval, "",
+                                  PrefService::UNSYNCABLE_PREF);
   local_state->RegisterDictionaryPref(kUserOAuthTokenStatus,
                                       PrefService::UNSYNCABLE_PREF);
   local_state->RegisterDictionaryPref(kUserDisplayName,
                                       PrefService::UNSYNCABLE_PREF);
   local_state->RegisterDictionaryPref(kUserDisplayEmail,
                                       PrefService::UNSYNCABLE_PREF);
 }
 
 UserManagerImpl::UserManagerImpl()
-    : logged_in_user_(NULL),
+    : cros_settings_(CrosSettings::Get()),
+      observing_cros_settings_(false),
+      users_loaded_(false),
+      logged_in_user_(NULL),
       session_started_(false),
       is_current_user_owner_(false),
       is_current_user_new_(false),
       is_current_user_ephemeral_(false),
       ephemeral_users_enabled_(false),
       observed_sync_service_(NULL),
-      user_image_manager_(new UserImageManagerImpl) {
+      user_image_manager_(new UserImageManagerImpl),
+      weak_ptr_factory_(this) {
   // UserManager instance should be used only on UI thread.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   registrar_.Add(this, chrome::NOTIFICATION_OWNERSHIP_STATUS_CHANGED,
       content::NotificationService::AllSources());
   registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
       content::NotificationService::AllSources());
   RetrieveTrustedDevicePolicies();
 }
 
 UserManagerImpl::~UserManagerImpl() {
@@ skipping 29 matching lines @@
     return;
   }
 
   if (IsEphemeralUser(email)) {
     EphemeralUserLoggedIn(email);
     return;
   }
 
   EnsureUsersLoaded();
 
-  // Clear the prefs view of the users.
-  PrefService* prefs = g_browser_process->local_state();
-  ListPrefUpdate prefs_users_update(prefs, kLoggedInUsers);
-  prefs_users_update->Clear();
-
-  // Make sure this user is first.
-  prefs_users_update->Append(new base::StringValue(email));
-  UserList::iterator logged_in_user = users_.end();
-  for (UserList::iterator it = users_.begin(); it != users_.end(); ++it) {
-    std::string user_email = (*it)->email();
-    // Skip the most recent user.
-    if (email != user_email)
-      prefs_users_update->Append(new base::StringValue(user_email));
-    else
-      logged_in_user = it;
-  }
-
-  if (logged_in_user == users_.end()) {
-    is_current_user_new_ = true;
-    logged_in_user_ = User::CreateRegularUser(email);
-    logged_in_user_->set_oauth_token_status(LoadUserOAuthStatus(email));
-  } else {
-    logged_in_user_ = *logged_in_user;
-    users_.erase(logged_in_user);
-  }
-  // This user must be in the front of the user list.
-  users_.insert(users_.begin(), logged_in_user_);
+  // Move the user to the front of the user list (adding the user if necessary).
+  logged_in_user_ = UpdateRegularUsers(email, true);
 
   if (is_current_user_new_) {
     SaveUserDisplayName(logged_in_user_->email(),
                         UTF8ToUTF16(logged_in_user_->GetAccountName(true)));
     WallpaperManager::Get()->SetInitialUserWallpaper(email, true);
   }
 
   user_image_manager_->UserLoggedIn(email, is_current_user_new_);
 
   if (!browser_restart) {
     // For GAIA login flow, logged in user wallpaper may not be loaded.
     WallpaperManager::Get()->EnsureLoggedInUserWallpaperLoaded();
   }
 
-  // Make sure we persist new user data to Local State.
-  prefs->CommitPendingWrite();
+  // Make sure the modified data is persisted to Local State.
+  g_browser_process->local_state()->CommitPendingWrite();
 
   NotifyOnLogin();
 }
 
 void UserManagerImpl::RetailModeUserLoggedIn() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   is_current_user_new_ = true;
   is_current_user_ephemeral_ = true;
   logged_in_user_ = User::CreateRetailModeUser();
   user_image_manager_->UserLoggedIn(kRetailModeUserEMail,
@@ skipping 22 matching lines @@
 }
 
 void UserManagerImpl::SessionStarted() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   session_started_ = true;
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_SESSION_STARTED,
       content::NotificationService::AllSources(),
       content::NotificationService::NoDetails());
   if (is_current_user_new_) {
-    // Make sure we persist new user data to Local State.
+    // Make sure the modified data is persisted to Local State.

[pastarmovj, 2012/11/28 09:57:16]

I am not convinced we need all those CommitPendingWrites all over this file. The
local state is persisted automatically often enough and those writes can only
slow down login for new users. I guess they have been added in an attempt to
prevent crashes in the browser from wiping new users but I think that instead of
doing that we should be more diligent in not allowing crashes in the first
place.

[bartfab (slow), 2012/11/28 12:30:17]

I removed the CommitPendingWrite() calls. I agree that relying on prefs doing
the right thing is much more elegant.

     g_browser_process->local_state()->CommitPendingWrite();
   }
 }
 
 void UserManagerImpl::RemoveUser(const std::string& email,
                                  RemoveUserDelegate* delegate) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (!IsKnownUser(email))
+  const User* user = FindUser(email);
+  if (!user || user->GetType() != User::USER_TYPE_REGULAR)
     return;
 
   // Sanity check: we must not remove single user. This check may seem
   // redundant at a first sight because this single user must be an owner and
   // we perform special check later in order not to remove an owner.  However
   // due to non-instant nature of ownership assignment this later check may
   // sometimes fail. See http://crosbug.com/12723
   if (users_.size() < 2)
     return;
 
   // Sanity check: do not allow the logged-in user to remove himself.
   if (logged_in_user_ && logged_in_user_->email() == email)
     return;
 
   RemoveUserInternal(email, delegate);
 }
 
 void UserManagerImpl::RemoveUserFromList(const std::string& email) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   EnsureUsersLoaded();
-  RemoveUserFromListInternal(email);
+  RemoveNonCryptohomeData(email);
+  UpdateRegularUsers(email, false);

[pastarmovj, 2012/11/28 09:57:16]

I have a proposal about the second parameter of this function. Why not make it
an enum so that you can have more descriptive names for the operations. It seems
quite strange to me that depending on this param being true or false the
behavior changes so vastly.

[bartfab (slow), 2012/11/28 12:30:17]

I refactored the code. There is now a |RemoveUserFromListInternal| method that
does what it says on the tin only. If the user needs to be added at the front of
the list, that is done explicitly outside the method.

+  // Make sure the modified data is persisted to Local State.
+  g_browser_process->local_state()->CommitPendingWrite();
 }
 
 bool UserManagerImpl::IsKnownUser(const std::string& email) const {
   return FindUser(email) != NULL;
 }
 
 const User* UserManagerImpl::FindUser(const std::string& email) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (logged_in_user_ && logged_in_user_->email() == email)
     return logged_in_user_;
@@ skipping 125 matching lines @@
         if (!profile->IsOffTheRecord() &&
             profile == ProfileManager::GetDefaultProfile()) {
           DCHECK(NULL == observed_sync_service_);
           observed_sync_service_ =
               ProfileSyncServiceFactory::GetForProfile(profile);
           if (observed_sync_service_)
             observed_sync_service_->AddObserver(this);
         }
       }
       break;
+    case chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED:
+      DCHECK_EQ(*content::Details<const std::string>(details).ptr(),
+                kAccountsPrefDeviceLocalAccounts);
+      RetrieveTrustedDevicePolicies();
+      break;
     default:
       NOTREACHED();
   }
 }
 
 void UserManagerImpl::OnStateChanged() {
-  DCHECK(IsUserLoggedIn() && !IsLoggedInAsGuest());
+  DCHECK(IsLoggedInAsRegularUser());
   GoogleServiceAuthError::State state =
       observed_sync_service_->GetAuthError().state();
   if (state != GoogleServiceAuthError::NONE &&
       state != GoogleServiceAuthError::CONNECTION_FAILED &&
       state != GoogleServiceAuthError::SERVICE_UNAVAILABLE &&
       state != GoogleServiceAuthError::REQUEST_CANCELED) {
     // Invalidate OAuth token to force Gaia sign-in flow. This is needed
     // because sign-out/sign-in solution is suggested to the user.
     // TODO(altimofeev): this code isn't needed after crosbug.com/25978 is
     // implemented.
@@ skipping 28 matching lines @@
 bool UserManagerImpl::CanCurrentUserLock() const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   return IsUserLoggedIn() && logged_in_user_->can_lock();
 }
 
 bool UserManagerImpl::IsUserLoggedIn() const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   return logged_in_user_;
 }
 
+bool UserManagerImpl::IsLoggedInAsRegularUser() const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  return IsUserLoggedIn() &&
+         logged_in_user_->GetType() == User::USER_TYPE_REGULAR;
+}
+
 bool UserManagerImpl::IsLoggedInAsDemoUser() const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   return IsUserLoggedIn() &&
          logged_in_user_->GetType() == User::USER_TYPE_RETAIL_MODE;
 }
 
 bool UserManagerImpl::IsLoggedInAsPublicAccount() const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   return IsUserLoggedIn() &&
       logged_in_user_->GetType() == User::USER_TYPE_PUBLIC_ACCOUNT;
@@ skipping 47 matching lines @@
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   observer_list_.RemoveObserver(obs);
 }
 
 void UserManagerImpl::NotifyLocalStateChanged() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   FOR_EACH_OBSERVER(UserManager::Observer, observer_list_,
                     LocalStateChanged(this));
 }
 
+bool UserManagerImpl::ParseUserList(
+    std::vector<std::string>* users_vector,
+    std::set<std::string>* users_set,
+    const ListValue& users_list,
+    const std::set<std::string>& existing_users,
+    const std::string& logged_in_user) const {
+  users_vector->clear();
+  users_set->clear();
+  bool logged_in_user_on_list = false;
+  for (size_t i = 0; i < users_list.GetSize(); ++i) {
+    std::string email;
+    if (!users_list.GetString(i, &email) || email.empty()) {
+      LOG(ERROR) << "Corrupt entry in user list.";

[pastarmovj, 2012/11/28 09:57:16]

Please print the index here too. Could be helpful for some debugging.

[bartfab (slow), 2012/11/28 12:30:17]

Done.

+      continue;
+    }
+    if ((existing_users.find(email) != existing_users.end()) ||

[pastarmovj, 2012/11/28 09:57:16]

I think you don't need the extra parenthesis around a != b.

[bartfab (slow), 2012/11/28 12:30:17]

Correct. I added them for readability only. Removed now.

+        !users_set->insert(email).second) {
+      LOG(ERROR) << "Duplicate user: " << email;
+      continue;
+    }
+    if (email == logged_in_user) {
+      logged_in_user_on_list = true;
+      continue;
+    }
+    users_vector->push_back(email);
+  }
+  users_set->erase(logged_in_user);
+  return logged_in_user_on_list;
+}
+
 void UserManagerImpl::EnsureUsersLoaded() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!users_.empty())
+  if (users_loaded_)
     return;
   if (!g_browser_process)

[pastarmovj, 2012/11/28 09:57:16]

Swap the two ifs and split them with an empty line please. The one is a
precondition and the other is part of the functionality of this proc.

[bartfab (slow), 2012/11/28 12:30:17]

Done.

     return;
+  users_loaded_ = true;
 
   PrefService* local_state = g_browser_process->local_state();
-  const ListValue* prefs_users =
-      local_state->GetList(kLoggedInUsers);
+  const ListValue* prefs_regular_users = local_state->GetList(kRegularUsers);
+  const ListValue* prefs_local_users = local_state->GetList(kLocalUsers);
   const DictionaryValue* prefs_display_names =
       local_state->GetDictionary(kUserDisplayName);
   const DictionaryValue* prefs_display_emails =
       local_state->GetDictionary(kUserDisplayEmail);
 
-  if (!prefs_users)
-    return;
+  // Load regular users.
+  std::vector<std::string> regular_users;
+  std::set<std::string> regular_users_set;
+  ParseUserList(&regular_users, &regular_users_set, *prefs_regular_users,
+                std::set<std::string>(), "");
+  for (std::vector<std::string>::const_iterator it = regular_users.begin();
+       it != regular_users.end(); ++it) {
+    User* user = User::CreateRegularUser(*it);
+    user->set_oauth_token_status(LoadUserOAuthStatus(*it));
+    users_.push_back(user);
 
-  for (ListValue::const_iterator it = prefs_users->begin();
-       it != prefs_users->end(); ++it) {
-    std::string email;
-    if ((*it)->GetAsString(&email)) {
-      User* user = User::CreateRegularUser(email);
-      user->set_oauth_token_status(LoadUserOAuthStatus(email));
-      users_.push_back(user);
+    string16 display_name;
+    if (prefs_display_names->GetStringWithoutPathExpansion(*it,
+                                                           &display_name)) {
+      user->set_display_name(display_name);
+    }
 
-      string16 display_name;
-      if (prefs_display_names &&
-          prefs_display_names->GetStringWithoutPathExpansion(
-              email, &display_name)) {
-        user->set_display_name(display_name);
-      }
+    std::string display_email;
+    if (prefs_display_emails->GetStringWithoutPathExpansion(*it,
+                                                            &display_email)) {
+      user->set_display_email(display_email);
+    }
+  }
 
-      std::string display_email;
-      if (prefs_display_emails &&
-          prefs_display_emails->GetStringWithoutPathExpansion(
-              email, &display_email)) {
-        user->set_display_email(display_email);
-      }
-    }
+  // Load device-local users.
+  std::vector<std::string> local_users;
+  std::set<std::string> local_users_set;
+  ParseUserList(&local_users, &local_users_set, *prefs_local_users,
+                regular_users_set, "");
+  for (std::vector<std::string>::const_iterator it = local_users.begin();
+       it != local_users.end(); ++it) {
+    // Currently, all device-local users are public account users. This may
+    // change in the future.
+    users_.push_back(User::CreatePublicAccountUser(*it));
   }
 
   user_image_manager_->LoadUserImages(users_);
 }
 
 void UserManagerImpl::RetrieveTrustedDevicePolicies() {
   ephemeral_users_enabled_ = false;
   owner_email_ = "";
 
-  CrosSettings* cros_settings = CrosSettings::Get();
   // Schedule a callback if device policy has not yet been verified.
-  if (CrosSettingsProvider::TRUSTED != cros_settings->PrepareTrustedValues(
+  if (CrosSettingsProvider::TRUSTED != cros_settings_->PrepareTrustedValues(
       base::Bind(&UserManagerImpl::RetrieveTrustedDevicePolicies,
                  base::Unretained(this)))) {
     return;
   }
 
-  cros_settings->GetBoolean(kAccountsPrefEphemeralUsersEnabled,
-                            &ephemeral_users_enabled_);
-  cros_settings->GetString(kDeviceOwner, &owner_email_);
+  cros_settings_->GetBoolean(kAccountsPrefEphemeralUsersEnabled,
+                             &ephemeral_users_enabled_);
+  cros_settings_->GetString(kDeviceOwner, &owner_email_);
+  const base::ListValue* local_users;
+  cros_settings_->GetList(kAccountsPrefDeviceLocalAccounts, &local_users);
+
+  bool changed = UpdateLocalUsers(*local_users);
 
   // If ephemeral users are enabled and we are on the login screen, take this
   // opportunity to clean up by removing all users except the owner.
   if (ephemeral_users_enabled_  && !IsUserLoggedIn()) {
     scoped_ptr<base::ListValue> users(
-        g_browser_process->local_state()->GetList(kLoggedInUsers)->DeepCopy());
+        g_browser_process->local_state()->GetList(kRegularUsers)->DeepCopy());
 
-    bool changed = false;
     for (base::ListValue::const_iterator user = users->begin();
         user != users->end(); ++user) {
       std::string user_email;
       (*user)->GetAsString(&user_email);
       if (user_email != owner_email_) {
-        RemoveUserFromListInternal(user_email);
+        RemoveNonCryptohomeData(user_email);
+        UpdateRegularUsers(user_email, false);
         changed = true;
       }
     }
+  }
 
-    if (changed) {
-      content::NotificationService::current()->Notify(
-          chrome::NOTIFICATION_POLICY_USER_LIST_CHANGED,
-          content::Source<UserManager>(this),
-          content::NotificationService::NoDetails());
-    }
+  // Make sure the modified data is persisted to Local State.
+  g_browser_process->local_state()->CommitPendingWrite();
+
+  if (changed) {
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_POLICY_USER_LIST_CHANGED,
+        content::Source<UserManager>(this),
+        content::NotificationService::NoDetails());
+  }
+
+  if (!observing_cros_settings_) {
+    cros_settings_->AddSettingsObserver(kAccountsPrefDeviceLocalAccounts,

[pastarmovj, 2012/11/28 09:57:16]

Is there any reason you don't simply do this in the constructor and spare the
bool flag?

[bartfab (slow), 2012/11/28 12:30:17]

The RetrieveTrustedDevicePolicies() will always run when policy first becomes
available. If I registered as an observer in the constructor, I would also
receive a notification that the list of device-local accounts has changed at
that point, running RetrieveTrustedDevicePolicies() once more for no good
reason. By registering only after RetrieveTrustedDevicePolicies() has
successfully run once, I never receive the first notification.

I switched away from WeakPtr and as such, no longer need the bool. I simplified
the code accordingly.

+                                        weak_ptr_factory_.GetWeakPtr());
+    observing_cros_settings_ = true;
   }
 }
 
 bool UserManagerImpl::AreEphemeralUsersEnabled() const {
   return ephemeral_users_enabled_ &&
       (g_browser_process->browser_policy_connector()->IsEnterpriseManaged() ||
       !owner_email_.empty());
 }
 
 const User* UserManagerImpl::FindUserInList(const std::string& email) const {
@@ skipping 26 matching lines @@
 
   SetCurrentUserIsOwner(is_owner);
 }
 
 void UserManagerImpl::CheckOwnership() {
   DeviceSettingsService::Get()->GetOwnershipStatusAsync(
       base::Bind(&UserManagerImpl::UpdateOwnership,
                  base::Unretained(this)));
 }
 
-void UserManagerImpl::RemoveUserFromListInternal(const std::string& email) {
-  // Clear the prefs view of the users.
+void UserManagerImpl::RemoveNonCryptohomeData(const std::string& email) {
+  WallpaperManager::Get()->RemoveUserWallpaperInfo(email);
+  user_image_manager_->DeleteUserImage(email);
+
   PrefService* prefs = g_browser_process->local_state();
-  ListPrefUpdate prefs_users_update(prefs, kLoggedInUsers);
-  prefs_users_update->Clear();
-
-  UserList::iterator user_to_remove = users_.end();
-  for (UserList::iterator it = users_.begin(); it != users_.end(); ++it) {
-    std::string user_email = (*it)->email();
-    // Skip user that we would like to delete.
-    if (email != user_email)
-      prefs_users_update->Append(new base::StringValue(user_email));
-    else
-      user_to_remove = it;
-  }
-
-  WallpaperManager::Get()->RemoveUserWallpaperInfo(email);
-
   DictionaryPrefUpdate prefs_oauth_update(prefs, kUserOAuthTokenStatus);
   int oauth_status;
   prefs_oauth_update->GetIntegerWithoutPathExpansion(email, &oauth_status);
   prefs_oauth_update->RemoveWithoutPathExpansion(email, NULL);
 
   DictionaryPrefUpdate prefs_display_name_update(prefs, kUserDisplayName);
   prefs_display_name_update->RemoveWithoutPathExpansion(email, NULL);
 
   DictionaryPrefUpdate prefs_display_email_update(prefs, kUserDisplayEmail);
   prefs_display_email_update->RemoveWithoutPathExpansion(email, NULL);
+}
 
-  if (user_to_remove != users_.end()) {
-    delete *user_to_remove;
-    users_.erase(user_to_remove);
+User *UserManagerImpl::UpdateRegularUsers(const std::string& email,
+                                          bool move_to_front) {
+  ListPrefUpdate prefs_users_update(g_browser_process->local_state(),
+                                    kRegularUsers);
+  prefs_users_update->Clear();
+  UserList::iterator user_it = users_.end();
+  for (UserList::iterator it = users_.begin(); it != users_.end(); ++it) {
+    const std::string user_email = (*it)->email();
+    if (user_email == email)
+      user_it = it;
+    else if ((*it)->GetType() == User::USER_TYPE_REGULAR)
+      prefs_users_update->Append(new base::StringValue(user_email));
   }
+  User* user = NULL;
+  if (user_it != users_.end()) {
+    user = *user_it;
+    users_.erase(user_it);
+  }
+
+  if (move_to_front) {
+    prefs_users_update->Insert(0, new base::StringValue(email));
+    if (!user) {

[pastarmovj, 2012/11/28 09:57:16]

Please also document that this function will actually create a new user if it
doesn't exist.

[bartfab (slow), 2012/11/28 12:30:17]

I refactored the code so that this function no longer creates users.

+      is_current_user_new_ = true;
+      user = User::CreateRegularUser(email);
+      user->set_oauth_token_status(LoadUserOAuthStatus(email));
+    }
+    users_.insert(users_.begin(), user);
+  } else {
+    delete user;
+    user = NULL;
+  }
+
+  return user;
+}
+
+bool UserManagerImpl::UpdateLocalUsers(
+    const base::ListValue& local_users_list) {
+  PrefService* local_state = g_browser_process->local_state();
+
+  EnsureUsersLoaded();
+
+  // Determine the currently logged-in user's email.
+  std::string logged_in_user;
+  if (IsUserLoggedIn())
+    logged_in_user = GetLoggedInUser()->email();
+
+  // If there is a user whose data is pending removal and that user is not
+  // currently logged in, take this opportunity to remove the data.
+  std::string user_pending_data_removal =
+      local_state->GetString(kUserPendingDataRemoval);
+  if (!user_pending_data_removal.empty() &&
+      user_pending_data_removal != logged_in_user) {
+    RemoveNonCryptohomeData(user_pending_data_removal);
+    local_state->ClearPref(kUserPendingDataRemoval);
+  }
+
+  // Split the current user list into device-local users and regular users.
+  std::vector<std::string> old_local_users;
+  std::set<std::string> regular_users;
+  for (UserList::const_iterator it = users_.begin(); it != users_.end(); ++it) {
+    if ((*it)->is_device_local_account())
+      old_local_users.push_back((*it)->email());
+    else
+      regular_users.insert((*it)->email());
+  }
+
+  // Get the new list of device-local users from policy.
+  std::vector<std::string> new_local_users;
+  std::set<std::string> new_local_users_set;
+  std::string pending_remove;
+  if (!ParseUserList(&new_local_users, &new_local_users_set, local_users_list,
+                     regular_users, logged_in_user) && IsUserLoggedIn()) {
+    User* user = GetLoggedInUser();
+    // If the currently logged-in user is a device-local user not found on the
+    // new list, mark that user's data as pending removal after logout.
+    if (user->is_device_local_account() && !user->is_builtin_account())
+      local_state->SetString(kUserPendingDataRemoval, logged_in_user);
+  }
+
+  // Persist the new list of device-local users in a pref.
+  ListPrefUpdate prefs_local_users_update(local_state, kLocalUsers);
+  scoped_ptr<base::ListValue> prefs_local_users(local_users_list.DeepCopy());
+  prefs_local_users_update->Swap(prefs_local_users.get());
+
+  // If the list of device-local users has not changed, return.
+  if (new_local_users.size() == old_local_users.size()) {
+    bool changed = false;
+    for (size_t i = 0; i < new_local_users.size(); ++i) {
+      if (new_local_users[i] != old_local_users[i]) {
+        changed = true;
+        break;
+      }
+    }
+    if (!changed)
+      return false;
+  }
+
+  // Remove the old device-local users from the user list.
+  for (UserList::iterator it = users_.begin(); it != users_.end(); ) {
+    if ((*it)->is_device_local_account()) {
+      delete *it;
+      it = users_.erase(it);
+    } else {
+      ++it;
+    }
+  }
+
+  // Add the new device-local users to the user list.
+  for (std::vector<std::string>::const_iterator it = new_local_users.begin();
+       it != new_local_users.end(); ++it) {
+    // Currently, all device-local users are public account users. This may
+    // change in the future.
+    users_.push_back(User::CreatePublicAccountUser(*it));
+  }
+
+  user_image_manager_->LoadUserImages(
+      UserList(users_.end() - new_local_users.size(), users_.end()));
+
+  return true;
 }
 
 }  // namespace chromeos