Adding new user menu section to the SystemTrayMenu & refactoring of user access

chrome/browser/chromeos/login/user_manager_impl.cc

 // Copyright (c) 2013 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 <set>
 #include <vector>
 
@@ skipping 203 matching lines @@
 UserManagerImpl::~UserManagerImpl() {
   // Can't use STLDeleteElements because of the private destructor of User.
   for (UserList::iterator it = users_.begin(); it != users_.end();
        it = users_.erase(it)) {
     if (active_user_ == *it)
       active_user_ = NULL;
     delete *it;
   }
   // These are pointers to the same User instances that were in users_ list.
   logged_in_users_.clear();
+  lru_logged_in_users_.clear();
+
   delete active_user_;
 }
 
 void UserManagerImpl::Shutdown() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   cros_settings_->RemoveSettingsObserver(kAccountsPrefDeviceLocalAccounts,
                                          this);
   // Stop the session length limiter.
   session_length_limiter_.reset();
 
   if (device_local_account_policy_service_)
     device_local_account_policy_service_->RemoveObserver(this);
 }
 
 UserImageManager* UserManagerImpl::GetUserImageManager() {
   return user_image_manager_.get();
 }
 
 const UserList& UserManagerImpl::GetUsers() const {
   const_cast<UserManagerImpl*>(this)->EnsureUsersLoaded();
   return users_;
 }
 
 const UserList& UserManagerImpl::GetLoggedInUsers() const {
   return logged_in_users_;
 }
 
+const UserList& UserManagerImpl::GetLRULoggedInUsers() {
+  // If there is no user logged in, we return the active user as the only one.
+  if (lru_logged_in_users_.empty() && active_user_) {
+    temp_single_logged_in_users_.clear();

[Nikita (slow), 2013/05/17 17:00:32]

Why not use the same lru_logged_in_users_?

[Mr4D (OOO till 08-26), 2013/05/17 18:45:09]

Because as I was looking at the code it appears that there are some cases where
the |lru_logged_in_users_| can be empty, but there is an active user. This
active user can then later on apparently "convert" into a real user. If that
changes it could happen that the |lru_logged_in_users_| list could get more
users then there are. As such I'd rather have a dummy list for this case. An
alternative would have been to check the length of the users list for 0 before
adding a user to the LRU_ list and clearing it before. However - since the
execution order might change that seems rather asking for problems. Therefore I
have chosen to go this way. Again. If you feel strongly I can change this.

[Nikita (slow), 2013/05/17 19:15:47]

When does this happen?
I would really like to avoid just another user list if possible.

[Mr4D (OOO till 08-26), 2013/05/17 20:04:43]

Okay, let's address that then with the second patch.

+    temp_single_logged_in_users_.insert(temp_single_logged_in_users_.begin(),
+                                        active_user_);
+    return temp_single_logged_in_users_;
+  }
+  return lru_logged_in_users_;
+}
+
 void UserManagerImpl::UserLoggedIn(const std::string& email,
                                    const std::string& username_hash,
                                    bool browser_restart) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (!CommandLine::ForCurrentProcess()->HasSwitch(::switches::kMultiProfiles))
     DCHECK(!IsUserLoggedIn());
 
   if (active_user_)
     active_user_->set_is_active(false);
@@ skipping 31 matching lines @@
     session_length_limiter_.reset(new SessionLengthLimiter(NULL,
                                                            browser_restart));
   }
   DCHECK(active_user_);
   active_user_->set_is_logged_in(true);
   active_user_->set_is_active(true);
   active_user_->set_username_hash(username_hash);
 
   // Place user who just signed in to the top of the logged in users.
   logged_in_users_.insert(logged_in_users_.begin(), active_user_);
+  SetLRUUser(active_user_);
 
   NotifyOnLogin();
 }
 
 void UserManagerImpl::SwitchActiveUser(const std::string& email) {
   if (!CommandLine::ForCurrentProcess()->HasSwitch(::switches::kMultiProfiles))
     return;
 
   User* user = FindUserAndModify(email);
   if (!user) {
@@ skipping 15 matching lines @@
   if (user->username_hash().empty()) {
     NOTREACHED() << "Switching to a user that doesn't have username_hash set";
     return;
   }
 
   DCHECK(active_user_);
   active_user_->set_is_active(false);
   user->set_is_active(true);
   active_user_ = user;
 
+  // Move the user to the front.
+  SetLRUUser(active_user_);
+
   NotifyActiveUserHashChanged(active_user_->username_hash());
 
   // TODO(nkostylev): Notify session_manager on active user change.
   // http://crbug.com/230857
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_ACTIVE_USER_CHANGED,
       content::Source<UserManager>(this),
       content::Details<const User>(active_user_));
 }
 
@@ skipping 656 matching lines @@
     SaveUserDisplayName(active_user_->email(),
                         UTF8ToUTF16(active_user_->GetAccountName(true)));
     WallpaperManager::Get()->SetInitialUserWallpaper(email, true);
   }
 
   // Add the user to the front of the user list.
   ListPrefUpdate prefs_users_update(g_browser_process->local_state(),
                                     kRegularUsers);
   prefs_users_update->Insert(0, new base::StringValue(email));
   users_.insert(users_.begin(), active_user_);
+  SetLRUUser(active_user_);

[Nikita (slow), 2013/05/17 17:00:32]

You already move this user in UserLoggedIn().

[Mr4D (OOO till 08-26), 2013/05/17 18:45:09]

Following all calls - indeed! Done!

 
   user_image_manager_->UserLoggedIn(email, is_current_user_new_, false);
 
   if (!browser_restart) {
     // For GAIA login flow, logged in user wallpaper may not be loaded.
     WallpaperManager::Get()->EnsureLoggedInUserWallpaperLoaded();
   }
 
   default_pinned_apps_field_trial::SetupForUser(email, is_current_user_new_);
 
@@ skipping 29 matching lines @@
       is_current_user_new_ = true;
       WallpaperManager::Get()->SetInitialUserWallpaper(username, true);
     }
   }
 
   // Add the user to the front of the user list.
   ListPrefUpdate prefs_users_update(g_browser_process->local_state(),
                                     kRegularUsers);
   prefs_users_update->Insert(0, new base::StringValue(username));
   users_.insert(users_.begin(), active_user_);
+  SetLRUUser(active_user_);

[Nikita (slow), 2013/05/17 17:00:32]

Not needed, is handled in UserLoggedIn()

[Mr4D (OOO till 08-26), 2013/05/17 18:45:09]

Ditto.

 
   // Now that user is in the list, save display name.
   if (is_current_user_new_) {
     SaveUserDisplayName(active_user_->email(),
                         active_user_->GetDisplayName());
   }
 
   user_image_manager_->UserLoggedIn(username, is_current_user_new_, true);
   WallpaperManager::Get()->EnsureLoggedInUserWallpaperLoaded();
 
@@ skipping 415 matching lines @@
         }
         case DEVICE_LOCAL_ACCOUNT_TYPE_KIOSK_APP:
           // TODO(mnissler, nkostylev, bartfab): Process Kiosk Apps within the
           // standard login framework: http://crbug.com/234694
           break;
       }
     }
   }
 }
 
+void UserManagerImpl::SetLRUUser(User* user) {
+  UserList::iterator it = std::find(lru_logged_in_users_.begin(),
+                                    lru_logged_in_users_.end(),
+                                    user);
+  if (it != lru_logged_in_users_.end())
+    lru_logged_in_users_.erase(it);
+  lru_logged_in_users_.insert(lru_logged_in_users_.begin(), user);
+}
+
 }  // namespace chromeos