[cros] UserManager cleanup regarding stub/demo users.

chrome/browser/chromeos/login/user_manager_impl.cc

Index: chrome/browser/chromeos/login/user_manager_impl.cc
diff --git a/chrome/browser/chromeos/login/user_manager_impl.cc b/chrome/browser/chromeos/login/user_manager_impl.cc
index 80e5b96bb8d8ced388d377d1558934904ba78b9d..2e67ddd4bac62c874b20a44bba77be4429376daa 100644
--- a/chrome/browser/chromeos/login/user_manager_impl.cc
+++ b/chrome/browser/chromeos/login/user_manager_impl.cc
@@ -66,10 +66,6 @@ namespace chromeos {
 
 namespace {
 
-// Incognito user is represented by an empty string (since some code already
-// depends on that and it's hard to figure out what).
-const char kGuestUser[] = "";
-
 // Names of nodes with info about user image.
 const char kImagePathNodeName[] = "path";
 const char kImageIndexNodeName[] = "index";
@@ -252,6 +248,7 @@ const UserList& UserManagerImpl::GetUsers() const {
 void UserManagerImpl::UserLoggedIn(const std::string& email,
                                    bool browser_restart) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!IsUserLoggedIn());

[pastarmovj, 2012/07/18 19:03:36]

I remember we had some issues with this check the last time I touched this code
with testing on Linux because there the browser starts logged in with the stub
user and can then transition to another user logging in.
Have you checked if this case won't trigger this DCHECK still?

[Ivan Korotkov, 2012/07/19 12:20:21]

Right. I've recently submitted  which slightly changes this behaviour (stub
login is performed from ChromeBrowserMainPartsChromeos now) and we're
refactoring all tests to add flag --login-manager if they're doing a manual
login later. So this CHECK is to guarantee that no such tests are left :)

 
   if (email == kGuestUser) {
     GuestUserLoggedIn();
@@ -289,7 +286,7 @@ void UserManagerImpl::UserLoggedIn(const std::string& email,
 
   if (logged_in_user == users_.end()) {
     is_current_user_new_ = true;
-    logged_in_user_ = CreateUser(email);
+    logged_in_user_ = CreateUser(email, /* is_ephemeral= */ false);

[pastarmovj, 2012/07/18 19:03:36]

Do you need those comments here? I can see how you wanted to make it clear this
is not the flag for guest but for ephemeral but I think whoever needs to know
that will read the function definition and know it as long it is well documented
there.

[Ivan Korotkov, 2012/07/19 12:20:21]

I'd rather leave them because bool arguments always look confusing without a
clear context, like here.

   } else {
     logged_in_user_ = *logged_in_user;
     users_.erase(logged_in_user);
@@ -353,7 +350,7 @@ void UserManagerImpl::DemoUserLoggedIn() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   is_current_user_new_ = true;
   is_current_user_ephemeral_ = true;
-  logged_in_user_ = new User(kDemoUser, false);
+  logged_in_user_ = CreateUser(kDemoUser, /* is_ephemeral= */ true);
   SetInitialUserImage(kDemoUser);
   SetInitialUserWallpaper(kDemoUser);
   NotifyOnLogin();
@@ -363,7 +360,7 @@ void UserManagerImpl::GuestUserLoggedIn() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   is_current_user_ephemeral_ = true;
   SetInitialUserWallpaper(kGuestUser);
-  logged_in_user_ = new User(kGuestUser, true);
+  logged_in_user_ = CreateUser(kGuestUser, /* is_ephemeral= */ true);
   NotifyOnLogin();
 }
 
@@ -371,7 +368,7 @@ void UserManagerImpl::EphemeralUserLoggedIn(const std::string& email) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   is_current_user_new_ = true;
   is_current_user_ephemeral_ = true;
-  logged_in_user_ = CreateUser(email);
+  logged_in_user_ = CreateUser(email, /* is_ephemeral= */ true);
   SetInitialUserImage(email);
   SetInitialUserWallpaper(email);
   NotifyOnLogin();
@@ -830,7 +827,7 @@ void UserManagerImpl::EnsureUsersLoaded() {
          it != prefs_users->end(); ++it) {
       std::string email;
       if ((*it)->GetAsString(&email)) {
-        User* user = CreateUser(email);
+        User* user = CreateUser(email, /* is_ephemeral= */ false);
         users_.push_back(user);
 
         if (prefs_images) {
@@ -1617,9 +1614,11 @@ void UserManagerImpl::OnProfileDownloadFailure(ProfileDownloader* downloader) {
       content::NotificationService::NoDetails());
 }
 
-User* UserManagerImpl::CreateUser(const std::string& email) const {
-  User* user = new User(email, email == kGuestUser);
-  user->set_oauth_token_status(LoadUserOAuthStatus(email));
+User* UserManagerImpl::CreateUser(const std::string& email,
+                                  bool is_ephemeral) const {
+  User* user = new User(email);
+  if (!is_ephemeral)
+    user->set_oauth_token_status(LoadUserOAuthStatus(email));
   return user;
 }