[cros] Encode user images when set from WebUI (regression fix).

This only fixes the avatar-related part of the bug.
UserImageLoader now always passes raw data to the callback.


BUG=139887


Review URL: https://chromiumcodereview.appspot.com/10830111

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149744 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 54 insertions, 36 deletions

diff --git a/chrome/browser/automation/automation_provider_observers_chromeos.cc b/chrome/browser/automation/automation_provider_observers_chromeos.cc
index 9da0036..e80d839 100644
--- a/chrome/browser/automation/automation_provider_observers_chromeos.cc
+++ b/chrome/browser/automation/automation_provider_observers_chromeos.cc
@@ -518,7 +518,8 @@ void PhotoCaptureObserver::OnCapturingStopped(
 
   // Set up an observer for UserManager (it will delete itself).
   user_manager->AddObserver(this);
-  user_manager->SaveUserImage(user.email(), chromeos::UserImage(photo));
+  user_manager->SaveUserImage(
+      user.email(), chromeos::UserImage::CreateAndEncode(photo));
 }
 
 void PhotoCaptureObserver::LocalStateChanged(
diff --git a/chrome/browser/chromeos/login/user.cc b/chrome/browser/chromeos/login/user.cc
index 835b0bd..da797c3 100644
--- a/chrome/browser/chromeos/login/user.cc
+++ b/chrome/browser/chromeos/login/user.cc
@@ -47,6 +47,7 @@ void User::SetImage(const UserImage& user_image, int image_index) {
   user_image_ = user_image;
   image_index_ = image_index;
   image_is_stub_ = false;
+  DCHECK(HasDefaultImage() || user_image.has_raw_image());
 }
 
 void User::SetImageURL(const GURL& image_url) {
@@ -70,6 +71,10 @@ std::string User::GetAccountName(bool use_display_email) const {
     return GetUserName(email_);
 }
 
+bool User::HasDefaultImage() const {
+  return image_index_ >= 0 && image_index_ < kDefaultImagesCount;
+}
+
 string16 User::GetDisplayName() const {
   // Fallback to the email account name in case display name haven't been set.
   return display_name_.empty() ?
diff --git a/chrome/browser/chromeos/login/user.h b/chrome/browser/chromeos/login/user.h
index 48d0f6e..d7d5f32 100644
--- a/chrome/browser/chromeos/login/user.h
+++ b/chrome/browser/chromeos/login/user.h
@@ -66,9 +66,8 @@ class User {
   // The image for this user.
   const gfx::ImageSkia& image() const { return user_image_.image(); }
 
-  bool has_default_image() const {
-    return image_index_ >= 0 && image_index_ < kDefaultImagesCount;
-  }
+  // Whether the user has a default image.
+  bool HasDefaultImage() const;
 
   int image_index() const { return image_index_; }
   bool has_raw_image() const { return user_image_.has_raw_image(); }
diff --git a/chrome/browser/chromeos/login/user_image.cc b/chrome/browser/chromeos/login/user_image.cc
index 088e2ff..fd00c1e 100644
--- a/chrome/browser/chromeos/login/user_image.cc
+++ b/chrome/browser/chromeos/login/user_image.cc
@@ -23,6 +23,13 @@ bool IsAnimatedImage(const UserImage::RawImage& data) {
 
 }  // namespace
 
+// static
+UserImage UserImage::CreateAndEncode(const gfx::ImageSkia& image) {
+  RawImage raw_image;
+  return gfx::PNGCodec::EncodeBGRASkBitmap(image, false, &raw_image) ?
+      UserImage(image, raw_image) : UserImage(image);
+}
+
 UserImage::UserImage()
     : has_raw_image_(false),
       has_animated_image_(false) {
@@ -52,4 +59,8 @@ UserImage::UserImage(const gfx::ImageSkia& image,
 
 UserImage::~UserImage() {}
 
+void UserImage::DiscardRawImage() {
+  RawImage().swap(raw_image_);  // Clear |raw_image_|.
+}
+
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/login/user_image.h b/chrome/browser/chromeos/login/user_image.h
index 0afb266..b2a52f9 100644
--- a/chrome/browser/chromeos/login/user_image.h
+++ b/chrome/browser/chromeos/login/user_image.h
@@ -20,6 +20,10 @@ class UserImage {
   // TODO(ivankr): replace with RefCountedMemory to prevent copying.
   typedef std::vector<unsigned char> RawImage;
 
+  // Creates a new instance from a given still frame and tries to encode raw
+  // representation for it.
+  static UserImage CreateAndEncode(const gfx::ImageSkia& image);
+
   // Create instance with an empty still frame and no raw data.
   UserImage();
 
@@ -40,6 +44,9 @@ class UserImage {
   bool has_raw_image() const { return has_raw_image_; }
   const RawImage& raw_image() const { return raw_image_; }
 
+  // Discards the stored raw image, freeing used memory.
+  void DiscardRawImage();
+
   // Optional raw representation of the animated image.
   bool has_animated_image() const { return has_animated_image_; }
   const RawImage& animated_image() const { return animated_image_; }
diff --git a/chrome/browser/chromeos/login/user_image_loader.cc b/chrome/browser/chromeos/login/user_image_loader.cc
index 94925fc..45f8228 100644
--- a/chrome/browser/chromeos/login/user_image_loader.cc
+++ b/chrome/browser/chromeos/login/user_image_loader.cc
@@ -22,10 +22,8 @@ using content::BrowserThread;
 namespace chromeos {
 
 UserImageLoader::ImageInfo::ImageInfo(int size,
-                                      bool load_raw_image,
                                       const LoadedCallback& loaded_cb)
     : size(size),
-      load_raw_image(load_raw_image),
       loaded_cb(loaded_cb) {
 }
 
@@ -41,11 +39,10 @@ UserImageLoader::~UserImageLoader() {
 
 void UserImageLoader::Start(const std::string& filepath,
                             int size,
-                            bool load_raw_image,
                             const LoadedCallback& loaded_cb) {
   target_message_loop_ = MessageLoop::current();
 
-  ImageInfo image_info(size, load_raw_image, loaded_cb);
+  ImageInfo image_info(size, loaded_cb);
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&UserImageLoader::LoadImage, this, filepath, image_info));
@@ -96,15 +93,10 @@ void UserImageLoader::OnImageDecoded(const ImageDecoder* decoder,
     }
   }
 
-  scoped_ptr<UserImage> user_image;
-  if (image_info.load_raw_image)
-    user_image.reset(new UserImage(final_image, decoder->get_image_data()));
-  else
-    user_image.reset(new UserImage(final_image));
-
   target_message_loop_->PostTask(
       FROM_HERE,
-      base::Bind(image_info.loaded_cb, *user_image));
+      base::Bind(image_info.loaded_cb,
+                 UserImage(final_image, decoder->get_image_data())));
 
   image_info_map_.erase(info_it);
 }
diff --git a/chrome/browser/chromeos/login/user_image_loader.h b/chrome/browser/chromeos/login/user_image_loader.h
index eb210e1..01b55fd 100644
--- a/chrome/browser/chromeos/login/user_image_loader.h
+++ b/chrome/browser/chromeos/login/user_image_loader.h
@@ -25,7 +25,7 @@ class UserImage;
 class UserImageLoader : public base::RefCountedThreadSafe<UserImageLoader>,
                         public ImageDecoder::Delegate {
  public:
-  // Callback used to inidicate that image has been loaded.
+  // Callback used to indicate that image has been loaded.
   typedef base::Callback<void(const UserImage& user_image)> LoadedCallback;
 
   UserImageLoader();
@@ -34,8 +34,7 @@ class UserImageLoader : public base::RefCountedThreadSafe<UserImageLoader>,
   // |loaded_cb| when image has been successfully loaded.
   // If |size| is positive, image is cropped and (if needed) downsized to
   // |size|x|size| pixels.
-  // If |load_raw_image| is true, raw image is also passed to callback.
-  void Start(const std::string& filepath, int size, bool load_raw_image,
+  void Start(const std::string& filepath, int size,
              const LoadedCallback& loaded_cb);
 
  private:
@@ -43,12 +42,11 @@ class UserImageLoader : public base::RefCountedThreadSafe<UserImageLoader>,
 
   // Contains attributes we need to know about each image we decode.
   struct ImageInfo {
-    ImageInfo(int size, bool load_raw_image, const LoadedCallback& loaded_cb);
+    ImageInfo(int size, const LoadedCallback& loaded_cb);
     ~ImageInfo();
 
-    int size;
-    bool load_raw_image;
-    LoadedCallback loaded_cb;
+    const int size;
+    const LoadedCallback loaded_cb;
   };
 
   typedef std::map<const ImageDecoder*, ImageInfo> ImageInfoMap;
diff --git a/chrome/browser/chromeos/login/user_image_screen.cc b/chrome/browser/chromeos/login/user_image_screen.cc
index dd23b94..6122e5e 100644
--- a/chrome/browser/chromeos/login/user_image_screen.cc
+++ b/chrome/browser/chromeos/login/user_image_screen.cc
@@ -116,8 +116,10 @@ void UserImageScreen::StopCamera() {
 
 void UserImageScreen::OnPhotoTaken(const gfx::ImageSkia& image) {
   UserManager* user_manager = UserManager::Get();
+  // TODO(ivankr): once old camera UI is gone, there's raw data in image
+  // decoder, pass UserImage and user it instead.
   user_manager->SaveUserImage(user_manager->GetLoggedInUser().email(),
-                              UserImage(image));
+                              UserImage::CreateAndEncode(image));
 
   get_screen_observer()->OnExit(ScreenObserver::USER_IMAGE_SELECTED);
 
diff --git a/chrome/browser/chromeos/login/user_manager_impl.cc b/chrome/browser/chromeos/login/user_manager_impl.cc
index 2cdd827..9ac4191 100644
--- a/chrome/browser/chromeos/login/user_manager_impl.cc
+++ b/chrome/browser/chromeos/login/user_manager_impl.cc
@@ -420,7 +420,7 @@ void UserManagerImpl::UserSelected(const std::string& email) {
       ash::WallpaperLayout layout = static_cast<ash::WallpaperLayout>(index);
       // Load user image asynchronously.
       image_loader_->Start(
-          wallpaper_path, 0, false,
+          wallpaper_path, 0,
           base::Bind(&UserManagerImpl::OnCustomWallpaperLoaded,
                      base::Unretained(this), email, layout));
       return;
@@ -631,7 +631,7 @@ void UserManagerImpl::SaveUserImageFromFile(const std::string& username,
                                             const FilePath& path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   image_loader_->Start(
-      path.value(), login::kMaxUserImageSize, true,
+      path.value(), login::kMaxUserImageSize,
       base::Bind(&UserManagerImpl::SaveUserImage,
                  base::Unretained(this), username));
 }
@@ -644,7 +644,7 @@ void UserManagerImpl::SaveUserWallpaperFromFile(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // For wallpapers, save the image without resizing.
   image_loader_->Start(
-      path.value(), 0 /* Original size */, false,
+      path.value(), 0 /* Original size */,
       base::Bind(&UserManagerImpl::SaveUserWallpaperInternal,
                  base::Unretained(this), username, layout, User::CUSTOMIZED,
                  delegate));
@@ -656,9 +656,10 @@ void UserManagerImpl::SaveUserImageFromProfileImage(
   if (!downloaded_profile_image_.empty()) {
     // Profile image has already been downloaded, so save it to file right now.
     DCHECK(profile_image_url_.is_valid());
-    SaveUserImageInternal(username,
-                          User::kProfileImageIndex, profile_image_url_,
-                          UserImage(downloaded_profile_image_));
+    SaveUserImageInternal(
+        username,
+        User::kProfileImageIndex, profile_image_url_,
+        UserImage::CreateAndEncode(downloaded_profile_image_));
   } else {
     // No profile image - use the stub image (gray avatar).
     SetUserImage(username, User::kProfileImageIndex,
@@ -849,7 +850,7 @@ void UserManagerImpl::EnsureUsersLoaded() {
               DCHECK(!image_path.empty());
               // Load user image asynchronously.
               image_loader_->Start(
-                  image_path, user_image_size, true,
+                  image_path, user_image_size,
                   base::Bind(&UserManagerImpl::SetUserImage,
                              base::Unretained(this),
                              email, image_index, GURL()));
@@ -879,7 +880,7 @@ void UserManagerImpl::EnsureUsersLoaded() {
               if (!image_path.empty()) {
                 // Load user image asynchronously.
                 image_loader_->Start(
-                    image_path, user_image_size, true,
+                    image_path, user_image_size,
                     base::Bind(&UserManagerImpl::SetUserImage,
                                base::Unretained(this),
                                email, image_index, image_gurl));
@@ -1270,7 +1271,7 @@ void UserManagerImpl::OnCustomWallpaperLoaded(const std::string& email,
   std::string wallpaper_thumbnail_path =
       GetWallpaperPathForUser(email, true).value();
   image_loader_->Start(
-      wallpaper_thumbnail_path, 0, false,
+      wallpaper_thumbnail_path, 0,
       base::Bind(&UserManagerImpl::OnCustomWallpaperThumbnailLoaded,
       base::Unretained(this), email));
 }
diff --git a/chrome/browser/chromeos/login/wallpaper_manager.cc b/chrome/browser/chromeos/login/wallpaper_manager.cc
index 05d6c86..46dec2b 100644
--- a/chrome/browser/chromeos/login/wallpaper_manager.cc
+++ b/chrome/browser/chromeos/login/wallpaper_manager.cc
@@ -114,7 +114,7 @@ void WallpaperManager::SetLastSelectedUser(
 void WallpaperManager::SetWallpaperFromFilePath(const std::string& path,
                                                 ash::WallpaperLayout layout) {
   image_loader_->Start(
-      path, 0, false,
+      path, 0,
       base::Bind(&WallpaperManager::OnWallpaperLoaded,
                  base::Unretained(this), layout));
 }
diff --git a/chrome/browser/ui/webui/options2/chromeos/change_picture_options_handler.cc b/chrome/browser/ui/webui/options2/chromeos/change_picture_options_handler.cc
index efe7bf9..55eac97a 100644
--- a/chrome/browser/ui/webui/options2/chromeos/change_picture_options_handler.cc
+++ b/chrome/browser/ui/webui/options2/chromeos/change_picture_options_handler.cc
@@ -302,7 +302,7 @@ void ChangePictureOptionsHandler::HandleSelectImage(const ListValue* args) {
 
     DCHECK(!previous_image_.empty());
     user_manager->SaveUserImage(user.email(),
-                                chromeos::UserImage(previous_image_));
+                                UserImage::CreateAndEncode(previous_image_));
 
     UMA_HISTOGRAM_ENUMERATION("UserImage.ChangeChoice",
                               kHistogramImageOld,
@@ -361,8 +361,10 @@ void ChangePictureOptionsHandler::FileSelected(const FilePath& path,
 
 void ChangePictureOptionsHandler::OnPhotoAccepted(const gfx::ImageSkia& photo) {
   UserManager* user_manager = UserManager::Get();
+  // TODO(ivankr): once old camera UI is gone, there's always raw data in
+  // |image_decoder_|, pass UserImage and user it instead.
   user_manager->SaveUserImage(user_manager->GetLoggedInUser().email(),
-                              chromeos::UserImage(photo));
+                              UserImage::CreateAndEncode(photo));
   UMA_HISTOGRAM_ENUMERATION("UserImage.ChangeChoice",
                             kHistogramImageFromCamera,
                             kHistogramImagesCount);
diff --git a/chrome/browser/ui/webui/options2/chromeos/user_image_source.cc b/chrome/browser/ui/webui/options2/chromeos/user_image_source.cc
index 01f0efe..57f4438 100644
--- a/chrome/browser/ui/webui/options2/chromeos/user_image_source.cc
+++ b/chrome/browser/ui/webui/options2/chromeos/user_image_source.cc
@@ -73,7 +73,7 @@ base::RefCountedMemory* UserImageSource::GetUserImage(
       return new base::RefCountedBytes(user->animated_image());
     } else if (user->has_raw_image()) {
       return new base::RefCountedBytes(user->raw_image());
-    } else if (user->has_default_image()) {
+    } else if (user->HasDefaultImage()) {
       return ResourceBundle::GetSharedInstance().
           LoadDataResourceBytes(kDefaultImageResources[user->image_index()],
                                 scale_factor);