Refactor ProfileInfoCache in c/b/ui/app_list

chrome/browser/profiles/profile_attributes_entry.h

Index: chrome/browser/profiles/profile_attributes_entry.h
diff --git a/chrome/browser/profiles/profile_attributes_entry.h b/chrome/browser/profiles/profile_attributes_entry.h
index 081c5c55557a49fa77cd061df1cae86cf7c506bf..133cafac20d99aadd56d6a2375c4acd5963b4542 100644
--- a/chrome/browser/profiles/profile_attributes_entry.h
+++ b/chrome/browser/profiles/profile_attributes_entry.h
@@ -11,17 +11,34 @@
 
 #include "base/files/file_path.h"
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
 #include "base/time/time.h"
+#include "third_party/icu/source/common/unicode/uversion.h"

[tapted, 2016/02/04 23:10:35]

Can this be moved to the.cc? That might remove the need for the .gyp changes.
Doesn't seem right to force the test support library to depend on this, since
the file that `owns` the dependency isn't compiled as part of test support

[lwchkg, 2016/02/05 00:56:22]

I guess it simply means that I did not add a proper test for SortComparator.
That comparator is complex and deserves some testing.

Anthony: If we provide ProfileAttributesStorage::SortEntries(std::vector<...>)
instead of ProfileAttributesEntry::SortComparator, then we don't need this file
here (and the gyp changes). I guess you still want sorting implemented in the
current way. Anyway, since you're the owner I'll wait for your decision.

[anthonyvd, 2016/02/05 15:36:31]

I'm fine with having a function in ProfileAttributesStorage to get the entries
as a sorted vector. Something like GetAllProfileAttributesSortedByName or
similar. It now looks like it's the better thing to do.

 
 namespace gfx {
 class Image;
 }
+namespace U_ICU_NAMESPACE {

[tapted, 2016/02/04 23:10:36]

does `icu` work in place of U_ICU_NAMESPACE ? That should allow the uversion.h
include to be moved, and it's weird that you would need U_ICU_NAMESPACE here and
not when declaring the scoped_ptr member.

[lwchkg, 2016/02/05 00:56:22]

No. The namespace icu is alias to U_ICU_NAMESPACE (which currently resolves to
icu54, but it may be changed when we upgrade icu).

Sadly U_ICU_NAMESPACE is defined in uversions.h. And those gyp/gn changes is
still needed. The files included by uversions.h should be only a small subset of
"unicode/coll.h".

+class Collator;
+}
 
 class ProfileInfoCache;
 
 class ProfileAttributesEntry {
  public:
+  // Compares two ProfileAttributesEntry using locale-sensitive comparison of
+  // their names. For ties, the profile path is compared next.
+  class SortComparator {
+   public:
+    SortComparator();
+    ~SortComparator();
+    bool operator()(const ProfileAttributesEntry* const a,
+                    const ProfileAttributesEntry* const b) const;
+   private:
+    scoped_ptr<icu::Collator> collator_;
+  };

[tapted, 2016/02/04 23:10:36]

nit: DISALLOW_COPY_AND_ASSIGN(..)

[lwchkg, 2016/02/05 00:56:22]

Thanks. Forgot to add it back.

+
   ProfileAttributesEntry();
   virtual ~ProfileAttributesEntry() {}
 
@@ -39,7 +56,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