[Profiles] Send out less profile avatar related notifications

With the new profiles UI and the high res avatar downloading code, we are sending out
waaaay too many OnProfileAvatarChanged notifications. This is particularly terrible on
Windows, where basically on every Chrome startup we refresh all the icons.

Most importantly, we need to stop downloading the avatar files every time profiles get
loaded in the ProfileInfoCache. We did this originally because the avatar files were pretty
volatile, but it's safe to assume they've stopped changing.

Secondly, the only things that care about the high res avatar files are the UserManager
and the profile switcher, so they should get a special notification that this particular
file is ready, rather that annoy all the other listeners who only care about the tiny avatars.

BUG=449569

Review URL: https://codereview.chromium.org/861053004

Cr-Commit-Position: refs/heads/master@{#313593}

chrome/browser/profiles/profile_info_cache.cc

@@ -136,6 +136,13 @@ void ReadBitmap(const base::FilePath& image_path,
   *out_image = new gfx::Image(image);
 }
 
+void RunCallbackIfFileMissing(const base::FilePath& file_path,
+                              const base::Closure& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  if (!base::PathExists(file_path))
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+}
+
 void DeleteBitmap(const base::FilePath& image_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   base::DeleteFile(image_path, false);
@@ -215,7 +222,7 @@ void ProfileInfoCache::AddProfileToCache(
   sorted_keys_.insert(FindPositionForProfile(key, name), key);
 
   if (switches::IsNewAvatarMenu())
-    DownloadHighResAvatar(icon_index, profile_path);
+    DownloadHighResAvatarIfNeeded(icon_index, profile_path);
 
   FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
                     observer_list_,
@@ -543,9 +550,8 @@ void ProfileInfoCache::SetAvatarIconOfProfileAtIndex(size_t index,
 
   base::FilePath profile_path = GetPathOfProfileAtIndex(index);
 
-  // If needed, start downloading the high-res avatar.
   if (switches::IsNewAvatarMenu())
-    DownloadHighResAvatar(icon_index, profile_path);
+    DownloadHighResAvatarIfNeeded(icon_index, profile_path);
 
   FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
                     observer_list_,
@@ -700,7 +706,6 @@ void ProfileInfoCache::SetIsUsingGAIAPictureOfProfileAtIndex(size_t index,
   // This takes ownership of |info|.
   SetInfoForProfileAtIndex(index, info.release());
 
-  // Retrieve some info to update observers who care about avatar changes.
   base::FilePath profile_path = GetPathOfProfileAtIndex(index);
   FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
                     observer_list_,
@@ -866,7 +871,7 @@ void ProfileInfoCache::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterDictionaryPref(prefs::kProfileInfoCache);
 }
 
-void ProfileInfoCache::DownloadHighResAvatar(
+void ProfileInfoCache::DownloadHighResAvatarIfNeeded(
     size_t icon_index,
     const base::FilePath& profile_path) {
   // Downloading is only supported on desktop.
@@ -874,25 +879,15 @@ void ProfileInfoCache::DownloadHighResAvatar(
   return;
 #endif
 
-  // TODO(noms): We should check whether the file already exists on disk
-  // before trying to re-download it. For now, since this is behind a flag and
-  // the resources are still changing, re-download it every time the profile
-  // avatar changes, to make sure we have the latest copy.
-  std::string file_name = profiles::GetDefaultAvatarIconFileNameAtIndex(
-      icon_index);
-  // If the file is already being downloaded, don't start another download.
-  if (avatar_images_downloads_in_progress_[file_name])
-    return;
-
-  // Start the download for this file. The cache takes ownership of the
-  // |avatar_downloader|, which will be deleted when the download completes, or
-  // if that never happens, when the ProfileInfoCache is destroyed.
-  ProfileAvatarDownloader* avatar_downloader = new ProfileAvatarDownloader(
-      icon_index,
-      profile_path,
-      this);
-  avatar_images_downloads_in_progress_[file_name] = avatar_downloader;
-  avatar_downloader->Start();
+  const base::FilePath& file_path =
+      profiles::GetPathOfHighResAvatarAtIndex(icon_index);
+  base::Closure callback =
+      base::Bind(&ProfileInfoCache::DownloadHighResAvatar,
+                 AsWeakPtr(),
+                 icon_index,
+                 profile_path);
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&RunCallbackIfFileMissing, file_path, callback));
 }
 
 void ProfileInfoCache::SaveAvatarImageAtPath(
@@ -906,12 +901,18 @@ void ProfileInfoCache::SaveAvatarImageAtPath(
   scoped_refptr<base::RefCountedMemory> png_data = image->As1xPNGBytes();
   data->assign(png_data->front(), png_data->front() + png_data->size());
 
+  // Remove the file from the list of downloads in progress. Note that this list
+  // only contains the high resolution avatars, and not the Gaia profile images.
+  if (avatar_images_downloads_in_progress_[key]) {
+    delete avatar_images_downloads_in_progress_[key];
+    avatar_images_downloads_in_progress_[key] = NULL;
+  }
+
   if (!data->size()) {
     LOG(ERROR) << "Failed to PNG encode the image.";
   } else {
     base::Closure callback = base::Bind(&ProfileInfoCache::OnAvatarPictureSaved,
         AsWeakPtr(), key, profile_path);
-
     BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
         base::Bind(&SaveBitmap, base::Passed(&data), image_path, callback));
   }
@@ -1037,6 +1038,30 @@ const gfx::Image* ProfileInfoCache::GetHighResAvatarOfProfileAtIndex(
                                    key, image_path);
 }
 
+void ProfileInfoCache::DownloadHighResAvatar(
+    size_t icon_index,
+    const base::FilePath& profile_path) {
+  // Downloading is only supported on desktop.
+#if defined(OS_ANDROID) || defined(OS_IOS) || defined(OS_CHROMEOS)
+  return;
+#endif
+  const std::string file_name =
+      profiles::GetDefaultAvatarIconFileNameAtIndex(icon_index);
+  // If the file is already being downloaded, don't start another download.
+  if (avatar_images_downloads_in_progress_[file_name])
+    return;
+
+  // Start the download for this file. The cache takes ownership of the
+  // |avatar_downloader|, which will be deleted when the download completes, or
+  // if that never happens, when the ProfileInfoCache is destroyed.
+  ProfileAvatarDownloader* avatar_downloader = new ProfileAvatarDownloader(
+      icon_index,
+      profile_path,
+      this);
+  avatar_images_downloads_in_progress_[file_name] = avatar_downloader;
+  avatar_downloader->Start();
+}
+
 const gfx::Image* ProfileInfoCache::LoadAvatarPictureFromPath(
     const base::FilePath& profile_path,
     const std::string& key,
@@ -1085,7 +1110,7 @@ void ProfileInfoCache::OnAvatarPictureLoaded(const base::FilePath& profile_path,
 
   FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
                     observer_list_,
-                    OnProfileAvatarChanged(profile_path));
+                    OnProfileHighResAvatarLoaded(profile_path));
 }
 
 void ProfileInfoCache::OnAvatarPictureSaved(
@@ -1099,16 +1124,8 @@ void ProfileInfoCache::OnAvatarPictureSaved(
       content::NotificationService::NoDetails());
 
   FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
-                    observer_list_,
-                    OnProfileAvatarChanged(profile_path));
-
-  // Remove the file from the list of downloads in progress. Note that this list
-  // only contains the high resolution avatars, and not the Gaia profile images.
-  if (!avatar_images_downloads_in_progress_[file_name])
-    return;
-
-  delete avatar_images_downloads_in_progress_[file_name];
-  avatar_images_downloads_in_progress_[file_name] = NULL;
+      observer_list_,
+      OnProfileHighResAvatarLoaded(profile_path));
 }
 
 void ProfileInfoCache::MigrateLegacyProfileNamesAndDownloadAvatars() {
@@ -1132,9 +1149,8 @@ void ProfileInfoCache::MigrateLegacyProfileNamesAndDownloadAvatars() {
       l10n_util::GetStringUTF16(IDS_LEGACY_DEFAULT_PROFILE_NAME));
 
   for (size_t i = 0; i < GetNumberOfProfiles(); i++) {
-    // If needed, start downloading the high-res avatar for this profile.
-    DownloadHighResAvatar(GetAvatarIconIndexOfProfileAtIndex(i),
-                          GetPathOfProfileAtIndex(i));
+    DownloadHighResAvatarIfNeeded(GetAvatarIconIndexOfProfileAtIndex(i),
+                                  GetPathOfProfileAtIndex(i));
 
     base::string16 name = base::i18n::ToLower(GetNameOfProfileAtIndex(i));
     if (name == default_profile_name || name == default_legacy_profile_name)

chrome/browser/profiles/profile_info_cache.h

@@ -138,10 +138,10 @@ class ProfileInfoCache : public ProfileInfoInterface,
   // Register cache related preferences in Local State.
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
-  // Starts downloading the high res avatar at index |icon_index| for profile
-  // with path |profile_path|.
-  void DownloadHighResAvatar(size_t icon_index,
-                             const base::FilePath& profile_path);
+  // Checks whether the high res avatar at index |icon_index| exists, and
+  // if it does not, calls |DownloadHighResAvatar|.
+  void DownloadHighResAvatarIfNeeded(size_t icon_index,
+                                     const base::FilePath& profile_path);
 
   // Saves the avatar |image| at |image_path|. This is used both for the
   // GAIA profile pictures and the ProfileAvatarDownloader that is used to
@@ -186,6 +186,11 @@ class ProfileInfoCache : public ProfileInfoInterface,
   // generic profile avatar.
   const gfx::Image* GetHighResAvatarOfProfileAtIndex(size_t index) const;
 
+  // Starts downloading the high res avatar at index |icon_index| for profile
+  // with path |profile_path|.
+  void DownloadHighResAvatar(size_t icon_index,
+                             const base::FilePath& profile_path);
+
   // Returns the decoded image at |image_path|. Used both by the GAIA profile
   // image and the high res avatars.
   const gfx::Image* LoadAvatarPictureFromPath(

chrome/browser/profiles/profile_info_cache_observer.h

@@ -25,6 +25,8 @@ class ProfileInfoCacheObserver {
   virtual void OnProfileNameChanged(const base::FilePath& profile_path,
                                     const base::string16& old_profile_name) {}
   virtual void OnProfileAvatarChanged(const base::FilePath& profile_path) {}
+  virtual void OnProfileHighResAvatarLoaded(
+      const base::FilePath& profile_path) {}
   virtual void OnProfileSigninRequiredChanged(
       const base::FilePath& profile_path) {}
   virtual void OnProfileSupervisedUserIdChanged(

chrome/browser/profiles/profile_info_cache_unittest.cc

@@ -546,6 +546,7 @@ TEST_F(ProfileInfoCacheTest, DownloadHighResAvatarTest) {
   GetCache()->AddProfileToCache(path_1, ASCIIToUTF16("name_1"),
                                 base::string16(), 0, std::string());
   EXPECT_EQ(1U, GetCache()->GetNumberOfProfiles());
+  base::RunLoop().RunUntilIdle();
 
   // We haven't downloaded any high-res avatars yet.
   EXPECT_EQ(0U, GetCache()->cached_avatar_images_.size());
@@ -676,4 +677,3 @@ TEST_F(ProfileInfoCacheTest,
   EXPECT_EQ(name_4, GetCache()->GetNameOfProfileAtIndex(
       GetCache()->GetIndexOfProfileWithPath(path_4)));
 }
-

chrome/browser/ui/cocoa/profiles/avatar_base_controller.mm

@@ -93,7 +93,7 @@ class ProfileInfoUpdateObserver : public ProfileInfoCacheObserver,
   }
 
   void OnProfileAvatarChanged(const base::FilePath& profile_path) override {
-    if (profile_->GetPath() == profile_path)
+    if (!switches::IsNewAvatarMenu() && profile_->GetPath() == profile_path)
       [avatarController_ updateAvatarButtonAndLayoutParent:YES];
   }
 

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

@@ -150,12 +150,6 @@ void NewAvatarButton::OnProfileNameChanged(
     UpdateAvatarButtonAndRelayoutParent();
 }
 
-void NewAvatarButton::OnProfileAvatarChanged(
-      const base::FilePath& profile_path) {
-  if (browser_->profile()->GetPath() == profile_path)
-    UpdateAvatarButtonAndRelayoutParent();
-}
-
 void NewAvatarButton::OnProfileSupervisedUserIdChanged(
       const base::FilePath& profile_path) {
   if (browser_->profile()->GetPath() == profile_path)

chrome/browser/ui/views/profiles/new_avatar_button.h

@@ -40,7 +40,6 @@ class NewAvatarButton : public views::LabelButton,
                            const base::string16& profile_name) override;
   void OnProfileNameChanged(const base::FilePath& profile_path,
                             const base::string16& old_profile_name) override;
-  void OnProfileAvatarChanged(const base::FilePath& profile_path) override;
   void OnProfileSupervisedUserIdChanged(
       const base::FilePath& profile_path) override;
 

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

@@ -256,6 +256,11 @@ class UserManagerScreenHandler::ProfileUpdateObserver
     user_manager_handler_->SendUserList();
   }
 
+  void OnProfileHighResAvatarLoaded(
+      const base::FilePath& profile_path) override {
+    user_manager_handler_->SendUserList();
+  }
+
   void OnProfileSigninRequiredChanged(
       const base::FilePath& profile_path) override {
     user_manager_handler_->SendUserList();