Remove call to UserManager::Get when initializing UserManager

This CL removes UserManager::Get()(for ephemeral user check) out from
WallpaperManager::SaveUserWallpaperProperties(called when initializing UserManager).
It was creating a deadlock and make chrome hang on startup screen.

The ephemeral user check in SaveUserWallpaperProperties is not correct either.
We should not check if current logged in user is ephemeral user while we try
to save wallpaper properties for some other user specified by email.

BUG=142440

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

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

chrome/browser/chromeos/login/user_manager_impl.cc

@@ -276,7 +276,7 @@ void UserManagerImpl::UserLoggedIn(const std::string& email,
     SaveUserDisplayName(GetLoggedInUser().email(),
         UTF8ToUTF16(GetLoggedInUser().GetAccountName(true)));
     SetInitialUserImage(email);
-    WallpaperManager::Get()->SetInitialUserWallpaper(email);
+    WallpaperManager::Get()->SetInitialUserWallpaper(email, true);
   } else {
     int image_index = logged_in_user_->image_index();
     // If current user image is profile image, it needs to be refreshed.
@@ -330,14 +330,14 @@ void UserManagerImpl::DemoUserLoggedIn() {
   is_current_user_ephemeral_ = true;
   logged_in_user_ = CreateUser(kDemoUser, /* is_ephemeral= */ true);
   SetInitialUserImage(kDemoUser);
-  WallpaperManager::Get()->SetInitialUserWallpaper(kDemoUser);
+  WallpaperManager::Get()->SetInitialUserWallpaper(kDemoUser, false);
   NotifyOnLogin();
 }
 
 void UserManagerImpl::GuestUserLoggedIn() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   is_current_user_ephemeral_ = true;
-  WallpaperManager::Get()->SetInitialUserWallpaper(kGuestUser);
+  WallpaperManager::Get()->SetInitialUserWallpaper(kGuestUser, false);
   logged_in_user_ = CreateUser(kGuestUser, /* is_ephemeral= */ true);
   NotifyOnLogin();
 }
@@ -348,7 +348,7 @@ void UserManagerImpl::EphemeralUserLoggedIn(const std::string& email) {
   is_current_user_ephemeral_ = true;
   logged_in_user_ = CreateUser(email, /* is_ephemeral= */ true);
   SetInitialUserImage(email);
-  WallpaperManager::Get()->SetInitialUserWallpaper(email);
+  WallpaperManager::Get()->SetInitialUserWallpaper(email, false);
   NotifyOnLogin();
 }
 
@@ -928,9 +928,10 @@ void UserManagerImpl::MigrateWallpaperData() {
           DictionaryPrefUpdate prefs_wallpapers_update(local_state,
                                                        kUserWallpapers);
           prefs_wallpapers_update->RemoveWithoutPathExpansion(username, NULL);
-          WallpaperManager::Get()->SaveUserWallpaperProperties(username,
-                                                               User::DEFAULT,
-                                                               index);
+          WallpaperManager::Get()->SetUserWallpaperProperties(username,
+                                                              User::DEFAULT,
+                                                              index,
+                                                              true);
         } else {
           // Before M20, wallpaper index is not saved into LocalState unless
           // user specifically sets a wallpaper. After M20, the default
@@ -944,8 +945,8 @@ void UserManagerImpl::MigrateWallpaperData() {
           // wallpaper for those users as described in cr/130685. So here we use
           // default wallpaper for users that exist in user list but does not
           // have an index saved in LocalState.
-          WallpaperManager::Get()->SaveUserWallpaperProperties(username,
-              User::DEFAULT, ash::GetDefaultWallpaperIndex());
+          WallpaperManager::Get()->SetUserWallpaperProperties(username,
+              User::DEFAULT, ash::GetDefaultWallpaperIndex(), true);
         }
       }
     }
@@ -954,8 +955,11 @@ void UserManagerImpl::MigrateWallpaperData() {
 
 void UserManagerImpl::SaveLoggedInUserWallpaperProperties(
     User::WallpaperType type, int index) {
-  WallpaperManager::Get()->SaveUserWallpaperProperties(
-      GetLoggedInUser().email(), type, index);
+  // Ephemeral users can not save data to local state.
+  // We just cache the index in memory for them.
+  bool is_persistent = !IsCurrentUserEphemeral();
+  WallpaperManager::Get()->SetUserWallpaperProperties(
+      GetLoggedInUser().email(), type, index, is_persistent);
 }
 
 void UserManagerImpl::SetUserImage(const std::string& username,
@@ -1076,7 +1080,8 @@ void UserManagerImpl::SaveWallpaperToLocalState(const std::string& username,
     ash::WallpaperLayout layout,
     User::WallpaperType type) {
   // TODO(bshe): We probably need to save wallpaper_path instead of index.
-  WallpaperManager::Get()->SaveUserWallpaperProperties(username, type, layout);
+  WallpaperManager::Get()->SetUserWallpaperProperties(
+      username, type, layout, true);
 }
 
 bool UserManagerImpl::SaveBitmapToFile(const UserImage& user_image,

chrome/browser/chromeos/login/wallpaper_manager.cc

@@ -319,16 +319,15 @@ void WallpaperManager::RestartTimer() {
   }
 }
 
-void WallpaperManager::SaveUserWallpaperProperties(const std::string& email,
-                                                   User::WallpaperType type,
-                                                   int index) {
+void WallpaperManager::SetUserWallpaperProperties(const std::string& email,
+                                                  User::WallpaperType type,
+                                                  int index,
+                                                  bool is_persistent) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   current_user_wallpaper_type_ = type;
   current_user_wallpaper_index_ = index;
-  // Ephemeral users can not save data to local state. We just cache the index
-  // in memory for them.
-  if (UserManager::Get()->IsCurrentUserEphemeral())
+  if (!is_persistent)
     return;
 
   PrefService* local_state = g_browser_process->local_state();
@@ -360,15 +359,17 @@ void WallpaperManager::SetUserWallpaperFromFile(
                  delegate));
 }
 
-void WallpaperManager::SetInitialUserWallpaper(const std::string& username) {
+void WallpaperManager::SetInitialUserWallpaper(const std::string& username,
+                                               bool is_persistent) {
   current_user_wallpaper_type_ = User::DEFAULT;
   if (username == kGuestUser)
     current_user_wallpaper_index_ = ash::GetGuestWallpaperIndex();
   else
     current_user_wallpaper_index_ = ash::GetDefaultWallpaperIndex();
-  SaveUserWallpaperProperties(username,
-                              current_user_wallpaper_type_,
-                              current_user_wallpaper_index_);
+  SetUserWallpaperProperties(username,
+                             current_user_wallpaper_type_,
+                             current_user_wallpaper_index_,
+                             is_persistent);
 
   // Some browser tests do not have shell instance. And it is not necessary to
   // create a wallpaper for these tests. Add HasInstance check to prevent tests
@@ -424,7 +425,8 @@ void WallpaperManager::SetUserWallpaper(const std::string& email) {
   GetUserWallpaperProperties(email, &type, &index, &date);
   if (type == User::DAILY && date != base::Time::Now().LocalMidnight()) {
     index = ash::GetNextWallpaperIndex(index);
-    SaveUserWallpaperProperties(email, User::DAILY, index);
+    SetUserWallpaperProperties(email, User::DAILY, index,
+                               ShouldPersistDataForUser(email));
   } else if (type == User::CUSTOMIZED) {
     // For security reason, use default wallpaper instead of custom wallpaper
     // at login screen. The security issue is tracked in issue 143198. Once it
@@ -477,7 +479,7 @@ void WallpaperManager::BatchUpdateWallpaper() {
       base::Time current_date = base::Time::Now().LocalMidnight();
       if (type == User::DAILY && current_date != last_modification_date) {
         index = ash::GetNextWallpaperIndex(index);
-        SaveUserWallpaperProperties(email, type, index);
+        SetUserWallpaperProperties(email, type, index, true);
       }
       // Force a wallpaper update for logged in / last selected user.
       // TODO(bshe): Notify lock screen, wallpaper picker UI to update wallpaper
@@ -637,12 +639,15 @@ void WallpaperManager::SetWallpaper(const std::string& username,
   std::string wallpaper_path =
       GetWallpaperPathForUser(username, false).value();
 
-  // TODO(bshe): Ephemeral user should not save custom wallpaper to disk.
-  BrowserThread::PostTask(
-      BrowserThread::FILE,
-      FROM_HERE,
-      base::Bind(&WallpaperManager::SaveWallpaper,
-                 base::Unretained(this), wallpaper_path, wallpaper));
+  bool is_persistent = ShouldPersistDataForUser(username);
+
+  if (is_persistent) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE,
+        FROM_HERE,
+        base::Bind(&WallpaperManager::SaveWallpaper,
+                   base::Unretained(this), wallpaper_path, wallpaper));
+  }
 
   BrowserThread::PostTask(
       BrowserThread::FILE,
@@ -653,7 +658,13 @@ void WallpaperManager::SetWallpaper(const std::string& username,
 
   ash::Shell::GetInstance()->desktop_background_controller()->
       SetCustomWallpaper(wallpaper.image(), layout);
-  SaveUserWallpaperProperties(username, type, layout);
+  SetUserWallpaperProperties(username, type, layout, is_persistent);
+}
+
+bool WallpaperManager::ShouldPersistDataForUser(const std::string& email) {
+  UserManager* user_manager = UserManager::Get();
+  return !(email == user_manager->GetLoggedInUser().email() &&
+           user_manager->IsCurrentUserEphemeral());
 }
 
 void WallpaperManager::OnWallpaperLoaded(ash::WallpaperLayout layout,

chrome/browser/chromeos/login/wallpaper_manager.h

@@ -91,13 +91,15 @@ class WallpaperManager: public system::TimezoneSettings::Observer,
                                 base::WeakPtr<WallpaperDelegate> delegate);
 
   // Set |email|'s wallpaper |type|, |index| and local date to local state.
-  void SaveUserWallpaperProperties(const std::string& email,
-                                   User::WallpaperType type,
-                                   int index);
+  // When |is_persistent| is false, changes are kept in memory only.
+  void SetUserWallpaperProperties(const std::string& email,
+                                  User::WallpaperType type,
+                                  int index,
+                                  bool is_persistent);
 
   // Sets one of the default wallpapers for the specified user and saves this
   // settings in local state.
-  void SetInitialUserWallpaper(const std::string& username);
+  void SetInitialUserWallpaper(const std::string& username, bool is_persistent);
 
   // Saves |username| selected wallpaper information to local state.
   void SaveUserWallpaperInfo(const std::string& username,
@@ -185,6 +187,11 @@ class WallpaperManager: public system::TimezoneSettings::Observer,
                     base::WeakPtr<WallpaperDelegate> delegate,
                     const UserImage& wallpaper);
 
+  // Whether wallpaper data should be persisted for user |email|.
+  // Note: this function can not be called in SetUserWallpaperProperties. It
+  // will create a deadlock. (issue 142440)
+  bool ShouldPersistDataForUser(const std::string& email);
+
   // Sets wallpaper to image in |user_image| with |layout|.
   void OnWallpaperLoaded(ash::WallpaperLayout layout,
                          const UserImage& user_image);