cros: Fix avatar loaded from disk not being shown in views-login.

Bug: 784495
Change-Id: I8a797815817b364b0dcdd5e836cad40c571cb2fa
Reviewed-on: https://chromium-review.googlesource.com/978862
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Oliver Chang <ochang@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550687}

ash/display/resolution_notification_controller_unittest.cc

@@ -397,6 +397,7 @@ TEST_F(ResolutionNotificationControllerTest, NoTimeoutInKioskMode) {
   mojom::UserSessionPtr session = mojom::UserSession::New();
   session->session_id = 1u;
   session->user_info = mojom::UserInfo::New();
+  session->user_info->avatar = mojom::UserAvatar::New();
   session->user_info->type = user_manager::USER_TYPE_KIOSK_APP;
   session->user_info->account_id = AccountId::FromUserEmail("user1@test.com");
   session->user_info->display_name = "User 1";

ash/login/login_screen_controller.cc

@@ -312,6 +312,12 @@ void LoginScreenController::SetPinEnabledForUser(const AccountId& account_id,
     DataDispatcher()->SetPinEnabledForUser(account_id, is_enabled);
 }
 
+void LoginScreenController::SetAvatarForUser(const AccountId& account_id,
+                                             mojom::UserAvatarPtr avatar) {
+  for (auto& observer : observers_)
+    observer.SetAvatarForUser(account_id, avatar);
+}
+
 void LoginScreenController::HandleFocusLeavingLockScreenApps(bool reverse) {
   for (auto& observer : observers_)
     observer.OnFocusLeavingLockScreenApps(reverse);

ash/login/login_screen_controller.h

@@ -96,6 +96,8 @@ class ASH_EXPORT LoginScreenController : public mojom::LoginScreen {
                  bool show_guest) override;
   void SetPinEnabledForUser(const AccountId& account_id,
                             bool is_enabled) override;
+  void SetAvatarForUser(const AccountId& account_id,
+                        mojom::UserAvatarPtr avatar) override;
   void HandleFocusLeavingLockScreenApps(bool reverse) override;
   void SetDevChannelInfo(const std::string& os_version_label_text,
                          const std::string& enterprise_info_text,

ash/login/login_screen_controller_observer.h

@@ -6,6 +6,9 @@
 #define ASH_LOGIN_LOGIN_SCREEN_CONTROLLER_OBSERVER_H_
 
 #include "ash/ash_export.h"
+#include "ash/public/interfaces/user_info.mojom.h"
+
+class AccountId;
 
 namespace ash {
 
@@ -15,6 +18,10 @@ class ASH_EXPORT LoginScreenControllerObserver {
  public:
   virtual ~LoginScreenControllerObserver();
 
+  // Called when |avatar| for |account_id| has changed.
+  virtual void SetAvatarForUser(const AccountId& account_id,
+                                const mojom::UserAvatarPtr& avatar) = 0;
+
   // Called when focus is leaving a lock screen app window due to tabbing.
   // |reverse| - whether the tab order is reversed.
   virtual void OnFocusLeavingLockScreenApps(bool reverse) = 0;

ash/login/ui/lock_contents_view.cc

@@ -592,6 +592,28 @@ void LockContentsView::OnDetachableBasePairingStatusChanged(
     GetWidget()->GetFocusManager()->ClearFocus();
 }
 
+void LockContentsView::SetAvatarForUser(const AccountId& account_id,
+                                        const mojom::UserAvatarPtr& avatar) {
+  auto replace = [&](const ash::mojom::LoginUserInfoPtr& user) {
+    auto changed = user->Clone();
+    changed->basic_user_info->avatar = avatar->Clone();
+    return changed;
+  };
+
+  LoginBigUserView* big =
+      TryToFindBigUser(account_id, false /*require_auth_active*/);
+  if (big) {
+    big->UpdateForUser(replace(big->GetCurrentUser()));
+    return;
+  }
+
+  LoginUserView* user = users_list_->GetUserView(account_id);
+  if (user) {
+    user->UpdateForUser(replace(user->current_user()), false /*animate*/);
+    return;
+  }
+}
+
 void LockContentsView::OnFocusLeavingLockScreenApps(bool reverse) {
   if (!reverse || lock_screen_apps_active_)
     FocusNextWidget(reverse);

ash/login/ui/lock_contents_view.h

@@ -109,6 +109,8 @@ class ASH_EXPORT LockContentsView : public NonAccessibleView,
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
   // LoginScreenController::Observer:
+  void SetAvatarForUser(const AccountId& account_id,
+                        const mojom::UserAvatarPtr& avatar) override;
   void OnFocusLeavingLockScreenApps(bool reverse) override;
 
   // LoginDataDispatcher::Observer:

ash/login/ui/login_test_utils.cc

@@ -36,6 +36,7 @@ LoginPasswordView::TestApi MakeLoginPasswordTestApi(LockContentsView* view,
 mojom::LoginUserInfoPtr CreateUser(const std::string& email) {
   auto user = mojom::LoginUserInfo::New();
   user->basic_user_info = mojom::UserInfo::New();
+  user->basic_user_info->avatar = mojom::UserAvatar::New();
   user->basic_user_info->account_id = AccountId::FromUserEmail(email);
   user->basic_user_info->display_name = base::SplitString(
       email, "@", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL)[0];
@@ -48,6 +49,7 @@ mojom::LoginUserInfoPtr CreatePublicAccountUser(const std::string& email) {
   user->basic_user_info = mojom::UserInfo::New();
   std::vector<std::string> email_parts = base::SplitString(
       email, "@", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+  user->basic_user_info->avatar = mojom::UserAvatar::New();
   user->basic_user_info->account_id = AccountId::FromUserEmail(email);
   user->basic_user_info->display_name = email_parts[0];
   user->basic_user_info->display_email = email;

ash/login/ui/login_user_view.cc

@@ -115,13 +115,13 @@ class LoginUserView::UserImage : public NonAccessibleView {
     // Set the initial image from |avatar| since we already have it available.
     // Then, decode the bytes via blink's PNG decoder and play any animated
     // frames if they are available.
-    if (!user->basic_user_info->avatar.isNull())
-      image_->SetImage(user->basic_user_info->avatar);
+    if (!user->basic_user_info->avatar->image.isNull())
+      image_->SetImage(user->basic_user_info->avatar->image);
 
     // Decode the avatar using blink, as blink's PNG decoder supports APNG,
     // which is the format used for the animated avators.
-    if (!user->basic_user_info->avatar_bytes.empty()) {
-      DecodeAnimation(user->basic_user_info->avatar_bytes,
+    if (!user->basic_user_info->avatar->bytes.empty()) {
+      DecodeAnimation(user->basic_user_info->avatar->bytes,
                       base::Bind(&LoginUserView::UserImage::OnImageDecoded,
                                  weak_factory_.GetWeakPtr()));
     }

ash/public/interfaces/login_screen.mojom

@@ -4,6 +4,7 @@
 
 module ash.mojom;
 
+import "ash/public/interfaces/user_info.mojom";
 import "ash/public/interfaces/login_user_info.mojom";
 import "chromeos/components/proximity_auth/public/interfaces/auth_type.mojom";
 import "components/password_manager/public/interfaces/sync_password_data.mojom";
@@ -68,6 +69,10 @@ interface LoginScreen {
   // |is_enabled|:   True if pin unlock is enabled.
   SetPinEnabledForUser(signin.mojom.AccountId account_id, bool is_enabled);
 
+  // Change the user's avatar. Some avatars may take a long time to load and the
+  // login screen may already be visible.
+  SetAvatarForUser(signin.mojom.AccountId account_id, UserAvatar avatar);
+
   // Called when focus is reported to be leaving a lock screen app window.
   // Requests focus to be handed off to the next suitable widget.
   // |reverse|:   Whether the tab order is reversed.

ash/public/interfaces/user_info.mojom

@@ -35,6 +35,14 @@ enum UserType {
   ACTIVE_DIRECTORY,
 };
 
+// Data for a user's avatar.
+struct UserAvatar {
+  gfx.mojom.ImageSkia image;
+  // The raw bytes for the avatar. Useful if the avatar is animated.
+  // TODO(crbug.com/770373): Use a shared buffer (mojo.Blob), as this may be
+  // large enough to congest IPC.
+  array<uint8> bytes;
+};
 
 // Info about a user. May be sent repeatedly for a single user because
 // individual fields may change (e.g. the avatar image or custodians).
@@ -43,11 +51,7 @@ struct UserInfo {
   signin.mojom.AccountId account_id;
   string display_name;
   string display_email;
-  gfx.mojom.ImageSkia avatar;
-  // The raw bytes for the avatar. Useful if the avatar is animated.
-  // TODO(crbug.com/770373): Use a shared buffer (mojo.Blob), as this may be
-  // large enough to congest IPC.
-  array<uint8> avatar_bytes;
+  UserAvatar avatar;
   // True if this user has a newly created profile (first time login on the
   // device)
   bool is_new_profile;

ash/session/test_session_controller_client.cc

@@ -129,6 +129,7 @@ void TestSessionControllerClient::AddUserSession(
   mojom::UserSessionPtr session = mojom::UserSession::New();
   session->session_id = ++fake_session_id_;
   session->user_info = mojom::UserInfo::New();
+  session->user_info->avatar = mojom::UserAvatar::New();
   session->user_info->type = user_type;
   session->user_info->account_id = account_id;
   session->user_info->display_name = "Über tray Über tray Über tray Über tray";

ash/system/unified/top_shortcuts_view.cc

@@ -35,7 +35,7 @@ views::View* CreateUserAvatarView() {
         gfx::CreateVectorIcon(kSystemMenuGuestIcon, kMenuIconColor);
     image_view->SetImage(icon, icon.size());
   } else {
-    image_view->SetImage(user_session->user_info->avatar,
+    image_view->SetImage(user_session->user_info->avatar->image,
                          gfx::Size(kTrayItemSize, kTrayItemSize));
   }
 

ash/system/user/tray_user.cc

@@ -227,7 +227,7 @@ void TrayUser::UpdateAvatarImage(LoginStatus status) {
 
   const mojom::UserSession* const user_session =
       session_controller->GetUserSession(0);
-  avatar_->SetImage(user_session->user_info->avatar,
+  avatar_->SetImage(user_session->user_info->avatar->image,
                     gfx::Size(kTrayItemSize, kTrayItemSize));
 
   // Unit tests might come here with no images for some users.

ash/system/user/tray_user_unittest.cc

@@ -274,7 +274,7 @@ TEST_F(TrayUserTest, AvatarChange) {
   const gfx::ImageSkia red_icon =
       CreateImageSkiaWithColor(kTrayItemSize, kTrayItemSize, SK_ColorRED);
   mojom::UserSessionPtr user = controller()->GetUserSession(0)->Clone();
-  user->user_info->avatar = red_icon;
+  user->user_info->avatar->image = red_icon;
   controller()->UpdateUserSession(std::move(user));
   EXPECT_TRUE(gfx::test::AreImagesEqual(
       gfx::Image(red_icon),

ash/system/user/user_card_view.cc

@@ -68,7 +68,7 @@ views::View* CreateUserAvatarView(int user_index) {
         gfx::CreateVectorIcon(kSystemMenuGuestIcon, kMenuIconColor);
     image_view->SetImage(icon, icon.size());
   } else {
-    image_view->SetImage(user_session->user_info->avatar,
+    image_view->SetImage(user_session->user_info->avatar->image,
                          gfx::Size(kTrayItemSize, kTrayItemSize));
   }
 

chrome/browser/chromeos/login/screens/user_selection_screen.cc

@@ -457,39 +457,26 @@ bool UserSelectionScreen::ShouldForceOnlineSignIn(
 }
 
 // static
-void UserSelectionScreen::FillUserMojoStruct(
-    const user_manager::User* user,
-    bool is_owner,
-    bool is_signin_to_add,
-    proximity_auth::mojom::AuthType auth_type,
-    const std::vector<std::string>* public_session_recommended_locales,
-    ash::mojom::LoginUserInfo* user_info) {
-  user_info->basic_user_info = ash::mojom::UserInfo::New();
-  user_info->basic_user_info->type = user->GetType();
-  user_info->basic_user_info->account_id = user->GetAccountId();
-  user_info->basic_user_info->display_name =
-      base::UTF16ToUTF8(user->GetDisplayName());
-  user_info->basic_user_info->display_email = user->display_email();
-
+ash::mojom::UserAvatarPtr UserSelectionScreen::BuildMojoUserAvatarForUser(
+    const user_manager::User* user) {
+  auto avatar = ash::mojom::UserAvatar::New();
   if (!user->GetImage().isNull()) {
-    user_info->basic_user_info->avatar = user->GetImage();
+    avatar->image = user->GetImage();
   } else {
-    user_info->basic_user_info->avatar =
-        *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
-            IDR_LOGIN_DEFAULT_USER);
+    avatar->image = *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+        IDR_LOGIN_DEFAULT_USER);
   }
 
   // TODO(jdufault): Unify image handling between this code and
   // user_image_source::GetUserImageInternal.
   auto load_image_from_resource = [&](int resource_id) {
     auto& rb = ui::ResourceBundle::GetSharedInstance();
-    base::StringPiece avatar =
+    base::StringPiece avatar_data =
         rb.GetRawDataResourceForScale(resource_id, rb.GetMaxScaleFactor());
-    user_info->basic_user_info->avatar_bytes.assign(avatar.begin(),
-                                                    avatar.end());
+    avatar->bytes.assign(avatar_data.begin(), avatar_data.end());
   };
   if (user->has_image_bytes()) {
-    user_info->basic_user_info->avatar_bytes.assign(
+    avatar->bytes.assign(
         user->image_bytes()->front(),
         user->image_bytes()->front() + user->image_bytes()->size());
   } else if (user->HasDefaultImage()) {
@@ -500,6 +487,24 @@ void UserSelectionScreen::FillUserMojoStruct(
     load_image_from_resource(IDR_LOGIN_DEFAULT_USER);
   }
 
+  return avatar;
+}
+
+// static
+void UserSelectionScreen::FillUserMojoStruct(
+    const user_manager::User* user,
+    bool is_owner,
+    bool is_signin_to_add,
+    proximity_auth::mojom::AuthType auth_type,
+    const std::vector<std::string>* public_session_recommended_locales,
+    ash::mojom::LoginUserInfo* user_info) {
+  user_info->basic_user_info = ash::mojom::UserInfo::New();
+  user_info->basic_user_info->type = user->GetType();
+  user_info->basic_user_info->account_id = user->GetAccountId();
+  user_info->basic_user_info->display_name =
+      base::UTF16ToUTF8(user->GetDisplayName());
+  user_info->basic_user_info->display_email = user->display_email();
+  user_info->basic_user_info->avatar = BuildMojoUserAvatarForUser(user);
   user_info->auth_type = auth_type;
   user_info->is_signed_in = user->is_logged_in();
   user_info->is_device_owner = is_owner;

chrome/browser/chromeos/login/screens/user_selection_screen.h

@@ -114,6 +114,11 @@ class UserSelectionScreen
   // Determines if user auth status requires online sign in.
   static bool ShouldForceOnlineSignIn(const user_manager::User* user);
 
+  // Builds a |UserAvatarPtr| instance which contains the current image for
+  // |user|.
+  static ash::mojom::UserAvatarPtr BuildMojoUserAvatarForUser(
+      const user_manager::User* user);
+
   // Fills |user_info| with information about |user|.
   // TODO: Public sesssions exist in login screen, but not lock screen.
   // We will need public session locales in the future when we change login

chrome/browser/chromeos/login/ui/login_display_mojo.cc

@@ -28,9 +28,12 @@ LoginDisplayMojo::LoginDisplayMojo(Delegate* delegate,
       user_selection_screen_(
           std::make_unique<ChromeUserSelectionScreen>(kLoginDisplay)) {
   user_selection_screen_->SetView(user_board_view_mojo_.get());
+  user_manager::UserManager::Get()->AddObserver(this);
 }
 
-LoginDisplayMojo::~LoginDisplayMojo() = default;
+LoginDisplayMojo::~LoginDisplayMojo() {
+  user_manager::UserManager::Get()->RemoveObserver(this);
+}
 
 void LoginDisplayMojo::ClearAndEnablePassword() {}
 
@@ -95,4 +98,10 @@ void LoginDisplayMojo::ShowUnrecoverableCrypthomeErrorDialog() {
   NOTIMPLEMENTED();
 }
 
+void LoginDisplayMojo::OnUserImageChanged(const user_manager::User& user) {
+  LoginScreenClient::Get()->login_screen()->SetAvatarForUser(
+      user.GetAccountId(),
+      UserSelectionScreen::BuildMojoUserAvatarForUser(&user));
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/login/ui/login_display_mojo.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "chrome/browser/chromeos/login/ui/login_display.h"
+#include "components/user_manager/user_manager.h"
 
 namespace chromeos {
 
@@ -18,7 +19,8 @@ class UserSelectionScreen;
 
 // Interface used by UI-agnostic code to send messages to views-based login
 // screen.
-class LoginDisplayMojo : public LoginDisplay {
+class LoginDisplayMojo : public LoginDisplay,
+                         public user_manager::UserManager::Observer {
  public:
   LoginDisplayMojo(Delegate* delegate, LoginDisplayHostMojo* host);
   ~LoginDisplayMojo() override;
@@ -41,6 +43,9 @@ class LoginDisplayMojo : public LoginDisplay {
   void ShowWhitelistCheckFailedError() override;
   void ShowUnrecoverableCrypthomeErrorDialog() override;
 
+  // user_manager::UserManager::Observer:
+  void OnUserImageChanged(const user_manager::User& user) override;
+
  private:
   LoginDisplayHostMojo* const host_ = nullptr;
   std::unique_ptr<UserBoardViewMojo> user_board_view_mojo_;

chrome/browser/ui/ash/session_controller_client.cc

@@ -50,9 +50,9 @@
 using session_manager::Session;
 using session_manager::SessionManager;
 using session_manager::SessionState;
-using user_manager::UserManager;
 using user_manager::User;
 using user_manager::UserList;
+using user_manager::UserManager;
 
 namespace {
 
@@ -95,9 +95,10 @@ ash::mojom::UserSessionPtr UserToUserSession(const User& user) {
   if (profile)
     session->user_info->is_new_profile = profile->IsNewProfile();
 
-  session->user_info->avatar = user.GetImage();
-  if (session->user_info->avatar.isNull()) {
-    session->user_info->avatar =
+  session->user_info->avatar = ash::mojom::UserAvatar::New();
+  session->user_info->avatar->image = user.GetImage();
+  if (session->user_info->avatar->image.isNull()) {
+    session->user_info->avatar->image =
         *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
             IDR_LOGIN_DEFAULT_USER);
   }