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";

[Ivan Korotkov, 2012/11/28 21:40:53]

I'm very concerned about splitting users into several lists.
Not sure whether we'll end up showing local users always first or mixed with
regular, but we still should keep an LRU list of *all* users to prevent future
migrations forth and back depending on the UI decisions.

So I suggest either:
(*) keeping *all* users in LoggedInUsers and only local users in LocalUsers or
(*) keeping 3 lists (LoggedInUsers, GaiaUsers, LocalUsers or  smth like that).

[Nikita (slow), 2012/11/29 12:00:50]

I agree with Ivan. We should have single LoggedInUsers that has users in LRU.

Do you intend to place public accounts in some specific way i.e. always first
one / always last one?

[Ivan Korotkov, 2012/11/29 12:06:31]

As discussed over chat, this CL actually adds support for public accounts, which
have fixed display order and don't need to be put into LRU list.

So it's OK to have kPublicAccounts (fixed order) and a mixed kSomethingUsers
(LRU order) with all other user types (gaia, local with cryptohome).

[bartfab (slow), 2012/11/29 14:18:09]

I renamed |LocalUsers| to |PublicAccounts|.
I left |RegularUsers| as-is for now. Once local accounts are added, we can
rethink the name - I cannot think of anything better that signifies "GAIA plus
local, persistent" right now.

+
+// 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()),
+      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) {
   // 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() {
   // Can't use STLDeleteElements because of the private destructor of User.
   for (size_t i = 0; i < users_.size(); ++i)
     delete users_[i];
   users_.clear();
   if (is_current_user_ephemeral_)
     delete logged_in_user_;
 }
 
+void UserManagerImpl::Shutdown() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  cros_settings_->RemoveSettingsObserver(kAccountsPrefDeviceLocalAccounts,
+                                         this);
+}
+
 UserImageManager* UserManagerImpl::GetUserImageManager() {
   return user_image_manager_.get();
 }
 
 const UserList& UserManagerImpl::GetUsers() const {
-  const_cast<UserManagerImpl*>(this)->EnsureUsersLoaded();
+  EnsureUsersLoaded();
   return users_;
 }
 
 void UserManagerImpl::UserLoggedIn(const std::string& email,
                                    bool browser_restart) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!IsUserLoggedIn());
 
   if (email == kGuestUserEMail) {
     GuestUserLoggedIn();
     return;
   }
 
   if (email == kRetailModeUserEMail) {
     RetailModeUserLoggedIn();
     return;
   }
 
   if (IsEphemeralUser(email)) {

[Nikita (slow), 2012/11/29 12:00:50]

Public accounts will be handled here, right?

If that's the case I could not figure out how is_current_user_ephemeral_ is set
for them because IsEphemeralUser relied on that value.

[bartfab (slow), 2012/11/29 14:18:09]

Public accounts will be treated separately because e.g. their user policy works
slightly differently. I will handle the login part in a separate CL. Right now,
clicking on a public account pod does not let you log in at all.

     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();
+  // Remove the user from the user list.
+  logged_in_user_ = RemoveUserFromListInternal(email);
 
-  // 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;
-  }
+  // Add the user to the front of the persistent user list.
+  ListPrefUpdate prefs_users_update(g_browser_process->local_state(),
+                                    kRegularUsers);
+  prefs_users_update->Insert(0, new base::StringValue(email));
 
-  if (logged_in_user == users_.end()) {
+  // If the user was not found on the user list, create a new user.
+  if (!logged_in_user_) {
     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_);
-
-  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();
   }
 

[Ivan Korotkov, 2012/11/28 21:40:53]

Why are you removing this bit?

[Nikita (slow), 2012/11/29 12:00:50]

To give more context: this call ensures that if sign out is done exactly after
new user sign in we would not lost that user from login screen.

[bartfab (slow), 2012/11/29 14:18:09]

I removed this at Julian's request. The local state is always flushed to disk on
browser shutdown, even if the user logs out straight after logging in (see
ShutdownPreThreadsStop() in browser_shutdown.cc).

[Ivan Korotkov, 2012/11/29 17:25:33]

Right, but if browser crashes in the first few seconds after session start, user
will be not saved. That's the reason this line was added in the first place.

[bartfab (slow), 2012/11/29 18:56:17]

Done.

-  // Make sure we persist new user data to Local State.
-  prefs->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,
                                     /* user_is_new= */ true);
@@ skipping 20 matching lines @@
   NotifyOnLogin();
 }
 
 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.

[Ivan Korotkov, 2012/11/28 21:40:53]

And this one.

[Nikita (slow), 2012/11/29 12:00:50]

Same here: ensures that new user avatar is saved immediately.

[bartfab (slow), 2012/11/29 14:18:09]

Same as above.

[bartfab (slow), 2012/11/29 18:56:17]

Done.

-    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);
+  delete RemoveUserFromListInternal(email);
 }
 
 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));
 }
 
-void UserManagerImpl::EnsureUsersLoaded() {
+bool UserManagerImpl::ParseUserList(

[Ivan Korotkov, 2012/11/28 21:40:53]

Since this is a utility method that has nothing to do with UserManager's state,
better make it a local function in namespace {}

[bartfab (slow), 2012/11/29 14:18:09]

Done.

+    const ListValue& users_list,
+    const std::set<std::string>& existing_users,
+    const std::string& logged_in_user,
+    std::vector<std::string>* users_vector,
+    std::set<std::string>* users_set) 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 at index " << i << ".";
+      continue;
+    }
+    if (existing_users.find(email) != existing_users.end() ||
+        !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() const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!users_.empty())
-    return;
   if (!g_browser_process)
     return;
 
+  if (users_loaded_)
+    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(*prefs_regular_users, std::set<std::string>(), "",
+                &regular_users, &regular_users_set);
+  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(*prefs_local_users,regular_users_set, "",
+                &local_users, &local_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));

[Ivan Korotkov, 2012/11/28 21:40:53]

Maybe CreateGaiaAccountUser? Public sounds somewhat counterintuitive.

[bartfab (slow), 2012/11/29 14:18:09]

As discussed offline, public accounts are non-GAIA.

   }
 
   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);
+
+  EnsureUsersLoaded();
+
+  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);
+        delete RemoveUserFromListInternal(user_email);
         changed = true;
       }
     }
+  }
 
-    if (changed) {
-      content::NotificationService::current()->Notify(
-          chrome::NOTIFICATION_POLICY_USER_LIST_CHANGED,
-          content::Source<UserManager>(this),
-          content::NotificationService::NoDetails());
-    }
+  if (changed) {
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_POLICY_USER_LIST_CHANGED,
+        content::Source<UserManager>(this),
+        content::NotificationService::NoDetails());
   }
+
+  cros_settings_->AddSettingsObserver(kAccountsPrefDeviceLocalAccounts,
+                                      this);
 }
 
 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 {
   const UserList& users = GetUsers();
@@ skipping 25 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::RemoveUserFromListInternal(const std::string& email) {

[Ivan Korotkov, 2012/11/28 21:40:53]

This looks like it really is RemoveRegularUser...?

[bartfab (slow), 2012/11/29 14:18:09]

Done. Though note that the method will handle other user types properly - but
you are right, it is intended to be used with regular users.

+  ListPrefUpdate prefs_users_update(g_browser_process->local_state(),
+                                    kRegularUsers);
+  prefs_users_update->Clear();
+  User* user = NULL;
+  for (UserList::iterator it = users_.begin(); it != users_.end(); ) {
+    const std::string user_email = (*it)->email();
+    if (user_email == email) {
+      user = *it;
+      it = users_.erase(it);
+    } else {
+      if ((*it)->GetType() == User::USER_TYPE_REGULAR)
+        prefs_users_update->Append(new base::StringValue(user_email));
+      ++it;
+    }
   }
+  return user;
+}
+
+bool UserManagerImpl::UpdateLocalUsers(
+    const base::ListValue& local_users_list) {
+  PrefService* local_state = g_browser_process->local_state();
+
+  // Determine the currently logged-in user's email.
+  std::string logged_in_user;

[Nikita (slow), 2012/11/29 16:42:50]

Please rename to logged_in_user_email so that you don't confuse with
logged_in_user_

[bartfab (slow), 2012/11/29 18:56:17]

Done.

+  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(local_users_list, regular_users, logged_in_user,
+                     &new_local_users, &new_local_users_set) &&
+      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