Refactor of ProfileInfoCache in c/b/notifications

chrome/browser/notifications/message_center_settings_controller.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/notifications/message_center_settings_controller.h"
 
 #include <algorithm>
+#include <string>
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/i18n/string_compare.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "base/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/extensions/app_icon_loader_impl.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/notifications/desktop_notification_profile_util.h"
 #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"
 #include "chrome/common/extensions/extension_constants.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/favicon_base/favicon_types.h"
 #include "components/history/core/browser/history_types.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
@@ skipping 17 matching lines @@
 using message_center::Notifier;
 using message_center::NotifierId;
 
 namespace message_center {
 
 class ProfileNotifierGroup : public message_center::NotifierGroup {
  public:
   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() {}
 
   Profile* profile() const { return profile_; }
 
  private:
   Profile* profile_;
 };
 
 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_ =
       g_browser_process->profile_manager()->GetProfileByPath(profile_path);
 }
 
 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) {
 }
 
 }  // namespace message_center
 
 namespace {
 class NotifierComparator {
  public:
   explicit NotifierComparator(icu::Collator* collator) : collator_(collator) {}
 
   bool operator() (Notifier* n1, Notifier* n2) {
     if (n1->notifier_id.type != n2->notifier_id.type)
       return n1->notifier_id.type < n2->notifier_id.type;
 
     if (collator_) {
       return base::i18n::CompareString16WithCollator(*collator_, n1->name,
                                                      n2->name) == UCOL_LESS;
     }
     return n1->name < n2->name;
   }
 
  private:
   icu::Collator* collator_;
 };
 
 }  // 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.
   registrar_.Add(this,
                  chrome::NOTIFICATION_PROFILE_CREATED,
                  content::NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this,
                  chrome::NOTIFICATION_PROFILE_ADDED,
                  content::NotificationService::AllBrowserContextsAndSources());
   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)
   // UserManager may not exist in some tests.
   if (user_manager::UserManager::IsInitialized())
     user_manager::UserManager::Get()->AddSessionStateObserver(this);
 #endif
 }
 
 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())
     user_manager::UserManager::Get()->RemoveSessionStateObserver(this);
 #endif
 }
 
 void MessageCenterSettingsController::AddObserver(
     message_center::NotifierSettingsObserver* observer) {
   observers_.AddObserver(observer);
@@ skipping 322 matching lines @@
 
   user_manager::User* user = user_manager->GetActiveUser();
   Profile* profile =
       chromeos::ProfileHelper::Get()->GetProfileByUserUnsafe(user);
   DCHECK(profile);
 
   scoped_ptr<message_center::ProfileNotifierGroup> group(
       new message_center::ProfileNotifierGroup(gfx::Image(user->GetImage()),
                                                user->GetDisplayName(),
                                                user->GetDisplayName(),
-                                               0,
                                                profile));
 
   notifier_groups_.push_back(std::move(group));
 
   FOR_EACH_OBSERVER(message_center::NotifierSettingsObserver,
                     observers_,
                     NotifierGroupChanged());
 }
 #endif
 
 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;
 
 #if defined(OS_CHROMEOS)
     // Allows the active user only.
     // UserManager may not exist in some tests.
     if (user_manager::UserManager::IsInitialized()) {
       user_manager::UserManager* user_manager =
           user_manager::UserManager::Get();
       if (chromeos::ProfileHelper::Get()->GetUserByProfile(group->profile()) !=
@@ skipping 31 matching lines @@
             weak_factory_.GetWeakPtr()));
   }
 #endif
 
   if (notify) {
     FOR_EACH_OBSERVER(message_center::NotifierSettingsObserver,
                       observers_,
                       NotifierGroupChanged());
   }
 }