Skip to content

Commit

Permalink
[M108 Merge] Avatars: Fix regression on existing users
Browse files Browse the repository at this point in the history
Users without the feature flag could get static images on login/lock
screen. Use the default way of creating the user image to prevent.

(cherry picked from commit 290150c)

Bug: 1385672
Test: Manual test
Change-Id: I81a3a5ebc44eba11b745194411b284dda591011d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4039701
Reviewed-by: Angela Xiao <angelaxiao@chromium.org>
Commit-Queue: Yue Li <updowndota@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1074353}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4048833
Auto-Submit: Yue Li <updowndota@chromium.org>
Cr-Commit-Position: refs/branch-heads/5359@{#943}
Cr-Branched-From: 27d3765-refs/heads/main@{#1058933}
  • Loading branch information
Yue Li authored and Chromium LUCI CQ committed Nov 23, 2022
1 parent 1007171 commit 3fe906b
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest, SaveUserDefaultImageIndex) {
EXPECT_TRUE(test::AreImagesEqual(default_image, user->GetImage()));
ExpectUserImageInfo(test_account_id1_,
default_user_image::kFirstDefaultImageIndex,
GetUserImagePath(test_account_id1_, "jpg"));
base::FilePath());
}

// Verifies that SaveUserImage() correctly sets and persists the chosen user
Expand Down Expand Up @@ -743,7 +743,7 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, PolicyOverridesUser) {
EXPECT_TRUE(test::AreImagesEqual(default_image, user->GetImage()));
ExpectUserImageInfo(enterprise_account_id_,
default_user_image::kFirstDefaultImageIndex,
GetUserImagePath(enterprise_account_id_, "jpg"));
base::FilePath());

// Set policy. Verify that the policy-provided user image is downloaded, set
// and persisted, overriding the previously set image.
Expand Down
26 changes: 9 additions & 17 deletions chrome/browser/ash/login/users/avatar/user_image_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -302,15 +302,11 @@ void UserImageManagerImpl::Job::LoadImage(base::FilePath image_path,
weak_factory_.GetWeakPtr(), true));
}
} else {
gfx::ImageSkia default_image =
default_user_image::GetDefaultImageDeprecated(image_index_);
std::unique_ptr<user_manager::UserImage> user_image(
user_manager::UserImage::CreateAndEncode(
default_image, user_manager::UserImage::ChooseImageFormat(
*default_image.bitmap())));
// Cache the in-use default image as part of the migration of avatar
// images to cloud.
UpdateUserAndSaveImage(std::move(user_image));
new user_manager::UserImage(
default_user_image::GetDefaultImageDeprecated(image_index_)));
UpdateUser(std::move(user_image));
NotifyJobDone();
}
} else if (image_index_ == user_manager::User::USER_IMAGE_EXTERNAL ||
image_index_ == user_manager::User::USER_IMAGE_PROFILE) {
Expand Down Expand Up @@ -345,17 +341,13 @@ void UserImageManagerImpl::Job::SetToDefaultImage(int default_image_index) {
image_url_, base::BindOnce(&Job::OnLoadImageDone,
weak_factory_.GetWeakPtr(), true));
} else {
gfx::ImageSkia default_image =
default_user_image::GetDefaultImageDeprecated(image_index_);
std::unique_ptr<user_manager::UserImage> user_image(
user_manager::UserImage::CreateAndEncode(
default_image, user_manager::UserImage::ChooseImageFormat(
*default_image.bitmap())));
new user_manager::UserImage(
default_user_image::GetDefaultImageDeprecated(image_index_)));

// Now that default images are served from the cloud, the current in-use
// user avatar image needs to be saved and cached in local state for
// offline usage.
UpdateUserAndSaveImage(std::move(user_image));
UpdateUser(std::move(user_image));
UpdateLocalState();
NotifyJobDone();
}
}

Expand Down

0 comments on commit 3fe906b

Please sign in to comment.