[cros] UserManager cleanup regarding stub/demo users.

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 <vector>
 
 #include "ash/shell.h"
 #include "ash/desktop_background/desktop_background_controller.h"
@@ skipping 48 matching lines @@
 #include "ui/gfx/image/image_skia.h"
 
 using content::BrowserThread;
 
 typedef GoogleServiceAuthError AuthError;
 
 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";
 const char kImageURLNodeName[] = "url";
 
 const char kWallpaperTypeNodeName[] = "type";
 const char kWallpaperIndexNodeName[] = "index";
 const char kWallpaperDateNodeName[] = "date";
 
 // Default wallpaper index used in OOBE (first boot).
@@ skipping 162 matching lines @@
 }
 
 const UserList& UserManagerImpl::GetUsers() const {
   const_cast<UserManagerImpl*>(this)->EnsureUsersLoaded();
   return users_;
 }
 
 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();
     return;
   }
 
   if (email == kDemoUser) {
     DemoUserLoggedIn();
     return;
   }
@@ skipping 17 matching lines @@
     std::string user_email = (*it)->email();
     // Skip the most recent user.
     if (email != user_email)
       prefs_users_update->Append(Value::CreateStringValue(user_email));
     else
       logged_in_user = it;
   }
 
   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);
   }
   // This user must be in the front of the user list.
   users_.insert(users_.begin(), logged_in_user_);
 
   if (is_current_user_new_) {
     SaveUserDisplayName(GetLoggedInUser().email(),
         UTF8ToUTF16(GetLoggedInUser().GetAccountName(true)));
@@ skipping 43 matching lines @@
     EnsureLoggedInUserWallpaperLoaded();
   }
 
   NotifyOnLogin();
 }
 
 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();
 }
 
 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();
 }
 
 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();
 }
 
 void UserManagerImpl::InitializeWallpaper() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (!IsUserLoggedIn()) {
     if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableNewOobe)) {
       if (!WizardController::IsDeviceRegistered()) {
@@ skipping 438 matching lines @@
   const DictionaryValue* prefs_display_emails =
       local_state->GetDictionary(UserManager::kUserDisplayEmail);
 
   int user_image_size = GetCurrentUserImageSize();
 
   if (prefs_users) {
     for (ListValue::const_iterator it = prefs_users->begin();
          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) {
           // Get account image path.
           // TODO(avayvod): Reading image path as a string is here for
           // backward compatibility.
           std::string image_path;
           base::DictionaryValue* image_properties;
           if (prefs_images->GetStringWithoutPathExpansion(email, &image_path)) {
             int image_id = User::kInvalidImageIndex;
@@ skipping 766 matching lines @@
   base::TimeDelta delta = base::Time::Now() - profile_image_load_start_time_;
   AddProfileImageTimeHistogram(kDownloadFailure, profile_image_download_reason_,
                                delta);
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PROFILE_IMAGE_UPDATE_FAILED,
       content::Source<UserManagerImpl>(this),
       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;
 }
 
 void UserManagerImpl::RemoveUserFromListInternal(const std::string& email) {
   // Clear the prefs view of the users.
   PrefService* prefs = g_browser_process->local_state();
   ListPrefUpdate prefs_users_update(prefs, kLoggedInUsers);
   prefs_users_update->Clear();
 
   UserList::iterator user_to_remove = users_.end();
@@ skipping 55 matching lines @@
     BrowserThread::PostTask(
         BrowserThread::FILE,
         FROM_HERE,
         base::Bind(&UserManagerImpl::DeleteUserImage,
                    base::Unretained(this),
                    image_path));
   }
 }
 
 }  // namespace chromeos