Improve the ProfileInfoCache API.

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 "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
@@ skipping 1220 matching lines @@
   std::vector<base::FilePath>::const_iterator it;
   for (it = profiles_to_rename.begin(); it != profiles_to_rename.end(); ++it) {
     size_t profile_index = GetIndexOfProfileWithPath(*it);
     SetProfileIsUsingDefaultNameAtIndex(profile_index, true);
     // This will assign a new "Person %d" type name and re-sort the cache.
     SetNameOfProfileAtIndex(profile_index, ChooseNameForNewProfile(
         GetAvatarIconIndexOfProfileAtIndex(profile_index)));
   }
 #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;
+}