[Signin]: Trigger 'OnAvatarChanged' on theme color changes.

For the profiles that use generic colored avatar,
'OnProfileAvatarChanged' needs to fired for the avatar icon to be
updated across different UIs on theme color changes. This CL doesn't
handle the switch between light/dark mode that also has an effect on the
avatar icon.

Bug: 1136851
Change-Id: I11acfffea825bcc87f41640986bb1e3f1858bc06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461320
Commit-Queue: Monica Basta <msalama@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815713}

chrome/browser/profiles/profile_attributes_entry.cc

@@ -97,6 +97,11 @@ int GetLowEntropyHashValue(const std::string& value) {
   return base::PersistentHash(value) % kNumberOfLowEntropyHashValues;
 }
 
+bool ShouldShowGenericColoredAvatar(size_t avatar_icon_index) {
+  return base::FeatureList::IsEnabled(features::kNewProfilePicker) &&
+         avatar_icon_index == profiles::GetPlaceholderAvatarIndex();
+}
+
 }  // namespace
 
 bool ProfileThemeColors::operator==(const ProfileThemeColors& other) const {
@@ -300,8 +305,7 @@ gfx::Image ProfileAttributesEntry::GetAvatarIcon(
 
   // TODO(crbug.com/1100835): After launch, remove the treatment of placeholder
   // avatars from GetHighResAvatar() and from any other places.
-  if (base::FeatureList::IsEnabled(features::kNewProfilePicker) &&
-      GetAvatarIconIndex() == profiles::GetPlaceholderAvatarIndex()) {
+  if (ShouldShowGenericColoredAvatar(GetAvatarIconIndex())) {
     return GetPlaceholderAvatarIcon(size_for_placeholder_avatar);
   }
 
@@ -615,16 +619,22 @@ void ProfileAttributesEntry::SetAvatarIconIndex(size_t icon_index) {
 
 void ProfileAttributesEntry::SetProfileThemeColors(
     const base::Optional<ProfileThemeColors>& colors) {
+  bool changed = false;
   if (colors.has_value()) {
-    SetInteger(kProfileHighlightColorKey, colors->profile_highlight_color);
-    SetInteger(kDefaultAvatarFillColorKey, colors->default_avatar_fill_color);
-    SetInteger(kDefaultAvatarStrokeColorKey,
-               colors->default_avatar_stroke_color);
+    changed |=
+        SetInteger(kProfileHighlightColorKey, colors->profile_highlight_color);
+    changed |= SetInteger(kDefaultAvatarFillColorKey,
+                          colors->default_avatar_fill_color);
+    changed |= SetInteger(kDefaultAvatarStrokeColorKey,
+                          colors->default_avatar_stroke_color);
   } else {
-    ClearValue(kProfileHighlightColorKey);
-    ClearValue(kDefaultAvatarFillColorKey);
-    ClearValue(kDefaultAvatarStrokeColorKey);
+    changed |= ClearValue(kProfileHighlightColorKey);
+    changed |= ClearValue(kDefaultAvatarFillColorKey);
+    changed |= ClearValue(kDefaultAvatarStrokeColorKey);
   }
+
+  if (changed && ShouldShowGenericColoredAvatar(GetAvatarIconIndex()))
+    profile_info_cache_->NotifyOnProfileAvatarChanged(GetPath());
 }
 
 void ProfileAttributesEntry::SetHostedDomain(std::string hosted_domain) {

chrome/browser/profiles/profile_attributes_storage_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_avatar_downloader.h"
@@ -20,6 +21,7 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/signin_util.h"
 #include "chrome/browser/supervised_user/supervised_user_constants.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "components/account_id/account_id.h"
@@ -922,13 +924,17 @@ TEST_F(ProfileAttributesStorageTest, ProfilesState_SingleProfile) {
 // Themes aren't used on Android
 #if !defined(OS_ANDROID)
 TEST_F(ProfileAttributesStorageTest, ProfileThemeColors) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kNewProfilePicker);
   AddTestingProfile();
   base::FilePath profile_path = GetProfilePath("testing_profile_path0");
 
-  DisableObserver();  // No need to test observers in this test.
-
   ProfileAttributesEntry* entry;
   ASSERT_TRUE(storage()->GetProfileAttributesWithPath(profile_path, &entry));
+  EXPECT_CALL(observer(), OnProfileAvatarChanged(profile_path)).Times(1);
+  entry->SetAvatarIconIndex(profiles::GetPlaceholderAvatarIndex());
+  VerifyAndResetCallExpectations();
+
   EXPECT_EQ(entry->GetProfileThemeColors(),
             ProfileAttributesEntry::GetDefaultProfileThemeColors(false));
 
@@ -940,16 +946,20 @@ TEST_F(ProfileAttributesStorageTest, ProfileThemeColors) {
 
   ProfileThemeColors colors = {SK_ColorTRANSPARENT, SK_ColorBLACK,
                                SK_ColorWHITE};
+  EXPECT_CALL(observer(), OnProfileAvatarChanged(profile_path)).Times(1);
   entry->SetProfileThemeColors(colors);
   EXPECT_EQ(entry->GetProfileThemeColors(), colors);
+  VerifyAndResetCallExpectations();
 
   // Colors shouldn't change after switching back to the light mode.
   ui::NativeTheme::GetInstanceForNativeUi()->set_use_dark_colors(false);
   EXPECT_EQ(entry->GetProfileThemeColors(), colors);
 
   // base::nullopt resets the colors to default.
+  EXPECT_CALL(observer(), OnProfileAvatarChanged(profile_path)).Times(1);
   entry->SetProfileThemeColors(base::nullopt);
   EXPECT_EQ(entry->GetProfileThemeColors(),
             ProfileAttributesEntry::GetDefaultProfileThemeColors(false));
+  VerifyAndResetCallExpectations();
 }
 #endif