[Settings]: Add generic colored avatar to mange profile.

This CL adds the generic colored avatar to the avatar icons shown on
'chrome://settings/manageProfile' that is only shown for local profile
(No GAIA account).

It also fixes the bug where no selected is shown when the user is using
the GAIA avatar as the chrome avatar.

In a future CL, |kProfileUsingDefaultAvatar, kProfileUsingGAIAAvatar|
prefs will be deleted. The |ManageProfileHandler| should not rely on
these prefs.

Fixed: 1149539
Change-Id: I9cbce03e62efb20a6c466eac9c7e32d1cbc93f18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542062
Commit-Queue: Monica Basta <msalama@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Esmael Elmoslimany <aee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829194}

chrome/browser/profiles/profile_attributes_entry.cc

@@ -607,11 +607,6 @@ void ProfileAttributesEntry::SetIsAuthError(bool value) {
 }
 
 void ProfileAttributesEntry::SetAvatarIconIndex(size_t icon_index) {
-  if (!profiles::IsDefaultAvatarIconIndex(icon_index)) {
-    DLOG(WARNING) << "Unknown avatar icon index: " << icon_index;
-    // switch to generic avatar
-    icon_index = 0;
-  }
   std::string default_avatar_icon_url =
       profiles::GetDefaultAvatarIconUrl(icon_index);
   if (default_avatar_icon_url == GetString(kAvatarIconKey)) {

chrome/browser/profiles/profile_avatar_icon_util.cc

@@ -34,6 +34,7 @@
 #include "third_party/skia/include/core/SkScalar.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/base/webui/web_ui_util.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/favicon_size.h"
@@ -551,7 +552,7 @@ const IconResourceInfo* GetDefaultAvatarIconResourceInfo(size_t index) {
      IDS_DEFAULT_AVATAR_LABEL_25},
 #endif
     // Placeholder avatar icon:
-    {IDR_PROFILE_AVATAR_26, NULL, -1},
+    {IDR_PROFILE_AVATAR_26, nullptr, IDS_DEFAULT_AVATAR_LABEL_26},
 
 #if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
     // Modern avatar icons:
@@ -663,7 +664,6 @@ std::string GetDefaultAvatarIconUrl(size_t index) {
 }
 
 int GetDefaultAvatarLabelResourceIDAtIndex(size_t index) {
-  CHECK_NE(index, kPlaceholderAvatarIndex);
   return GetDefaultAvatarIconResourceInfo(index)->label_id;
 }
 
@@ -691,22 +691,49 @@ bool IsDefaultAvatarIconUrl(const std::string& url, size_t* icon_index) {
   return false;
 }
 
-std::unique_ptr<base::ListValue> GetDefaultProfileAvatarIconsAndLabels(
+std::unique_ptr<base::DictionaryValue> GetAvatarIconAndLabelDict(
+    const std::string& url,
+    const base::string16& label,
+    size_t index,
+    bool selected,
+    bool is_gaia_avatar) {
+  std::unique_ptr<base::DictionaryValue> avatar_info(
+      new base::DictionaryValue());
+  avatar_info->SetStringPath("url", url);
+  avatar_info->SetStringPath("label", label);
+  avatar_info->SetIntPath("index", index);
+  avatar_info->SetBoolPath("selected", selected);
+  avatar_info->SetBoolPath("isGaiaAvatar", is_gaia_avatar);
+  return avatar_info;
+}
+
+std::unique_ptr<base::DictionaryValue> GetDefaultProfileAvatarIconAndLabel(
+    SkColor fill_color,
+    SkColor stroke_color,
+    bool selected) {
+  std::unique_ptr<base::DictionaryValue> avatar_info(
+      new base::DictionaryValue());
+  gfx::Image icon = profiles::GetPlaceholderAvatarIconWithColors(
+      fill_color, stroke_color, kAvatarIconSize);
+  size_t index = profiles::GetPlaceholderAvatarIndex();
+  return GetAvatarIconAndLabelDict(
+      webui::GetBitmapDataUrl(icon.AsBitmap()),
+      l10n_util::GetStringUTF16(
+          profiles::GetDefaultAvatarLabelResourceIDAtIndex(index)),
+      index, selected, /*is_gaia_avatar=*/false);
+}
+
+std::unique_ptr<base::ListValue> GetCustomProfileAvatarIconsAndLabels(
     size_t selected_avatar_idx) {
   std::unique_ptr<base::ListValue> avatars(new base::ListValue());
 
   for (size_t i = GetModernAvatarIconStartIndex();
        i < GetDefaultAvatarIconCount(); ++i) {
-    std::unique_ptr<base::DictionaryValue> avatar_info(
-        new base::DictionaryValue());
-    avatar_info->SetString("url", profiles::GetDefaultAvatarIconUrl(i));
-    avatar_info->SetString(
-        "label", l10n_util::GetStringUTF16(
-                     profiles::GetDefaultAvatarLabelResourceIDAtIndex(i)));
-    if (i == selected_avatar_idx)
-      avatar_info->SetBoolean("selected", true);
-
-    avatars->Append(std::move(avatar_info));
+    avatars->Append(GetAvatarIconAndLabelDict(
+        profiles::GetDefaultAvatarIconUrl(i),
+        l10n_util::GetStringUTF16(
+            profiles::GetDefaultAvatarLabelResourceIDAtIndex(i)),
+        i, i == selected_avatar_idx, /*is_gaia_avatar=*/false));
   }
   return avatars;
 }

chrome/browser/profiles/profile_avatar_icon_util.h

@@ -150,11 +150,27 @@ bool IsDefaultAvatarIconIndex(size_t index);
 // is, returns true and its index through |icon_index|. If not, returns false.
 bool IsDefaultAvatarIconUrl(const std::string& icon_url, size_t *icon_index);
 
-// Returns a list of dictionaries containing the default profile avatar icons as
+// Returns Dict containing the avatar icon info in the format expected by the
+// WebUI component 'cr-profile-avatar-selector'.
+std::unique_ptr<base::DictionaryValue> GetAvatarIconAndLabelDict(
+    const std::string& url,
+    const base::string16& label,
+    size_t index,
+    bool selected,
+    bool is_gaia_avatar);
+
+// Returns Dict containing the default generic avatar icon, label, index and
+// selected state.
+std::unique_ptr<base::DictionaryValue> GetDefaultProfileAvatarIconAndLabel(
+    SkColor fill_color,
+    SkColor stroke_color,
+    bool selected);
+
+// Returns a list of dictionaries containing modern profile avatar icons as
 // well as avatar labels used for accessibility purposes. The list is ordered
 // according to the avatars' default order. If |selected_avatar_idx| is one of
 // the available indices, the corresponding avatar is marked as selected.
-std::unique_ptr<base::ListValue> GetDefaultProfileAvatarIconsAndLabels(
+std::unique_ptr<base::ListValue> GetCustomProfileAvatarIconsAndLabels(
     size_t selected_avatar_idx = SIZE_MAX);
 
 // This method tries to find a random avatar index that is not in

chrome/browser/resources/settings/people_page/manage_profile.js

@@ -176,7 +176,8 @@ Polymer({
     if (this.profileAvatar_.isGaiaAvatar) {
       this.browserProxy_.setProfileIconToGaiaAvatar();
     } else {
-      this.browserProxy_.setProfileIconToDefaultAvatar(this.profileAvatar_.url);
+      this.browserProxy_.setProfileIconToDefaultAvatar(
+          this.profileAvatar_.index);
     }
   },
 

chrome/browser/resources/settings/people_page/manage_profile_browser_proxy.js

@@ -38,9 +38,9 @@ export class ManageProfileBrowserProxy {
 
   /**
    * Sets the profile's icon to one of the default avatars.
-   * @param {string} iconUrl The new profile URL.
+   * @param {number} index The new profile avatar index.
    */
-  setProfileIconToDefaultAvatar(iconUrl) {}
+  setProfileIconToDefaultAvatar(index) {}
 
   /**
    * Sets the profile's name.
@@ -80,8 +80,8 @@ export class ManageProfileBrowserProxyImpl {
   }
 
   /** @override */
-  setProfileIconToDefaultAvatar(iconUrl) {
-    chrome.send('setProfileIconToDefaultAvatar', [iconUrl]);
+  setProfileIconToDefaultAvatar(index) {
+    chrome.send('setProfileIconToDefaultAvatar', [index]);
   }
 
   /** @override */

chrome/browser/ui/webui/settings/settings_manage_profile_handler.cc

@@ -23,6 +23,8 @@
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/signin/profile_colors_util.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
@@ -94,16 +96,28 @@ void ManageProfileHandler::OnJavascriptDisallowed() {
 
 void ManageProfileHandler::OnProfileHighResAvatarLoaded(
     const base::FilePath& profile_path) {
+  if (profile_path != profile_->GetPath())
+    return;
+
   // GAIA image is loaded asynchronously.
   FireWebUIListener("available-icons-changed", *GetAvailableIcons());
 }
 
 void ManageProfileHandler::OnProfileAvatarChanged(
     const base::FilePath& profile_path) {
+  if (profile_path != profile_->GetPath())
+    return;
+
   // This is necessary to send the potentially updated GAIA photo.
   FireWebUIListener("available-icons-changed", *GetAvailableIcons());
 }
 
+void ManageProfileHandler::OnProfileThemeColorsChanged(
+    const base::FilePath& profile_path) {
+  // This is necessary to send the potentially updated Generic colored avatar.
+  OnProfileAvatarChanged(profile_path);
+}
+
 void ManageProfileHandler::HandleGetAvailableIcons(
     const base::ListValue* args) {
   AllowJavascript();
@@ -117,35 +131,47 @@ void ManageProfileHandler::HandleGetAvailableIcons(
 }
 
 std::unique_ptr<base::ListValue> ManageProfileHandler::GetAvailableIcons() {
-  PrefService* pref_service = profile_->GetPrefs();
-  bool using_gaia = pref_service->GetBoolean(prefs::kProfileUsingGAIAAvatar);
+  ProfileAttributesEntry* entry = nullptr;
+  // TODO(msalama): Convert to a DCHECK.
+  if (!g_browser_process->profile_manager()
+           ->GetProfileAttributesStorage()
+           .GetProfileAttributesWithPath(profile_->GetPath(), &entry)) {
+    LOG(ERROR) << "No profile attributes entry found for profile with path: "
+               << profile_->GetPath();
+    return std::make_unique<base::ListValue>();
+  }
+
+  bool using_gaia = entry->IsUsingGAIAPicture();
   size_t selected_avatar_idx =
-      using_gaia ? SIZE_MAX
-                 : pref_service->GetInteger(prefs::kProfileAvatarIndex);
+      using_gaia ? SIZE_MAX : entry->GetAvatarIconIndex();
 
-  // Obtain a list of the default avatar icons.
+  // Obtain a list of the modern avatar icons.
   std::unique_ptr<base::ListValue> avatars(
-      profiles::GetDefaultProfileAvatarIconsAndLabels(selected_avatar_idx));
+      profiles::GetCustomProfileAvatarIconsAndLabels(selected_avatar_idx));
+
+  if (entry->GetSigninState() == SigninState::kNotSignedIn) {
+    if (base::FeatureList::IsEnabled(features::kNewProfilePicker) &&
+        base::FeatureList::IsEnabled(features::kProfilesUIRevamp)) {
+      ProfileThemeColors colors = entry->GetProfileThemeColors();
+      auto generic_avatar_info = profiles::GetDefaultProfileAvatarIconAndLabel(
+          colors.default_avatar_fill_color, colors.default_avatar_stroke_color,
+          selected_avatar_idx == profiles::GetPlaceholderAvatarIndex());
+      avatars->Insert(0, std::move(generic_avatar_info));
+    }
+    return avatars;
+  }
 
   // Add the GAIA picture to the beginning of the list if it is available.
-  ProfileAttributesEntry* entry;
-  if (g_browser_process->profile_manager()->GetProfileAttributesStorage().
-          GetProfileAttributesWithPath(profile_->GetPath(), &entry)) {
   const gfx::Image* icon = entry->GetGAIAPicture();
   if (icon) {
-      auto gaia_picture_info = std::make_unique<base::DictionaryValue>();
     gfx::Image avatar_icon = profiles::GetAvatarIconForWebUI(*icon, true);
-      gaia_picture_info->SetString(
-          "url", webui::GetBitmapDataUrl(avatar_icon.AsBitmap()));
-      gaia_picture_info->SetString(
-          "label",
-          l10n_util::GetStringUTF16(IDS_SETTINGS_CHANGE_PICTURE_PROFILE_PHOTO));
-      gaia_picture_info->SetBoolean("isGaiaAvatar", true);
-      if (using_gaia)
-        gaia_picture_info->SetBoolean("selected", true);
+    auto gaia_picture_info = profiles::GetAvatarIconAndLabelDict(
+        /*url=*/webui::GetBitmapDataUrl(avatar_icon.AsBitmap()),
+        /*label=*/
+        l10n_util::GetStringUTF16(IDS_SETTINGS_CHANGE_PICTURE_PROFILE_PHOTO),
+        /*index=*/0, using_gaia, /*is_gaia_avatar=*/true);
     avatars->Insert(0, std::move(gaia_picture_info));
   }
-  }
 
   return avatars;
 }
@@ -173,18 +199,18 @@ void ManageProfileHandler::HandleSetProfileIconToGaiaAvatar(
 void ManageProfileHandler::HandleSetProfileIconToDefaultAvatar(
     const base::ListValue* args) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK(args);
-  DCHECK_EQ(1u, args->GetSize());
+  CHECK(args);
+  CHECK_EQ(1u, args->GetSize());
+  CHECK(args->GetList()[0].is_int());
 
-  std::string icon_url;
-  CHECK(args->GetString(0, &icon_url));
-
-  size_t new_icon_index = 0;
-  CHECK(profiles::IsDefaultAvatarIconUrl(icon_url, &new_icon_index));
+  size_t new_icon_index = args->GetList()[0].GetInt();
+  CHECK(profiles::IsDefaultAvatarIconIndex(new_icon_index));
 
   PrefService* pref_service = profile_->GetPrefs();
   pref_service->SetInteger(prefs::kProfileAvatarIndex, new_icon_index);
-  pref_service->SetBoolean(prefs::kProfileUsingDefaultAvatar, false);
+  pref_service->SetBoolean(
+      prefs::kProfileUsingDefaultAvatar,
+      new_icon_index == profiles::GetPlaceholderAvatarIndex());
   pref_service->SetBoolean(prefs::kProfileUsingGAIAAvatar, false);
 
   ProfileMetrics::LogProfileAvatarSelection(new_icon_index);
@@ -193,8 +219,8 @@ void ManageProfileHandler::HandleSetProfileIconToDefaultAvatar(
 
 void ManageProfileHandler::HandleSetProfileName(const base::ListValue* args) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK(args);
-  DCHECK_EQ(1u, args->GetSize());
+  CHECK(args);
+  CHECK_EQ(1u, args->GetSize());
 
   if (profile_->IsLegacySupervised())
     return;

chrome/browser/ui/webui/settings/settings_manage_profile_handler.h

@@ -31,22 +31,25 @@ class ManageProfileHandler : public settings::SettingsPageUIHandler,
 
   // ProfileAttributesStorage::Observer:
   void OnProfileAvatarChanged(const base::FilePath& profile_path) override;
-
-  // ProfileAttributesStorage::Observer:
   void OnProfileHighResAvatarLoaded(
       const base::FilePath& profile_path) override;
+  void OnProfileThemeColorsChanged(const base::FilePath& profile_path) override;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest,
                            HandleSetProfileIconToGaiaAvatar);
   FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest,
-                           HandleSetProfileIconToDefaultAvatar);
+                           GetAvailableIconsSignedInProfile);
+  FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest,
+                           GetAvailableIconsLocalProfile);
+  FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest,
+                           HandleSetProfileIconToDefaultCustomAvatar);
+  FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest,
+                           HandleSetProfileIconToDefaultGenericAvatar);
   FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest, HandleSetProfileName);
   FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest, HandleGetAvailableIcons);
   FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest,
                            HandleGetAvailableIconsOldIconSelected);
-  FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest,
-                           HandleGetAvailableIconsGaiaAvatarSelected);
 
   // Callback for the "getAvailableIcons" message.
   // Sends the array of default profile icon URLs and profile names to WebUI.

chrome/browser/ui/webui/signin/signin_create_profile_handler.cc

@@ -123,7 +123,7 @@ void SigninCreateProfileHandler::RequestDefaultProfileIcons(
     const base::ListValue* args) {
   web_ui()->CallJavascriptFunctionUnsafe(
       "cr.webUIListenerCallback", base::Value("profile-icons-received"),
-      *profiles::GetDefaultProfileAvatarIconsAndLabels());
+      *profiles::GetCustomProfileAvatarIconsAndLabels());
 }
 
 void SigninCreateProfileHandler::CreateProfile(const base::ListValue* args) {

chrome/test/data/webui/settings/people_page_manage_profile_test.js

@@ -35,8 +35,8 @@ class TestManageProfileBrowserProxy extends TestBrowserProxy {
   getAvailableIcons() {
     this.methodCalled('getAvailableIcons');
     return Promise.resolve([
-      {url: 'fake-icon-1.png', label: 'fake-icon-1'},
-      {url: 'fake-icon-2.png', label: 'fake-icon-2', selected: true},
+      {url: 'fake-icon-1.png', label: 'fake-icon-1', index: 1},
+      {url: 'fake-icon-2.png', label: 'fake-icon-2', index: 2, selected: true},
       {url: 'gaia-icon.png', label: 'gaia-icon', isGaiaAvatar: true},
     ]);
   }
@@ -47,8 +47,8 @@ class TestManageProfileBrowserProxy extends TestBrowserProxy {
   }
 
   /** @override */
-  setProfileIconToDefaultAvatar(iconUrl) {
-    this.methodCalled('setProfileIconToDefaultAvatar', [iconUrl]);
+  setProfileIconToDefaultAvatar(index) {
+    this.methodCalled('setProfileIconToDefaultAvatar', [index]);
   }
 
   /** @override */
@@ -130,7 +130,7 @@ suite('ManageProfileTests', function() {
       items[1].click();
       const args =
           await browserProxy.whenCalled('setProfileIconToDefaultAvatar');
-      assertEquals('fake-icon-2.png', args[0]);
+      assertEquals(2, args[0]);
 
       items[2].click();
       await browserProxy.whenCalled('setProfileIconToGaiaAvatar');

ui/webui/resources/cr_elements/cr_profile_avatar_selector/cr_profile_avatar_selector.js

@@ -10,8 +10,9 @@
 /**
  * @typedef {{url: string,
  *            label: string,
- *            isGaiaAvatar: (boolean|undefined),
- *            selected: (boolean|undefined)}}
+ *            index: (number),
+ *            isGaiaAvatar: (boolean),
+ *            selected: (boolean)}}
  */
 /* #export */ let AvatarIcon;