Settings WebUI: show selected avatar on initial load of manage profile

Bug: 710660
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I2a43c8a51c6702d2e79e05b72162133d18f82041
Reviewed-on: https://chromium-review.googlesource.com/949419
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541862}

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

@@ -36,7 +36,7 @@ Polymer({
 
     /**
      * The available icons for selection.
-     * @type {!Array<string>}
+     * @type {!Array<!AvatarIcon>}
      */
     availableIcons: {
       type: Array,

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

@@ -94,6 +94,12 @@ void ManageProfileHandler::OnJavascriptDisallowed() {
   observer_.RemoveAll();
 }
 
+void ManageProfileHandler::OnProfileHighResAvatarLoaded(
+    const base::FilePath& profile_path) {
+  // GAIA image is loaded asynchronously.
+  FireWebUIListener("available-icons-changed", *GetAvailableIcons());
+}
+
 void ManageProfileHandler::OnProfileAvatarChanged(
     const base::FilePath& profile_path) {
   // This is necessary to send the potentially updated GAIA photo.
@@ -113,9 +119,20 @@ void ManageProfileHandler::HandleGetAvailableIcons(
 }
 
 std::unique_ptr<base::ListValue> ManageProfileHandler::GetAvailableIcons() {
-  std::unique_ptr<base::ListValue> image_url_list(
+  std::unique_ptr<base::ListValue> avatars(
       profiles::GetDefaultProfileAvatarIconsAndLabels());
 
+  PrefService* pref_service = profile_->GetPrefs();
+  bool using_gaia = pref_service->GetBoolean(prefs::kProfileUsingGAIAAvatar);
+
+  // Select the avatar from the default set.
+  if (!using_gaia) {
+    size_t index = pref_service->GetInteger(prefs::kProfileAvatarIndex);
+    base::DictionaryValue* avatar = nullptr;
+    if (avatars->GetDictionary(index, &avatar))
+      avatar->SetBoolean("selected", true);
+  }
+
   // Add the GAIA picture to the beginning of the list if it is available.
   ProfileAttributesEntry* entry;
   if (g_browser_process->profile_manager()->GetProfileAttributesStorage().
@@ -130,11 +147,13 @@ std::unique_ptr<base::ListValue> ManageProfileHandler::GetAvailableIcons() {
           "label",
           l10n_util::GetStringUTF16(IDS_SETTINGS_CHANGE_PICTURE_PROFILE_PHOTO));
       gaia_picture_info->SetBoolean("isGaiaAvatar", true);
-      image_url_list->Insert(0, std::move(gaia_picture_info));
+      if (using_gaia)
+        gaia_picture_info->SetBoolean("selected", true);
+      avatars->Insert(0, std::move(gaia_picture_info));
     }
   }
 
-  return image_url_list;
+  return avatars;
 }
 
 void ManageProfileHandler::HandleSetProfileIconToGaiaAvatar(

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

@@ -32,6 +32,10 @@ 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;
+
  private:
   FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest,
                            HandleSetProfileIconToGaiaAvatar);
@@ -39,6 +43,8 @@ class ManageProfileHandler : public settings::SettingsPageUIHandler,
                            HandleSetProfileIconToDefaultAvatar);
   FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest, HandleSetProfileName);
   FRIEND_TEST_ALL_PREFIXES(ManageProfileHandlerTest, HandleGetAvailableIcons);
+  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/settings/settings_manage_profile_handler_unittest.cc

@@ -45,7 +45,30 @@ class ManageProfileHandlerTest : public testing::Test {
     web_ui()->ClearTrackedCalls();
   }
 
-  void VerifyIconList(const base::Value* value) {
+  void VerifyIconListWithNoneSelected(const base::Value* value) {
+    VerifyIconList(value, 0 /* ignored */, true);
+  }
+
+  void VerifyIconListWithSingleSelection(const base::Value* value,
+                                         size_t selected_index) {
+    VerifyIconList(value, selected_index, false);
+  }
+
+  content::TestWebUI* web_ui() { return &web_ui_; }
+  Profile* profile() const { return profile_; }
+  TestManageProfileHandler* handler() const { return handler_.get(); }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+  TestingProfileManager profile_manager_;
+  content::TestWebUI web_ui_;
+
+  Profile* profile_;
+  std::unique_ptr<TestManageProfileHandler> handler_;
+
+  void VerifyIconList(const base::Value* value,
+                      size_t selected_index,
+                      bool all_not_selected) {
     const base::ListValue* icons = nullptr;
     ASSERT_TRUE(value->GetAsList(&icons));
 
@@ -61,20 +84,16 @@ class ManageProfileHandlerTest : public testing::Test {
       std::string icon_label;
       EXPECT_TRUE(icon->GetString("label", &icon_label));
       EXPECT_FALSE(icon_label.empty());
+      bool icon_selected;
+      bool has_icon_selected = icon->GetBoolean("selected", &icon_selected);
+      if (all_not_selected) {
+        EXPECT_FALSE(has_icon_selected);
+      } else if (selected_index == i) {
+        EXPECT_TRUE(has_icon_selected);
+        EXPECT_TRUE(icon_selected);
+      }
     }
   }
-
-  content::TestWebUI* web_ui() { return &web_ui_; }
-  Profile* profile() const { return profile_; }
-  TestManageProfileHandler* handler() const { return handler_.get(); }
-
- private:
-  content::TestBrowserThreadBundle thread_bundle_;
-  TestingProfileManager profile_manager_;
-  content::TestWebUI web_ui_;
-
-  Profile* profile_;
-  std::unique_ptr<TestManageProfileHandler> handler_;
 };
 
 TEST_F(ManageProfileHandlerTest, HandleSetProfileIconToGaiaAvatar) {
@@ -106,6 +125,30 @@ TEST_F(ManageProfileHandlerTest, HandleSetProfileName) {
 }
 
 TEST_F(ManageProfileHandlerTest, HandleGetAvailableIcons) {
+  PrefService* pref_service = profile()->GetPrefs();
+  pref_service->SetInteger(prefs::kProfileAvatarIndex, 7);
+
+  base::ListValue list_args_1;
+  list_args_1.AppendString("get-icons-callback-id");
+  handler()->HandleGetAvailableIcons(&list_args_1);
+
+  EXPECT_EQ(1U, web_ui()->call_data().size());
+
+  const content::TestWebUI::CallData& data_1 = *web_ui()->call_data().back();
+  EXPECT_EQ("cr.webUIResponse", data_1.function_name());
+
+  std::string callback_id_1;
+  ASSERT_TRUE(data_1.arg1()->GetAsString(&callback_id_1));
+  EXPECT_EQ("get-icons-callback-id", callback_id_1);
+
+  VerifyIconListWithSingleSelection(data_1.arg3(), 7);
+}
+
+TEST_F(ManageProfileHandlerTest, HandleGetAvailableIconsGaiaAvatarSelected) {
+  PrefService* pref_service = profile()->GetPrefs();
+  pref_service->SetInteger(prefs::kProfileAvatarIndex, 7);
+  pref_service->SetBoolean(prefs::kProfileUsingGAIAAvatar, true);
+
   base::ListValue list_args;
   list_args.AppendString("get-icons-callback-id");
   handler()->HandleGetAvailableIcons(&list_args);
@@ -119,10 +162,13 @@ TEST_F(ManageProfileHandlerTest, HandleGetAvailableIcons) {
   ASSERT_TRUE(data.arg1()->GetAsString(&callback_id));
   EXPECT_EQ("get-icons-callback-id", callback_id);
 
-  VerifyIconList(data.arg3());
+  VerifyIconListWithNoneSelected(data.arg3());
 }
 
 TEST_F(ManageProfileHandlerTest, ProfileAvatarChangedWebUIEvent) {
+  PrefService* pref_service = profile()->GetPrefs();
+  pref_service->SetInteger(prefs::kProfileAvatarIndex, 12);
+
   handler()->OnProfileAvatarChanged(base::FilePath());
 
   EXPECT_EQ(1U, web_ui()->call_data().size());
@@ -133,8 +179,7 @@ TEST_F(ManageProfileHandlerTest, ProfileAvatarChangedWebUIEvent) {
   std::string event_id;
   ASSERT_TRUE(data.arg1()->GetAsString(&event_id));
   EXPECT_EQ("available-icons-changed", event_id);
-
-  VerifyIconList(data.arg2());
+  VerifyIconListWithSingleSelection(data.arg2(), 12);
 }
 
 }  // namespace settings

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

@@ -31,8 +31,8 @@ cr.define('settings_people_page_manage_profile', function() {
       this.methodCalled('getAvailableIcons');
       return Promise.resolve([
         {url: 'fake-icon-1.png', label: 'fake-icon-1'},
-        {url: 'fake-icon-2.png', label: 'fake-icon-2'},
-        {url: 'gaia-icon.png', label: 'gaia-icon', isGaiaAvatar: true}
+        {url: 'fake-icon-2.png', label: 'fake-icon-2', selected: true},
+        {url: 'gaia-icon.png', label: 'gaia-icon', isGaiaAvatar: true},
       ]);
     }
 
@@ -97,11 +97,10 @@ cr.define('settings_people_page_manage_profile', function() {
             items = manageProfile.$.selector.$['avatar-grid'].
                 querySelectorAll('.avatar');
 
-            // Initially no item is selected, because of crbug.com/710660.
             assertFalse(!!manageProfile.profileAvatar);
             assertEquals(3, items.length);
             assertFalse(items[0].classList.contains('iron-selected'));
-            assertFalse(items[1].classList.contains('iron-selected'));
+            assertTrue(items[1].classList.contains('iron-selected'));
             assertFalse(items[2].classList.contains('iron-selected'));
 
             MockInteractions.tap(items[1]);

ui/webui/resources/cr_elements/cr_profile_avatar_selector/cr_profile_avatar_selector.html

@@ -43,7 +43,8 @@
     <cr-profile-avatar-selector-grid id="avatar-grid"
         ignore-modified-key-events="[[ignoreModifiedKeyEvents]]">
       <template is="dom-repeat" items="[[avatars]]">
-        <paper-button class="avatar" title="[[item.label]]"
+        <paper-button class$="avatar [[getSelectedClass_(item.selected)]]"
+            title="[[item.label]]"
             style$="background-image: [[getIconImageSet_(item.url)]]"
             on-tap="onAvatarTap_">
         </paper-button>

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

@@ -10,7 +10,8 @@
 /**
  * @typedef {{url: string,
  *            label: string,
- *            isGaiaAvatar: (boolean|undefined)}}
+ *            isGaiaAvatar: (boolean|undefined),
+ *            selected: (boolean|undefined)}}
  */
 let AvatarIcon;
 
@@ -47,6 +48,13 @@ Polymer({
     },
   },
 
+  /** @private */
+  getSelectedClass_: function(isSelected) {
+    // TODO(dpapad): Rename 'iron-selected' to 'selected' now that this CSS
+    // class is not assigned by any iron-* behavior.
+    return isSelected ? 'iron-selected' : '';
+  },
+
   /**
    * @param {string} iconUrl
    * @return {string} A CSS image-set for multiple scale factors.
@@ -61,13 +69,14 @@ Polymer({
    * @private
    */
   onAvatarTap_: function(e) {
-    // TODO(dpapad): Rename 'iron-selected' to 'selected' now that this CSS
-    // class is not assigned by any iron-* behavior.
+    // Manual selection for profile creation
     if (this.selectedAvatarElement_)
       this.selectedAvatarElement_.classList.remove('iron-selected');
-
     this.selectedAvatarElement_ = /** @type {!HTMLElement} */ (e.target);
     this.selectedAvatarElement_.classList.add('iron-selected');
+
+    // |selectedAvatar| is set to pass back selection to the owner of this
+    // component.
     this.selectedAvatar =
         /** @type {!{model: {item: !AvatarIcon}}} */ (e).model.item;
   },