Refactor ProfileInfoCache in c/b/ui/app_list

chrome/browser/profiles/profile_attributes_entry.h

 // Copyright 2015 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.
 
 #ifndef CHROME_BROWSER_PROFILES_PROFILE_ATTRIBUTES_ENTRY_H_
 #define CHROME_BROWSER_PROFILES_PROFILE_ATTRIBUTES_ENTRY_H_
 
 #include <stddef.h>
 
 #include <string>
 
 #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() {}
 
   // Gets the name of the profile, which is the one displayed in the User Menu.
   base::string16 GetName() const;
 
   base::string16 GetShortcutName() const;
   // Gets the path to the profile. Should correspond to the path passed to
   // ProfileAttributesStorage::GetProfileAttributesWithPath to get this entry.
   base::FilePath GetPath() const;
   base::Time GetActiveTime() const;
   // Gets the user name of the signed in profile. This is typically the email
   // address used to sign in and the empty string for profiles that aren't
   // signed in to chrome.
   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
   // that there are currently no background apps running. However, the action
   // which results is the same in both cases (thus far).
   bool GetBackgroundStatus() const;
   // Gets the GAIA full name associated with this profile if it's signed in.
   base::string16 GetGAIAName() const;
   // Gets the GAIA given name associated with this profile if it's signed in.
   base::string16 GetGAIAGivenName() const;
@@ skipping 81 matching lines @@
   friend class ProfileInfoCache;
   void Initialize(ProfileInfoCache* cache, const base::FilePath& path);
   size_t profile_index() const;
   ProfileInfoCache* profile_info_cache_;
   base::FilePath profile_path_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileAttributesEntry);
 };
 
 #endif  // CHROME_BROWSER_PROFILES_PROFILE_ATTRIBUTES_ENTRY_H_