Commit 1baad057 authored by Monica Basta's avatar Monica Basta Committed by Commit Bot

[ProfileInfoCache]: Only override account image if it is of the right size.

The profile info cache relies now on the AccountFetcherService to fetch
the account info as well the account image. The AccountFetcherService
downloads the image on signin or every 24 hours. The profile info cache
used to rely on the profile_downloader to download 256 image while the
AccountFetcherService used to download 64 image. The
AccountFetcherService currently downloads 256 images but for existing
profiles there is a delay of 24 hours until the image of size 256 is
downloaded. To eliviate this case, the GaiaInfoUpdateService will do the
following:
(1) If the image's size is equal to |kAccountInfoImageSize|, it means it
is a newly downloaded image either for a new signin event or a 24 hour
refresh, so we should save it to the profile info cache.
(2) Else, it means it is an old image and should not override the
existing one.

Bug: 1028328
Change-Id: I2c77b2bf803b60aa98b46a80649697707e223005
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1936313Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Commit-Queue: Monica Basta <msalama@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719203}
parent 1dbf2a77
......@@ -19,6 +19,7 @@
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/common/pref_names.h"
#include "components/prefs/pref_service.h"
#include "components/signin/public/base/avatar_icon_util.h"
#include "components/signin/public/base/signin_pref_names.h"
#include "components/signin/public/identity_manager/account_info.h"
#include "content/public/browser/notification_details.h"
......@@ -86,17 +87,23 @@ void GAIAInfoUpdateService::Update(const AccountInfo& info) {
entry->SetGAIAGivenName(base::UTF8ToUTF16(info.given_name));
entry->SetGAIAName(base::UTF8ToUTF16(info.full_name));
const base::string16 hosted_domain = base::UTF8ToUTF16(info.hosted_domain);
profile_prefs_->SetString(prefs::kGoogleServicesHostedDomain,
base::UTF16ToUTF8(hosted_domain));
if (info.picture_url == kNoPictureURLFound) {
entry->SetGAIAPicture(gfx::Image());
} else if (!info.account_image.IsEmpty()) {
if (info.account_image.ToSkBitmap()->width() !=
signin::kAccountInfoImageSize) {
// All newly downloaded images should be of
// |signin::kAccountInfoImageSize| size.
return;
}
// Only set the image if it is not empty, to avoid clearing the image if we
// fail to download it on one of the 24 hours interval to refresh the data.
entry->SetGAIAPicture(info.account_image);
}
const base::string16 hosted_domain = base::UTF8ToUTF16(info.hosted_domain);
profile_prefs_->SetString(prefs::kGoogleServicesHostedDomain,
base::UTF16ToUTF8(hosted_domain));
}
// static
......
......@@ -50,8 +50,6 @@ const char kImageFetcherUmaClient[] = "AccountFetcherService";
const char AccountFetcherService::kLastUpdatePref[] =
"account_tracker_service_last_update";
const int AccountFetcherService::kAccountImageDownloadSize = 256;
// AccountFetcherService implementation
AccountFetcherService::AccountFetcherService() = default;
......@@ -317,7 +315,7 @@ void AccountFetcherService::FetchAccountImage(const CoreAccountId& account_id) {
"profile image."
})");
GURL image_url_with_size(signin::GetAvatarImageURLWithOptions(
picture_url, kAccountImageDownloadSize, true /* no_silhouette */));
picture_url, signin::kAccountInfoImageSize, true /* no_silhouette */));
auto callback = base::BindRepeating(&AccountFetcherService::OnImageFetched,
base::Unretained(this), account_id);
image_fetcher::ImageFetcherParams params(traffic_annotation,
......
......@@ -47,9 +47,6 @@ class AccountFetcherService : public ProfileOAuth2TokenServiceObserver {
// time the AccountTrackerService was updated.
static const char kLastUpdatePref[];
// Size used for downloading account pictures. Exposed for tests.
static const int kAccountImageDownloadSize;
AccountFetcherService();
~AccountFetcherService() override;
......
......@@ -110,8 +110,7 @@ std::string AccountKeyToPictureURL(AccountKey account_key) {
std::string AccountKeyToPictureURLWithSize(AccountKey account_key) {
return signin::GetAvatarImageURLWithOptions(
GURL(AccountKeyToPictureURL(account_key)),
AccountFetcherService::kAccountImageDownloadSize,
true /* no_silhouette */)
signin::kAccountInfoImageSize, true /* no_silhouette */)
.spec();
}
......
......@@ -119,6 +119,8 @@ std::vector<std::string> TryProcessAsContentImageURL(
namespace signin {
const int kAccountInfoImageSize = 256;
GURL GetAvatarImageURLWithOptions(const GURL& old_url,
int image_size,
bool no_silhouette) {
......
......@@ -9,6 +9,9 @@ class GURL;
namespace signin {
// Size of |AccountInfo| image.
extern const int kAccountInfoImageSize;
// Given an image URL this function builds a new URL, appending passed
// |image_size| and |no_silhouette| parameters. |old_url| must be valid.
// For example, if |image_size| was set to 256, |no_silhouette| was set to
......
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