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

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 7a4ab3efe9677be769a50e1a951c067bb0316d68..6ca3439af251982dafc03aafe88982101c6bc670 100644
--- a/chrome/browser/chromeos/login/user_manager_impl.cc
+++ b/chrome/browser/chromeos/login/user_manager_impl.cc
@@ -221,6 +221,8 @@ UserManagerImpl::~UserManagerImpl() {
   }
   // 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_;
 }
 
@@ -248,6 +250,17 @@ 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) {
@@ -299,6 +312,7 @@ void UserManagerImpl::UserLoggedIn(const std::string& email,
 
   // 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();
 }
@@ -334,6 +348,9 @@ void UserManagerImpl::SwitchActiveUser(const std::string& email) {
   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.
@@ -1010,6 +1027,7 @@ void UserManagerImpl::RegularUserLoggedIn(const std::string& email,
                                     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);
 
@@ -1059,6 +1077,7 @@ void UserManagerImpl::LocallyManagedUserLoggedIn(
                                     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_) {
@@ -1494,4 +1513,13 @@ void UserManagerImpl::ReadPublicAccounts(base::ListValue* public_accounts) {
   }
 }
 
+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