[cros] Remove redundant PNG enconding in UserImage.

*) PNG enconding removed from UserImage.
*) chrome://userimage/ source understand @<scale> spec now.


BUG=137929


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

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

chrome/browser/chromeos/login/user.cc

@@ -8,16 +8,11 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chromeos/login/default_user_images.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
-#include "grit/theme_resources.h"
-#include "ui/base/resource/resource_bundle.h"
 
 namespace chromeos {
 
 namespace {
 
-// Resource ID of the image to use as stub image.
-const int kStubImageResourceID = IDR_PROFILE_PICTURE_LOADING;
-
 // Returns account name portion of an email.
 std::string GetUserName(const std::string& email) {
   std::string::size_type i = email.find('@');
@@ -37,8 +32,6 @@ const char kGuestUser[] = "";
 
 User::User(const std::string& email)
     : email_(email),
-      user_image_(*ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
-          kDefaultImageResources[0])),
       oauth_token_status_(OAUTH_TOKEN_STATUS_UNKNOWN),
       image_index_(kInvalidImageIndex),
       image_is_stub_(false) {
@@ -61,8 +54,7 @@ void User::SetImageURL(const GURL& image_url) {
 }
 
 void User::SetStubImage(int image_index) {
-  user_image_.SetImage(*ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
-      kStubImageResourceID));
+  user_image_ = UserImage();
   image_index_ = image_index;
   image_is_stub_ = true;
 }

chrome/browser/chromeos/login/user.h

@@ -22,6 +22,8 @@ extern const char kDemoUser[];
 // Username for incognito login.
 extern const char kGuestUser[];
 
+extern const int kDefaultImagesCount;
+
 // A class representing information about a previously logged in user.
 // Each user has a canonical email (username), returned by |email()| and
 // may have a different displayed email (in the raw form as entered by user),
@@ -64,6 +66,10 @@ 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;
+  }
+
   int image_index() const { return image_index_; }
   bool has_raw_image() const { return user_image_.has_raw_image(); }
   // Returns raw representation of static user image.

chrome/browser/chromeos/login/user_image.cc

@@ -23,12 +23,15 @@ bool IsAnimatedImage(const UserImage::RawImage& data) {
 
 }  // namespace
 
+UserImage::UserImage()
+    : has_raw_image_(false),
+      has_animated_image_(false) {
+}
+
 UserImage::UserImage(const gfx::ImageSkia& image)
     : image_(image),
       has_raw_image_(false),
       has_animated_image_(false) {
-  if (gfx::PNGCodec::EncodeBGRASkBitmap(image_, false, &raw_image_))
-    has_raw_image_ = true;
 }
 
 UserImage::UserImage(const gfx::ImageSkia& image,
@@ -39,25 +42,14 @@ UserImage::UserImage(const gfx::ImageSkia& image,
   if (IsAnimatedImage(raw_image)) {
     has_animated_image_ = true;
     animated_image_ = raw_image;
-  }
-  if (gfx::PNGCodec::EncodeBGRASkBitmap(image_, false, &raw_image_))
+    if (gfx::PNGCodec::EncodeBGRASkBitmap(image_, false, &raw_image_))
+      has_raw_image_ = true;
+  } else {
     has_raw_image_ = true;
+    raw_image_ = raw_image;
+  }
 }
 
 UserImage::~UserImage() {}
 
-void UserImage::SetImage(const gfx::ImageSkia& image) {
-  image_ = image;
-  if (gfx::PNGCodec::EncodeBGRASkBitmap(image_, false, &raw_image_)) {
-    has_raw_image_ = true;
-  } else {
-    has_raw_image_ = false;
-    RawImage().swap(raw_image_);  // Clear |raw_image_|.
-  }
-
-  has_animated_image_ = false;
-  if (!animated_image_.empty())
-    RawImage().swap(animated_image_);  // Clear |animated_image_|.
-}
-
 }  // namespace chromeos

chrome/browser/chromeos/login/user_image.h

@@ -12,36 +12,35 @@
 
 namespace chromeos {
 
-// Wrapper class for bitmaps and raw images when it's necessary. Could
-// be used for storing profile images (including animated profile
-// images) and user wallpapers.
+// Wrapper class storing a still image and it's raw representation.  Could be
+// used for storing profile images (including animated profile images) and user
+// wallpapers.
 class UserImage {
  public:
+  // TODO(ivankr): replace with RefCountedMemory to prevent copying.
   typedef std::vector<unsigned char> RawImage;
 
-  // Constructs UserImage from bitmap. Should be used where only
-  // static image is needed (e.g. wallpapers).
+  // Create instance with an empty still frame and no raw data.
+  UserImage();
+
+  // Creates a new instance from a given still frame without any raw data.
   explicit UserImage(const gfx::ImageSkia& image);
 
-  // Constructs UserImage from raw image and bitmap that should
-  // represent single frame from that one. Can be used for wrapping
-  // animated images.
+  // Creates a new instance from a given still frame and raw representation.
+  // |raw_image| can be animated, in which case animated_image() will return the
+  // original |raw_image| and raw_image() will return the encoded representation
+  // of |image|.
   UserImage(const gfx::ImageSkia& image, const RawImage& raw_image);
 
   virtual ~UserImage();
 
-  // Replaces already stored image to new |image|. Note, that
-  // |raw_image| and |animated_image| will be reset after that
-  // operation.
-  void SetImage(const gfx::ImageSkia& image);
   const gfx::ImageSkia& image() const { return image_; }
 
-  // Returns true iff |image| argument of constructors or |SetImage|
-  // can be encoded as a bitmap.
+  // Optional raw representation of the still image.
   bool has_raw_image() const { return has_raw_image_; }
   const RawImage& raw_image() const { return raw_image_; }
 
-  // Returns true iff UserImage is constructed from animated image.
+  // Optional raw representation of the animated image.
   bool has_animated_image() const { return has_animated_image_; }
   const RawImage& animated_image() const { return animated_image_; }
 

chrome/browser/ui/webui/options2/chromeos/user_image_source.cc

@@ -7,7 +7,9 @@
 #include "base/memory/ref_counted_memory.h"
 #include "base/message_loop.h"
 #include "base/string_split.h"
+#include "chrome/browser/chromeos/login/default_user_images.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
+#include "chrome/browser/ui/webui/web_ui_util.cc"
 #include "chrome/common/url_constants.h"
 #include "googleurl/src/url_parse.h"
 #include "grit/theme_resources.h"
@@ -21,35 +23,35 @@ namespace {
 // non-animated version of user image should be returned.
 const char kKeyAnimated[] = "animated";
 
-// Extracts from user image request user email and type of requested
-// image (animated or non-animated). |path| is an user image request
-// and should look like "username@host?key1=value1&...&key_n=value_n".
-// So, "username@host" is stored into |email|. If a query part of
-// |path| contains "animated" key, |is_image_animated| is set to true,
-// otherwise |is_image_animated| is set to false. Doesn't change
-// arguments if email can't be parsed (for instance, in guest mode).
-void ParseRequest(const std::string& path,
+// Parses the user image URL, which looks like
+// "chrome://userimage/user@host?key1=value1&...&key_n=value_n@<scale>x",
+// to user email, optional parameters and scale factor.
+void ParseRequest(const GURL& url,
                   std::string* email,
-                  bool* is_image_animated) {
-  url_parse::Parsed parsed;
-  url_parse::ParseStandardURL(path.c_str(), path.size(), &parsed);
-  if (!parsed.username.is_valid() || !parsed.host.is_valid())
-    return;
-
-  DCHECK(email != NULL);
-  *email = path.substr(parsed.username.begin, parsed.username.len);
-  email->append("@");
-  email->append(path.substr(parsed.host.begin, parsed.host.len));
-
-  if (!parsed.query.is_valid())
-    return;
+                  bool* is_image_animated,
+                  ui::ScaleFactor* scale_factor) {
+  DCHECK(url.is_valid());
+
+  *email = url.path();
+  email->erase(0, 1);  // Strip initial slash.
+
+  // TODO(ivankr): when all chrome://userimage URLs have a valid @<scale>x,
+  // remove this and pass |email| instead of |&path| to ParsePathAndScale.
+  size_t pos = email->find('@');
+  if (pos != std::string::npos) {
+    pos = email->find('@', pos + 1);
+    if (pos != std::string::npos)
+      email->erase(pos);
+  }
+  std::string path;
+  web_ui_util::ParsePathAndScale(url, &path, scale_factor);
 
-  url_parse::Component query = parsed.query;
+  std::string url_spec = url.possibly_invalid_spec();
+  url_parse::Component query = url.parsed_for_possibly_invalid_spec().query;
   url_parse::Component key, value;
-  DCHECK(is_image_animated != NULL);
   *is_image_animated = false;
-  while (ExtractQueryKeyValue(path.c_str(), &query, &key, &value)) {
-    if (path.substr(key.begin, key.len) == kKeyAnimated) {
+  while (ExtractQueryKeyValue(url_spec.c_str(), &query, &key, &value)) {
+    if (url_spec.substr(key.begin, key.len) == kKeyAnimated) {
       *is_image_animated = true;
       break;
     }
@@ -61,22 +63,26 @@ void ParseRequest(const std::string& path,
 namespace chromeos {
 namespace options2 {
 
-std::vector<unsigned char> UserImageSource::GetUserImage(
-    const std::string& email, bool is_image_animated) const {
+base::RefCountedMemory* UserImageSource::GetUserImage(
+    const std::string& email,
+    bool is_image_animated,
+    ui::ScaleFactor scale_factor) const {
   const chromeos::User* user = chromeos::UserManager::Get()->FindUser(email);
   if (user) {
-    if (user->has_animated_image() && is_image_animated)
-      return user->animated_image();
-    else if (user->has_raw_image())
-      return user->raw_image();
+    if (user->has_animated_image() && is_image_animated) {
+      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()) {
+      return ResourceBundle::GetSharedInstance().
+          LoadDataResourceBytes(kDefaultImageResources[user->image_index()],
+                                scale_factor);
+    } else {
+      NOTREACHED() << "User with custom image missing raw data";
+    }
   }
-  std::vector<unsigned char> user_image;
-  gfx::PNGCodec::EncodeBGRASkBitmap(
-      *ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
-          IDR_LOGIN_DEFAULT_USER),
-      false,
-      &user_image);
-  return user_image;
+  return ResourceBundle::GetSharedInstance().
+      LoadDataResourceBytes(IDR_LOGIN_DEFAULT_USER, scale_factor);
 }
 
 UserImageSource::UserImageSource()
@@ -90,10 +96,11 @@ void UserImageSource::StartDataRequest(const std::string& path,
                                        int request_id) {
   std::string email;
   bool is_image_animated = false;
-  ParseRequest(path, &email, &is_image_animated);
-
-  std::vector<unsigned char> image = GetUserImage(email, is_image_animated);
-  SendResponse(request_id, new base::RefCountedBytes(image));
+  ui::ScaleFactor scale_factor;
+  GURL url(chrome::kChromeUIUserImageURL + path);
+  ParseRequest(url, &email, &is_image_animated, &scale_factor);
+  SendResponse(request_id,
+               GetUserImage(email, is_image_animated, scale_factor));
 }
 
 std::string UserImageSource::GetMimeType(const std::string& path) const {
@@ -101,7 +108,10 @@ std::string UserImageSource::GetMimeType(const std::string& path) const {
   // drag the image they get no extension.
   std::string email;
   bool is_image_animated = false;
-  ParseRequest(path, &email, &is_image_animated);
+  ui::ScaleFactor scale_factor;
+
+  GURL url(chrome::kChromeUIUserImageURL + path);
+  ParseRequest(url, &email, &is_image_animated, &scale_factor);
 
   if (is_image_animated) {
     const chromeos::User* user = chromeos::UserManager::Get()->FindUser(email);

chrome/browser/ui/webui/options2/chromeos/user_image_source.h

@@ -11,6 +11,11 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
+#include "ui/base/layout.h"
+
+namespace base {
+class RefCountedMemory;
+}
 
 namespace chromeos {
 namespace options2 {
@@ -31,9 +36,10 @@ class UserImageSource : public ChromeURLDataManager::DataSource {
 
   // Returns PNG or GIF (when possible and if |is_image_animated| flag
   // is true) encoded image for user with specified email.  If there's
-  // no user with such email, returns the default image.
-  std::vector<unsigned char> GetUserImage(const std::string& email,
-                                          bool is_image_animated) const;
+  // no user with such email, returns the first default image.
+  base::RefCountedMemory* GetUserImage(const std::string& email,
+                                       bool is_image_animated,
+                                       ui::ScaleFactor scale_factor) const;
 
  private:
   virtual ~UserImageSource();