[Profiles] Download high-res avatars using the --new-profile-management flag

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/file_util.h"
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/path_service.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/profiles/profile_avatar_downloader.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
-#include "chrome/common/chrome_paths.h"
+#include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/common/pref_names.h"
 #include "components/signin/core/common/profile_management_switches.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
-#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_util.h"
 
 using content::BrowserThread;
 
 namespace {
 
 const char kNameKey[] = "name";
@@ skipping 143 matching lines @@
       info->Remove(kIsManagedKey, NULL);
       info->SetString(kManagedUserId, is_managed ? "DUMMY_ID" : std::string());
     }
     info->SetBoolean(kIsUsingDefaultName, IsDefaultName(name));
   }
 }
 
 ProfileInfoCache::~ProfileInfoCache() {
   STLDeleteContainerPairSecondPointers(
       cached_avatar_images_.begin(), cached_avatar_images_.end());
+  STLDeleteContainerPairSecondPointers(
+      avatar_images_downloads_in_progress_.begin(),
+      avatar_images_downloads_in_progress_.end());
 }
 
 void ProfileInfoCache::AddProfileToCache(const base::FilePath& profile_path,
                                          const base::string16& name,
                                          const base::string16& username,
                                          size_t icon_index,
                                          const std::string& managed_user_id) {
   std::string key = CacheKeyFromProfilePath(profile_path);
   DictionaryPrefUpdate update(prefs_, prefs::kProfileInfoCache);
   base::DictionaryValue* cache = update.Get();
@@ skipping 119 matching lines @@
 
 const gfx::Image& ProfileInfoCache::GetAvatarIconOfProfileAtIndex(
     size_t index) const {
   if (IsUsingGAIAPictureOfProfileAtIndex(index)) {
     const gfx::Image* image = GetGAIAPictureOfProfileAtIndex(index);
     if (image)
       return *image;
   }
 
   // Use the high resolution version of the avatar if it exists.
-  if (switches::IsNewProfileManagement()) {
+  if (switches::IsNewAvatarMenu()) {
     const gfx::Image* image = GetHighResAvatarOfProfileAtIndex(index);
     if (image)
       return *image;
   }
 
   int resource_id = profiles::GetDefaultAvatarIconResourceIDAtIndex(
       GetAvatarIconIndexOfProfileAtIndex(index));
   return ResourceBundle::GetSharedInstance().GetNativeImageNamed(resource_id);
 }
 
@@ skipping 38 matching lines @@
       kGAIAPictureFileNameKey, &file_name);
 
   // If the picture is not on disk then return NULL.
   if (file_name.empty())
     return NULL;
 
   base::FilePath image_path = path.AppendASCII(file_name);
   return LoadAvatarPictureFromPath(key, image_path);
 }
 
-const gfx::Image* ProfileInfoCache::GetHighResAvatarOfProfileAtIndex(
-    size_t index) const {
-  int avatar_index = GetAvatarIconIndexOfProfileAtIndex(index);
-  std::string key = profiles::GetDefaultAvatarIconFileNameAtIndex(avatar_index);
-
-  if (!strcmp(key.c_str(), profiles::GetNoHighResAvatarFileName()))
-    return NULL;
-
-  base::FilePath user_data_dir;
-  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
-  base::FilePath image_path = user_data_dir.
-      AppendASCII(profiles::kHighResAvatarFolderName).AppendASCII(key);
-  return LoadAvatarPictureFromPath(key, image_path);
-}
-
-const gfx::Image* ProfileInfoCache::LoadAvatarPictureFromPath(
-    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];
-  }
-
-  // If the picture is already being loaded then don't try loading it again.
-  if (cached_avatar_images_loading_[key])
-    return NULL;
-  cached_avatar_images_loading_[key] = true;
-
-  gfx::Image** image = new gfx::Image*;
-  BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
-      base::Bind(&ReadBitmap, image_path, image),
-      base::Bind(&ProfileInfoCache::OnAvatarPictureLoaded,
-          const_cast<ProfileInfoCache*>(this)->AsWeakPtr(), key, image));
-  return NULL;
-}
-
 bool ProfileInfoCache::ProfileIsManagedAtIndex(size_t index) const {
   return !GetManagedUserIdOfProfileAtIndex(index).empty();
 }
 
 bool ProfileInfoCache::IsOmittedProfileAtIndex(size_t index) const {
   bool value = false;
   GetInfoForProfileAtIndex(index)->GetBoolean(kIsOmittedFromProfileListKey,
                                               &value);
   return value;
 }
@@ skipping 16 matching lines @@
   GetInfoForProfileAtIndex(index)->GetBoolean(kProfileIsEphemeral, &value);
   return value;
 }
 
 bool ProfileInfoCache::ProfileIsUsingDefaultNameAtIndex(size_t index) const {
   bool value = false;
   GetInfoForProfileAtIndex(index)->GetBoolean(kIsUsingDefaultName, &value);
   return value;
 }
 
-void ProfileInfoCache::OnAvatarPictureLoaded(const std::string& key,
-                                             gfx::Image** image) const {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  cached_avatar_images_loading_[key] = false;
-
-  delete cached_avatar_images_[key];
-  if (*image) {
-    cached_avatar_images_[key] = *image;
-  } else {
-    // Place an empty image in the cache to avoid reloading it again.
-    cached_avatar_images_[key] = new gfx::Image();
-  }
-  delete image;
-
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED,
-      content::NotificationService::AllSources(),
-      content::NotificationService::NoDetails());
-}
-
-void ProfileInfoCache::OnGAIAPictureSaved(const base::FilePath& path,
-                                          bool* success) const  {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  if (*success) {
-    content::NotificationService::current()->Notify(
-        chrome::NOTIFICATION_PROFILE_CACHE_PICTURE_SAVED,
-        content::NotificationService::AllSources(),
-        content::NotificationService::NoDetails());
-  }
-  delete success;
-}
-
 bool ProfileInfoCache::IsUsingGAIAPictureOfProfileAtIndex(
     size_t index) const {
   bool value = false;
   GetInfoForProfileAtIndex(index)->GetBoolean(kUseGAIAPictureKey, &value);
   return value;
 }
 
 size_t ProfileInfoCache::GetAvatarIconIndexOfProfileAtIndex(size_t index)
     const {
   std::string icon_url;
@@ skipping 176 matching lines @@
 
   if (!image) {
     // Delete the old bitmap from disk.
     if (!old_file_name.empty()) {
       base::FilePath image_path = path.AppendASCII(old_file_name);
       BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
                               base::Bind(&DeleteBitmap, image_path));
     }
   } else {
     // Save the new bitmap to disk.
-    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.";
-    } else {
-      new_file_name = old_file_name.empty() ?
-          profiles::kGAIAPictureFileName : old_file_name;
-      base::FilePath image_path = path.AppendASCII(new_file_name);
-      bool* success = new bool;
-      BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
-          base::Bind(&SaveBitmap, data.release(), image_path, success),
-          base::Bind(&ProfileInfoCache::OnGAIAPictureSaved, AsWeakPtr(),
-                     path, success));
-    }
+    new_file_name =
+        old_file_name.empty() ? profiles::kGAIAPictureFileName : old_file_name;
+    base::FilePath image_path = path.AppendASCII(new_file_name);
+    SaveAvatarImageAtPath(image, key, image_path);
   }
 
   scoped_ptr<base::DictionaryValue> info(
       GetInfoForProfileAtIndex(index)->DeepCopy());
   info->SetString(kGAIAPictureFileNameKey, new_file_name);
   // This takes ownership of |info|.
   SetInfoForProfileAtIndex(index, info.release());
 
   FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
                     observer_list_,
@@ skipping 200 matching lines @@
   DCHECK(key_it != sorted_keys_.end());
   sorted_keys_.erase(key_it);
   sorted_keys_.insert(FindPositionForProfile(key, name), key);
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED,
       content::NotificationService::AllSources(),
       content::NotificationService::NoDetails());
 }
 
+const gfx::Image* ProfileInfoCache::GetHighResAvatarOfProfileAtIndex(
+    size_t index) const {
+  int avatar_index = GetAvatarIconIndexOfProfileAtIndex(index);
+  std::string key = profiles::GetDefaultAvatarIconFileNameAtIndex(avatar_index);

[msw, 2014/04/28 20:46:37]

This is no longer checked against GetNoHighResAvatarFileName , is that okay? (I
guess this is somehow related to GetPathOfHighResAvatarAtIndex using
GetDefaultAvatarIconResourceInfo instead of GetDefaultAvatarIconFileNameAtIndex
for the image path?)

[noms (inactive), 2014/04/29 19:12:27]

Argh, no, that was a rebase disaster. Thank you for catching it!
Fixed.
On 2014/04/28 20:46:37, msw wrote:

+  base::FilePath image_path =
+      profiles::GetPathOfHighResAvatarAtIndex(avatar_index);
+  return LoadAvatarPictureFromPath(key, image_path);
+}
+
+const gfx::Image* ProfileInfoCache::LoadAvatarPictureFromPath(
+    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];
+  }
+
+  // If the picture is already being loaded then don't try loading it again.
+  if (cached_avatar_images_loading_[key])
+    return NULL;
+  cached_avatar_images_loading_[key] = true;
+
+  gfx::Image** image = new gfx::Image*;
+  BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ReadBitmap, image_path, image),
+      base::Bind(&ProfileInfoCache::OnAvatarPictureLoaded,
+          const_cast<ProfileInfoCache*>(this)->AsWeakPtr(), key, image));
+  return NULL;
+}
+
+void ProfileInfoCache::OnAvatarPictureLoaded(const std::string& key,
+                                             gfx::Image** image) const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  cached_avatar_images_loading_[key] = false;
+  delete cached_avatar_images_[key];
+
+  if (*image) {
+    cached_avatar_images_[key] = *image;
+  } else {
+    // Place an empty image in the cache to avoid reloading it again.
+    cached_avatar_images_[key] = new gfx::Image();
+  }
+  delete image;
+
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED,
+      content::NotificationService::AllSources(),
+      content::NotificationService::NoDetails());
+}
+
+void ProfileInfoCache::OnAvatarPictureSaved(const std::string& file_name,
+                                            bool* success) {
+

[msw, 2014/04/28 20:46:37]

nit: remove new blank line

[noms (inactive), 2014/04/29 19:12:27]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (*success) {
+    content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_PROFILE_CACHE_PICTURE_SAVED,

[msw, 2014/04/28 20:46:37]

nit: these should be indented two more spaces.

[noms (inactive), 2014/04/29 19:12:27]

Done.

+      content::NotificationService::AllSources(),
+      content::NotificationService::NoDetails());
+  }
+  delete success;
+
+  // 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;
+}
+
 // static
 std::vector<base::string16> ProfileInfoCache::GetProfileNames() {
   std::vector<base::string16> names;
   PrefService* local_state = g_browser_process->local_state();
   const base::DictionaryValue* cache = local_state->GetDictionary(
       prefs::kProfileInfoCache);
   base::string16 name;
   for (base::DictionaryValue::Iterator it(*cache); !it.IsAtEnd();
        it.Advance()) {
     const base::DictionaryValue* info = NULL;
     it.value().GetAsDictionary(&info);
     info->GetString(kNameKey, &name);
     names.push_back(name);
   }
   return names;
 }
 
 // static
 void ProfileInfoCache::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterDictionaryPref(prefs::kProfileInfoCache);
 }
+
+void ProfileInfoCache::DownloadHighResAvatar(

[msw, 2014/04/28 20:46:37]

The order of all the function definitions in this file should match the
declaration order in the header.

[noms (inactive), 2014/04/29 19:12:27]

Done.

+    ProfileAvatarDownloader* avatar_downloader,
+    size_t icon_index) {
+  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;
+  avatar_images_downloads_in_progress_[file_name] = avatar_downloader;
+  avatar_downloader->Start();
+}
+
+void ProfileInfoCache::SaveAvatarImageAtPath(
+    const gfx::Image* image,
+    const std::string& key,
+    const base::FilePath& image_path) {
+

[msw, 2014/04/28 20:46:37]

nit: remove blank line

[noms (inactive), 2014/04/29 19:12:27]

Done.

+  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.";
+  } else {
+    bool* success = new bool;

[msw, 2014/04/28 20:46:37]

Why is this allocated here? SaveBitmap can call OnAvatarPictureSaved with its
own success value. I realize you haven't changed the original pattern, but I
would encourage you to revise this to something sane.

[noms (inactive), 2014/04/29 19:12:27]

I've changed it to passing the callback, and only running it in the successful
case. Is this better?

On 2014/04/28 20:46:37, msw wrote:

+    BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
+        base::Bind(&SaveBitmap, data.release(), image_path, success),
+        base::Bind(&ProfileInfoCache::OnAvatarPictureSaved, AsWeakPtr(),
+                   key, success));
+  }
+}