Multi-Profiles: Fix choosing random profile avatar

chrome/browser/profiles/profile_info_cache.cc

 // Copyright (c) 2011 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/format_macros.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/rand_util.h"
@@ skipping 42 matching lines @@
   IDR_PROFILE_AVATAR_20,
   IDR_PROFILE_AVATAR_21,
   IDR_PROFILE_AVATAR_22,
   IDR_PROFILE_AVATAR_23,
   IDR_PROFILE_AVATAR_24,
   IDR_PROFILE_AVATAR_25,
 };
 
 const size_t kDefaultAvatarIconsCount = arraysize(kDefaultAvatarIconResources);
 
-// Returns true if the resource ID belongs to a generic avatar icon.
-bool IsAvatarIconGeneric(int icon_id) {
-  return icon_id == IDR_PROFILE_AVATAR_0 ||
-         icon_id == IDR_PROFILE_AVATAR_1 ||
-         icon_id == IDR_PROFILE_AVATAR_2 ||
-         icon_id == IDR_PROFILE_AVATAR_3 ||
-         icon_id == IDR_PROFILE_AVATAR_4 ||
-         icon_id == IDR_PROFILE_AVATAR_5 ||
-         icon_id == IDR_PROFILE_AVATAR_6 ||
-         icon_id == IDR_PROFILE_AVATAR_7;
-}
+// The first 8 icons are gendar netural.

[Miranda Callahan, 2011/10/11 19:17:51]

I would actually stick with calling these "generic" -- because the majority of
the icons are gender neutral (cupcake, horse, dog, pizza, etc.)

[sail, 2011/10/11 21:44:54]

Done.

+const size_t kGenderNeutralCount = 8;

[Miranda Callahan, 2011/10/11 19:17:51]

same: kGenericIconCount

[sail, 2011/10/11 21:44:54]

Done.

 
 } // namespace
 
 ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
                                    const FilePath& user_data_dir)
     : prefs_(prefs),
       user_data_dir_(user_data_dir) {
   // Populate the cache
   const DictionaryValue* cache =
       prefs_->GetDictionary(prefs::kProfileInfoCache);
@@ skipping 135 matching lines @@
       if (GetNameOfProfileAtIndex(i) == name) {
         name_found = true;
         break;
       }
     }
     if (!name_found)
       return name;
   }
 }
 
-int ProfileInfoCache::ChooseAvatarIconIndexForNewProfile() {
-  // Start with a random icon to introduce variety.
-  size_t rand_start_index = base::RandInt(0, GetDefaultAvatarIconCount() - 1);
-  for (size_t icon_index = 0; icon_index < GetDefaultAvatarIconCount();
-       ++icon_index) {
-    size_t rand_icon_index =
-        (icon_index + rand_start_index) % GetDefaultAvatarIconCount();
-    if (IsAvatarIconGeneric(GetDefaultAvatarIconResourceIDAtIndex(
-        rand_icon_index)))
-      continue;
+bool ProfileInfoCache::IconIndexIsUnique(size_t icon_index) const {
+  for (size_t i = 0; i < GetNumberOfProfiles(); ++i) {
+    if (GetAvatarIconIndexOfProfileAtIndex(i) == icon_index)
+      return false;
+  }
+  return true;
+}
 
-    bool icon_found = false;
-    for (size_t i = 0; i < GetNumberOfProfiles(); ++i) {
-      if (GetAvatarIconIndexOfProfileAtIndex(i) == rand_icon_index) {
-        icon_found = true;
-        break;
-      }
+bool ProfileInfoCache::ChooseAvatarIconIndexForNewProfile(
+    bool allow_gender_neutral,
+    bool must_be_unique,
+    size_t* out_icon_index) const {
+  size_t start = allow_gender_neutral ? 0 : kGenderNeutralCount;
+  size_t end = GetDefaultAvatarIconCount();
+  size_t count = end - start;
+
+  int rand = base::RandInt(0, count);
+  for (size_t i = 0; i < count; ++i) {
+    size_t icon_index = start + (rand + i) %  count;
+    if (!must_be_unique || IconIndexIsUnique(icon_index)) {
+      *out_icon_index = icon_index;
+      return true;
     }
-    if (!icon_found)
-      return rand_icon_index;
   }
 
-  // If there's no unique icon then just use the random one.
-  return rand_start_index;
+  return false;
+}
+
+size_t ProfileInfoCache::ChooseAvatarIconIndexForNewProfile() const {
+  size_t icon_index = 0;
+  // Try to find a unique, non-generic icon.
+  if (ChooseAvatarIconIndexForNewProfile(false, true, &icon_index))
+    return icon_index;
+  // Try to find any unique icon.
+  if (ChooseAvatarIconIndexForNewProfile(true, true, &icon_index))
+    return icon_index;
+  // Settle for any random icon, even if it's not unique.
+  if (ChooseAvatarIconIndexForNewProfile(true, false, &icon_index))
+    return icon_index;
+
+  DCHECK(false);

[Miranda Callahan, 2011/10/11 19:17:51]

Are we doing DCHECK(false) instead of NOTREACHED() now?

[sail, 2011/10/11 21:44:54]

Done.

+  return 0;
 }
 
 const FilePath& ProfileInfoCache::GetUserDataDir() const {
   return user_data_dir_;
 }
 
 // static
 size_t ProfileInfoCache::GetDefaultAvatarIconCount() {
   return kDefaultAvatarIconsCount;
 }
@@ skipping 72 matching lines @@
       if (key_compare < 0)
         return sorted_keys_.begin() + i;
     }
   }
   return sorted_keys_.end();
 }
 
 void ProfileInfoCache::RegisterPrefs(PrefService* prefs) {
   prefs->RegisterDictionaryPref(prefs::kProfileInfoCache);
 }