Improve the ProfileInfoCache API.

chrome/browser/profiles/profile_info_cache.cc

Index: chrome/browser/profiles/profile_info_cache.cc
diff --git a/chrome/browser/profiles/profile_info_cache.cc b/chrome/browser/profiles/profile_info_cache.cc
index 2c1d80f166e7e4edac254274f0375594ff4ffb6f..ccdd553c54086b6dd1c82ecd1569463009dfd67a 100644
--- a/chrome/browser/profiles/profile_info_cache.cc
+++ b/chrome/browser/profiles/profile_info_cache.cc
@@ -1238,3 +1238,44 @@ void ProfileInfoCache::MigrateLegacyProfileNamesAndDownloadAvatars() {
   }
 #endif
 }
+
+// ProfileMetadataStorage implementation

[Mike Lerman, 2015/07/02 18:01:30]

I think you can remove this comment.

[anthonyvd, 2015/07/07 21:05:32]

Done.

+void ProfileInfoCache::AddProfile(
+    const base::FilePath& profile_path,
+    const base::string16& name,
+    const std::string& gaia_id,
+    const base::string16& user_name,
+    size_t icon_index,
+    const std::string& supervised_user_id) {
+  AddProfileToCache(
+      profile_path, name, gaia_id, user_name, icon_index, supervised_user_id);
+}
+
+void ProfileInfoCache::DeleteProfile(const base::FilePath& profile_path) {
+  DeleteProfileFromCache(profile_path);
+}
+
+std::vector<ProfileAttributesEntry>
+ProfileInfoCache::GetAllProfilesAttributes() {
+  std::vector<ProfileAttributesEntry> ret;
+  for (size_t i = 0; i < GetNumberOfProfiles(); ++i) {
+    ProfileAttributesEntry entry(this, i);
+    ret.push_back(entry);
+  }
+  return ret;
+}
+
+bool ProfileInfoCache::GetProfileAttributesWithPath(
+    const base::FilePath& path, ProfileAttributesEntry* entry) {
+  if (GetNumberOfProfiles() == 0) {
+    return false;
+  }
+
+  size_t index = GetIndexOfProfileWithPath(path);
+  if (index == std::string::npos) {
+    return false;
+  }
+
+  *entry = ProfileAttributesEntry(this, index);

[Mike Lerman, 2015/07/02 18:01:30]

you're explicitly making use of operator=(). You should probably either define
operator=() explicitly or put a comment as to why you're not putting
DISALLOW_COPY_AND_ASSIGN.

[Mike Lerman, 2015/07/02 18:01:30]

This is fragile because the index of a Profile can change. I can call this
method on a Profile like /MyProfile which has index 2, call SetName(), and now
/MyProfile has index of 1 and some other profile has index of 2, and now further
accessing methods on the ProfileAttributesEntry are working on the wrong object.
I think you have to pass the whole profile_path into the ProfileAttributesEntry
constructor and do the lookup each time.

Please add a UnitTest that triggers this scenario, to make sure we won't trip it
up later!

[anthonyvd, 2015/07/07 21:05:32]

Having the storage own the entries makes DISALLOW_COPY_AND_ASSIGN easier so
done.

[anthonyvd, 2015/07/07 21:05:32]

You're absolutely right, done.

In the future though, the entry should hopefully never be invalidated this way
(unless it's deleted I guess) since we won't be resorting the internal storage.
At that point however, this path/index dance won't exist anymore. Until then,
this is better!

+  return true;
+}