ash: HiDPI user avatar for SessionController

chrome/browser/ui/ash/session_controller_client.cc

 // Copyright 2016 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/ui/ash/session_controller_client.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "ash/public/cpp/session_types.h"
@@ skipping 44 matching lines @@
   if (user_session_id == 0u)
     return nullptr;
 
   ash::mojom::UserSessionPtr session = ash::mojom::UserSession::New();
   session->session_id = user_session_id;
   session->type = user.GetType();
   session->account_id = user.GetAccountId();
   session->display_name = base::UTF16ToUTF8(user.display_name());
   session->display_email = user.display_email();
 
-  // TODO(xiyuan): Support multiple scale factor.
-  session->avatar = *user.GetImage().bitmap();
+  // Make sure all representations are loaded before sending via mojo.

[msw, 2017/03/23 20:47:51]

Hmm, I wonder if there's a way to ensure we always do this; can this be moved
into the struct traits? (also, do we need to call SetReadOnly() or
MakeThreadSafe() before allowing shared memory access?)

[xiyuan, 2017/03/24 05:45:19]

I left this part out of traits because only caller knows exactly what should be
transferred and can limit memory usage by only loading what are needed. However,
I agree it would make it error prone and harder to write code.

Moved EnsureRepsForSupportedScales() to struct trait. 

MakeThreadSafe() is not need to for using shared memory. A SkBitmap in
ImageSkia/ImageSkiaRep is immutable [1] so copying its pixel data should not be
problem.

Also MakeThreadSafe() has a side effect that removes ImageSkiaSource from
storage. This might not be the right thing. A few ImageSkia is implemented to
get representation asynchronously, e.g extensions::IconImage::Source[2].
EnsureRepsForSupportedScales would trigger the load and the ImageSkia would be
filled with correct reps later. But if MakeThreadSafe is used, the source is
deleted at the call and we get stub image reps instead.

[1]:
https://cs.chromium.org/chromium/src/ui/gfx/image/image_skia_rep.cc?rcl=d62fe...
[2]:
https://cs.chromium.org/chromium/src/extensions/browser/extension_icon_image....

+  user.GetImage().EnsureRepsForSupportedScales();
+
+  session->avatar = user.GetImage();
   if (session->avatar.isNull()) {
-    session->avatar = *ResourceBundle::GetSharedInstance()
-                           .GetImageSkiaNamed(IDR_PROFILE_PICTURE_LOADING)
-                           ->bitmap();
+    session->avatar = *ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+        IDR_PROFILE_PICTURE_LOADING);
   }
 
   return session;
 }
 
 void DoSwitchUser(const AccountId& account_id) {
   UserManager::Get()->SwitchActiveUser(account_id);
 }
 
 }  // namespace
@@ skipping 206 matching lines @@
   const UserList logged_in_users = user_manager->GetLoggedInUsers();
   std::vector<uint32_t> user_session_ids;
   for (auto* user : user_manager->GetLRULoggedInUsers()) {
     const uint32_t user_session_id = GetSessionId(*user);
     DCHECK_NE(0u, user_session_id);
     user_session_ids.push_back(user_session_id);
   }
 
   session_controller_->SetUserSessionOrder(user_session_ids);
 }