Refactor ProfileInfoCache in c/b/ui/app_list

chrome/browser/profiles/profile_attributes_entry.cc

Index: chrome/browser/profiles/profile_attributes_entry.cc
diff --git a/chrome/browser/profiles/profile_attributes_entry.cc b/chrome/browser/profiles/profile_attributes_entry.cc
index 3adf2a0bffd53c2632b46826be473144bed427bc..c769c174cee95adddc1b39c4755ea50ab16d539a 100644
--- a/chrome/browser/profiles/profile_attributes_entry.cc
+++ b/chrome/browser/profiles/profile_attributes_entry.cc
@@ -3,8 +3,40 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/profiles/profile_attributes_entry.h"
+
+#include "base/i18n/string_compare.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 
+scoped_ptr<icu::Collator>
+    ProfileAttributesEntry::SortComparator::GetCollator() {
+  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));
+  return collator;
+}
+
+bool ProfileAttributesEntry::SortComparator::operator()(
+    const ProfileAttributesEntry* const a,
+    const ProfileAttributesEntry* const b) const {
+  if (!collator_)
+    return false;  // If collator is absent sorting fails with undefined result.

[tapted, 2016/01/31 23:04:37]

can this actually happen? I think this should DCHECK. A lot of sorting
algorithms will actually crash if the comparator doesn't provide "strict weak"
ordering, so better to crash sooner.

[lwchkg, 2016/02/01 06:31:20]

The constructor has already DCHECKed. This is only to prevent release version
from crashing. Anyway, thanks for the reminder of strict weak ordering. So I'll
provide a backup implementation (e.g. a default collation) that guarantees to
finish sorting.

[tapted, 2016/02/02 00:20:55]

That's not normally what we use DCHECK for. DCHECKs should never happen, and you
should never write code that considers that they could fail. See
https://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREAC...


If there's a real possibility that Collator::createInstance could fail, it
should be handled and tested in debug builds.

+
+  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())
+    return a_path.length() < b_path.length();
+  return a_path < b_path;
+}
+
 ProfileAttributesEntry::ProfileAttributesEntry()
     : profile_info_cache_(nullptr),
       profile_path_(base::FilePath()) {}
@@ -39,7 +71,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());
 }