Avoid sending OnProfileAvatarChanged when the avatar image does not change

On Windows, Taskbar and Desktop icons are refreshed every time |OnProfileAvatarChanged|
notification is fired.

This CL only fires the notification |OnProfileAvatarChanged| when the profile
avatar image actually changes and avoids firing it otherwise.

Bug: 900374

Change-Id: I9c930a3eab9d59aa91c785715336d520dcc813c5
Reviewed-on: https://chromium-review.googlesource.com/c/1323711
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607196}

chrome/browser/profiles/gaia_info_update_service_unittest.cc

@@ -204,13 +204,27 @@ class GAIAInfoUpdateServiceTest : public GAIAInfoUpdateServiceTestBase {
   DISALLOW_COPY_AND_ASSIGN(GAIAInfoUpdateServiceTest);
 };
 
-class GAIAInfoUpdateServiceMiscTest : public GAIAInfoUpdateServiceTestBase {
+class GAIAInfoUpdateServiceMiscTest : public GAIAInfoUpdateServiceTestBase,
+                                      public ProfileInfoCacheObserver {
  public:
   GAIAInfoUpdateServiceMiscTest()
       : GAIAInfoUpdateServiceTestBase(
             /*create_gaia_info_service_on_setup_=*/false) {}
   ~GAIAInfoUpdateServiceMiscTest() override = default;
 
+  void OnProfileNameChanged(const base::FilePath& profile_path,
+                            const base::string16& old_profile_name) override {
+    profile_name_changed_count_++;
+  }
+
+  void OnProfileAvatarChanged(const base::FilePath& profile_path) override {
+    profile_avatar_changed_count_++;
+  }
+
+ protected:
+  unsigned int profile_name_changed_count_ = 0;
+  unsigned int profile_avatar_changed_count_ = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(GAIAInfoUpdateServiceMiscTest);
 };
@@ -373,7 +387,7 @@ TEST_F(GAIAInfoUpdateServiceTest, LogIn) {
 TEST_F(GAIAInfoUpdateServiceMiscTest, ClearGaiaInfoOnStartup) {
   // Simulate a state where the profile entry has GAIA related information
   // when there is not primary account set.
-  EXPECT_FALSE(identity_test_env()->identity_manager()->HasPrimaryAccount());
+  ASSERT_FALSE(identity_test_env()->identity_manager()->HasPrimaryAccount());
   ASSERT_EQ(1u, storage()->GetNumberOfProfiles());
   ProfileAttributesEntry* entry = storage()->GetAllProfilesAttributes().front();
   entry->SetGAIAName(base::UTF8ToUTF16("foo"));
@@ -381,11 +395,47 @@ TEST_F(GAIAInfoUpdateServiceMiscTest, ClearGaiaInfoOnStartup) {
   gfx::Image gaia_picture = gfx::test::CreateImage(256, 256);
   entry->SetGAIAPicture(&gaia_picture);
 
+  GetCache()->AddObserver(this);
+
   // Verify that creating the GAIAInfoUpdateService resets the GAIA related
-  // profile attributes if the profile no longer has a primary account.
+  // profile attributes if the profile no longer has a primary account and that
+  // the profile info cache observer wass notified about profile name and
+  // avatar changes.
+  service_.reset(new NiceMock<GAIAInfoUpdateServiceMock>(profile()));
+  EXPECT_TRUE(entry->GetGAIAName().empty());
+  EXPECT_TRUE(entry->GetGAIAGivenName().empty());
+  EXPECT_FALSE(entry->GetGAIAPicture());
+  EXPECT_EQ(1U, profile_name_changed_count_);
+  EXPECT_EQ(1U, profile_avatar_changed_count_);
+
+  GetCache()->RemoveObserver(this);
+}
+
+// Regression test for http://crbug.com/900374
+TEST_F(GAIAInfoUpdateServiceMiscTest,
+       ClearGaiaInfoForSignedOutProfileDoesNotNotifyProfileObservers) {
+  // Simulate a state where the profile entry has no GAIA related information
+  // and when there is not primary account set.
+  ASSERT_FALSE(identity_test_env()->identity_manager()->HasPrimaryAccount());
+  ASSERT_EQ(1u, storage()->GetNumberOfProfiles());
+  ProfileAttributesEntry* entry = storage()->GetAllProfilesAttributes().front();
+  ASSERT_TRUE(entry->GetGAIAName().empty());
+  ASSERT_TRUE(entry->GetGAIAGivenName().empty());
+  ASSERT_FALSE(entry->GetGAIAPicture());
+
+  GetCache()->AddObserver(this);
+
+  // Verify that the state for the profile entry did not change and that the
+  // profile info cache observer was not notified about any profile name
+  // and avatar changes.
   service_.reset(new NiceMock<GAIAInfoUpdateServiceMock>(profile()));
   EXPECT_TRUE(entry->GetGAIAName().empty());
   EXPECT_TRUE(entry->GetGAIAGivenName().empty());
   EXPECT_FALSE(entry->GetGAIAPicture());
+  EXPECT_EQ(0U, profile_name_changed_count_);
+  EXPECT_EQ(0U, profile_avatar_changed_count_);
+
+  GetCache()->RemoveObserver(this);
 }
+
 #endif

chrome/browser/profiles/profile_attributes_entry.cc

@@ -304,13 +304,24 @@ void ProfileAttributesEntry::SetAvatarIconIndex(size_t icon_index) {
     // switch to generic avatar
     icon_index = 0;
   }
-  SetString(kAvatarIconKey, profiles::GetDefaultAvatarIconUrl(icon_index));
+  std::string default_avatar_icon_url =
+      profiles::GetDefaultAvatarIconUrl(icon_index);
+  if (default_avatar_icon_url == GetString(kAvatarIconKey)) {
+    // On Windows, Taskbar and Desktop icons are refreshed every time
+    // |OnProfileAvatarChanged| notification is fired.
+    // As the current avatar icon is already set to |default_avatar_icon_url|,
+    // it is important to avoid firing |OnProfileAvatarChanged| in this case.
+    // See http://crbug.com/900374
+    return;
+  }
 
-  base::FilePath profile_path = GetPath();
+  SetString(kAvatarIconKey, default_avatar_icon_url);
 
-  if (!profile_info_cache_->GetDisableAvatarDownloadForTesting())
+  base::FilePath profile_path = GetPath();
+  if (!profile_info_cache_->GetDisableAvatarDownloadForTesting()) {
     profile_info_cache_->DownloadHighResAvatarIfNeeded(icon_index,
                                                        profile_path);
+  }
 
   profile_info_cache_->NotifyOnProfileAvatarChanged(profile_path);
 }

chrome/browser/profiles/profile_info_cache.cc

@@ -535,21 +535,28 @@ void ProfileInfoCache::SetGAIAPictureOfProfileAtIndex(size_t index,
   base::FilePath path = GetPathOfProfileAtIndex(index);
   std::string key = CacheKeyFromProfilePath(path);
 
-  // Delete the old bitmap from cache.
-  cached_avatar_images_.erase(key);
-
   std::string old_file_name;
   GetInfoForProfileAtIndex(index)->GetString(
       kGAIAPictureFileNameKey, &old_file_name);
   std::string new_file_name;
 
+  if (!image && old_file_name.empty()) {
+    // On Windows, Taskbar and Desktop icons are refreshed every time
+    // |OnProfileAvatarChanged| notification is fired.
+    // Updating from an empty image to a null image is a no-op and it is
+    // important to avoid firing |OnProfileAvatarChanged| in this case.
+    // See http://crbug.com/900374
+    DCHECK_EQ(0U, cached_avatar_images_.count(key));
+    return;
+  }
+
+  // Delete the old bitmap from cache.
+  cached_avatar_images_.erase(key);
   if (!image) {
     // Delete the old bitmap from disk.
-    if (!old_file_name.empty()) {
-      base::FilePath image_path = path.AppendASCII(old_file_name);
-      file_task_runner_->PostTask(FROM_HERE,
-                                  base::BindOnce(&DeleteBitmap, image_path));
-    }
+    base::FilePath image_path = path.AppendASCII(old_file_name);
+    file_task_runner_->PostTask(FROM_HERE,
+                                base::BindOnce(&DeleteBitmap, image_path));
   } else {
     // Save the new bitmap to disk.
     new_file_name =