Remove user from the device when removing from the user list.

BUG=chromium-os:7943
TEST=manual (see bug description)

Review URL: http://codereview.chromium.org/6602083

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76551 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 109 insertions, 69 deletions

diff --git a/chrome/browser/chromeos/login/existing_user_controller.cc b/chrome/browser/chromeos/login/existing_user_controller.cc
index 619f7d7..d02e7c8 100644
--- a/chrome/browser/chromeos/login/existing_user_controller.cc
+++ b/chrome/browser/chromeos/login/existing_user_controller.cc
@@ -11,7 +11,6 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/boot_times_loader.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
-#include "chrome/browser/chromeos/cros/cryptohome_library.h"
 #include "chrome/browser/chromeos/cros/login_library.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/login/background_view.h"
@@ -53,30 +52,6 @@ const char kCreateAccountURL[] =
 // Landing URL when launching Guest mode to fix captive portal.
 const char kCaptivePortalLaunchURL[] = "http://www.google.com/";
 
-// Used to handle the asynchronous response of deleting a cryptohome directory.
-class RemoveAttempt : public CryptohomeLibrary::Delegate {
- public:
-  explicit RemoveAttempt(const std::string& user_email)
-      : user_email_(user_email) {
-    if (CrosLibrary::Get()->EnsureLoaded()) {
-      CrosLibrary::Get()->GetCryptohomeLibrary()->AsyncRemove(
-          user_email_, this);
-    }
-  }
-
-  void OnComplete(bool success, int return_code) {
-    // Log the error, but there's not much we can do.
-    if (!success) {
-      VLOG(1) << "Removal of cryptohome for " << user_email_
-              << " failed, return code: " << return_code;
-    }
-    delete this;
-  }
-
- private:
-  std::string user_email_;
-};
-
 }  // namespace
 
 // static
@@ -279,34 +254,6 @@ void ExistingUserController::OnUserSelected(const std::string& username) {
   num_login_attempts_ = 0;
 }
 
-void ExistingUserController::RemoveUser(const std::string& username) {
-  // Owner is not allowed to be removed from the device.
-  // Must not proceed without signature verification.
-  UserCrosSettingsProvider user_settings;
-  bool trusted_owner_available = user_settings.RequestTrustedOwner(
-      method_factory_.NewRunnableMethod(&ExistingUserController::RemoveUser,
-                                        username));
-  if (!trusted_owner_available) {
-    // Value of owner email is still not verified.
-    // Another attempt will be invoked after verification completion.
-    return;
-  }
-  if (username == UserCrosSettingsProvider::cached_owner()) {
-    // Owner is not allowed to be removed from the device.
-    return;
-  }
-
-  login_display_->OnBeforeUserRemoved(username);
-
-  // Delete user from user list.
-  UserManager::Get()->RemoveUser(username);
-
-  // Delete the encrypted user directory.
-  new RemoveAttempt(username);
-
-  login_display_->OnUserRemoved(username);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // ExistingUserController, LoginPerformer::Delegate implementation:
 //
diff --git a/chrome/browser/chromeos/login/existing_user_controller.h b/chrome/browser/chromeos/login/existing_user_controller.h
index 696a337..9c643f7 100644
--- a/chrome/browser/chromeos/login/existing_user_controller.h
+++ b/chrome/browser/chromeos/login/existing_user_controller.h
@@ -66,7 +66,6 @@ class ExistingUserController : public LoginDisplay::Delegate,
                      const std::string& password);
   virtual void LoginAsGuest();
   virtual void OnUserSelected(const std::string& username);
-  virtual void RemoveUser(const std::string& username);
 
   // NotificationObserver implementation.
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/chromeos/login/login_display.h b/chrome/browser/chromeos/login/login_display.h
index d0605ff..a705cdb 100644
--- a/chrome/browser/chromeos/login/login_display.h
+++ b/chrome/browser/chromeos/login/login_display.h
@@ -17,8 +17,18 @@
 
 namespace chromeos {
 
+// Delegate to be used while user removing.
+class RemoveUserDelegate {
+ public:
+  // Called right before actual user removal process is initiated.
+  virtual void OnBeforeUserRemoved(const std::string& username) = 0;
+
+  // Called right after user removal process has been initiated.
+  virtual void OnUserRemoved(const std::string& username) = 0;
+};
+
 // An abstract class that defines login UI implementation.
-class LoginDisplay {
+class LoginDisplay : public RemoveUserDelegate {
  public:
   class Delegate {
    public:
@@ -41,9 +51,6 @@ class LoginDisplay {
 
     // Called when existing user pod is selected in the UI.
     virtual void OnUserSelected(const std::string& username) = 0;
-
-    // Completely removes user (from the list of users and cryptohome).
-    virtual void RemoveUser(const std::string& username) = 0;
    protected:
     virtual ~Delegate() {}
   };
@@ -61,16 +68,11 @@ class LoginDisplay {
                     bool show_guest,
                     bool show_new_user) = 0;
 
-  // Called right before actual user removal process is initiated.
-  virtual void OnBeforeUserRemoved(const std::string& username) = 0;
 
   // Called when user image has been changed.
   // |user| contains updated user.
   virtual void OnUserImageChanged(UserManager::User* user) = 0;
 
-  // Called right after user removal process has been initiated.
-  virtual void OnUserRemoved(const std::string& username) = 0;
-
   // After this call login display should be ready to be smoothly destroyed
   // (e.g. hide throbber, etc.).
   virtual void OnFadeOut() = 0;
diff --git a/chrome/browser/chromeos/login/user_manager.cc b/chrome/browser/chromeos/login/user_manager.cc
index f76384f..08b52e3 100644
--- a/chrome/browser/chromeos/login/user_manager.cc
+++ b/chrome/browser/chromeos/login/user_manager.cc
@@ -17,7 +17,9 @@
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
+#include "chrome/browser/chromeos/cros/cryptohome_library.h"
 #include "chrome/browser/chromeos/cros/input_method_library.h"
+#include "chrome/browser/chromeos/login/login_display.h"
 #include "chrome/browser/chromeos/login/ownership_service.h"
 #include "chrome/browser/chromeos/user_cros_settings_provider.h"
 #include "chrome/browser/chromeos/wm_ipc.h"
@@ -153,6 +155,73 @@ void CheckOwnership() {
       NewRunnableFunction(&UpdateOwnership, is_owner));
 }
 
+// Used to handle the asynchronous response of deleting a cryptohome directory.
+class RemoveAttempt : public CryptohomeLibrary::Delegate {
+ public:
+  // Creates new remove attempt for the given user. Note, |delegate| can
+  // be NULL.
+  RemoveAttempt(const std::string& user_email,
+                chromeos::RemoveUserDelegate* delegate)
+      : user_email_(user_email),
+        delegate_(delegate),
+        method_factory_(this) {
+    RemoveUser();
+  }
+
+  void RemoveUser() {
+    // Owner is not allowed to be removed from the device.
+    // Must not proceed without signature verification.
+    UserCrosSettingsProvider user_settings;
+    bool trusted_owner_available = user_settings.RequestTrustedOwner(
+        method_factory_.NewRunnableMethod(&RemoveAttempt::RemoveUser));
+    if (!trusted_owner_available) {
+      // Value of owner email is still not verified.
+      // Another attempt will be invoked after verification completion.
+      return;
+    }
+    if (user_email_ == UserCrosSettingsProvider::cached_owner()) {
+      // Owner is not allowed to be removed from the device. Probably on
+      // the stack, so deffer the deletion.
+      MessageLoop::current()->DeleteSoon(FROM_HERE, this);
+      return;
+    }
+
+    if (delegate_)
+      delegate_->OnBeforeUserRemoved(user_email_);
+
+    chromeos::UserManager::Get()->RemoveUserFromList(user_email_);
+    RemoveUserCryptohome();
+
+    if (delegate_)
+      delegate_->OnUserRemoved(user_email_);
+  }
+
+  void RemoveUserCryptohome() {
+    if (CrosLibrary::Get()->EnsureLoaded()) {
+      CrosLibrary::Get()->GetCryptohomeLibrary()->AsyncRemove(user_email_,
+                                                              this);
+    }
+  }
+
+  void OnComplete(bool success, int return_code) {
+    // Log the error, but there's not much we can do.
+    if (!success) {
+      VLOG(1) << "Removal of cryptohome for " << user_email_
+              << " failed, return code: " << return_code;
+    }
+    delete this;
+  }
+
+ private:
+  std::string user_email_;
+  chromeos::RemoveUserDelegate* delegate_;
+
+  // Factory of callbacks.
+  ScopedRunnableMethodFactory<RemoveAttempt> method_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemoveAttempt);
+};
+
 }  // namespace
 
 UserManager::User::User() {
@@ -295,7 +364,22 @@ void UserManager::UserLoggedIn(const std::string& email) {
     SetDefaultUserImage(email);
 }
 
-void UserManager::RemoveUser(const std::string& email) {
+void UserManager::RemoveUser(const std::string& email,
+                             RemoveUserDelegate* delegate) {
+  // Get a copy of the current users.
+  std::vector<User> users = GetUsers();
+
+  bool user_found = false;
+  for (size_t i = 0; !user_found && i < users.size(); ++i)
+    user_found = (email == users[i].email());
+  if (!user_found)
+    return;
+
+  // |RemoveAttempt| deletes itself when done.
+  new RemoveAttempt(email, delegate);
+}
+
+void UserManager::RemoveUserFromList(const std::string& email) {
   // Get a copy of the current users.
   std::vector<User> users = GetUsers();
 
diff --git a/chrome/browser/chromeos/login/user_manager.h b/chrome/browser/chromeos/login/user_manager.h
index 58b28e1..6048f52 100644
--- a/chrome/browser/chromeos/login/user_manager.h
+++ b/chrome/browser/chromeos/login/user_manager.h
@@ -21,6 +21,7 @@ class FilePath;
 class PrefService;
 
 namespace chromeos {
+class RemoveUserDelegate;
 
 // This class provides a mechanism for discovering users who have logged
 // into this chromium os device before and updating that list.
@@ -71,8 +72,15 @@ class UserManager : public UserImageLoader::Delegate,
   // The persistent list will be updated accordingly.
   virtual void UserLoggedIn(const std::string& email);
 
-  // Remove user from persistent list. NOTE: user's data won't be removed.
-  virtual void RemoveUser(const std::string& email);
+  // Removes the user from the device. Note, it will verify that the given user
+  // isn't the owner, so calling this method for the owner will take no effect.
+  // Note, |delegate| can be NULL.
+  virtual void RemoveUser(const std::string& email,
+                          RemoveUserDelegate* delegate);
+
+  // Removes the user from the persistent list only. Also removes the user's
+  // picture.
+  virtual void RemoveUserFromList(const std::string& email);
 
   // Returns true if given user has logged into the device before.
   virtual bool IsKnownUser(const std::string& email);
diff --git a/chrome/browser/chromeos/login/views_login_display.cc b/chrome/browser/chromeos/login/views_login_display.cc
index 2e39fac..8c54ac9 100644
--- a/chrome/browser/chromeos/login/views_login_display.cc
+++ b/chrome/browser/chromeos/login/views_login_display.cc
@@ -142,7 +142,7 @@ void ViewsLoginDisplay::OnUserImageChanged(UserManager::User* user) {
 
 void ViewsLoginDisplay::OnUserRemoved(const std::string& username) {
   // We need to unmap entry windows, the windows will be unmapped in destructor.
-  delete controller_for_removal_;
+  MessageLoop::current()->DeleteSoon(FROM_HERE, controller_for_removal_);
   controller_for_removal_ = NULL;
 
   // Nothing to insert.
@@ -279,7 +279,7 @@ void ViewsLoginDisplay::OnUserSelected(UserController* source) {
 
 void ViewsLoginDisplay::RemoveUser(UserController* source) {
   ClearErrors();
-  delegate()->RemoveUser(source->user().email());
+  UserManager::Get()->RemoveUser(source->user().email(), this);
 }
 
 void ViewsLoginDisplay::SelectUser(int index) {
diff --git a/chrome/browser/chromeos/webui/accounts_options_handler.cc b/chrome/browser/chromeos/webui/accounts_options_handler.cc
index 75b5115..9e06f38 100644
--- a/chrome/browser/chromeos/webui/accounts_options_handler.cc
+++ b/chrome/browser/chromeos/webui/accounts_options_handler.cc
@@ -87,6 +87,7 @@ void AccountsOptionsHandler::UnwhitelistUser(const ListValue* args) {
   }
 
   users_settings()->UnwhitelistUser(Authenticator::Canonicalize(email));
+  UserManager::Get()->RemoveUser(email, NULL);
 }
 
 void AccountsOptionsHandler::FetchUserPictures(const ListValue* args) {
diff --git a/chrome/browser/chromeos/webui/accounts_options_handler.h b/chrome/browser/chromeos/webui/accounts_options_handler.h
index e91b9ef..9146a83 100644
--- a/chrome/browser/chromeos/webui/accounts_options_handler.h
+++ b/chrome/browser/chromeos/webui/accounts_options_handler.h
@@ -48,4 +48,3 @@ class AccountsOptionsHandler : public CrosOptionsPageUIHandler {
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_WEBUI_ACCOUNTS_OPTIONS_HANDLER_H_
-