Refactor of ProfileInfoCache in c/b/notifications

chrome/browser/notifications/message_center_settings_controller.cc

Index: chrome/browser/notifications/message_center_settings_controller.cc
diff --git a/chrome/browser/notifications/message_center_settings_controller.cc b/chrome/browser/notifications/message_center_settings_controller.cc
index f67ee396c3e2f1b351e7e79d4e8798a5c9193cea..883ca011ab7ba2abd54ccd75995737199b8bf6bd 100644
--- a/chrome/browser/notifications/message_center_settings_controller.cc
+++ b/chrome/browser/notifications/message_center_settings_controller.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/notifications/message_center_settings_controller.h"
 
 #include <algorithm>
+#include <string>
 #include <utility>
 
 #include "base/command_line.h"
@@ -22,6 +23,8 @@
 #include "chrome/browser/notifications/notifier_state_tracker.h"
 #include "chrome/browser/notifications/notifier_state_tracker_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_attributes_entry.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/extensions/api/notifications.h"
@@ -59,12 +62,10 @@ class ProfileNotifierGroup : public message_center::NotifierGroup {
   ProfileNotifierGroup(const gfx::Image& icon,
                        const base::string16& display_name,
                        const base::string16& login_info,
-                       size_t index,
                        const base::FilePath& profile_path);
   ProfileNotifierGroup(const gfx::Image& icon,
                        const base::string16& display_name,
                        const base::string16& login_info,
-                       size_t index,
                        Profile* profile);
   virtual ~ProfileNotifierGroup() {}
 
@@ -77,9 +78,8 @@ class ProfileNotifierGroup : public message_center::NotifierGroup {
 ProfileNotifierGroup::ProfileNotifierGroup(const gfx::Image& icon,
                                            const base::string16& display_name,
                                            const base::string16& login_info,
-                                           size_t index,
                                            const base::FilePath& profile_path)
-    : message_center::NotifierGroup(icon, display_name, login_info, index),
+    : message_center::NotifierGroup(icon, display_name, login_info),
       profile_(NULL) {
   // Try to get the profile
   profile_ =
@@ -89,9 +89,8 @@ ProfileNotifierGroup::ProfileNotifierGroup(const gfx::Image& icon,
 ProfileNotifierGroup::ProfileNotifierGroup(const gfx::Image& icon,
                                            const base::string16& display_name,
                                            const base::string16& login_info,
-                                           size_t index,
                                            Profile* profile)
-    : message_center::NotifierGroup(icon, display_name, login_info, index),
+    : message_center::NotifierGroup(icon, display_name, login_info),
       profile_(profile) {
 }
 
@@ -120,11 +119,10 @@ class NotifierComparator {
 }  // namespace
 
 MessageCenterSettingsController::MessageCenterSettingsController(
-    ProfileInfoCache* profile_info_cache)
+    ProfileAttributesStorage& storage)
     : current_notifier_group_(0),
-      profile_info_cache_(profile_info_cache),
+      storage_(storage),
       weak_factory_(this) {
-  DCHECK(profile_info_cache_);
   // The following events all represent changes that may need to be reflected in
   // the profile selector context menu, so listen for them all.  We'll just
   // rebuild the list when we get any of them.
@@ -137,7 +135,7 @@ MessageCenterSettingsController::MessageCenterSettingsController(
   registrar_.Add(this,
                  chrome::NOTIFICATION_PROFILE_DESTROYED,
                  content::NotificationService::AllBrowserContextsAndSources());
-  g_browser_process->profile_manager()->GetProfileInfoCache().AddObserver(this);
+  storage_.AddObserver(this);
   RebuildNotifierGroups(false);
 
 #if defined(OS_CHROMEOS)
@@ -148,8 +146,7 @@ MessageCenterSettingsController::MessageCenterSettingsController(
 }
 
 MessageCenterSettingsController::~MessageCenterSettingsController() {
-  g_browser_process->profile_manager()->
-      GetProfileInfoCache().RemoveObserver(this);
+  storage_.RemoveObserver(this);
 #if defined(OS_CHROMEOS)
   // UserManager may not exist in some tests.
   if (user_manager::UserManager::IsInitialized())
@@ -492,7 +489,6 @@ void MessageCenterSettingsController::CreateNotifierGroupForGuestLogin() {
       new message_center::ProfileNotifierGroup(gfx::Image(user->GetImage()),
                                                user->GetDisplayName(),
                                                user->GetDisplayName(),
-                                               0,
                                                profile));
 
   notifier_groups_.push_back(std::move(group));
@@ -507,15 +503,15 @@ void MessageCenterSettingsController::RebuildNotifierGroups(bool notify) {
   notifier_groups_.clear();
   current_notifier_group_ = 0;
 
-  const size_t count = profile_info_cache_->GetNumberOfProfiles();
-  for (size_t i = 0; i < count; ++i) {
+  std::vector<ProfileAttributesEntry*> entries =
+      storage_.GetAllProfilesAttributes();

[lwchkg, 2016/02/07 16:36:23]

Is it needed to sort the entries in a particular order? I guess it's a no here.

[dewittj, 2016/02/08 18:07:10]

At this point, I believe the number of profiles will always be 1 since this code
is only used in ChromeOS now.  |index| used to be used for sorting purposes.  So
long as GetAllPRofilesAttributes returns a reasonably stable sort order it
should be fine.

[lwchkg, 2016/02/08 19:06:43]

Confused. Does the "1" count the profiles that are not yet open?
Anyway, I want to know what is meant exactly by "reasonably stable sort order".
We don't provide a guarantee that entries GetAllProfileAttributes is in any
order (sorting is in another CL, still being reviewed), so if you rely on any
sort order, then the code here will not work properly. I do suspect that
"reasonably stable sort order" means it's okay not to sort the entries.

[dewittj, 2016/02/08 19:11:45]

This code is used to show a UI so users can choose a profile to configure.  The
reason I say "reasonably stable" is that we don't really want the order changing
every time the user interacts with the config page, if we can avoid it.

In ChromeOS there is only one active user profile, so there will only really
ever be one shown - this is why there the sort order is moot anyway.

[lwchkg, 2016/02/08 19:22:00]

I see. How can I access that UI?

[dewittj, 2016/02/08 19:28:03]

1. Get a notification (could be from
https://tests.peter.sh/notification-generator/)
2. Click on the notification center icon in the shelf
3. Click the cog icon in the notification center

If there is more than one profile, a drop-down will be shown.  Again, though, on
ChromeOS since tehre is only one active user we will never show the drop down. 
The UI is inaccessible on other platforms.

[lwchkg, 2016/02/10 14:57:24]

Thanks for explanation. I'll just land the CL.

+  for (const auto entry : entries) {
     scoped_ptr<message_center::ProfileNotifierGroup> group(
         new message_center::ProfileNotifierGroup(
-            profile_info_cache_->GetAvatarIconOfProfileAtIndex(i),
-            profile_info_cache_->GetNameOfProfileAtIndex(i),
-            profile_info_cache_->GetUserNameOfProfileAtIndex(i),
-            i,
-            profile_info_cache_->GetPathOfProfileAtIndex(i)));
+            entry->GetAvatarIcon(),
+            entry->GetName(),
+            entry->GetUserName(),
+            entry->GetPath()));
     if (group->profile() == NULL)
       continue;