[cros] Profile image downloader is always reset after a download.

Additionally, downloaded profile image is no more saved to file every time it is downloaded after login.

BUG=chromium-os:23684
TEST=Manual: see bug description

Review URL: http://codereview.chromium.org/8822015

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

chrome/browser/chromeos/login/user.cc

@@ -7,15 +7,24 @@
 #include "base/stringprintf.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;
+
+}  // namespace
+
 User::User(const std::string& email)
     : email_(email),
       display_email_(email),
       oauth_token_status_(OAUTH_TOKEN_STATUS_UNKNOWN),
-      image_index_(kInvalidImageIndex) {
+      image_index_(kInvalidImageIndex),
+      image_is_stub_(false) {
   image_ = *ResourceBundle::GetSharedInstance().GetBitmapNamed(
       kDefaultImageResources[0]);
 }
@@ -25,6 +34,14 @@ User::~User() {}
 void User::SetImage(const SkBitmap& image, int image_index) {
   image_ = image;
   image_index_ = image_index;
+  image_is_stub_ = false;
+}
+
+void User::SetStubImage(int image_index) {
+  image_ = *ResourceBundle::GetSharedInstance().
+      GetBitmapNamed(kStubImageResourceID);
+  image_index_ = image_index;
+  image_is_stub_ = true;
 }
 
 std::string User::GetDisplayName() const {

chrome/browser/chromeos/login/user.h

@@ -53,6 +53,9 @@ class User {
   const SkBitmap& image() const { return image_; }
   int image_index() const { return image_index_; }
 
+  // True if user image is a stub (while real image is being loaded from file).
+  bool image_is_stub() const { return image_is_stub_; }
+
   // OAuth token status for this user.
   OAuthTokenStatus oauth_token_status() const { return oauth_token_status_; }
 
@@ -68,6 +71,9 @@ class User {
 
   // Setters are private so only UserManager can call them.
   void SetImage(const SkBitmap& image, int image_index);
+  // Sets a stub image until the next |SetImage| call. |image_index| may be
+  // one of |kExternalImageIndex| or |kProfileImageIndex|.
+  void SetStubImage(int image_index);
 
   void set_oauth_token_status(OAuthTokenStatus status) {
     oauth_token_status_ = status;
@@ -87,6 +93,9 @@ class User {
   // |kProfileImageIndex|.
   int image_index_;
 
+  // True if current user image is a stub set by a |SetStubImage| call.
+  bool image_is_stub_;
+
   DISALLOW_COPY_AND_ASSIGN(User);
 };
 

chrome/browser/chromeos/login/user_manager.cc

@@ -47,8 +47,6 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/common/url_constants.h"
 #include "crypto/nss_util.h"
-#include "grit/theme_resources.h"
-#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/codec/png_codec.h"
 
 #if defined(TOOLKIT_USES_GTK)
@@ -284,6 +282,7 @@ void UserManager::UserLoggedIn(const std::string& email) {
     // Download profile image if it's user image and see if it has changed.
     int image_index = logged_in_user_->image_index();
     if (image_index == User::kProfileImageIndex) {
+      InitDownloadedProfileImage();
       BrowserThread::PostDelayedTask(
           BrowserThread::UI,
           FROM_HERE,
@@ -493,10 +492,8 @@ void UserManager::SaveUserImageFromProfileImage(const std::string& username) {
     SaveUserImageInternal(username, User::kProfileImageIndex,
                           downloaded_profile_image_);
   } else {
-    // No profile image - use the default gray avatar.
-    SetUserImage(username, User::kProfileImageIndex,
-                 *ResourceBundle::GetSharedInstance().
-                     GetBitmapNamed(IDR_PROFILE_PICTURE_LOADING));
+    // No profile image - use the stub image (gray avatar).
+    SetUserImage(username, User::kProfileImageIndex, SkBitmap());
     SaveImageToLocalState(username, "", User::kProfileImageIndex, false);
   }
 }
@@ -617,10 +614,8 @@ void UserManager::EnsureUsersLoaded() {
               user->SetImage(GetDefaultImage(image_id), image_id);
             } else {
               int image_index = User::kExternalImageIndex;
-              // Until image has been loaded, use a stub.
-              user->SetImage(*ResourceBundle::GetSharedInstance().
-                                 GetBitmapNamed(IDR_PROFILE_PICTURE_LOADING),
-                             image_index);
+              // Until image has been loaded, use the stub image.
+              user->SetStubImage(image_index);
               DCHECK(!image_path.empty());
               // Load user image asynchronously.
               image_loader_->Start(
@@ -643,10 +638,8 @@ void UserManager::EnsureUsersLoaded() {
               // after user logs in).
               DCHECK(!image_path.empty() ||
                      image_index == User::kProfileImageIndex);
-              // Until image has been loaded, use a gray avatar.
-              user->SetImage(*ResourceBundle::GetSharedInstance().
-                                 GetBitmapNamed(IDR_PROFILE_PICTURE_LOADING),
-                             image_index);
+              // Until image has been loaded, use the stub image (gray avatar).
+              user->SetStubImage(image_index);
               if (!image_path.empty()) {
                 // Load user image asynchronously.
                 image_loader_->Start(
@@ -731,14 +724,14 @@ void UserManager::SetUserImage(const std::string& username,
     DCHECK(user->image_index() != User::kInvalidImageIndex ||
            current_user_is_new_);
     bool image_changed = user->image_index() != User::kInvalidImageIndex;
-    user->SetImage(image, image_index);
-    // If it is the profile image of the current user, save it to
-    // |downloaded_profile_image_| so that it can be reused if the started
-    // download attempt fails.
-    if (image_index == User::kProfileImageIndex && user == logged_in_user_) {
-      downloaded_profile_image_ = image;
-      downloaded_profile_image_data_url_ = web_ui_util::GetImageDataUrl(image);
-    }
+    if (!image.empty())
+      user->SetImage(image, image_index);
+    else
+      user->SetStubImage(image_index);
+    // For the logged-in user with a profile picture, initialize
+    // |downloaded_profile_picture_|.
+    if (user == logged_in_user_ && image_index == User::kProfileImageIndex)
+      InitDownloadedProfileImage();
     if (image_changed) {
       // Unless this is first-time setting with |SetInitialUserImage|,
       // send a notification about image change.
@@ -826,6 +819,16 @@ void UserManager::SaveImageToLocalState(const std::string& username,
   NotifyLocalStateChanged();
 }
 
+void UserManager::InitDownloadedProfileImage() {
+  DCHECK(logged_in_user_);
+  if (downloaded_profile_image_.empty() && !logged_in_user_->image_is_stub()) {
+    VLOG(1) << "Profile image initialized";
+    downloaded_profile_image_ = logged_in_user_->image();
+    downloaded_profile_image_data_url_ =
+        web_ui_util::GetImageDataUrl(downloaded_profile_image_);
+  }
+}
+
 void UserManager::DeleteUserImage(const FilePath& image_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   if (!file_util::Delete(image_path, false)) {
@@ -881,6 +884,11 @@ std::string UserManager::GetCachedPictureURL() const {
 
 void UserManager::OnDownloadComplete(ProfileDownloader* downloader,
                                      bool success) {
+  // Make sure that |ProfileDownloader| gets deleted after return.
+  scoped_ptr<ProfileDownloader> profile_image_downloader(
+      profile_image_downloader_.release());
+  DCHECK(profile_image_downloader.get() == downloader);
+
   ProfileDownloadResult result;
   std::string time_histogram_name;
   if (!success) {
@@ -935,8 +943,6 @@ void UserManager::OnDownloadComplete(ProfileDownloader* downloader,
         content::Source<UserManager>(this),
         content::NotificationService::NoDetails());
   }
-
-  profile_image_downloader_.reset();
 }
 
 User* UserManager::CreateUser(const std::string& email) const {

chrome/browser/chromeos/login/user_manager.h

@@ -186,6 +186,7 @@ class UserManager : public ProfileDownloaderDelegate,
 
   // Sets image for user |username| and sends LOGIN_USER_IMAGE_CHANGED
   // notification unless this is a new user and image is set for the first time.
+  // If |image| is empty, sets a stub image for the user.
   void SetUserImage(const std::string& username,
                     int image_index,
                     const SkBitmap& image);
@@ -212,6 +213,10 @@ class UserManager : public ProfileDownloaderDelegate,
                              int image_index,
                              bool is_async);
 
+  // Initializes |downloaded_profile_picture_| with the picture of the logged-in
+  // user.
+  void InitDownloadedProfileImage();
+
   // Deletes user's image file. Runs on FILE thread.
   void DeleteUserImage(const FilePath& image_path);