Commit d05e7751 authored by ivankr@chromium.org's avatar ivankr@chromium.org

[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
parent 3f343425
...@@ -518,7 +518,8 @@ void PhotoCaptureObserver::OnCapturingStopped( ...@@ -518,7 +518,8 @@ void PhotoCaptureObserver::OnCapturingStopped(
// Set up an observer for UserManager (it will delete itself). // Set up an observer for UserManager (it will delete itself).
user_manager->AddObserver(this); user_manager->AddObserver(this);
user_manager->SaveUserImage(user.email(), chromeos::UserImage(photo)); user_manager->SaveUserImage(
user.email(), chromeos::UserImage::CreateAndEncode(photo));
} }
void PhotoCaptureObserver::LocalStateChanged( void PhotoCaptureObserver::LocalStateChanged(
......
...@@ -47,6 +47,7 @@ void User::SetImage(const UserImage& user_image, int image_index) { ...@@ -47,6 +47,7 @@ void User::SetImage(const UserImage& user_image, int image_index) {
user_image_ = user_image; user_image_ = user_image;
image_index_ = image_index; image_index_ = image_index;
image_is_stub_ = false; image_is_stub_ = false;
DCHECK(HasDefaultImage() || user_image.has_raw_image());
} }
void User::SetImageURL(const GURL& image_url) { void User::SetImageURL(const GURL& image_url) {
...@@ -70,6 +71,10 @@ std::string User::GetAccountName(bool use_display_email) const { ...@@ -70,6 +71,10 @@ std::string User::GetAccountName(bool use_display_email) const {
return GetUserName(email_); return GetUserName(email_);
} }
bool User::HasDefaultImage() const {
return image_index_ >= 0 && image_index_ < kDefaultImagesCount;
}
string16 User::GetDisplayName() const { string16 User::GetDisplayName() const {
// Fallback to the email account name in case display name haven't been set. // Fallback to the email account name in case display name haven't been set.
return display_name_.empty() ? return display_name_.empty() ?
......
...@@ -66,9 +66,8 @@ class User { ...@@ -66,9 +66,8 @@ class User {
// The image for this user. // The image for this user.
const gfx::ImageSkia& image() const { return user_image_.image(); } const gfx::ImageSkia& image() const { return user_image_.image(); }
bool has_default_image() const { // Whether the user has a default image.
return image_index_ >= 0 && image_index_ < kDefaultImagesCount; bool HasDefaultImage() const;
}
int image_index() const { return image_index_; } int image_index() const { return image_index_; }
bool has_raw_image() const { return user_image_.has_raw_image(); } bool has_raw_image() const { return user_image_.has_raw_image(); }
......
...@@ -23,6 +23,13 @@ bool IsAnimatedImage(const UserImage::RawImage& data) { ...@@ -23,6 +23,13 @@ bool IsAnimatedImage(const UserImage::RawImage& data) {
} // namespace } // 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() UserImage::UserImage()
: has_raw_image_(false), : has_raw_image_(false),
has_animated_image_(false) { has_animated_image_(false) {
...@@ -52,4 +59,8 @@ UserImage::UserImage(const gfx::ImageSkia& image, ...@@ -52,4 +59,8 @@ UserImage::UserImage(const gfx::ImageSkia& image,
UserImage::~UserImage() {} UserImage::~UserImage() {}
void UserImage::DiscardRawImage() {
RawImage().swap(raw_image_); // Clear |raw_image_|.
}
} // namespace chromeos } // namespace chromeos
...@@ -20,6 +20,10 @@ class UserImage { ...@@ -20,6 +20,10 @@ class UserImage {
// TODO(ivankr): replace with RefCountedMemory to prevent copying. // TODO(ivankr): replace with RefCountedMemory to prevent copying.
typedef std::vector<unsigned char> RawImage; 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. // Create instance with an empty still frame and no raw data.
UserImage(); UserImage();
...@@ -40,6 +44,9 @@ class UserImage { ...@@ -40,6 +44,9 @@ class UserImage {
bool has_raw_image() const { return has_raw_image_; } bool has_raw_image() const { return has_raw_image_; }
const RawImage& raw_image() const { return 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. // Optional raw representation of the animated image.
bool has_animated_image() const { return has_animated_image_; } bool has_animated_image() const { return has_animated_image_; }
const RawImage& animated_image() const { return animated_image_; } const RawImage& animated_image() const { return animated_image_; }
......
...@@ -22,10 +22,8 @@ using content::BrowserThread; ...@@ -22,10 +22,8 @@ using content::BrowserThread;
namespace chromeos { namespace chromeos {
UserImageLoader::ImageInfo::ImageInfo(int size, UserImageLoader::ImageInfo::ImageInfo(int size,
bool load_raw_image,
const LoadedCallback& loaded_cb) const LoadedCallback& loaded_cb)
: size(size), : size(size),
load_raw_image(load_raw_image),
loaded_cb(loaded_cb) { loaded_cb(loaded_cb) {
} }
...@@ -41,11 +39,10 @@ UserImageLoader::~UserImageLoader() { ...@@ -41,11 +39,10 @@ UserImageLoader::~UserImageLoader() {
void UserImageLoader::Start(const std::string& filepath, void UserImageLoader::Start(const std::string& filepath,
int size, int size,
bool load_raw_image,
const LoadedCallback& loaded_cb) { const LoadedCallback& loaded_cb) {
target_message_loop_ = MessageLoop::current(); target_message_loop_ = MessageLoop::current();
ImageInfo image_info(size, load_raw_image, loaded_cb); ImageInfo image_info(size, loaded_cb);
BrowserThread::PostTask( BrowserThread::PostTask(
BrowserThread::FILE, FROM_HERE, BrowserThread::FILE, FROM_HERE,
base::Bind(&UserImageLoader::LoadImage, this, filepath, image_info)); base::Bind(&UserImageLoader::LoadImage, this, filepath, image_info));
...@@ -96,15 +93,10 @@ void UserImageLoader::OnImageDecoded(const ImageDecoder* decoder, ...@@ -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( target_message_loop_->PostTask(
FROM_HERE, 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); image_info_map_.erase(info_it);
} }
......
...@@ -25,7 +25,7 @@ class UserImage; ...@@ -25,7 +25,7 @@ class UserImage;
class UserImageLoader : public base::RefCountedThreadSafe<UserImageLoader>, class UserImageLoader : public base::RefCountedThreadSafe<UserImageLoader>,
public ImageDecoder::Delegate { public ImageDecoder::Delegate {
public: 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; typedef base::Callback<void(const UserImage& user_image)> LoadedCallback;
UserImageLoader(); UserImageLoader();
...@@ -34,8 +34,7 @@ class UserImageLoader : public base::RefCountedThreadSafe<UserImageLoader>, ...@@ -34,8 +34,7 @@ class UserImageLoader : public base::RefCountedThreadSafe<UserImageLoader>,
// |loaded_cb| when image has been successfully loaded. // |loaded_cb| when image has been successfully loaded.
// If |size| is positive, image is cropped and (if needed) downsized to // If |size| is positive, image is cropped and (if needed) downsized to
// |size|x|size| pixels. // |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,
void Start(const std::string& filepath, int size, bool load_raw_image,
const LoadedCallback& loaded_cb); const LoadedCallback& loaded_cb);
private: private:
...@@ -43,12 +42,11 @@ class UserImageLoader : public base::RefCountedThreadSafe<UserImageLoader>, ...@@ -43,12 +42,11 @@ class UserImageLoader : public base::RefCountedThreadSafe<UserImageLoader>,
// Contains attributes we need to know about each image we decode. // Contains attributes we need to know about each image we decode.
struct ImageInfo { struct ImageInfo {
ImageInfo(int size, bool load_raw_image, const LoadedCallback& loaded_cb); ImageInfo(int size, const LoadedCallback& loaded_cb);
~ImageInfo(); ~ImageInfo();
int size; const int size;
bool load_raw_image; const LoadedCallback loaded_cb;
LoadedCallback loaded_cb;
}; };
typedef std::map<const ImageDecoder*, ImageInfo> ImageInfoMap; typedef std::map<const ImageDecoder*, ImageInfo> ImageInfoMap;
......
...@@ -116,8 +116,10 @@ void UserImageScreen::StopCamera() { ...@@ -116,8 +116,10 @@ void UserImageScreen::StopCamera() {
void UserImageScreen::OnPhotoTaken(const gfx::ImageSkia& image) { void UserImageScreen::OnPhotoTaken(const gfx::ImageSkia& image) {
UserManager* user_manager = UserManager::Get(); 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(), user_manager->SaveUserImage(user_manager->GetLoggedInUser().email(),
UserImage(image)); UserImage::CreateAndEncode(image));
get_screen_observer()->OnExit(ScreenObserver::USER_IMAGE_SELECTED); get_screen_observer()->OnExit(ScreenObserver::USER_IMAGE_SELECTED);
......
...@@ -420,7 +420,7 @@ void UserManagerImpl::UserSelected(const std::string& email) { ...@@ -420,7 +420,7 @@ void UserManagerImpl::UserSelected(const std::string& email) {
ash::WallpaperLayout layout = static_cast<ash::WallpaperLayout>(index); ash::WallpaperLayout layout = static_cast<ash::WallpaperLayout>(index);
// Load user image asynchronously. // Load user image asynchronously.
image_loader_->Start( image_loader_->Start(
wallpaper_path, 0, false, wallpaper_path, 0,
base::Bind(&UserManagerImpl::OnCustomWallpaperLoaded, base::Bind(&UserManagerImpl::OnCustomWallpaperLoaded,
base::Unretained(this), email, layout)); base::Unretained(this), email, layout));
return; return;
...@@ -631,7 +631,7 @@ void UserManagerImpl::SaveUserImageFromFile(const std::string& username, ...@@ -631,7 +631,7 @@ void UserManagerImpl::SaveUserImageFromFile(const std::string& username,
const FilePath& path) { const FilePath& path) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
image_loader_->Start( image_loader_->Start(
path.value(), login::kMaxUserImageSize, true, path.value(), login::kMaxUserImageSize,
base::Bind(&UserManagerImpl::SaveUserImage, base::Bind(&UserManagerImpl::SaveUserImage,
base::Unretained(this), username)); base::Unretained(this), username));
} }
...@@ -644,7 +644,7 @@ void UserManagerImpl::SaveUserWallpaperFromFile( ...@@ -644,7 +644,7 @@ void UserManagerImpl::SaveUserWallpaperFromFile(
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
// For wallpapers, save the image without resizing. // For wallpapers, save the image without resizing.
image_loader_->Start( image_loader_->Start(
path.value(), 0 /* Original size */, false, path.value(), 0 /* Original size */,
base::Bind(&UserManagerImpl::SaveUserWallpaperInternal, base::Bind(&UserManagerImpl::SaveUserWallpaperInternal,
base::Unretained(this), username, layout, User::CUSTOMIZED, base::Unretained(this), username, layout, User::CUSTOMIZED,
delegate)); delegate));
...@@ -656,9 +656,10 @@ void UserManagerImpl::SaveUserImageFromProfileImage( ...@@ -656,9 +656,10 @@ void UserManagerImpl::SaveUserImageFromProfileImage(
if (!downloaded_profile_image_.empty()) { if (!downloaded_profile_image_.empty()) {
// Profile image has already been downloaded, so save it to file right now. // Profile image has already been downloaded, so save it to file right now.
DCHECK(profile_image_url_.is_valid()); DCHECK(profile_image_url_.is_valid());
SaveUserImageInternal(username, SaveUserImageInternal(
User::kProfileImageIndex, profile_image_url_, username,
UserImage(downloaded_profile_image_)); User::kProfileImageIndex, profile_image_url_,
UserImage::CreateAndEncode(downloaded_profile_image_));
} else { } else {
// No profile image - use the stub image (gray avatar). // No profile image - use the stub image (gray avatar).
SetUserImage(username, User::kProfileImageIndex, SetUserImage(username, User::kProfileImageIndex,
...@@ -849,7 +850,7 @@ void UserManagerImpl::EnsureUsersLoaded() { ...@@ -849,7 +850,7 @@ void UserManagerImpl::EnsureUsersLoaded() {
DCHECK(!image_path.empty()); DCHECK(!image_path.empty());
// Load user image asynchronously. // Load user image asynchronously.
image_loader_->Start( image_loader_->Start(
image_path, user_image_size, true, image_path, user_image_size,
base::Bind(&UserManagerImpl::SetUserImage, base::Bind(&UserManagerImpl::SetUserImage,
base::Unretained(this), base::Unretained(this),
email, image_index, GURL())); email, image_index, GURL()));
...@@ -879,7 +880,7 @@ void UserManagerImpl::EnsureUsersLoaded() { ...@@ -879,7 +880,7 @@ void UserManagerImpl::EnsureUsersLoaded() {
if (!image_path.empty()) { if (!image_path.empty()) {
// Load user image asynchronously. // Load user image asynchronously.
image_loader_->Start( image_loader_->Start(
image_path, user_image_size, true, image_path, user_image_size,
base::Bind(&UserManagerImpl::SetUserImage, base::Bind(&UserManagerImpl::SetUserImage,
base::Unretained(this), base::Unretained(this),
email, image_index, image_gurl)); email, image_index, image_gurl));
...@@ -1270,7 +1271,7 @@ void UserManagerImpl::OnCustomWallpaperLoaded(const std::string& email, ...@@ -1270,7 +1271,7 @@ void UserManagerImpl::OnCustomWallpaperLoaded(const std::string& email,
std::string wallpaper_thumbnail_path = std::string wallpaper_thumbnail_path =
GetWallpaperPathForUser(email, true).value(); GetWallpaperPathForUser(email, true).value();
image_loader_->Start( image_loader_->Start(
wallpaper_thumbnail_path, 0, false, wallpaper_thumbnail_path, 0,
base::Bind(&UserManagerImpl::OnCustomWallpaperThumbnailLoaded, base::Bind(&UserManagerImpl::OnCustomWallpaperThumbnailLoaded,
base::Unretained(this), email)); base::Unretained(this), email));
} }
......
...@@ -114,7 +114,7 @@ void WallpaperManager::SetLastSelectedUser( ...@@ -114,7 +114,7 @@ void WallpaperManager::SetLastSelectedUser(
void WallpaperManager::SetWallpaperFromFilePath(const std::string& path, void WallpaperManager::SetWallpaperFromFilePath(const std::string& path,
ash::WallpaperLayout layout) { ash::WallpaperLayout layout) {
image_loader_->Start( image_loader_->Start(
path, 0, false, path, 0,
base::Bind(&WallpaperManager::OnWallpaperLoaded, base::Bind(&WallpaperManager::OnWallpaperLoaded,
base::Unretained(this), layout)); base::Unretained(this), layout));
} }
......
...@@ -302,7 +302,7 @@ void ChangePictureOptionsHandler::HandleSelectImage(const ListValue* args) { ...@@ -302,7 +302,7 @@ void ChangePictureOptionsHandler::HandleSelectImage(const ListValue* args) {
DCHECK(!previous_image_.empty()); DCHECK(!previous_image_.empty());
user_manager->SaveUserImage(user.email(), user_manager->SaveUserImage(user.email(),
chromeos::UserImage(previous_image_)); UserImage::CreateAndEncode(previous_image_));
UMA_HISTOGRAM_ENUMERATION("UserImage.ChangeChoice", UMA_HISTOGRAM_ENUMERATION("UserImage.ChangeChoice",
kHistogramImageOld, kHistogramImageOld,
...@@ -361,8 +361,10 @@ void ChangePictureOptionsHandler::FileSelected(const FilePath& path, ...@@ -361,8 +361,10 @@ void ChangePictureOptionsHandler::FileSelected(const FilePath& path,
void ChangePictureOptionsHandler::OnPhotoAccepted(const gfx::ImageSkia& photo) { void ChangePictureOptionsHandler::OnPhotoAccepted(const gfx::ImageSkia& photo) {
UserManager* user_manager = UserManager::Get(); 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(), user_manager->SaveUserImage(user_manager->GetLoggedInUser().email(),
chromeos::UserImage(photo)); UserImage::CreateAndEncode(photo));
UMA_HISTOGRAM_ENUMERATION("UserImage.ChangeChoice", UMA_HISTOGRAM_ENUMERATION("UserImage.ChangeChoice",
kHistogramImageFromCamera, kHistogramImageFromCamera,
kHistogramImagesCount); kHistogramImagesCount);
......
...@@ -73,7 +73,7 @@ base::RefCountedMemory* UserImageSource::GetUserImage( ...@@ -73,7 +73,7 @@ base::RefCountedMemory* UserImageSource::GetUserImage(
return new base::RefCountedBytes(user->animated_image()); return new base::RefCountedBytes(user->animated_image());
} else if (user->has_raw_image()) { } else if (user->has_raw_image()) {
return new base::RefCountedBytes(user->raw_image()); return new base::RefCountedBytes(user->raw_image());
} else if (user->has_default_image()) { } else if (user->HasDefaultImage()) {
return ResourceBundle::GetSharedInstance(). return ResourceBundle::GetSharedInstance().
LoadDataResourceBytes(kDefaultImageResources[user->image_index()], LoadDataResourceBytes(kDefaultImageResources[user->image_index()],
scale_factor); scale_factor);
......
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