Implement ephemeral users

chrome/browser/chromeos/login/user_manager.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.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 18 matching lines @@
 #include "chrome/browser/chromeos/cryptohome/async_method_caller.h"
 #include "chrome/browser/chromeos/dbus/cryptohome_client.h"
 #include "chrome/browser/chromeos/dbus/dbus_thread_manager.h"
 #include "chrome/browser/chromeos/input_method/input_method_manager.h"
 #include "chrome/browser/chromeos/login/default_user_images.h"
 #include "chrome/browser/chromeos/login/helper.h"
 #include "chrome/browser/chromeos/login/login_display.h"
 #include "chrome/browser/chromeos/login/ownership_service.h"
 #include "chrome/browser/chromeos/login/remove_user_delegate.h"
 #include "chrome/browser/chromeos/system/runtime_environment.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_downloader.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/browser/ui/webui/web_ui_util.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
@@ skipping 266 matching lines @@
   const_cast<UserManager*>(this)->EnsureUsersLoaded();
   return users_;
 }
 
 void UserManager::UserLoggedIn(const std::string& email) {
   DCHECK(!user_is_logged_in_);
 
   user_is_logged_in_ = true;
 
   if (email == kGuestUser) {
+    current_user_is_ephemeral_ = true;
     GuestUserLoggedIn();
     return;
   }
 
   if (email == kDemoUser) {
+    current_user_is_ephemeral_ = true;
     DemoUserLoggedIn();
     return;
   }
 
+  if (IsEphemeralUser(email)) {
+    current_user_is_ephemeral_ = true;
+    logged_in_user_ = CreateUser(email);
+    NotifyOnLogin();
+    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(Value::CreateStringValue(email));
   UserList::iterator logged_in_user = users_.end();
@@ skipping 129 matching lines @@
                    base::Unretained(this),
                    image_path));
   }
 }
 
 bool UserManager::IsKnownUser(const std::string& email) const {
   return FindUser(email) != NULL;
 }
 
 const User* UserManager::FindUser(const std::string& email) const {
-  // Speed up search by checking the logged-in user first.
   if (logged_in_user_ && logged_in_user_->email() == email)
     return logged_in_user_;
-  const UserList& users = GetUsers();
-  for (UserList::const_iterator it = users.begin(); it != users.end(); ++it) {
-    if ((*it)->email() == email)
-      return *it;
-  }
-  return NULL;
+  return FindUserInList(email);
 }
 
 bool UserManager::IsDisplayNameUnique(const std::string& display_name) const {
   return display_name_count_[display_name] < 2;
 }
 
 void UserManager::SaveUserOAuthStatus(
     const std::string& username,
     User::OAuthTokenStatus oauth_token_status) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  PrefService* local_state = g_browser_process->local_state();
-  DictionaryPrefUpdate oauth_status_update(local_state, kUserOAuthTokenStatus);
-  oauth_status_update->SetWithoutPathExpansion(username,
-      new base::FundamentalValue(static_cast<int>(oauth_token_status)));
+
   DVLOG(1) << "Saving user OAuth token status in Local State";
   User* user = const_cast<User*>(FindUser(username));
   if (user)
     user->set_oauth_token_status(oauth_token_status);
+
+  // Do not update local store if the user is ephemeral.
+  if (IsEphemeralUser(username))
+    return;
+
+  PrefService* local_state = g_browser_process->local_state();
+
+  DictionaryPrefUpdate oauth_status_update(local_state, kUserOAuthTokenStatus);
+  oauth_status_update->SetWithoutPathExpansion(username,
+      new base::FundamentalValue(static_cast<int>(oauth_token_status)));
 }
 
 User::OAuthTokenStatus UserManager::LoadUserOAuthStatus(
     const std::string& username) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kSkipOAuthLogin)) {
     // Use OAUTH_TOKEN_STATUS_VALID flag if kSkipOAuthLogin is present.
     return User::OAUTH_TOKEN_STATUS_VALID;
@@ skipping 16 matching lines @@
 void UserManager::SaveUserDisplayEmail(const std::string& username,
                                        const std::string& display_email) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   User* user = const_cast<User*>(FindUser(username));
   if (!user)
     return;  // Ignore if there is no such user.
 
   user->set_display_email(display_email);
 
+  // Do not update local store if the user is ephemeral.

[Ivan Korotkov, 2012/03/01 13:37:50]

You don't need most of these checks, only those where persistent storage is
modified.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+  if (IsEphemeralUser(username))
+    return;
+
   PrefService* local_state = g_browser_process->local_state();
 
   DictionaryPrefUpdate display_email_update(local_state, kUserDisplayEmail);
   display_email_update->SetWithoutPathExpansion(
       username,
       base::Value::CreateStringValue(display_email));
 }
 
 std::string UserManager::GetUserDisplayEmail(
     const std::string& username) const {
   const User* user = FindUser(username);
   return user ? user->display_email() : username;
 }
 
 void UserManager::SaveUserDefaultImageIndex(const std::string& username,
                                             int image_index) {
   DCHECK(image_index >= 0 && image_index < kDefaultImagesCount);
+
+  // Ignore for ephemeral users.

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+  if (IsEphemeralUser(username))
+    return;
+
   SetUserImage(username, image_index, GetDefaultImage(image_index));
   SaveImageToLocalState(username, "", image_index, false);
 }
 
 void UserManager::SaveUserImage(const std::string& username,
                                 const SkBitmap& image) {
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   SaveUserImageInternal(username, User::kExternalImageIndex, image);
 }
 
 void UserManager::SaveUserImageFromFile(const std::string& username,
                                         const FilePath& path) {
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   image_loader_->Start(
       path.value(), login::kUserImageSize,
       base::Bind(&UserManager::SaveUserImage,
                  base::Unretained(this), username));
 }
 
 void UserManager::SaveUserImageFromProfileImage(const std::string& username) {
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   if (!downloaded_profile_image_.empty()) {
     // Profile image has already been downloaded, so save it to file right now.
     SaveUserImageInternal(username, User::kProfileImageIndex,
                           downloaded_profile_image_);
   } else {
     // No profile image - use the stub image (gray avatar).
     SetUserImage(username, User::kProfileImageIndex, SkBitmap());
     SaveImageToLocalState(username, "", User::kProfileImageIndex, false);
   }
 }
 
 void UserManager::DownloadProfileImage(const std::string& reason) {
   if (profile_image_downloader_.get()) {
     // Another download is already in progress
     return;
   }
 
   if (logged_in_user().email().empty()) {
     // This is a guest login so there's no profile image to download.
     return;
   }
 
+  if (current_user_is_ephemeral_) {

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    // Ignore for ephemeral users.
+    return;
+  }
+
   profile_image_download_reason_ = reason;
   profile_image_load_start_time_ = base::Time::Now();
   profile_image_downloader_.reset(new ProfileDownloader(this));
   profile_image_downloader_->Start();
 }
 
 void UserManager::Observe(int type,
                           const content::NotificationSource& source,
                           const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_OWNER_KEY_FETCH_ATTEMPT_SUCCEEDED:
       BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
                               base::Bind(&UserManager::CheckOwnership,
                                          base::Unretained(this)));
+      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+          base::Bind(&UserManager::RetrieveTrustedDevicePolicies,
+          base::Unretained(this)));
       break;
     case chrome::NOTIFICATION_PROFILE_ADDED:
       if (user_is_logged_in() && !IsLoggedInAsGuest()) {
         Profile* profile = content::Source<Profile>(source).ptr();
         if (!profile->IsOffTheRecord() &&
             profile == ProfileManager::GetDefaultProfile()) {
           DCHECK(NULL == observed_sync_service_);
           observed_sync_service_ =
               ProfileSyncServiceFactory::GetForProfile(profile);
           if (observed_sync_service_)
@@ skipping 45 matching lines @@
   observer_list_.RemoveObserver(obs);
 }
 
 void UserManager::NotifyLocalStateChanged() {
   FOR_EACH_OBSERVER(
     Observer,
     observer_list_,
     LocalStateChanged(this));
 }
 
+void UserManager::RetrieveTrustedDevicePolicies() {
+  ephemeral_users_enabled_.reset();
+  owner_.reset();
+
+  CrosSettings* cros_settings = CrosSettings::Get();
+  // Schedule a callback if device policy has not yet been verified.
+  if (!cros_settings->GetTrusted(
+      kAccountsPrefEphemeralUsersEnabled,
+      base::Bind(&UserManager::RetrieveTrustedDevicePolicies,
+                 base::Unretained(this))))
+    return;

[Ivan Korotkov, 2012/03/01 13:37:50]

Multiline if conditions require braces, too.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+  bool* ephemeral_users_enabled = new bool;
+  cros_settings->GetBoolean(kAccountsPrefEphemeralUsersEnabled,
+                            ephemeral_users_enabled);
+  ephemeral_users_enabled_.reset(ephemeral_users_enabled);
+
+  if (g_browser_process->browser_policy_connector()->IsEnterpriseManaged()) {
+    owner_.reset(new std::string);

[Ivan Korotkov, 2012/03/01 13:37:50]

There are alredy pieces of code that fetch owner (see RemoveUserInternal). I
suggest to add method GetOwner() that will call GetTrusted/GetString from all
places.

[use bartfab instead, 2012/03/01 18:33:43]

The problem is that GetTrusted() must be provided with a callback to retry
whatever action was desired if trusted information is not available yet. If
GetTrusted() was called from within a new GetOwner(), this method would need to
take a callback parameter to tell it how to repeat the entire operation if
trusted information is not yet available. This would seem more complicated to me
but if you wish, I can change the code.

[Ivan Korotkov, 2012/03/01 20:16:30]

Right, I'm sorry for the confusion. In that case, please make RemoveUserInternal
a member function so that it makes use of owner_.

[use bartfab instead, 2012/03/05 18:07:32]

This will not work I fear. What if the owner_ has not been successfully fetched
yet? RemoveUserInternal needs to find out somehow and schedule a callback to
itself in this case. Even if RemoveUserInternal is made a member function, it
will still need to make its own call to GetTrusted() so that it can schedule its
own callback.

+  } else {
+    std::string* owner = new std::string;
+    cros_settings->GetString(kDeviceOwner, owner);
+    owner_.reset(owner);
+  }
+
+  if (*ephemeral_users_enabled_)
+    RemoveAllExceptOwnerFromList();
+}
+
 // Protected constructor and destructor.
 UserManager::UserManager()
     : ALLOW_THIS_IN_INITIALIZER_LIST(image_loader_(new UserImageLoader)),
       demo_user_(kDemoUser, false),
       guest_user_(kGuestUser, true),
       stub_user_(kStubUser, false),
       logged_in_user_(NULL),
       current_user_is_owner_(false),
       current_user_is_new_(false),
+      current_user_is_ephemeral_(false),
       user_is_logged_in_(false),
       observed_sync_service_(NULL),
       last_image_set_async_(false),
       downloaded_profile_image_data_url_(chrome::kAboutBlankURL) {
   // Use stub as the logged-in user for test paths without login.
   if (!system::runtime_environment::IsRunningOnChromeOS())
     StubUserLoggedIn();
   registrar_.Add(this, chrome::NOTIFICATION_OWNER_KEY_FETCH_ATTEMPT_SUCCEEDED,
       content::NotificationService::AllSources());
   registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
       content::NotificationService::AllSources());
+  RetrieveTrustedDevicePolicies();
 }
 
 UserManager::~UserManager() {
 }
 
 FilePath UserManager::GetImagePathForUser(const std::string& username) {
   std::string filename = username + ".png";
   FilePath user_data_dir;
   PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
   return user_data_dir.AppendASCII(filename);
@@ skipping 75 matching lines @@
         if (prefs_display_emails &&
             prefs_display_emails->GetStringWithoutPathExpansion(
                 email, &display_email)) {
           user->set_display_email(display_email);
         }
       }
     }
   }
 }
 
+bool UserManager::AreEphemeralUsersEnabled() const {
+  return ephemeral_users_enabled_.get() &&
+      *ephemeral_users_enabled_ &&
+      owner_.get();
+}
+
+bool UserManager::IsEphemeralUser(const std::string& email) const {
+  // The guest user always is ephemeral.
+  if (email == guest_user_.email())
+    return true;
+
+  // The currently logged-in user is ephemeral iff logged in as ephemeral.
+  if (logged_in_user_ && (email == logged_in_user_->email()))
+    return current_user_is_ephemeral_;
+
+  // Any other user is ephemeral iff ephemeral users are enabled, the user is
+  // not the owner and is not in the persistent list.
+  return AreEphemeralUsersEnabled() &&
+      (email != *owner_) &&
+      !FindUserInList(email);
+}
+
+const User* UserManager::FindUserInList(const std::string& email) const {
+  const UserList& users = GetUsers();
+  for (UserList::const_iterator it = users.begin(); it != users.end(); ++it) {
+    if ((*it)->email() == email)
+      return *it;
+  }
+  return NULL;
+}
+
 void UserManager::StubUserLoggedIn() {
   logged_in_user_ = &stub_user_;
   stub_user_.SetImage(GetDefaultImage(kStubDefaultImageIndex),
                       kStubDefaultImageIndex);
 }
 
 void UserManager::NotifyOnLogin() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_LOGIN_USER_CHANGED,
@@ skipping 20 matching lines @@
     cert_library->RequestCertificates();
   }
 
   // Schedules current user ownership check on file thread.
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
                           base::Bind(&UserManager::CheckOwnership,
                                      base::Unretained(this)));
 }
 
 void UserManager::SetInitialUserImage(const std::string& username) {
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   // Choose a random default image.
   int image_id = base::RandInt(0, kDefaultImagesCount - 1);
   SaveUserDefaultImageIndex(username, image_id);
 }
 
 void UserManager::SetUserImage(const std::string& username,
                                int image_index,
                                const SkBitmap& image) {
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   User* user = const_cast<User*>(FindUser(username));
   // User may have been removed by now.
   if (user) {
     // For existing users, a valid image index should have been set upon loading
     // them from Local State.
     DCHECK(user->image_index() != User::kInvalidImageIndex ||
            current_user_is_new_);
     bool image_changed = user->image_index() != User::kInvalidImageIndex;
     if (!image.empty())
       user->SetImage(image, image_index);
@@ skipping 12 matching lines @@
           content::Details<const User>(user));
     }
   }
 }
 
 void UserManager::SaveUserImageInternal(const std::string& username,
                                         int image_index,
                                         const SkBitmap& image) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

This should be after SetUserImage.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

[Ivan Korotkov, 2012/03/01 20:16:30]

Hey, it's gone now. I meant that it should be after the SetUserImage call
(before the SaveImageToFile task is posted).

[use bartfab instead, 2012/03/05 18:07:32]

Done. I do not understand how this makes sense though. SaveUserImageInternal()
now sets the image index to either kExternalImageIndex or kProfileImageIndex but
then returns before creating an image file. So you have an index implying an
external image which does not actually exist. Would it not be better to move the
IsEphemeralUser() check to the top of the function so that the index is not set
in the first place?

[Ivan Korotkov, 2012/03/06 20:37:17]

The point is that the picture should be still set in memory (so that it will be
displayed on the lock screen/prefs page).

+    return;
+
   SetUserImage(username, image_index, image);
 
   FilePath image_path = GetImagePathForUser(username);
   DVLOG(1) << "Saving user image to " << image_path.value();
 
   last_image_set_async_ = true;
 
   BrowserThread::PostTask(
       BrowserThread::FILE,
       FROM_HERE,
       base::Bind(&UserManager::SaveImageToFile,
                  base::Unretained(this),
                  username, image, image_path, image_index));
 }
 
 void UserManager::SaveImageToFile(const std::string& username,
                                   const SkBitmap& image,
                                   const FilePath& image_path,
                                   int image_index) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
+  // Ignore for ephemeral users.
+  if (IsEphemeralUser(username))

[Ivan Korotkov, 2012/03/01 13:37:50]

Not needed (never reached).

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
   std::vector<unsigned char> encoded_image;
   if (!gfx::PNGCodec::EncodeBGRASkBitmap(image, false, &encoded_image)) {
     LOG(ERROR) << "Failed to PNG encode the image.";
     return;
   }
 
   if (file_util::WriteFile(image_path,
                            reinterpret_cast<char*>(&encoded_image[0]),
                            encoded_image.size()) == -1) {
     LOG(ERROR) << "Failed to save image to file.";
     return;
   }
 
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(&UserManager::SaveImageToLocalState,
                  base::Unretained(this),
                  username, image_path.value(), image_index, true));
 }
 
 void UserManager::SaveImageToLocalState(const std::string& username,
                                         const std::string& image_path,
                                         int image_index,
                                         bool is_async) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  // Ignore for ephemeral users.

[Ivan Korotkov, 2012/03/01 13:37:50]

Here it's ok.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+  if (IsEphemeralUser(username))
+    return;
+
   // TODO(ivankr): use unique filenames for user images each time
   // a new image is set so that only the last image update is saved
   // to Local State and notified.
   if (is_async && !last_image_set_async_) {
     DVLOG(1) << "Ignoring saved image because it has changed";
     return;
   } else if (!is_async) {
     // Reset the async image save flag if called directly from the UI thread.
     last_image_set_async_ = false;
   }
@@ skipping 131 matching lines @@
 }
 
 User* UserManager::CreateUser(const std::string& email) const {
   User* user = new User(email, email == kGuestUser);
   user->set_oauth_token_status(LoadUserOAuthStatus(email));
   // Used to determine whether user's display name is unique.
   ++display_name_count_[user->GetDisplayName()];
   return user;
 }
 
+void UserManager::RemoveAllExceptOwnerFromList() {
+  if (!AreEphemeralUsersEnabled())

[Ivan Korotkov, 2012/03/01 13:37:50]

No point in this check.

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+    return;
+
+  const std::string owner_email = *owner_;

[Ivan Korotkov, 2012/03/01 13:37:50]

This looks overly complicated. Since this should be doing any real job only
once, when ephemeral users are enabled, I'd just call RemoveUserFromList for
every user other than owner and logged-in user.

[use bartfab instead, 2012/03/01 18:33:43]

I considered this but it will not work:

RemoveUserFromList() calls EnsureUsersLoaded(). If any user has a custom profile
image, this will kick off a download of that image in the background. If the
user is deleted a moment later, the download will continue and will leave a
stale image file behind. This is why I rolled my own removal that does not
download anything.

[Ivan Korotkov, 2012/03/01 20:16:30]

Ah, I see. Add a comment about that, please.

[rkc, 2012/03/01 22:17:37]

Another option could be to refactor RemoveUserFromList to first call
EnsureUsersLoaded() then call a RemoveUserFromListWithoutLoadedCheck(); your
function could call the latter method directly.

It would avoid all this repeated code and keep the remove user logic in a
central method.

[use bartfab instead, 2012/03/05 18:07:32]

Good idea. I implemented it like this using a new RemoveUserFromListInternal()
function for the shared code.

+  // If a user is currently logged in and the login occurred before ephemeral
+  // users were enabled, this user should not be deleted yet either.
+  std::string logged_in_user_email;
+  if (user_is_logged_in_ && !current_user_is_ephemeral_)
+    logged_in_user_email = logged_in_user_->email();
+
+  PrefService* prefs = g_browser_process->local_state();
+  ListPrefUpdate prefs_users_update(prefs, kLoggedInUsers);
+  DictionaryPrefUpdate prefs_images_update(prefs, kUserImages);
+  DictionaryPrefUpdate prefs_oauth_update(prefs, kUserOAuthTokenStatus);
+  DictionaryPrefUpdate prefs_display_email_update(prefs, kUserDisplayEmail);
+
+  base::ListValue* users = prefs_users_update->DeepCopy();
+  prefs_users_update->Clear();
+  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 || user_email == logged_in_user_email) {
+      prefs_users_update->Append(Value::CreateStringValue(user_email));
+      continue;
+    }
+    std::string image_path_string;
+    prefs_images_update->GetStringWithoutPathExpansion(user_email,
+                                                       &image_path_string);
+    prefs_images_update->RemoveWithoutPathExpansion(user_email, NULL);
+    prefs_oauth_update->RemoveWithoutPathExpansion(user_email, NULL);
+    prefs_display_email_update->RemoveWithoutPathExpansion(user_email, NULL);
+
+    int default_image_id = User::kInvalidImageIndex;
+    if (!image_path_string.empty() &&
+        !IsDefaultImagePath(image_path_string, &default_image_id)) {
+      FilePath image_path(image_path_string);
+      BrowserThread::PostTask(
+          BrowserThread::FILE,
+          FROM_HERE,
+          base::Bind(&UserManager::DeleteUserImage,
+                     base::Unretained(this),
+                     image_path));
+    }
+  }
+  delete users;

[Nikita (slow), 2012/03/01 10:01:58]

scoped_ptr<>?

[use bartfab instead, 2012/03/01 18:33:43]

Done.

+
+  if (!users_.empty()) {
+    UserList users_update;
+    for (UserList::iterator user = users_.begin(); user != users_.end();
+        ++user) {
+      if ((*user)->email() ==  owner_email || *user == logged_in_user_) {
+        users_update.push_back(*user);
+      } else {
+        --display_name_count_[(*user)->GetDisplayName()];
+        delete *user;
+      }
+    }
+    users_ = users_update;
+    // Trigger a redraw of the login window.
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED,
+        content::Source<UserManager>(this),
+        content::NotificationService::NoDetails());
+  }
+}
+
 }  // namespace chromeos