Skip to content

GitLab

T
tangled
Commit 0cb19571 authored by emaxx's avatar emaxx Committed by Commit bot
Browse files
Options

Don't try to download 'NothingToDownload' placeholder avatar. 

Fix ProfileInfoCache behaviour when it tried to download the
non-existing "NothingToDownload" placeholder profile avatar from the
Web, instead of just loading it from the resources.

BUG=486854

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

Cr-Commit-Position: refs/heads/master@{#330397}
parent 92015685
Hide whitespace changes
Inline Side-by-side
Showing with 76 additions and 49 deletions
chrome/browser/profiles/profile_avatar_icon_util.cc
View file @ 0cb19571
...@@ -202,9 +202,6 @@ const char kDefaultUrlPrefix[] = "chrome://theme/IDR_PROFILE_AVATAR_"; ...@@ -202,9 +202,6 @@ const char kDefaultUrlPrefix[] = "chrome://theme/IDR_PROFILE_AVATAR_";
const char kGAIAPictureFileName[] = "Google Profile Picture.png"; const char kGAIAPictureFileName[] = "Google Profile Picture.png";
const char kHighResAvatarFolderName[] = "Avatars"; const char kHighResAvatarFolderName[] = "Avatars";
// This avatar does not exist on the server, the high res copy is in the build.
const char kNoHighResAvatar[] = "NothingToDownload";
// The size of the function-static kDefaultAvatarIconResources array below. // The size of the function-static kDefaultAvatarIconResources array below.
const size_t kDefaultAvatarIconsCount = 27; const size_t kDefaultAvatarIconsCount = 27;
...@@ -212,7 +209,7 @@ const size_t kDefaultAvatarIconsCount = 27; ...@@ -212,7 +209,7 @@ const size_t kDefaultAvatarIconsCount = 27;
const size_t kGenericAvatarIconsCount = 8; const size_t kGenericAvatarIconsCount = 8;
// The avatar used as a placeholder (grey silhouette). // The avatar used as a placeholder (grey silhouette).
const int kPlaceholderAvatarIcon = 26; const size_t kPlaceholderAvatarIcon = 26;
gfx::Image GetSizedAvatarIcon(const gfx::Image& image, gfx::Image GetSizedAvatarIcon(const gfx::Image& image,
bool is_rectangle, bool is_rectangle,
...@@ -300,7 +297,7 @@ size_t GetGenericAvatarIconCount() { ...@@ -300,7 +297,7 @@ size_t GetGenericAvatarIconCount() {
return kGenericAvatarIconsCount; return kGenericAvatarIconsCount;
} }
int GetPlaceholderAvatarIndex() { size_t GetPlaceholderAvatarIndex() {
return kPlaceholderAvatarIcon; return kPlaceholderAvatarIcon;
} }
...@@ -309,34 +306,35 @@ int GetPlaceholderAvatarIconResourceID() { ...@@ -309,34 +306,35 @@ int GetPlaceholderAvatarIconResourceID() {
} }
const IconResourceInfo* GetDefaultAvatarIconResourceInfo(size_t index) { const IconResourceInfo* GetDefaultAvatarIconResourceInfo(size_t index) {
DCHECK(index < kDefaultAvatarIconsCount);
static const IconResourceInfo resource_info[kDefaultAvatarIconsCount] = { static const IconResourceInfo resource_info[kDefaultAvatarIconsCount] = {
{ IDR_PROFILE_AVATAR_0, "avatar_generic.png"}, {IDR_PROFILE_AVATAR_0, "avatar_generic.png"},
{ IDR_PROFILE_AVATAR_1, "avatar_generic_aqua.png"}, {IDR_PROFILE_AVATAR_1, "avatar_generic_aqua.png"},
{ IDR_PROFILE_AVATAR_2, "avatar_generic_blue.png"}, {IDR_PROFILE_AVATAR_2, "avatar_generic_blue.png"},
{ IDR_PROFILE_AVATAR_3, "avatar_generic_green.png"}, {IDR_PROFILE_AVATAR_3, "avatar_generic_green.png"},
{ IDR_PROFILE_AVATAR_4, "avatar_generic_orange.png"}, {IDR_PROFILE_AVATAR_4, "avatar_generic_orange.png"},
{ IDR_PROFILE_AVATAR_5, "avatar_generic_purple.png"}, {IDR_PROFILE_AVATAR_5, "avatar_generic_purple.png"},
{ IDR_PROFILE_AVATAR_6, "avatar_generic_red.png"}, {IDR_PROFILE_AVATAR_6, "avatar_generic_red.png"},
{ IDR_PROFILE_AVATAR_7, "avatar_generic_yellow.png"}, {IDR_PROFILE_AVATAR_7, "avatar_generic_yellow.png"},
{ IDR_PROFILE_AVATAR_8, "avatar_secret_agent.png"}, {IDR_PROFILE_AVATAR_8, "avatar_secret_agent.png"},
{ IDR_PROFILE_AVATAR_9, "avatar_superhero.png"}, {IDR_PROFILE_AVATAR_9, "avatar_superhero.png"},
{ IDR_PROFILE_AVATAR_10, "avatar_volley_ball.png"}, {IDR_PROFILE_AVATAR_10, "avatar_volley_ball.png"},
{ IDR_PROFILE_AVATAR_11, "avatar_businessman.png"}, {IDR_PROFILE_AVATAR_11, "avatar_businessman.png"},
{ IDR_PROFILE_AVATAR_12, "avatar_ninja.png"}, {IDR_PROFILE_AVATAR_12, "avatar_ninja.png"},
{ IDR_PROFILE_AVATAR_13, "avatar_alien.png"}, {IDR_PROFILE_AVATAR_13, "avatar_alien.png"},
{ IDR_PROFILE_AVATAR_14, "avatar_smiley.png"}, {IDR_PROFILE_AVATAR_14, "avatar_smiley.png"},
{ IDR_PROFILE_AVATAR_15, "avatar_flower.png"}, {IDR_PROFILE_AVATAR_15, "avatar_flower.png"},
{ IDR_PROFILE_AVATAR_16, "avatar_pizza.png"}, {IDR_PROFILE_AVATAR_16, "avatar_pizza.png"},
{ IDR_PROFILE_AVATAR_17, "avatar_soccer.png"}, {IDR_PROFILE_AVATAR_17, "avatar_soccer.png"},
{ IDR_PROFILE_AVATAR_18, "avatar_burger.png"}, {IDR_PROFILE_AVATAR_18, "avatar_burger.png"},
{ IDR_PROFILE_AVATAR_19, "avatar_cat.png"}, {IDR_PROFILE_AVATAR_19, "avatar_cat.png"},
{ IDR_PROFILE_AVATAR_20, "avatar_cupcake.png"}, {IDR_PROFILE_AVATAR_20, "avatar_cupcake.png"},
{ IDR_PROFILE_AVATAR_21, "avatar_dog.png"}, {IDR_PROFILE_AVATAR_21, "avatar_dog.png"},
{ IDR_PROFILE_AVATAR_22, "avatar_horse.png"}, {IDR_PROFILE_AVATAR_22, "avatar_horse.png"},
{ IDR_PROFILE_AVATAR_23, "avatar_margarita.png"}, {IDR_PROFILE_AVATAR_23, "avatar_margarita.png"},
{ IDR_PROFILE_AVATAR_24, "avatar_note.png"}, {IDR_PROFILE_AVATAR_24, "avatar_note.png"},
{ IDR_PROFILE_AVATAR_25, "avatar_sun_cloud.png"}, {IDR_PROFILE_AVATAR_25, "avatar_sun_cloud.png"},
{ IDR_PROFILE_AVATAR_26, kNoHighResAvatar}, {IDR_PROFILE_AVATAR_26, NULL},
}; };
return &resource_info[index]; return &resource_info[index];
} }
...@@ -348,17 +346,14 @@ int GetDefaultAvatarIconResourceIDAtIndex(size_t index) { ...@@ -348,17 +346,14 @@ int GetDefaultAvatarIconResourceIDAtIndex(size_t index) {
const char* GetDefaultAvatarIconFileNameAtIndex(size_t index) { const char* GetDefaultAvatarIconFileNameAtIndex(size_t index) {
DCHECK(index < kDefaultAvatarIconsCount); DCHECK(index < kDefaultAvatarIconsCount);
DCHECK(index != kPlaceholderAvatarIcon);
return GetDefaultAvatarIconResourceInfo(index)->filename; return GetDefaultAvatarIconResourceInfo(index)->filename;
} }
const char* GetNoHighResAvatarFileName() {
return kNoHighResAvatar;
}
base::FilePath GetPathOfHighResAvatarAtIndex(size_t index) { base::FilePath GetPathOfHighResAvatarAtIndex(size_t index) {
std::string file_name = GetDefaultAvatarIconResourceInfo(index)->filename; const char* file_name = GetDefaultAvatarIconFileNameAtIndex(index);
base::FilePath user_data_dir; base::FilePath user_data_dir;
PathService::Get(chrome::DIR_USER_DATA, &user_data_dir); CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir));
return user_data_dir.AppendASCII( return user_data_dir.AppendASCII(
kHighResAvatarFolderName).AppendASCII(file_name); kHighResAvatarFolderName).AppendASCII(file_name);
} }
......
chrome/browser/profiles/profile_avatar_icon_util.h
View file @ 0cb19571
...@@ -67,7 +67,7 @@ size_t GetDefaultAvatarIconCount(); ...@@ -67,7 +67,7 @@ size_t GetDefaultAvatarIconCount();
size_t GetGenericAvatarIconCount(); size_t GetGenericAvatarIconCount();
// Gets the index for the (grey silhouette) avatar used as a placeholder. // Gets the index for the (grey silhouette) avatar used as a placeholder.
int GetPlaceholderAvatarIndex(); size_t GetPlaceholderAvatarIndex();
// Gets the resource ID of the placeholder avatar icon. // Gets the resource ID of the placeholder avatar icon.
int GetPlaceholderAvatarIconResourceID(); int GetPlaceholderAvatarIconResourceID();
...@@ -78,9 +78,6 @@ int GetDefaultAvatarIconResourceIDAtIndex(size_t index); ...@@ -78,9 +78,6 @@ int GetDefaultAvatarIconResourceIDAtIndex(size_t index);
// Gets the resource filename of the default avatar icon at |index|. // Gets the resource filename of the default avatar icon at |index|.
const char* GetDefaultAvatarIconFileNameAtIndex(size_t index); const char* GetDefaultAvatarIconFileNameAtIndex(size_t index);
// Gets the file name of an avatar that has no high res version.
const char* GetNoHighResAvatarFileName();
// Gets the full path of the high res avatar icon at |index|. // Gets the full path of the high res avatar icon at |index|.
base::FilePath GetPathOfHighResAvatarAtIndex(size_t index); base::FilePath GetPathOfHighResAvatarAtIndex(size_t index);
......
chrome/browser/profiles/profile_info_cache.cc
View file @ 0cb19571
...@@ -905,6 +905,12 @@ void ProfileInfoCache::DownloadHighResAvatarIfNeeded( ...@@ -905,6 +905,12 @@ void ProfileInfoCache::DownloadHighResAvatarIfNeeded(
#endif #endif
DCHECK(!disable_avatar_download_for_testing_); DCHECK(!disable_avatar_download_for_testing_);
// If this is the placeholder avatar, it is already included in the
// resources, so it doesn't need to be downloaded (and it will never be
// requested from disk by GetHighResAvatarOfProfileAtIndex).
if (icon_index == profiles::GetPlaceholderAvatarIndex())
return;
const base::FilePath& file_path = const base::FilePath& file_path =
profiles::GetPathOfHighResAvatarAtIndex(icon_index); profiles::GetPathOfHighResAvatarAtIndex(icon_index);
base::Closure callback = base::Closure callback =
...@@ -1036,20 +1042,21 @@ void ProfileInfoCache::UpdateSortForProfileIndex(size_t index) { ...@@ -1036,20 +1042,21 @@ void ProfileInfoCache::UpdateSortForProfileIndex(size_t index) {
const gfx::Image* ProfileInfoCache::GetHighResAvatarOfProfileAtIndex( const gfx::Image* ProfileInfoCache::GetHighResAvatarOfProfileAtIndex(
size_t index) const { size_t index) const {
int avatar_index = GetAvatarIconIndexOfProfileAtIndex(index); const size_t avatar_index = GetAvatarIconIndexOfProfileAtIndex(index);
std::string key = profiles::GetDefaultAvatarIconFileNameAtIndex(avatar_index);
// If this is the placeholder avatar, it is already included in the // If this is the placeholder avatar, it is already included in the
// resources, so it doesn't need to be downloaded. // resources, so it doesn't need to be downloaded.
if (!strcmp(key.c_str(), profiles::GetNoHighResAvatarFileName())) { if (avatar_index == profiles::GetPlaceholderAvatarIndex()) {
return &ui::ResourceBundle::GetSharedInstance().GetImageNamed( return &ui::ResourceBundle::GetSharedInstance().GetImageNamed(
profiles::GetPlaceholderAvatarIconResourceID()); profiles::GetPlaceholderAvatarIconResourceID());
} }
base::FilePath image_path = const std::string file_name =
profiles::GetDefaultAvatarIconFileNameAtIndex(avatar_index);
const base::FilePath image_path =
profiles::GetPathOfHighResAvatarAtIndex(avatar_index); profiles::GetPathOfHighResAvatarAtIndex(avatar_index);
return LoadAvatarPictureFromPath(GetPathOfProfileAtIndex(index), return LoadAvatarPictureFromPath(GetPathOfProfileAtIndex(index), file_name,
key, image_path); image_path);
} }
void ProfileInfoCache::DownloadHighResAvatar( void ProfileInfoCache::DownloadHighResAvatar(
...@@ -1064,8 +1071,9 @@ void ProfileInfoCache::DownloadHighResAvatar( ...@@ -1064,8 +1071,9 @@ void ProfileInfoCache::DownloadHighResAvatar(
tracked_objects::ScopedTracker tracking_profile1( tracked_objects::ScopedTracker tracking_profile1(
FROM_HERE_WITH_EXPLICIT_FUNCTION( FROM_HERE_WITH_EXPLICIT_FUNCTION(
"461175 ProfileInfoCache::DownloadHighResAvatar::GetFileName")); "461175 ProfileInfoCache::DownloadHighResAvatar::GetFileName"));
const std::string file_name = const char* file_name =
profiles::GetDefaultAvatarIconFileNameAtIndex(icon_index); profiles::GetDefaultAvatarIconFileNameAtIndex(icon_index);
DCHECK(file_name);
// If the file is already being downloaded, don't start another download. // If the file is already being downloaded, don't start another download.
if (avatar_images_downloads_in_progress_.count(file_name)) if (avatar_images_downloads_in_progress_.count(file_name))
return; return;
......
chrome/browser/profiles/profile_info_cache.h
View file @ 0cb19571
...@@ -159,6 +159,8 @@ class ProfileInfoCache : public ProfileInfoInterface, ...@@ -159,6 +159,8 @@ class ProfileInfoCache : public ProfileInfoInterface,
private: private:
FRIEND_TEST_ALL_PREFIXES(ProfileInfoCacheTest, DownloadHighResAvatarTest); FRIEND_TEST_ALL_PREFIXES(ProfileInfoCacheTest, DownloadHighResAvatarTest);
FRIEND_TEST_ALL_PREFIXES(ProfileInfoCacheTest,
NothingToDownloadHighResAvatarTest);
const base::DictionaryValue* GetInfoForProfileAtIndex(size_t index) const; const base::DictionaryValue* GetInfoForProfileAtIndex(size_t index) const;
// Saves the profile info to a cache and takes ownership of |info|. // Saves the profile info to a cache and takes ownership of |info|.
......
chrome/browser/profiles/profile_info_cache_unittest.cc
View file @ 0cb19571
...@@ -616,6 +616,31 @@ TEST_F(ProfileInfoCacheTest, DownloadHighResAvatarTest) { ...@@ -616,6 +616,31 @@ TEST_F(ProfileInfoCacheTest, DownloadHighResAvatarTest) {
EXPECT_FALSE(base::PathExists(icon_path)); EXPECT_FALSE(base::PathExists(icon_path));
} }
TEST_F(ProfileInfoCacheTest, NothingToDownloadHighResAvatarTest) {
switches::EnableNewAvatarMenuForTesting(
base::CommandLine::ForCurrentProcess());
// The TestingProfileManager's ProfileInfoCache doesn't download avatars.
ProfileInfoCache profile_info_cache(
g_browser_process->local_state(),
testing_profile_manager_.profile_manager()->user_data_dir());
const size_t kIconIndex = profiles::GetPlaceholderAvatarIndex();
EXPECT_EQ(0U, profile_info_cache.GetNumberOfProfiles());
base::FilePath path_1 = GetProfilePath("path_1");
profile_info_cache.AddProfileToCache(path_1, ASCIIToUTF16("name_1"),
std::string(), base::string16(),
kIconIndex, std::string());
EXPECT_EQ(1U, profile_info_cache.GetNumberOfProfiles());
base::RunLoop().RunUntilIdle();
// We haven't tried to download any high-res avatars as the specified icon is
// just a placeholder.
EXPECT_EQ(0U, profile_info_cache.cached_avatar_images_.size());
EXPECT_EQ(0U, profile_info_cache.avatar_images_downloads_in_progress_.size());
}
TEST_F(ProfileInfoCacheTest, MigrateLegacyProfileNamesWithNewAvatarMenu) { TEST_F(ProfileInfoCacheTest, MigrateLegacyProfileNamesWithNewAvatarMenu) {
switches::EnableNewAvatarMenuForTesting( switches::EnableNewAvatarMenuForTesting(
base::CommandLine::ForCurrentProcess()); base::CommandLine::ForCurrentProcess());
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment