Refactor ProfileInfoCache in c/b/ui/app_list

ProfileInfoCache is being refactored into ProfileAttributesStorage and
ProfileAttributesEntry, which use profile paths instead of numerical
indices in the interface. See
https://codereview.chromium.org/1599013002/ for details.

This CL adds an comparator for sorting ProfileAttributesEntry, and also
converts the calls in c/b/ui/app_list to use the new interface.

BUG=305720

Review URL: https://codereview.chromium.org/1631373003

Cr-Commit-Position: refs/heads/master@{#374726}

chrome/browser/profiles/profile_attributes_entry.cc

@@ -39,7 +39,7 @@ base::string16 ProfileAttributesEntry::GetUserName() const {
   return profile_info_cache_->GetUserNameOfProfileAtIndex(profile_index());
 }
 
-const gfx::Image& ProfileAttributesEntry::GetAvatarIcon() {
+const gfx::Image& ProfileAttributesEntry::GetAvatarIcon() const {
   return profile_info_cache_->GetAvatarIconOfProfileAtIndex(profile_index());
 }
 

chrome/browser/profiles/profile_attributes_entry.h

@@ -39,7 +39,7 @@ class ProfileAttributesEntry {
   base::string16 GetUserName() const;
   // Gets the icon used as this profile's avatar. This might not be the icon
   // displayed in the UI if IsUsingGAIAPicture() is true.
-  const gfx::Image& GetAvatarIcon();
+  const gfx::Image& GetAvatarIcon() const;
   std::string GetLocalAuthCredentials() const;
   std::string GetPasswordChangeDetectionToken() const;
   // Note that a return value of false could mean an error in collection or

chrome/browser/profiles/profile_attributes_storage.h

@@ -44,6 +44,8 @@ class ProfileAttributesStorage {
   // Returns a vector containing one attributes entry per known profile. They
   // are not sorted in any particular order.
   virtual std::vector<ProfileAttributesEntry*> GetAllProfilesAttributes() = 0;
+  virtual std::vector<ProfileAttributesEntry*>
+      GetAllProfilesAttributesSortedByName() = 0;
 
   // Populates |entry| with the data for the profile at |path| and returns true
   // if the operation is successful and |entry| can be used. Returns false

chrome/browser/profiles/profile_info_cache.cc

@@ -4,11 +4,13 @@
 
 #include "chrome/browser/profiles/profile_info_cache.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/i18n/case_conversion.h"
+#include "base/i18n/string_compare.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
@@ -31,6 +33,7 @@
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/signin/core/common/profile_management_switches.h"
 #include "content/public/browser/browser_thread.h"
+#include "third_party/icu/source/i18n/unicode/coll.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image.h"
@@ -157,6 +160,32 @@ void DeleteBitmap(const base::FilePath& image_path) {
   base::DeleteFile(image_path, false);
 }
 
+// Compares two ProfileAttributesEntry using locale-sensitive comparison of
+// their names. For ties, the profile path is compared next.
+class ProfileAttributesSortComparator {
+ public:
+  explicit ProfileAttributesSortComparator(icu::Collator* collator);
+  bool operator()(const ProfileAttributesEntry* const a,
+                  const ProfileAttributesEntry* const b) const;
+ private:
+  icu::Collator* collator_;
+};
+
+ProfileAttributesSortComparator::ProfileAttributesSortComparator(
+    icu::Collator* collator) : collator_(collator) {}
+
+bool ProfileAttributesSortComparator::operator()(
+    const ProfileAttributesEntry* const a,
+    const ProfileAttributesEntry* const b) const {
+  UCollationResult result = base::i18n::CompareString16WithCollator(
+      *collator_, a->GetName(), b->GetName());
+  if (result != UCOL_EQUAL)
+    return result == UCOL_LESS;
+
+  // If the names are the same, then compare the paths, which must be unique.
+  return a->GetPath().value() < b->GetPath().value();
+}
+
 }  // namespace
 
 ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
@@ -1361,6 +1390,20 @@ ProfileInfoCache::GetAllProfilesAttributes() {
   return ret;
 }
 
+std::vector<ProfileAttributesEntry*>
+ProfileInfoCache::GetAllProfilesAttributesSortedByName() {
+  UErrorCode error_code = U_ZERO_ERROR;
+  // Use the default collator. The default locale should have been properly
+  // set by the time this constructor is called.
+  scoped_ptr<icu::Collator> collator(icu::Collator::createInstance(error_code));
+  DCHECK(U_SUCCESS(error_code));
+
+  std::vector<ProfileAttributesEntry*> ret = GetAllProfilesAttributes();
+  std::sort(ret.begin(), ret.end(),
+      ProfileAttributesSortComparator(collator.get()));
+  return ret;
+}
+
 bool ProfileInfoCache::GetProfileAttributesWithPath(
     const base::FilePath& path, ProfileAttributesEntry** entry) {
   if (GetNumberOfProfiles() == 0)

chrome/browser/profiles/profile_info_cache.h

@@ -201,6 +201,8 @@ class ProfileInfoCache : public ProfileInfoInterface,
   // Returns a vector containing one attributes entry per known profile. They
   // are not sorted in any particular order.
   std::vector<ProfileAttributesEntry*> GetAllProfilesAttributes() override;
+  std::vector<ProfileAttributesEntry*> GetAllProfilesAttributesSortedByName()
+      override;
   bool GetProfileAttributesWithPath(
       const base::FilePath& path,
       ProfileAttributesEntry** entry) override;

chrome/browser/ui/app_list/app_list_service_impl.cc

@@ -19,6 +19,7 @@
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_shutdown.h"
+#include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_list/app_list_view_delegate.h"
 #include "chrome/browser/ui/app_list/profile_loader.h"
@@ -93,8 +94,9 @@ class ProfileStoreImpl : public ProfileStore {
         weak_factory_(this) {
   }
 
-  void AddProfileObserver(ProfileInfoCacheObserver* observer) override {
-    profile_manager_->GetProfileInfoCache().AddObserver(observer);
+  void AddProfileObserver(ProfileAttributesStorage::Observer* observer)
+      override {
+    profile_manager_->GetProfileAttributesStorage().AddObserver(observer);
   }
 
   void LoadProfileAsync(const base::FilePath& path,
@@ -142,19 +144,19 @@ class ProfileStoreImpl : public ProfileStore {
   }
 
   bool IsProfileSupervised(const base::FilePath& profile_path) override {
-    ProfileInfoCache& profile_info =
-        g_browser_process->profile_manager()->GetProfileInfoCache();
-    size_t profile_index = profile_info.GetIndexOfProfileWithPath(profile_path);
-    return profile_index != std::string::npos &&
-        profile_info.ProfileIsSupervisedAtIndex(profile_index);
+    ProfileAttributesEntry* entry = nullptr;
+    bool has_entry = g_browser_process->profile_manager()->
+        GetProfileAttributesStorage().
+        GetProfileAttributesWithPath(profile_path, &entry);
+    return has_entry && entry->IsSupervised();
   }
 
   bool IsProfileLocked(const base::FilePath& profile_path) override {
-    ProfileInfoCache& profile_info =
-        g_browser_process->profile_manager()->GetProfileInfoCache();
-    size_t profile_index = profile_info.GetIndexOfProfileWithPath(profile_path);
-    return profile_index != std::string::npos &&
-        profile_info.ProfileIsSigninRequiredAtIndex(profile_index);
+    ProfileAttributesEntry* entry = nullptr;
+    bool has_entry = g_browser_process->profile_manager()->
+        GetProfileAttributesStorage().
+        GetProfileAttributesWithPath(profile_path, &entry);
+    return has_entry && entry->IsSigninRequired();
   }
 
  private:
@@ -342,7 +344,7 @@ void AppListServiceImpl::OnProfileWillBeRemoved(
     return;
 
   // Switch the app list over to a valid profile.
-  // Before ProfileInfoCache::DeleteProfileFromCache() calls this function,
+  // Before ProfileAttributesStorage::RemoveProfile() calls this function,
   // ProfileManager::ScheduleProfileForDeletion() will have checked to see if
   // the deleted profile was also "last used", and updated that setting with
   // something valid.

chrome/browser/ui/app_list/app_list_service_impl.h

@@ -13,7 +13,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_info_cache_observer.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/ui/app_list/app_list_service.h"
 #include "chrome/browser/ui/app_list/profile_loader.h"
 
@@ -30,7 +30,7 @@ class AppListServiceImplTestApi;
 
 // Parts of the AppListService implementation shared between platforms.
 class AppListServiceImpl : public AppListService,
-                           public ProfileInfoCacheObserver {
+                           public ProfileAttributesStorage::Observer {
  public:
   ~AppListServiceImpl() override;
 

chrome/browser/ui/app_list/app_list_service_mac.mm

@@ -19,7 +19,8 @@
 #import "chrome/browser/app_controller_mac.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/profiles/profile_info_cache.h"
+#include "chrome/browser/profiles/profile_attributes_entry.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_list/app_list_positioner.h"
 #include "chrome/browser/ui/app_list/app_list_service.h"
@@ -388,13 +389,11 @@ void AppListServiceMac::InitWithProfilePath(
     // Do not show the launcher window when the profile is locked, or if it
     // can't be displayed unpopulated. In the latter case, the Show will occur
     // in OnShimLaunch() or AppListService::HandleLaunchCommandLine().
-    const ProfileInfoCache& profile_info_cache =
-        g_browser_process->profile_manager()->GetProfileInfoCache();
-    size_t profile_index = profile_info_cache.
-        GetIndexOfProfileWithPath(profile_path);
-    if (profile_index != std::string::npos &&
-        !profile_info_cache.ProfileIsSigninRequiredAtIndex(profile_index) &&
-        ReadyToShow())
+    ProfileAttributesEntry* entry = nullptr;
+    bool has_entry = g_browser_process->profile_manager()->
+        GetProfileAttributesStorage().
+        GetProfileAttributesWithPath(profile_path, &entry);
+    if (has_entry && !entry->IsSigninRequired() && ReadyToShow())
       ShowWindowNearDock();
   }
 }

chrome/browser/ui/app_list/app_list_view_delegate.cc

@@ -19,7 +19,8 @@
 #include "chrome/browser/apps/scoped_keep_alive.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/profiles/profile_info_cache.h"
+#include "chrome/browser/profiles/profile_attributes_entry.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/search/hotword_service.h"
 #include "chrome/browser/search/hotword_service_factory.h"
@@ -107,16 +108,17 @@ void CreateShortcutInWebAppDir(
 }
 #endif
 
-void PopulateUsers(const ProfileInfoCache& profile_info,
-                   const base::FilePath& active_profile_path,
+void PopulateUsers(const base::FilePath& active_profile_path,
                    app_list::AppListViewDelegate::Users* users) {
   users->clear();
-  const size_t count = profile_info.GetNumberOfProfiles();
-  for (size_t i = 0; i < count; ++i) {
+  std::vector<ProfileAttributesEntry*> entries = g_browser_process->
+      profile_manager()->GetProfileAttributesStorage().
+      GetAllProfilesAttributesSortedByName();
+  for (const auto entry : entries) {
     app_list::AppListViewDelegate::User user;
-    user.name = profile_info.GetNameOfProfileAtIndex(i);
-    user.email = profile_info.GetUserNameOfProfileAtIndex(i);
-    user.profile_path = profile_info.GetPathOfProfileAtIndex(i);
+    user.name = entry->GetName();
+    user.email = entry->GetUserName();
+    user.profile_path = entry->GetPath();
     user.active = active_profile_path == user.profile_path;
     users->push_back(user);
   }
@@ -199,7 +201,7 @@ AppListViewDelegate::AppListViewDelegate(AppListControllerDelegate* controller)
     }
   }
 
-  profile_manager->GetProfileInfoCache().AddObserver(this);
+  profile_manager->GetProfileAttributesStorage().AddObserver(this);
   speech_ui_.reset(new app_list::SpeechUIModel);
 
 #if defined(GOOGLE_CHROME_BUILD)
@@ -225,8 +227,8 @@ AppListViewDelegate::~AppListViewDelegate() {
   // by a leaky singleton. Essential shutdown work must be done by observing
   // chrome::NOTIFICATION_APP_TERMINATING.
   SetProfile(NULL);
-  g_browser_process->profile_manager()->GetProfileInfoCache().RemoveObserver(
-      this);
+  g_browser_process->profile_manager()->GetProfileAttributesStorage().
+      RemoveObserver(this);
 
   SigninManagerFactory* factory = SigninManagerFactory::GetInstance();
   if (factory)
@@ -330,9 +332,7 @@ void AppListViewDelegate::SetUpProfileSwitcher() {
     return;
 
   // Populate the app list users.
-  PopulateUsers(g_browser_process->profile_manager()->GetProfileInfoCache(),
-                profile_->GetPath(),
-                &users_);
+  PopulateUsers(profile_->GetPath(), &users_);
 
   FOR_EACH_OBSERVER(
       app_list::AppListViewDelegateObserver, observers_, OnProfilesChanged());

chrome/browser/ui/app_list/app_list_view_delegate.h

@@ -17,7 +17,7 @@
 #include "base/memory/scoped_vector.h"
 #include "base/observer_list.h"
 #include "base/scoped_observer.h"
-#include "chrome/browser/profiles/profile_info_cache_observer.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/search/hotword_client.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/ui/app_list/start_page_observer.h"
@@ -61,7 +61,7 @@ class AppSyncUIStateWatcher;
 class AppListViewDelegate : public app_list::AppListViewDelegate,
                             public app_list::StartPageObserver,
                             public HotwordClient,
-                            public ProfileInfoCacheObserver,
+                            public ProfileAttributesStorage::Observer,
                             public SigninManagerBase::Observer,
                             public SigninManagerFactory::Observer,
                             public content::NotificationObserver,
@@ -158,7 +158,7 @@ class AppListViewDelegate : public app_list::AppListViewDelegate,
   void GoogleSignedOut(const std::string& account_id,
                        const std::string& username) override;
 
-  // Overridden from ProfileInfoCacheObserver:
+  // Overridden from ProfileAttributesStorage::Observer:
   void OnProfileAdded(const base::FilePath& profile_path) override;
   void OnProfileWasRemoved(const base::FilePath& profile_path,
                            const base::string16& profile_name) override;

chrome/browser/ui/app_list/profile_store.h

@@ -7,7 +7,7 @@
 
 #include "base/callback_forward.h"
 #include "base/files/file_path.h"
-#include "chrome/browser/profiles/profile_info_cache_observer.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
 
 class Profile;
 
@@ -15,7 +15,8 @@ class Profile;
 class ProfileStore {
  public:
   virtual ~ProfileStore() {}
-  virtual void AddProfileObserver(ProfileInfoCacheObserver* observer) = 0;
+  virtual void AddProfileObserver(
+      ProfileAttributesStorage::Observer* observer) = 0;
 
   // Loads the profile at |path| and calls |callback| when its done. A NULL
   // Profile* represents an error.

chrome/browser/ui/app_list/test/fake_profile_store.cc

@@ -28,15 +28,15 @@ void FakeProfileStore::LoadProfile(Profile* profile) {
 
 void FakeProfileStore::RemoveProfile(Profile* profile) {
   base::FilePath path(profile->GetPath());
-  FOR_EACH_OBSERVER(ProfileInfoCacheObserver, observer_list_,
+  FOR_EACH_OBSERVER(ProfileAttributesStorage::Observer, observer_list_,
                     OnProfileWillBeRemoved(path));
   loaded_profiles_.erase(path);
-  FOR_EACH_OBSERVER(ProfileInfoCacheObserver, observer_list_,
+  FOR_EACH_OBSERVER(ProfileAttributesStorage::Observer, observer_list_,
                     OnProfileWasRemoved(path, base::string16()));
 }
 
 void FakeProfileStore::AddProfileObserver(
-    ProfileInfoCacheObserver* observer) {
+    ProfileAttributesStorage::Observer* observer) {
   observer_list_.AddObserver(observer);
 }
 

chrome/browser/ui/app_list/test/fake_profile_store.h

@@ -9,6 +9,7 @@
 
 #include "base/callback.h"
 #include "base/observer_list.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/ui/app_list/profile_store.h"
 
 class PrefService;
@@ -23,7 +24,8 @@ class FakeProfileStore : public ProfileStore {
   void RemoveProfile(Profile* profile);
 
   // ProfileStore overrides.
-  void AddProfileObserver(ProfileInfoCacheObserver* observer) override;
+  void AddProfileObserver(ProfileAttributesStorage::Observer* observer)
+      override;
   void LoadProfileAsync(const base::FilePath& path,
                         base::Callback<void(Profile*)> callback) override;
   Profile* GetProfileByPath(const base::FilePath& path) override;
@@ -38,7 +40,7 @@ class FakeProfileStore : public ProfileStore {
   typedef std::map<base::FilePath, base::Callback<void(Profile*)> >
       CallbacksByPath;
   CallbacksByPath callbacks_;
-  base::ObserverList<ProfileInfoCacheObserver> observer_list_;
+  base::ObserverList<ProfileAttributesStorage::Observer> observer_list_;
   typedef std::map<base::FilePath, Profile*> ProfilesByPath;
   ProfilesByPath loaded_profiles_;
 };