Refactor ProfileInfoCache in c/b/ui/app_list

chrome/browser/profiles/profile_info_cache.cc

 // Copyright (c) 2012 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/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"
 #include "base/profiler/scoped_tracker.h"
 #include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_avatar_downloader.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #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"
 #include "ui/gfx/image/image_util.h"
 
 #if defined(ENABLE_SUPERVISED_USERS)
 #include "chrome/browser/supervised_user/supervised_user_constants.h"
 #endif
 
 using content::BrowserThread;
@@ skipping 106 matching lines @@
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   if (!base::PathExists(file_path))
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
 }
 
 void DeleteBitmap(const base::FilePath& image_path) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   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;
+
+  // Compare the profile paths. Since the paths are "[userdir]\Profile #", so
+  // comparisons take care of the numbers represented by # only.
+  base::FilePath::StringType a_path = a->GetPath().value();
+  base::FilePath::StringType b_path = b->GetPath().value();
+  if (a_path.length() != b_path.length())

[anthonyvd, 2016/02/08 21:58:42]

I think this check isn't needed. base::FilePath::StringType is a string type
(std::string/std::wstring) and their string comparison functions already handle
the different length cases that way. For example, std::string::compare (called
by std::string::operator<) considers a string smaller if "Either the value of
the first character that does not match is lower in the compared string, or all
compared characters match but the compared string is shorter." 

[tapted, 2016/02/08 22:33:36]

I read this as solving the issue that for profiles named "Default", "Profile 1",
"Profile 2", ...., "Profile 10", then "Profile 10" would sort before "Profile
2". With this extra check, "Profile 10" will sort after "Profile 9" since the
string is longer.

But if someone has 11 profiles that they've all given the same name, they are
maybe an extreme example and can deal with it themselves.

[lwchkg, 2016/02/10 19:44:20]

You're right. If the profile names are the same, the user deliberately confuses
him/herself, and can probably deal with any sorting results. So I'm removing the
length check before commit.

+    return a_path.length() < b_path.length();
+  return a_path < b_path;
+}
+
 }  // namespace
 
 ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
                                    const base::FilePath& user_data_dir)
     : prefs_(prefs),
       user_data_dir_(user_data_dir),
       disable_avatar_download_for_testing_(false) {
   // Populate the cache
   DictionaryPrefUpdate update(prefs_, prefs::kProfileInfoCache);
   base::DictionaryValue* cache = update.Get();
@@ skipping 1184 matching lines @@
   std::vector<ProfileAttributesEntry*> ret;
   for (size_t i = 0; i < GetNumberOfProfiles(); ++i) {
     ProfileAttributesEntry* entry;
     if (GetProfileAttributesWithPath(GetPathOfProfileAtIndex(i), &entry)) {
       ret.push_back(entry);
     }
   }
   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)
     return false;
 
   if (GetIndexOfProfileWithPath(path) == std::string::npos)
     return false;
 
   if (profile_attributes_entries_.find(path) ==
       profile_attributes_entries_.end()) {
     // The profile info is in the cache but its entry isn't created yet, insert
     // it in the map.
     scoped_ptr<ProfileAttributesEntry> new_entry(new ProfileAttributesEntry());
     profile_attributes_entries_.add(path, std::move(new_entry));
     profile_attributes_entries_.get(path)->Initialize(this, path);
   }
 
   *entry = profile_attributes_entries_.get(path);
   return true;
 }