[Profiles] Use high resolution avatars if they exist

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/format_macros.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/stringprintf.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/common/chrome_paths.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/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"
@@ skipping 18 matching lines @@
 const char kGAIAPictureFileNameKey[] = "gaia_picture_file_name";
 const char kIsManagedKey[] = "is_managed";
 const char kIsOmittedFromProfileListKey[] = "is_omitted_from_profile_list";
 const char kSigninRequiredKey[] = "signin_required";
 const char kManagedUserId[] = "managed_user_id";
 const char kProfileIsEphemeral[] = "is_ephemeral";
 const char kActiveTimeKey[] = "active_time";
 
 const char kDefaultUrlPrefix[] = "chrome://theme/IDR_PROFILE_AVATAR_";
 const char kGAIAPictureFileName[] = "Google Profile Picture.png";
+const char kHighResAvatarFolderName[] = "Avatars";
 
 const int kDefaultAvatarIconResources[] = {
   IDR_PROFILE_AVATAR_0,
   IDR_PROFILE_AVATAR_1,
   IDR_PROFILE_AVATAR_2,
   IDR_PROFILE_AVATAR_3,
   IDR_PROFILE_AVATAR_4,
   IDR_PROFILE_AVATAR_5,
   IDR_PROFILE_AVATAR_6,
   IDR_PROFILE_AVATAR_7,
@@ skipping 10 matching lines @@
   IDR_PROFILE_AVATAR_18,
   IDR_PROFILE_AVATAR_19,
   IDR_PROFILE_AVATAR_20,
   IDR_PROFILE_AVATAR_21,
   IDR_PROFILE_AVATAR_22,
   IDR_PROFILE_AVATAR_23,
   IDR_PROFILE_AVATAR_24,
   IDR_PROFILE_AVATAR_25,
 };
 
+// File names for the high-res avatar icon resources. In the same order as
+// the avatars in kDefaultAvatarIconResources.
+const char* kDefaultAvatarIconResourceFileNames[] = {
+  "chrome_avatar_generic.png",
+  "chrome_avatar_generic_aqua.png",
+  "chrome_avatar_generic_blue.png",
+  "chrome_avatar_generic_green.png",
+  "chrome_avatar_generic_orange.png",
+  "chrome_avatar_generic_purple.png",
+  "chrome_avatar_generic_red.png",
+  "chrome_avatar_generic_yellow.png",
+  "chrome_avatar_secret_agent.png",
+  "chrome_avatar_superhero.png",
+  "chrome_avatar_volley_ball.png",
+  "chrome_avatar_businessman.png",
+  "chrome_avatar_ninja.png",
+  "chrome_avatar_alien.png",
+  "chrome_avatar_smiley.png",
+  "chrome_avatar_flower.png",
+  "chrome_avatar_pizza.png",
+  "chrome_avatar_soccer.png",
+  "chrome_avatar_burger.png",
+  "chrome_avatar_cat.png",
+  "chrome_avatar_cupcake.png",
+  "chrome_avatar_dog.png",
+  "chrome_avatar_horse.png",
+  "chrome_avatar_margarita.png",
+  "chrome_avatar_note.png",
+  "chrome_avatar_sun_cloud.png",

[Roger Tawa OOO till Jul 10th, 2014/03/25 14:39:49]

Do we really need chrome_ prefix ?

[noms (inactive), 2014/03/25 19:24:22]

Nope! On 2014/03/25 14:39:49, Roger Tawa wrote:

+};
+
 const size_t kDefaultAvatarIconsCount = arraysize(kDefaultAvatarIconResources);
 
 // The first 8 icons are generic.
 const size_t kGenericIconCount = 8;
 
 // First eight are generic icons, which use IDS_NUMBERED_PROFILE_NAME.
 const int kDefaultNames[] = {
   IDS_DEFAULT_AVATAR_NAME_8,
   IDS_DEFAULT_AVATAR_NAME_9,
   IDS_DEFAULT_AVATAR_NAME_10,
@@ skipping 42 matching lines @@
 }
 
 // Reads a PNG from disk and decodes it. If the bitmap was successfully read
 // from disk the then |out_image| will contain the bitmap image, otherwise it
 // will be NULL.
 void ReadBitmap(const base::FilePath& image_path,
                 gfx::Image** out_image) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   *out_image = NULL;
 
+  // If the path doesn't exist, don't even try reading it.
+  if (!base::PathExists(image_path))
+    return;
+
   std::string image_data;
   if (!base::ReadFileToString(image_path, &image_data)) {
     LOG(ERROR) << "Failed to read PNG file from disk.";
     return;
   }
 
   gfx::Image image = gfx::Image::CreateFrom1xPNGBytes(
       base::RefCountedString::TakeString(&image_data));
   if (image.IsEmpty()) {
     LOG(ERROR) << "Failed to decode PNG file.";
@@ skipping 53 matching lines @@
     if (info->GetBoolean(kIsManagedKey, &is_managed)) {
       info->Remove(kIsManagedKey, NULL);
       info->SetString(kManagedUserId, is_managed ? "DUMMY_ID" : std::string());
     }
     info->SetBoolean(kIsUsingDefaultName, IsDefaultName(name));
   }
 }
 
 ProfileInfoCache::~ProfileInfoCache() {
   STLDeleteContainerPairSecondPointers(
-      gaia_pictures_.begin(), gaia_pictures_.end());
+      cached_avatar_images_.begin(), cached_avatar_images_.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 118 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()) {
+    const gfx::Image* image = GetHighResAvatarOfProfileAtIndex(index);
+    if (image)
+      return *image;
+  }
+
   int resource_id = GetDefaultAvatarIconResourceIDAtIndex(
       GetAvatarIconIndexOfProfileAtIndex(index));
   return ResourceBundle::GetSharedInstance().GetNativeImageNamed(resource_id);
 }
 
 std::string ProfileInfoCache::GetLocalAuthCredentialsOfProfileAtIndex(
     size_t index) const {
   std::string credentials;
   GetInfoForProfileAtIndex(index)->GetString(kAuthCredentialsKey, &credentials);
   return credentials;
@@ skipping 28 matching lines @@
   GetInfoForProfileAtIndex(index)->GetBoolean(kUseGAIANameKey, &value);
   return value;
 }
 
 const gfx::Image* ProfileInfoCache::GetGAIAPictureOfProfileAtIndex(
     size_t index) const {
   base::FilePath path = GetPathOfProfileAtIndex(index);
   std::string key = CacheKeyFromProfilePath(path);
 
   // If the picture is already loaded then use it.
-  if (gaia_pictures_.count(key)) {
-    if (gaia_pictures_[key]->IsEmpty())
+  if (cached_avatar_images_.count(key)) {
+    if (cached_avatar_images_[key]->IsEmpty())
       return NULL;
-    return gaia_pictures_[key];
+    return cached_avatar_images_[key];
   }
 
   std::string file_name;
   GetInfoForProfileAtIndex(index)->GetString(
       kGAIAPictureFileNameKey, &file_name);
 
-  // If the picture is not on disk or it is already being loaded then return
-  // NULL.
-  if (file_name.empty() || gaia_pictures_loading_[key])
+  // If the picture is not on disk then return NULL.
+  if (file_name.empty())
     return NULL;
 
-  gaia_pictures_loading_[key] = true;
   base::FilePath image_path = path.AppendASCII(file_name);
+  LoadAvatarPictureFromPath(key, image_path);
+  return NULL;
+}
+
+const gfx::Image* ProfileInfoCache::GetHighResAvatarOfProfileAtIndex(
+    size_t index) const {
+  int avatar_index = GetAvatarIconIndexOfProfileAtIndex(index);
+  std::string key = kDefaultAvatarIconResourceFileNames[avatar_index];
+
+  // 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];
+  }
+
+  base::FilePath user_data_dir;
+  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
+  base::FilePath image_path =
+      user_data_dir.AppendASCII(kHighResAvatarFolderName).AppendASCII(key);
+  LoadAvatarPictureFromPath(key, image_path);
+  return NULL;
+}

[Roger Tawa OOO till Jul 10th, 2014/03/25 14:39:49]

GetGAIAPictureOfProfileAtIndex() and GetHighResAvatarOfProfileAtIndex() are
really similar.  Both are only called from GetAvatarIconOfProfileAtIndex().  I
think it might be better to combine them into one function like:

const gfx::Image* GetAvatarOfProfileAtIndex(
    const std::string& key,
    const base::FilePath& path)

and have GetAvatarIconOfProfileAtIndex() calculate the correct key and path
depending on whether IsUsingGAIAPictureOfProfileAtIndex(index) returns true or
not, and error conditions.  You probably don't need LoadAvatarPictureFromPath()
as a separate function either.

[noms (inactive), 2014/03/25 19:24:22]

Unfortunately GetGAIAPictureOfProfileAtIndex() is used by a bunch of other
classes that don't have access to things like the key and the path. However, I
moved more code to the common area. Is it better?

On 2014/03/25 14:39:49, Roger Tawa wrote:

+
+void ProfileInfoCache::LoadAvatarPictureFromPath(
+    const std::string& key,
+    const base::FilePath& image_path) const {
+  // If the picture is already being loaded then don't try loading it again.
+  if (cached_avatar_images_loading_[key])
+    return;
+  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::OnGAIAPictureLoaded,
-          const_cast<ProfileInfoCache*>(this)->AsWeakPtr(), path, image));
-
-  return NULL;
+      base::Bind(&ProfileInfoCache::OnAvatarPictureLoaded,
+          const_cast<ProfileInfoCache*>(this)->AsWeakPtr(), key, image));
 }
 
 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);
@@ skipping 18 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::OnGAIAPictureLoaded(const base::FilePath& path,
-                                           gfx::Image** image) const {
+void ProfileInfoCache::OnAvatarPictureLoaded(const std::string& key,
+                                             gfx::Image** image) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  std::string key = CacheKeyFromProfilePath(path);
-  gaia_pictures_loading_[key] = false;
+  cached_avatar_images_loading_[key] = false;
 
   if (*image) {
-    delete gaia_pictures_[key];
-    gaia_pictures_[key] = *image;
+    delete cached_avatar_images_[key];
+    cached_avatar_images_[key] = *image;
   } else {
     // Place an empty image in the cache to avoid reloading it again.
-    gaia_pictures_[key] = new gfx::Image();
+    cached_avatar_images_[key] = new gfx::Image();

[Roger Tawa OOO till Jul 10th, 2014/03/25 14:39:49]

I still think there is a potential memory leak here.  Can we guarantee that if
|*image| is null, that |cached_avatar_images_[key]| does not already contain an
image?  Can we add a DCHECK for this condition?

[noms (inactive), 2014/03/25 19:24:22]

I've just deleted |cached_avatar_images_[key]| in all cases. If it's null, it's
still fine, if it's not, it will get cleaned out and reassigned in both cases.

On 2014/03/25 14:39:49, Roger Tawa wrote:

   }
   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,
@@ skipping 198 matching lines @@
                       OnProfileNameChanged(profile_path, old_display_name));
   }
 }
 
 void ProfileInfoCache::SetGAIAPictureOfProfileAtIndex(size_t index,
                                                       const gfx::Image* image) {
   base::FilePath path = GetPathOfProfileAtIndex(index);
   std::string key = CacheKeyFromProfilePath(path);
 
   // Delete the old bitmap from cache.
-  std::map<std::string, gfx::Image*>::iterator it = gaia_pictures_.find(key);
-  if (it != gaia_pictures_.end()) {
+  std::map<std::string, gfx::Image*>::iterator it =
+      cached_avatar_images_.find(key);
+  if (it != cached_avatar_images_.end()) {
     delete it->second;
-    gaia_pictures_.erase(it);
+    cached_avatar_images_.erase(it);
   }
 
   std::string old_file_name;
   GetInfoForProfileAtIndex(index)->GetString(
       kGAIAPictureFileNameKey, &old_file_name);
   std::string new_file_name;
 
   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.
-    gaia_pictures_[key] = new gfx::Image(*image);
+    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() ? kGAIAPictureFileName : old_file_name;
       base::FilePath image_path = path.AppendASCII(new_file_name);
       bool* success = new bool;
@@ skipping 297 matching lines @@
     info->GetString(kNameKey, &name);
     names.push_back(name);
   }
   return names;
 }
 
 // static
 void ProfileInfoCache::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterDictionaryPref(prefs::kProfileInfoCache);
 }