[Profiles] Improve random selection of colors for new profiles

This CL improves the algorithm of color selection to disallow colors
that are taken by other profiles. To this end, logic to generate
ProfileThemeColors is consolidated in profile_colors_util.h. The exact
same logic is then used to generate ProfileThemeColors from the list of
all predefined profile colors.

A next CL will improve the logic further: to exclude the first line
of predefined profile colors (unless all colors are taken) and to
prefer colors on the same line (for sign-in interception).

Bug: 1108295
Change-Id: I6ed155dfa4376ea195e1a5a0f51586319d5ad0ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2529123
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826295}

chrome/browser/profiles/profile_attributes_entry.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/signin/signin_util.h"
+#include "chrome/browser/ui/signin/profile_colors_util.h"
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/common/pref_names.h"
 #include "components/policy/core/browser/browser_policy_connector.h"
@@ -41,7 +42,6 @@
 
 #if !defined(OS_ANDROID)
 #include "chrome/browser/themes/theme_properties.h"  // nogncheck crbug.com/1125897
-#include "chrome/browser/ui/signin/profile_colors_util.h"
 #endif
 
 namespace {
@@ -104,19 +104,6 @@ bool ShouldShowGenericColoredAvatar(size_t avatar_icon_index) {
 
 }  // namespace
 
-bool ProfileThemeColors::operator==(const ProfileThemeColors& other) const {
-  return std::tie(this->profile_highlight_color,
-                  this->default_avatar_fill_color,
-                  this->default_avatar_stroke_color) ==
-         std::tie(other.profile_highlight_color,
-                  other.default_avatar_fill_color,
-                  other.default_avatar_stroke_color);
-}
-
-bool ProfileThemeColors::operator!=(const ProfileThemeColors& other) const {
-  return !(*this == other);
-}
-
 const char ProfileAttributesEntry::kSupervisedUserId[] = "managed_user_id";
 const char ProfileAttributesEntry::kIsOmittedFromProfileListKey[] =
     "is_omitted_from_profile_list";
@@ -449,6 +436,11 @@ size_t ProfileAttributesEntry::GetAvatarIconIndex() const {
 }
 
 ProfileThemeColors ProfileAttributesEntry::GetProfileThemeColors() const {
+#if defined(OS_ANDROID)
+  // Profile theme colors shouldn't be queried on Android.
+  NOTREACHED();
+  return {SK_ColorRED, SK_ColorRED, SK_ColorRED};
+#else
   base::Optional<SkColor> profile_highlight_color =
       GetProfileThemeColor(kProfileHighlightColorKey);
   base::Optional<SkColor> default_avatar_fill_color =
@@ -464,11 +456,13 @@ ProfileThemeColors ProfileAttributesEntry::GetProfileThemeColors() const {
 
   DCHECK(default_avatar_fill_color.has_value() &&
          default_avatar_stroke_color.has_value());
+
   ProfileThemeColors colors;
   colors.profile_highlight_color = profile_highlight_color.value();
   colors.default_avatar_fill_color = default_avatar_fill_color.value();
   colors.default_avatar_stroke_color = default_avatar_stroke_color.value();
   return colors;
+#endif
 }
 
 size_t ProfileAttributesEntry::GetMetricsBucketIndex() {
@@ -712,25 +706,6 @@ size_t ProfileAttributesEntry::profile_index() const {
   return index;
 }
 
-// static
-ProfileThemeColors ProfileAttributesEntry::GetDefaultProfileThemeColors(
-    bool dark_mode) {
-#if defined(OS_ANDROID)
-  // Profile theme colors shouldn't be queried on Android.
-  NOTREACHED();
-  return {SK_ColorRED, SK_ColorRED, SK_ColorRED};
-#else
-  ProfileThemeColors default_colors;
-  default_colors.profile_highlight_color = ThemeProperties::GetDefaultColor(
-      ThemeProperties::COLOR_FRAME_ACTIVE, /*incognito=*/false, dark_mode);
-  default_colors.default_avatar_fill_color = ThemeProperties::GetDefaultColor(
-      ThemeProperties::COLOR_FRAME_ACTIVE, /*incognito=*/false, dark_mode);
-  default_colors.default_avatar_stroke_color =
-      GetAvatarStrokeColor(default_colors.default_avatar_fill_color);
-  return default_colors;
-#endif
-}
-
 const gfx::Image* ProfileAttributesEntry::GetHighResAvatar() const {
   const size_t avatar_index = GetAvatarIconIndex();
 

chrome/browser/profiles/profile_attributes_entry.h

@@ -23,6 +23,7 @@
 class PrefRegistrySimple;
 class PrefService;
 class ProfileInfoCache;
+struct ProfileThemeColors;
 
 enum class SigninState {
   kNotSignedIn,
@@ -38,16 +39,6 @@ enum class NameForm {
 
 enum class AccountCategory { kConsumer, kEnterprise };
 
-struct ProfileThemeColors {
-  SkColor profile_highlight_color;
-  SkColor default_avatar_fill_color;
-  SkColor default_avatar_stroke_color;
-
-  // Equality operators for testing.
-  bool operator==(const ProfileThemeColors& other) const;
-  bool operator!=(const ProfileThemeColors& other) const;
-};
-
 class ProfileAttributesEntry {
  public:
   static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
@@ -228,9 +219,6 @@ class ProfileAttributesEntry {
   FRIEND_TEST_ALL_PREFIXES(ProfileAttributesStorageTest, ProfileActiveTime);
   FRIEND_TEST_ALL_PREFIXES(ProfileAttributesStorageTest,
                            DownloadHighResAvatarTest);
-  FRIEND_TEST_ALL_PREFIXES(ProfileAttributesStorageTest, ProfileThemeColors);
-
-  static ProfileThemeColors GetDefaultProfileThemeColors(bool dark_mode);
 
   void Initialize(ProfileInfoCache* cache,
                   const base::FilePath& path,

chrome/browser/profiles/profile_attributes_storage_unittest.cc

@@ -33,6 +33,10 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/native_theme/native_theme.h"
 
+#if !defined(OS_ANDROID)
+#include "chrome/browser/ui/signin/profile_colors_util.h"
+#endif
+
 using ::testing::Mock;
 using ::testing::_;
 
@@ -938,13 +942,12 @@ TEST_F(ProfileAttributesStorageTest, ProfileThemeColors) {
   VerifyAndResetCallExpectations();
 
   EXPECT_EQ(entry->GetProfileThemeColors(),
-            ProfileAttributesEntry::GetDefaultProfileThemeColors(false));
+            GetDefaultProfileThemeColors(false));
 
   ui::NativeTheme::GetInstanceForNativeUi()->set_use_dark_colors(true);
-  EXPECT_EQ(entry->GetProfileThemeColors(),
-            ProfileAttributesEntry::GetDefaultProfileThemeColors(true));
+  EXPECT_EQ(entry->GetProfileThemeColors(), GetDefaultProfileThemeColors(true));
   EXPECT_NE(entry->GetProfileThemeColors(),
-            ProfileAttributesEntry::GetDefaultProfileThemeColors(false));
+            GetDefaultProfileThemeColors(false));
 
   ProfileThemeColors colors = {SK_ColorTRANSPARENT, SK_ColorBLACK,
                                SK_ColorWHITE};
@@ -963,7 +966,7 @@ TEST_F(ProfileAttributesStorageTest, ProfileThemeColors) {
   EXPECT_CALL(observer(), OnProfileThemeColorsChanged(profile_path)).Times(1);
   entry->SetProfileThemeColors(base::nullopt);
   EXPECT_EQ(entry->GetProfileThemeColors(),
-            ProfileAttributesEntry::GetDefaultProfileThemeColors(false));
+            GetDefaultProfileThemeColors(false));
   VerifyAndResetCallExpectations();
 }
 #endif

chrome/browser/profiles/profile_theme_update_service.cc

@@ -12,7 +12,9 @@
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/themes/theme_service.h"
+#include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/signin/profile_colors_util.h"
+#include "chrome/common/themes/autogenerated_theme_util.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
@@ -46,15 +48,9 @@ void ProfileThemeUpdateService::UpdateProfileThemeColors() {
     return;
   }
 
-  const ui::ThemeProvider& theme_provider =
-      ThemeService::GetThemeProviderForProfile(profile_);
-  ProfileThemeColors colors;
-  colors.profile_highlight_color =
-      theme_provider.GetColor(ThemeProperties::COLOR_FRAME_ACTIVE);
-  colors.default_avatar_fill_color =
-      theme_provider.GetColor(ThemeProperties::COLOR_FRAME_ACTIVE);
-  colors.default_avatar_stroke_color =
-      GetAvatarStrokeColor(colors.default_avatar_fill_color);
+  ThemeService* service = ThemeServiceFactory::GetForProfile(profile_);
+  ProfileThemeColors colors =
+      GetProfileThemeColorsForThemeSupplier(service->GetThemeSupplier());
   entry->SetProfileThemeColors(colors);
 }
 

chrome/browser/profiles/profile_theme_update_service_browsertest.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/signin/profile_colors_util.h"
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/test/browser_test.h"
@@ -37,8 +38,8 @@ class ProfileThemeUpdateServiceBrowserTest : public InProcessBrowserTest {
     return entry;
   }
 
-  ProfileThemeColors GetDefaultProfileThemeColors() {
-    return ProfileAttributesEntry::GetDefaultProfileThemeColors(false);
+  ProfileThemeColors GetDefaultProfileThemeColorsForLightMode() {
+    return GetDefaultProfileThemeColors(/*dark_mode=*/false);
   }
 
   ThemeService* theme_service() {
@@ -54,40 +55,40 @@ class ProfileThemeUpdateServiceBrowserTest : public InProcessBrowserTest {
 IN_PROC_BROWSER_TEST_F(ProfileThemeUpdateServiceBrowserTest,
                        PRE_AutogeneratedTheme) {
   EXPECT_EQ(GetProfileAttributesEntry()->GetProfileThemeColors(),
-            GetDefaultProfileThemeColors());
+            GetDefaultProfileThemeColorsForLightMode());
 
   theme_service()->BuildAutogeneratedThemeFromColor(SK_ColorDKGRAY);
   ProfileThemeColors theme_colors =
       GetProfileAttributesEntry()->GetProfileThemeColors();
-  EXPECT_NE(theme_colors, GetDefaultProfileThemeColors());
+  EXPECT_NE(theme_colors, GetDefaultProfileThemeColorsForLightMode());
 
   // Check that a switch to another autogenerated theme updates the colors.
   theme_service()->BuildAutogeneratedThemeFromColor(SK_ColorMAGENTA);
   ProfileThemeColors theme_colors2 =
       GetProfileAttributesEntry()->GetProfileThemeColors();
   EXPECT_NE(theme_colors, theme_colors2);
-  EXPECT_NE(theme_colors, GetDefaultProfileThemeColors());
+  EXPECT_NE(theme_colors, GetDefaultProfileThemeColorsForLightMode());
 
   // Reset the cached colors to test that they're recreated on the next startup.
   GetProfileAttributesEntry()->SetProfileThemeColors(base::nullopt);
   EXPECT_EQ(GetProfileAttributesEntry()->GetProfileThemeColors(),
-            GetDefaultProfileThemeColors());
+            GetDefaultProfileThemeColorsForLightMode());
 }
 
 // Tests that the profile theme colors are updated on startup.
 IN_PROC_BROWSER_TEST_F(ProfileThemeUpdateServiceBrowserTest,
                        AutogeneratedTheme) {
   EXPECT_NE(GetProfileAttributesEntry()->GetProfileThemeColors(),
-            GetDefaultProfileThemeColors());
+            GetDefaultProfileThemeColorsForLightMode());
 }
 
 // Tests that switching to the default theme resets the colors.
 IN_PROC_BROWSER_TEST_F(ProfileThemeUpdateServiceBrowserTest, DefaultTheme) {
   theme_service()->BuildAutogeneratedThemeFromColor(SK_ColorDKGRAY);
   EXPECT_NE(GetProfileAttributesEntry()->GetProfileThemeColors(),
-            GetDefaultProfileThemeColors());
+            GetDefaultProfileThemeColorsForLightMode());
 
   theme_service()->UseDefaultTheme();
   EXPECT_EQ(GetProfileAttributesEntry()->GetProfileThemeColors(),
-            GetDefaultProfileThemeColors());
+            GetDefaultProfileThemeColorsForLightMode());
 }

chrome/browser/signin/dice_web_signin_interceptor.cc

@@ -31,6 +31,7 @@
 #include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h"
 #include "chrome/browser/ui/signin/profile_colors_util.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/common/search/generated_colors_info.h"
 #include "chrome/common/themes/autogenerated_theme_util.h"
 #include "components/password_manager/core/common/password_manager_ui.h"
 #include "components/prefs/pref_service.h"

chrome/browser/ui/signin/profile_colors_util.cc

@@ -6,19 +6,79 @@
 
 #include "base/rand_util.h"
 #include "base/stl_util.h"
+#include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/themes/browser_theme_pack.h"
+#include "chrome/browser/themes/custom_theme_supplier.h"
+#include "chrome/browser/themes/theme_properties.h"
+#include "chrome/common/search/generated_colors_info.h"
 #include "ui/gfx/color_utils.h"
 
-ProfileThemeColors GetThemeColorsForProfile(Profile* profile) {
-  DCHECK(profile->IsRegularProfile());
-  ProfileAttributesEntry* entry = nullptr;
-  g_browser_process->profile_manager()
-      ->GetProfileAttributesStorage()
-      .GetProfileAttributesWithPath(profile->GetPath(), &entry);
-  DCHECK(entry);
-  return entry->GetProfileThemeColors();
+namespace {
+
+// This is the core definition of how ProfileThemeColors are obtained.
+ProfileThemeColors GetProfileThemeColorsFromHighlightColor(
+    SkColor highlight_color) {
+  ProfileThemeColors colors;
+  colors.profile_highlight_color = highlight_color;
+  colors.default_avatar_fill_color = highlight_color;
+  colors.default_avatar_stroke_color =
+      GetAvatarStrokeColor(colors.default_avatar_fill_color);
+  return colors;
+}
+
+ProfileThemeColors GetProfileThemeColorsForAutogeneratedColor(
+    SkColor autogenerated_color) {
+  auto pack = base::MakeRefCounted<BrowserThemePack>(
+      CustomThemeSupplier::ThemeType::AUTOGENERATED);
+  BrowserThemePack::BuildFromColor(autogenerated_color, pack.get());
+  return GetProfileThemeColorsForThemeSupplier(pack.get());
+}
+
+size_t GenerateRandomIndex(size_t size) {
+  DCHECK_GT(size, 0u);
+  return static_cast<size_t>(base::RandInt(0, size - 1));
+}
+
+}  // namespace
+
+bool ProfileThemeColors::operator<(const ProfileThemeColors& other) const {
+  return std::tie(this->profile_highlight_color,
+                  this->default_avatar_fill_color,
+                  this->default_avatar_stroke_color) <
+         std::tie(other.profile_highlight_color,
+                  other.default_avatar_fill_color,
+                  other.default_avatar_stroke_color);
+}
+
+bool ProfileThemeColors::operator==(const ProfileThemeColors& other) const {
+  return std::tie(this->profile_highlight_color,
+                  this->default_avatar_fill_color,
+                  this->default_avatar_stroke_color) ==
+         std::tie(other.profile_highlight_color,
+                  other.default_avatar_fill_color,
+                  other.default_avatar_stroke_color);
+}
+
+bool ProfileThemeColors::operator!=(const ProfileThemeColors& other) const {
+  return !(*this == other);
+}
+
+ProfileThemeColors GetProfileThemeColorsForThemeSupplier(
+    const CustomThemeSupplier* supplier) {
+  SkColor highlight_color;
+  bool is_defined =
+      supplier->GetColor(ThemeProperties::COLOR_FRAME_ACTIVE, &highlight_color);
+  DCHECK(is_defined);
+  return GetProfileThemeColorsFromHighlightColor(highlight_color);
+}
+
+ProfileThemeColors GetDefaultProfileThemeColors(bool dark_mode) {
+  return GetProfileThemeColorsFromHighlightColor(
+      ThemeProperties::GetDefaultColor(ThemeProperties::COLOR_FRAME_ACTIVE,
+                                       /*incognito=*/false, dark_mode));
 }
 
 SkColor GetProfileForegroundTextColor(SkColor profile_highlight_color) {
@@ -44,11 +104,48 @@ SkColor GetAvatarStrokeColor(SkColor avatar_fill_color) {
   return color_utils::HSLToSkColor(color_hsl, SkColorGetA(avatar_fill_color));
 }
 
-chrome_colors::ColorInfo GenerateNewProfileColor() {
+chrome_colors::ColorInfo GenerateNewProfileColorWithGenerator(
+    ProfileAttributesStorage& storage,
+    base::OnceCallback<size_t(size_t count)> random_generator) {
   // TODO(crbug.com/1108295):
-  // - Implement more sophisticated algorithm to pick the new profile color.
+  // - Exclude colors from the first line of the picker.
+  // - For sign-in interception, prefer colors from the same line (if not
+  // excluded).
+  // - Both of the previous constraints could be formulated using saturation and
+  //   brightness, instead of assuming a concrete structure of lines.
   // - Return only a SkColor if the full ColorInfo is not needed.
-  size_t size = base::size(chrome_colors::kGeneratedColorsInfo);
-  size_t index = static_cast<size_t>(base::RandInt(0, size - 1));
+  std::set<ProfileThemeColors> used_theme_colors;
+  for (ProfileAttributesEntry* entry : storage.GetAllProfilesAttributes()) {
+    used_theme_colors.insert(entry->GetProfileThemeColors());
+  }
+
+  // Collect indices of profile colors that are not in `used_theme_colors`.
+  std::vector<int> available_color_indices;
+  for (size_t i = 0; i < base::size(chrome_colors::kGeneratedColorsInfo); ++i) {
+    ProfileThemeColors theme_colors =
+        GetProfileThemeColorsForAutogeneratedColor(
+            chrome_colors::kGeneratedColorsInfo[i].color);
+    if (!base::Contains(used_theme_colors, theme_colors))
+      available_color_indices.push_back(i);
+  }
+
+  size_t index;
+  if (available_color_indices.empty()) {
+    // No suitable color, fallback to random color.
+    size_t size = base::size(chrome_colors::kGeneratedColorsInfo);
+    index = std::move(random_generator).Run(size);
+    DCHECK_LT(index, size);
+  } else {
+    size_t size = available_color_indices.size();
+    size_t available_index = std::move(random_generator).Run(size);
+    DCHECK_LT(available_index, size);
+    index = available_color_indices[available_index];
+  }
   return chrome_colors::kGeneratedColorsInfo[index];
 }
+
+chrome_colors::ColorInfo GenerateNewProfileColor() {
+  return GenerateNewProfileColorWithGenerator(
+      g_browser_process->profile_manager()->GetProfileAttributesStorage(),
+      base::BindOnce(&GenerateRandomIndex));
+}

chrome/browser/ui/signin/profile_colors_util.h

@@ -6,14 +6,33 @@
 #define CHROME_BROWSER_UI_SIGNIN_PROFILE_COLORS_UTIL_H_
 
 #include "chrome/browser/profiles/profile_attributes_entry.h"
-#include "chrome/common/search/generated_colors_info.h"
 #include "third_party/skia/include/core/SkColor.h"
 
-class Profile;
+namespace chrome_colors {
+struct ColorInfo;
+}
 
-// Gets the profile theme colors associated with a profile. Does not support
-// incognito or guest profiles.
-ProfileThemeColors GetThemeColorsForProfile(Profile* profile);
+class CustomThemeSupplier;
+class ProfileAttributesStorage;
+
+struct ProfileThemeColors {
+  SkColor profile_highlight_color;
+  SkColor default_avatar_fill_color;
+  SkColor default_avatar_stroke_color;
+
+  bool operator<(const ProfileThemeColors& other) const;
+
+  // Equality operators for testing.
+  bool operator==(const ProfileThemeColors& other) const;
+  bool operator!=(const ProfileThemeColors& other) const;
+};
+
+// Extracts ProfileThemeColors out of a theme supplier.
+ProfileThemeColors GetProfileThemeColorsForThemeSupplier(
+    const CustomThemeSupplier* supplier);
+
+// Returns ProfileThemeColors for profiles without autogenerated theme.
+ProfileThemeColors GetDefaultProfileThemeColors(bool dark_mode);
 
 // Returns the color that should be used to display text over the profile
 // highlight color.
@@ -27,6 +46,15 @@ SkColor GetProfileForegroundIconColor(SkColor profile_highlight_color);
 SkColor GetAvatarStrokeColor(SkColor avatar_fill_color);
 
 // Returns a new color for a profile, based on the colors of the existing
+// profiles in `storage`. `random_generator` is called to provide randomness and
+// must return a value smaller than provided `count`. This implementation
+// function is mainly exposed for easier mocking in tests. In production code,
+// GenerateNewProfileColor() should be sufficient.
+chrome_colors::ColorInfo GenerateNewProfileColorWithGenerator(
+    ProfileAttributesStorage& storage,
+    base::OnceCallback<size_t(size_t count)> random_generator);
+
+// Returns a new random color for a profile, based on the colors of the existing
 // profiles.
 chrome_colors::ColorInfo GenerateNewProfileColor();
 

chrome/browser/ui/signin/profile_colors_util_unittest.cc

+// Copyright 2020 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/ui/signin/profile_colors_util.h"
+
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/task_environment.h"
+#include "chrome/browser/profiles/profile_attributes_entry.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
+#include "chrome/browser/profiles/profile_info_cache.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/themes/browser_theme_pack.h"
+#include "chrome/browser/themes/custom_theme_supplier.h"
+#include "chrome/common/search/generated_colors_info.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile_manager.h"
+#include "components/account_id/account_id.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+constexpr size_t kColorsCount = base::size(chrome_colors::kGeneratedColorsInfo);
+
+size_t ReturnNth(size_t n, size_t size) {
+  DCHECK_LT(n, size);
+  return n;
+}
+
+const chrome_colors::ColorInfo& GetColor(size_t index) {
+  return chrome_colors::kGeneratedColorsInfo[index];
+}
+
+class ProfileColorsUtilTest : public testing::Test {
+ public:
+  ProfileColorsUtilTest()
+      : testing_profile_manager_(TestingBrowserProcess::GetGlobal()) {}
+  ~ProfileColorsUtilTest() override = default;
+
+ protected:
+  void SetUp() override { ASSERT_TRUE(testing_profile_manager_.SetUp()); }
+
+  void AddProfile(base::Optional<SkColor> color) {
+    size_t number_of_profiles = storage()->GetNumberOfProfiles();
+
+    base::FilePath profile_path =
+        testing_profile_manager_.profile_manager()->user_data_dir().AppendASCII(
+            base::StringPrintf("testing_profile_path%" PRIuS,
+                               number_of_profiles));
+    base::string16 name = base::ASCIIToUTF16(
+        base::StringPrintf("testing_profile_name%" PRIuS, number_of_profiles));
+    storage()->AddProfile(profile_path, name, std::string(), name, true,
+                          number_of_profiles, std::string(), EmptyAccountId());
+
+    EXPECT_EQ(number_of_profiles + 1, storage()->GetNumberOfProfiles());
+
+    if (!color.has_value())
+      return;
+
+    ProfileAttributesEntry* entry = nullptr;
+    EXPECT_TRUE(storage()->GetProfileAttributesWithPath(profile_path, &entry));
+
+    auto pack = base::MakeRefCounted<BrowserThemePack>(
+        CustomThemeSupplier::ThemeType::AUTOGENERATED);
+    BrowserThemePack::BuildFromColor(*color, pack.get());
+    ProfileThemeColors colors =
+        GetProfileThemeColorsForThemeSupplier(pack.get());
+    entry->SetProfileThemeColors(colors);
+  }
+
+  void ExpectAllColorsAvailable() {
+    for (size_t i = 0; i < kColorsCount; ++i)
+      EXPECT_EQ(GetAvailableColor(i).id, GetColor(i).id);
+  }
+
+  ProfileAttributesStorage* storage() {
+    return testing_profile_manager_.profile_attributes_storage();
+  }
+
+  chrome_colors::ColorInfo GetAvailableColor(size_t n) {
+    // Instead of providing a random number generator, return the nth option
+    // deterministically.
+    return GenerateNewProfileColorWithGenerator(*storage(),
+                                                base::Bind(&ReturnNth, n));
+  }
+
+ private:
+  TestingProfileManager testing_profile_manager_;
+  base::test::TaskEnvironment task_environment_;
+};
+
+// Test that all colors are available with no other profiles.
+TEST_F(ProfileColorsUtilTest, GenerateNewProfileColorWithNoColoredProfile) {
+  ExpectAllColorsAvailable();
+
+  // Add some profiles with the default theme.
+  AddProfile(base::nullopt);
+  AddProfile(base::nullopt);
+  // Add a profile with a custom color.
+  AddProfile(SK_ColorRED);
+
+  // It still behaves the same, all colors are available.
+  ExpectAllColorsAvailable();
+}
+
+// Test that the taken colors are not available.
+TEST_F(ProfileColorsUtilTest,
+       GenerateNewProfileColorWithMultipleColoredProfiles) {
+  AddProfile(GetColor(5).color);
+  AddProfile(GetColor(6).color);
+
+  EXPECT_EQ(GetAvailableColor(0).id, GetColor(0).id);
+  EXPECT_EQ(GetAvailableColor(4).id, GetColor(4).id);
+  EXPECT_EQ(GetAvailableColor(5).id, GetColor(7).id);
+  size_t max = kColorsCount - 1;
+  EXPECT_EQ(GetAvailableColor(max - 2).id, GetColor(max).id);
+}
+
+// Test that if all colors are taken, then again all are available.
+TEST_F(ProfileColorsUtilTest, GenerateNewProfileColorWithAllColorsTaken) {
+  for (size_t i = 0; i < kColorsCount - 1; i++)
+    AddProfile(GetColor(i).color);
+
+  // Only the last color is available.
+  EXPECT_EQ(GetAvailableColor(0).id, GetColor(kColorsCount - 1).id);
+
+  // Take the last available color.
+  AddProfile(GetColor(kColorsCount - 1).color);
+
+  // Again, all colors are available.
+  ExpectAllColorsAvailable();
+}
+
+}  // namespace

chrome/browser/ui/views/profiles/profile_menu_view.cc

@@ -466,7 +466,7 @@ void ProfileMenuView::BuildIdentity() {
 #endif
 
   SkColor background_color =
-      GetThemeColorsForProfile(profile).profile_highlight_color;
+      profile_attributes->GetProfileThemeColors().profile_highlight_color;
   if (account_info.has_value()) {
     SetProfileIdentityInfo(
         profile_name, background_color, edit_button_params,

chrome/test/BUILD.gn

@@ -5534,6 +5534,7 @@ test("unit_tests") {
       "../browser/net/disk_cache_dir_policy_handler_unittest.cc",
       "../browser/profiles/profile_list_desktop_unittest.cc",
       "../browser/profiles/profile_statistics_unittest.cc",
+      "../browser/ui/signin/profile_colors_util_unittest.cc",
       "../browser/ui/startup/startup_browser_creator_impl_unittest.cc",
       "../browser/ui/startup/startup_browser_policy_unittest.cc",
       "../browser/ui/startup/startup_tab_provider_unittest.cc",