[Profiles] Send out less profile avatar related notifications

chrome/browser/profiles/profile_info_cache.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/profiles/profile_info_cache.h"
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
@@ skipping 117 matching lines @@
   gfx::Image image = gfx::Image::CreateFrom1xPNGBytes(
       base::RefCountedString::TakeString(&image_data));
   if (image.IsEmpty()) {
     LOG(ERROR) << "Failed to decode PNG file.";
     return;
   }
 
   *out_image = new gfx::Image(image);
 }
 
+void CheckIfFileExists(const base::FilePath& file_path,
+                       const base::Closure& callback) {
+  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);
 }
 
 }  // namespace
 
 ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
                                    const base::FilePath& user_data_dir)
     : prefs_(prefs),
@@ skipping 59 matching lines @@
   info->SetBoolean(kIsOmittedFromProfileListKey, !supervised_user_id.empty());
   info->SetBoolean(kProfileIsEphemeral, false);
   info->SetBoolean(kIsUsingDefaultNameKey, IsDefaultProfileName(name));
   // Assume newly created profiles use a default avatar.
   info->SetBoolean(kIsUsingDefaultAvatarKey, true);
   cache->SetWithoutPathExpansion(key, info.release());
 
   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_,
                     OnProfileAdded(profile_path));
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED,
       content::NotificationService::AllSources(),
       content::NotificationService::NoDetails());
 }
@@ skipping 301 matching lines @@
                                                      size_t icon_index) {
   scoped_ptr<base::DictionaryValue> info(
       GetInfoForProfileAtIndex(index)->DeepCopy());
   info->SetString(kAvatarIconKey,
       profiles::GetDefaultAvatarIconUrl(icon_index));
   // This takes ownership of |info|.
   SetInfoForProfileAtIndex(index, info.release());
 
   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_,
                     OnProfileAvatarChanged(profile_path));
 }
 
 void ProfileInfoCache::SetIsOmittedProfileAtIndex(size_t index,
                                                   bool is_omitted) {
   if (IsOmittedProfileAtIndex(index) == is_omitted)
     return;
@@ skipping 134 matching lines @@
 }
 
 void ProfileInfoCache::SetIsUsingGAIAPictureOfProfileAtIndex(size_t index,
                                                              bool value) {
   scoped_ptr<base::DictionaryValue> info(
       GetInfoForProfileAtIndex(index)->DeepCopy());
   info->SetBoolean(kUseGAIAPictureKey, value);
   // 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_,
                     OnProfileAvatarChanged(profile_path));
 }
 
 void ProfileInfoCache::SetProfileSigninRequiredAtIndex(size_t index,
                                                        bool value) {
   if (value == ProfileIsSigninRequiredAtIndex(index))
     return;
@@ skipping 134 matching lines @@
     names.push_back(name);
   }
   return names;
 }
 
 // static
 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.
 #if defined(OS_ANDROID) || defined(OS_IOS) || defined(OS_CHROMEOS)
   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 =

[Mike Lerman, 2015/01/27 16:39:16]

To Simplify the ProfileInfoCache, should all this logic be in the
ProfileAvatarDownloader? It could have a method called "StartIfNecessary" and
then all the logic goes in there.

[noms (inactive), 2015/01/28 02:18:36]

Hmm, I don't think so. That would mean we'd create a ProfileAvatarDownloader
every time we look at an avatar and wonder if we should download it (i.e. every
time Chrome starts), which is a little much.

+      profiles::GetPathOfHighResAvatarAtIndex(icon_index);
+  base::Closure callback =
+      base::Bind(&ProfileInfoCache::DownloadHighResAvatar,
+                 AsWeakPtr(),
+                 profiles::GetDefaultAvatarIconFileNameAtIndex(icon_index),
+                 icon_index,
+                 profile_path);
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&CheckIfFileExists, file_path, callback));
 }
 
 void ProfileInfoCache::SaveAvatarImageAtPath(
     const gfx::Image* image,
     const std::string& key,
     const base::FilePath& image_path,
     const base::FilePath& profile_path) {
   cached_avatar_images_[key] = new gfx::Image(*image);
 
   scoped_ptr<ImageData> data(new ImageData);
   scoped_refptr<base::RefCountedMemory> png_data = image->As1xPNGBytes();
   data->assign(png_data->front(), png_data->front() + png_data->size());
 
   if (!data->size()) {
     LOG(ERROR) << "Failed to PNG encode the image.";

[Mike Lerman, 2015/01/27 16:39:16]

In this case we don't delete the ProfileAvatarDownloader. We should! In fact
there, are several flows from creating the ProfileAvatarDownloader that don't
lead to OnAvatarPictureSaved, which all mean the ProfileAvatarDownloader sits in
memory until the end of Chrome.

[noms (inactive), 2015/01/28 02:18:35]

I think it makes more sense to delete the downloader before the file is actually
saved (it's not like the download is restarted) so now the dangling pointers
should be taken care of.

   } 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));
   }
 }
 
 const base::DictionaryValue* ProfileInfoCache::GetInfoForProfileAtIndex(
@@ skipping 105 matching lines @@
 
   if (!strcmp(key.c_str(), profiles::GetNoHighResAvatarFileName()))
       return NULL;
 
   base::FilePath image_path =
       profiles::GetPathOfHighResAvatarAtIndex(avatar_index);
   return LoadAvatarPictureFromPath(GetPathOfProfileAtIndex(index),
                                    key, image_path);
 }
 
+void ProfileInfoCache::DownloadHighResAvatar(
+    const std::string file_name,
+    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
+
+  // 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,
     const base::FilePath& image_path) const {
   // If the picture is already loaded then use it.
   if (cached_avatar_images_.count(key)) {
     if (cached_avatar_images_[key]->IsEmpty())
       return NULL;
     return cached_avatar_images_[key];
   }
@@ skipping 28 matching lines @@
   }
   delete image;
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED,
       content::NotificationService::AllSources(),
       content::NotificationService::NoDetails());
 
   FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
                     observer_list_,
-                    OnProfileAvatarChanged(profile_path));
+                    OnProfileHighResAvatarLoaded(profile_path));
 }
 
 void ProfileInfoCache::OnAvatarPictureSaved(
       const std::string& file_name,
       const base::FilePath& profile_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PROFILE_CACHE_PICTURE_SAVED,
       content::NotificationService::AllSources(),
       content::NotificationService::NoDetails());
 
   FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
-                    observer_list_,
-                    OnProfileAvatarChanged(profile_path));
+      observer_list_,
+      OnProfileHighResAvatarLoaded(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;
 }
 
@@ skipping 11 matching lines @@
   // depend on the names of all the other profiles in the info cache, so they
   // need to be re-computed after each rename.
   std::vector<base::FilePath> profiles_to_rename;
 
   const base::string16 default_profile_name = base::i18n::ToLower(
       l10n_util::GetStringUTF16(IDS_DEFAULT_PROFILE_NAME));
   const base::string16 default_legacy_profile_name = base::i18n::ToLower(
       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)
       profiles_to_rename.push_back(GetPathOfProfileAtIndex(i));
   }
 
   // Rename the necessary profiles.
   std::vector<base::FilePath>::const_iterator it;
   for (it = profiles_to_rename.begin(); it != profiles_to_rename.end(); ++it) {
     size_t profile_index = GetIndexOfProfileWithPath(*it);
     SetProfileIsUsingDefaultNameAtIndex(profile_index, true);
     // This will assign a new "Person %d" type name and re-sort the cache.
     SetNameOfProfileAtIndex(profile_index, ChooseNameForNewProfile(
         GetAvatarIconIndexOfProfileAtIndex(profile_index)));
   }
 #endif
 }