diff --git a/chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc b/chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc index c1991c635d2801..94e4699d8bc1a1 100644 --- a/chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc +++ b/chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc @@ -150,25 +150,12 @@ class UserImageManagerTest : public LoginManagerTest, account_id, account_id.GetUserEmail(), false); } - // Stores old (pre-migration) user image info. - void SetOldUserImageInfo(const AccountId& account_id, - int image_index, - const base::FilePath& image_path) { - RegisterUser(account_id.GetUserEmail()); - DictionaryPrefUpdate images_pref(local_state_, "UserImages"); - base::DictionaryValue* image_properties = new base::DictionaryValue(); - image_properties->Set("index", new base::FundamentalValue(image_index)); - image_properties->Set( - "path" , new base::StringValue(image_path.value())); - images_pref->SetWithoutPathExpansion(account_id.GetUserEmail(), - image_properties); - } - - // Verifies user image info in |images_pref| dictionary. - void ExpectUserImageInfo(const base::DictionaryValue* images_pref, - const AccountId& account_id, + // Verifies user image info. + void ExpectUserImageInfo(const AccountId& account_id, int image_index, const base::FilePath& image_path) { + const base::DictionaryValue* images_pref = + local_state_->GetDictionary(UserImageManagerImpl::kUserImageProperties); ASSERT_TRUE(images_pref); const base::DictionaryValue* image_properties = NULL; images_pref->GetDictionaryWithoutPathExpansion(account_id.GetUserEmail(), @@ -176,8 +163,11 @@ class UserImageManagerTest : public LoginManagerTest, ASSERT_TRUE(image_properties); int actual_image_index; std::string actual_image_path; - ASSERT_TRUE(image_properties->GetInteger("index", &actual_image_index) && - image_properties->GetString("path", &actual_image_path)); + ASSERT_TRUE( + image_properties->GetInteger(UserImageManagerImpl::kImageIndexNodeName, + &actual_image_index) && + image_properties->GetString(UserImageManagerImpl::kImagePathNodeName, + &actual_image_path)); EXPECT_EQ(image_index, actual_image_index); EXPECT_EQ(image_path.value(), actual_image_path); } @@ -193,43 +183,6 @@ class UserImageManagerTest : public LoginManagerTest, ASSERT_FALSE(image_properties); } - // Verifies that old user image info matches |image_index| and |image_path| - // and that new user image info does not exist. - void ExpectOldUserImageInfo(const AccountId& account_id, - int image_index, - const base::FilePath& image_path) { - ExpectUserImageInfo(local_state_->GetDictionary("UserImages"), account_id, - image_index, image_path); - ExpectNoUserImageInfo(local_state_->GetDictionary("user_image_info"), - account_id); - } - - // Verifies that new user image info matches |image_index| and |image_path| - // and that old user image info does not exist. - void ExpectNewUserImageInfo(const AccountId& account_id, - int image_index, - const base::FilePath& image_path) { - ExpectUserImageInfo(local_state_->GetDictionary("user_image_info"), - account_id, image_index, image_path); - ExpectNoUserImageInfo(local_state_->GetDictionary("UserImages"), - account_id); - } - - // Sets bitmap |resource_id| as image for |account_id| and saves it to disk. - void SaveUserImagePNG(const AccountId& account_id, int resource_id) { - base::FilePath image_path = GetUserImagePath(account_id, "png"); - scoped_refptr image_data( - ResourceBundle::GetSharedInstance().LoadDataResourceBytesForScale( - resource_id, ui::SCALE_FACTOR_100P)); - int written = base::WriteFile( - image_path, - reinterpret_cast(image_data->front()), - image_data->size()); - EXPECT_EQ(static_cast(image_data->size()), written); - SetOldUserImageInfo(account_id, user_manager::User::USER_IMAGE_EXTERNAL, - image_path); - } - // Returns the image path for user |account_id| with specified |extension|. base::FilePath GetUserImagePath(const AccountId& account_id, const std::string& extension) { @@ -334,99 +287,22 @@ class UserImageManagerTest : public LoginManagerTest, DISALLOW_COPY_AND_ASSIGN(UserImageManagerTest); }; -IN_PROC_BROWSER_TEST_F(UserImageManagerTest, PRE_DefaultUserImagePreserved) { - // Setup an old default (stock) user image. - ScopedUserManagerEnabler(new MockUserManager); - SetOldUserImageInfo(test_account_id1_, - default_user_image::kFirstDefaultImageIndex, - base::FilePath()); -} - -IN_PROC_BROWSER_TEST_F(UserImageManagerTest, DefaultUserImagePreserved) { - user_manager::UserManager::Get()->GetUsers(); // Load users. - // Old info preserved. - ExpectOldUserImageInfo(test_account_id1_, - default_user_image::kFirstDefaultImageIndex, - base::FilePath()); - LogIn(test_account_id1_); - // Image info is migrated now. - ExpectNewUserImageInfo(test_account_id1_, - default_user_image::kFirstDefaultImageIndex, - base::FilePath()); -} - -IN_PROC_BROWSER_TEST_F(UserImageManagerTest, PRE_OtherUsersUnaffected) { - // Setup two users with stock images. - ScopedUserManagerEnabler(new MockUserManager); - SetOldUserImageInfo(test_account_id1_, - default_user_image::kFirstDefaultImageIndex, - base::FilePath()); - SetOldUserImageInfo(test_account_id2_, - default_user_image::kFirstDefaultImageIndex + 1, - base::FilePath()); -} - -IN_PROC_BROWSER_TEST_F(UserImageManagerTest, OtherUsersUnaffected) { - user_manager::UserManager::Get()->GetUsers(); // Load users. - // Old info preserved. - ExpectOldUserImageInfo(test_account_id1_, - default_user_image::kFirstDefaultImageIndex, - base::FilePath()); - ExpectOldUserImageInfo(test_account_id2_, - default_user_image::kFirstDefaultImageIndex + 1, - base::FilePath()); - LogIn(test_account_id1_); - // Image info is migrated for the first user and unaffected for the rest. - ExpectNewUserImageInfo(test_account_id1_, - default_user_image::kFirstDefaultImageIndex, - base::FilePath()); - ExpectOldUserImageInfo(test_account_id2_, - default_user_image::kFirstDefaultImageIndex + 1, - base::FilePath()); -} - -IN_PROC_BROWSER_TEST_F(UserImageManagerTest, PRE_PRE_NonJPEGImageFromFile) { - // Setup a user with non-JPEG image. - ScopedUserManagerEnabler(new MockUserManager); - SaveUserImagePNG(test_account_id1_, - default_user_image::kDefaultImageResourceIDs - [default_user_image::kFirstDefaultImageIndex]); -} +IN_PROC_BROWSER_TEST_F(UserImageManagerTest, PRE_SaveAndLoadUserImage) { + RegisterUser(test_account_id1_.GetUserEmail()); -IN_PROC_BROWSER_TEST_F(UserImageManagerTest, PRE_NonJPEGImageFromFile) { - user_manager::UserManager::Get()->GetUsers(); // Load users. - // Old info preserved. - ExpectOldUserImageInfo(test_account_id1_, - user_manager::User::USER_IMAGE_EXTERNAL, - GetUserImagePath(test_account_id1_, "png")); - const user_manager::User* user = - user_manager::UserManager::Get()->FindUser(test_account_id1_); - EXPECT_TRUE(user->image_is_stub()); - - base::RunLoop run_loop; - PrefChangeRegistrar pref_change_registrar_; - pref_change_registrar_.Init(local_state_); - pref_change_registrar_.Add("UserImages", run_loop.QuitClosure()); - LogIn(test_account_id1_); - - // Wait for migration. - run_loop.Run(); - - // Image info is migrated and the image is converted to JPG. - ExpectNewUserImageInfo(test_account_id1_, - user_manager::User::USER_IMAGE_EXTERNAL, - GetUserImagePath(test_account_id1_, "jpg")); - user = user_manager::UserManager::Get()->GetLoggedInUser(); - ASSERT_TRUE(user); - EXPECT_FALSE(user->image_is_safe_format()); - // Check image dimensions. - const gfx::ImageSkia& saved_image = default_user_image::GetDefaultImage( + // Setup a user with JPEG image. + run_loop_.reset(new base::RunLoop); + const gfx::ImageSkia& image = default_user_image::GetDefaultImage( default_user_image::kFirstDefaultImageIndex); - EXPECT_EQ(saved_image.width(), user->GetImage().width()); - EXPECT_EQ(saved_image.height(), user->GetImage().height()); + UserImageManager* user_image_manager = + ChromeUserManager::Get()->GetUserImageManager(test_account_id1_); + user_image_manager->SaveUserImage( + user_manager::UserImage::CreateAndEncode(image)); + run_loop_->Run(); } -IN_PROC_BROWSER_TEST_F(UserImageManagerTest, NonJPEGImageFromFile) { +// Ensures that the user image in JPEG format is loaded correctly. +IN_PROC_BROWSER_TEST_F(UserImageManagerTest, SaveAndLoadUserImage) { user_manager::UserManager::Get()->GetUsers(); // Load users. const user_manager::User* user = user_manager::UserManager::Get()->FindUser(test_account_id1_); @@ -437,7 +313,7 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest, NonJPEGImageFromFile) { chrome::NOTIFICATION_LOGIN_USER_IMAGE_CHANGED, content::NotificationService::AllSources()).Wait(); } - // Now the migrated image is used. + // The image should be in the safe format. EXPECT_TRUE(user->image_is_safe_format()); // Check image dimensions. Images can't be compared since JPEG is lossy. const gfx::ImageSkia& saved_image = default_user_image::GetDefaultImage( @@ -468,9 +344,9 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest, SaveUserDefaultImageIndex) { EXPECT_TRUE(user->HasDefaultImage()); EXPECT_EQ(default_user_image::kFirstDefaultImageIndex, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(default_image, user->GetImage())); - ExpectNewUserImageInfo(test_account_id1_, - default_user_image::kFirstDefaultImageIndex, - base::FilePath()); + ExpectUserImageInfo(test_account_id1_, + default_user_image::kFirstDefaultImageIndex, + base::FilePath()); } IN_PROC_BROWSER_TEST_F(UserImageManagerTest, PRE_SaveUserImage) { @@ -501,9 +377,9 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest, SaveUserImage) { EXPECT_FALSE(user->HasDefaultImage()); EXPECT_EQ(user_manager::User::USER_IMAGE_EXTERNAL, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(custom_image, user->GetImage())); - ExpectNewUserImageInfo(test_account_id1_, - user_manager::User::USER_IMAGE_EXTERNAL, - GetUserImagePath(test_account_id1_, "jpg")); + ExpectUserImageInfo(test_account_id1_, + user_manager::User::USER_IMAGE_EXTERNAL, + GetUserImagePath(test_account_id1_, "jpg")); const scoped_ptr saved_image = test::ImageLoader(GetUserImagePath(test_account_id1_, "jpg")).Load(); @@ -540,9 +416,9 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest, SaveUserImageFromFile) { EXPECT_FALSE(user->HasDefaultImage()); EXPECT_EQ(user_manager::User::USER_IMAGE_EXTERNAL, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(*custom_image, user->GetImage())); - ExpectNewUserImageInfo(test_account_id1_, - user_manager::User::USER_IMAGE_EXTERNAL, - GetUserImagePath(test_account_id1_, "jpg")); + ExpectUserImageInfo(test_account_id1_, + user_manager::User::USER_IMAGE_EXTERNAL, + GetUserImagePath(test_account_id1_, "jpg")); const scoped_ptr saved_image = test::ImageLoader(GetUserImagePath(test_account_id1_, "jpg")).Load(); @@ -587,9 +463,8 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest, SaveUserImageFromProfileImage) { EXPECT_FALSE(user->HasDefaultImage()); EXPECT_EQ(user_manager::User::USER_IMAGE_PROFILE, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(profile_image, user->GetImage())); - ExpectNewUserImageInfo(test_account_id1_, - user_manager::User::USER_IMAGE_PROFILE, - GetUserImagePath(test_account_id1_, "jpg")); + ExpectUserImageInfo(test_account_id1_, user_manager::User::USER_IMAGE_PROFILE, + GetUserImagePath(test_account_id1_, "jpg")); const scoped_ptr saved_image = test::ImageLoader(GetUserImagePath(test_account_id1_, "jpg")).Load(); @@ -641,9 +516,9 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest, EXPECT_TRUE(user->HasDefaultImage()); EXPECT_EQ(default_user_image::kFirstDefaultImageIndex, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(default_image, user->GetImage())); - ExpectNewUserImageInfo(test_account_id1_, - default_user_image::kFirstDefaultImageIndex, - base::FilePath()); + ExpectUserImageInfo(test_account_id1_, + default_user_image::kFirstDefaultImageIndex, + base::FilePath()); } class UserImageManagerPolicyTest : public UserImageManagerTest, @@ -757,9 +632,9 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, DISABLED_SetAndClear) { EXPECT_FALSE(user->HasDefaultImage()); EXPECT_EQ(user_manager::User::USER_IMAGE_EXTERNAL, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(*policy_image_, user->GetImage())); - ExpectNewUserImageInfo(enterprise_account_id_, - user_manager::User::USER_IMAGE_EXTERNAL, - GetUserImagePath(enterprise_account_id_, "jpg")); + ExpectUserImageInfo(enterprise_account_id_, + user_manager::User::USER_IMAGE_EXTERNAL, + GetUserImagePath(enterprise_account_id_, "jpg")); scoped_ptr saved_image = test::ImageLoader(GetUserImagePath(enterprise_account_id_, "jpg")).Load(); @@ -791,8 +666,8 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, DISABLED_SetAndClear) { const gfx::ImageSkia& default_image = default_user_image::GetDefaultImage(default_image_index); EXPECT_TRUE(test::AreImagesEqual(default_image, user->GetImage())); - ExpectNewUserImageInfo(enterprise_account_id_, default_image_index, - base::FilePath()); + ExpectUserImageInfo(enterprise_account_id_, default_image_index, + base::FilePath()); // Choose a different user image. Verify that the chosen user image is set and // persisted. @@ -810,8 +685,8 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, DISABLED_SetAndClear) { EXPECT_TRUE(user->HasDefaultImage()); EXPECT_EQ(user_image_index, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(user_image, user->GetImage())); - ExpectNewUserImageInfo(enterprise_account_id_, user_image_index, - base::FilePath()); + ExpectUserImageInfo(enterprise_account_id_, user_image_index, + base::FilePath()); } IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, PRE_PolicyOverridesUser) { @@ -846,9 +721,9 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, PolicyOverridesUser) { EXPECT_TRUE(user->HasDefaultImage()); EXPECT_EQ(default_user_image::kFirstDefaultImageIndex, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(default_image, user->GetImage())); - ExpectNewUserImageInfo(enterprise_account_id_, - default_user_image::kFirstDefaultImageIndex, - base::FilePath()); + ExpectUserImageInfo(enterprise_account_id_, + default_user_image::kFirstDefaultImageIndex, + base::FilePath()); // Set policy. Verify that the policy-provided user image is downloaded, set // and persisted, overriding the previously set image. @@ -864,9 +739,9 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, PolicyOverridesUser) { EXPECT_FALSE(user->HasDefaultImage()); EXPECT_EQ(user_manager::User::USER_IMAGE_EXTERNAL, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(*policy_image_, user->GetImage())); - ExpectNewUserImageInfo(enterprise_account_id_, - user_manager::User::USER_IMAGE_EXTERNAL, - GetUserImagePath(enterprise_account_id_, "jpg")); + ExpectUserImageInfo(enterprise_account_id_, + user_manager::User::USER_IMAGE_EXTERNAL, + GetUserImagePath(enterprise_account_id_, "jpg")); scoped_ptr saved_image = test::ImageLoader(GetUserImagePath(enterprise_account_id_, "jpg")).Load(); @@ -911,9 +786,9 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, UserDoesNotOverridePolicy) { EXPECT_FALSE(user->HasDefaultImage()); EXPECT_EQ(user_manager::User::USER_IMAGE_EXTERNAL, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(*policy_image_, user->GetImage())); - ExpectNewUserImageInfo(enterprise_account_id_, - user_manager::User::USER_IMAGE_EXTERNAL, - GetUserImagePath(enterprise_account_id_, "jpg")); + ExpectUserImageInfo(enterprise_account_id_, + user_manager::User::USER_IMAGE_EXTERNAL, + GetUserImagePath(enterprise_account_id_, "jpg")); scoped_ptr saved_image = test::ImageLoader(GetUserImagePath(enterprise_account_id_, "jpg")).Load(); @@ -933,9 +808,9 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, UserDoesNotOverridePolicy) { EXPECT_FALSE(user->HasDefaultImage()); EXPECT_EQ(user_manager::User::USER_IMAGE_EXTERNAL, user->image_index()); EXPECT_TRUE(test::AreImagesEqual(*policy_image_, user->GetImage())); - ExpectNewUserImageInfo(enterprise_account_id_, - user_manager::User::USER_IMAGE_EXTERNAL, - GetUserImagePath(enterprise_account_id_, "jpg")); + ExpectUserImageInfo(enterprise_account_id_, + user_manager::User::USER_IMAGE_EXTERNAL, + GetUserImagePath(enterprise_account_id_, "jpg")); saved_image = test::ImageLoader(GetUserImagePath(enterprise_account_id_, "jpg")).Load(); diff --git a/chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.cc b/chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.cc index 306aee0a720d67..d702cabe9fe867 100644 --- a/chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.cc +++ b/chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.cc @@ -49,20 +49,6 @@ namespace chromeos { namespace { -// A dictionary that maps user_ids to old user image data with images stored in -// PNG format. Deprecated. -// TODO(ivankr): remove this const char after migration is gone. -const char kUserImages[] = "UserImages"; - -// A dictionary that maps user_ids to user image data with images stored in -// JPEG format. -const char kUserImageProperties[] = "user_image_info"; - -// Names of user image properties. -const char kImagePathNodeName[] = "path"; -const char kImageIndexNodeName[] = "index"; -const char kImageURLNodeName[] = "url"; - // Delay betweeen user login and attempt to update user's profile data. const int kProfileDataDownloadDelaySec = 10; @@ -190,10 +176,14 @@ bool SaveImage(const user_manager::UserImage& user_image, } // namespace +const char UserImageManagerImpl::kUserImageProperties[] = "user_image_info"; +const char UserImageManagerImpl::kImagePathNodeName[] = "path"; +const char UserImageManagerImpl::kImageIndexNodeName[] = "index"; +const char UserImageManagerImpl::kImageURLNodeName[] = "url"; + // static void UserImageManager::RegisterPrefs(PrefRegistrySimple* registry) { - registry->RegisterDictionaryPref(kUserImages); - registry->RegisterDictionaryPref(kUserImageProperties); + registry->RegisterDictionaryPref(UserImageManagerImpl::kUserImageProperties); } // Every image load or update is encapsulated by a Job. The Job is allowed to @@ -482,7 +472,6 @@ UserImageManagerImpl::UserImageManagerImpl( downloading_profile_image_(false), profile_image_requested_(false), has_managed_image_(false), - user_needs_migration_(false), weak_factory_(this) { base::SequencedWorkerPool* blocking_pool = content::BrowserThread::GetBlockingPool(); @@ -496,29 +485,14 @@ UserImageManagerImpl::~UserImageManagerImpl() {} void UserImageManagerImpl::LoadUserImage() { PrefService* local_state = g_browser_process->local_state(); - const base::DictionaryValue* prefs_images_unsafe = - local_state->GetDictionary(kUserImages); const base::DictionaryValue* prefs_images = local_state->GetDictionary(kUserImageProperties); - if (!prefs_images && !prefs_images_unsafe) + if (!prefs_images) return; user_manager::User* user = GetUserAndModify(); - bool needs_migration = false; - // If entries are found in both |prefs_images_unsafe| and |prefs_images|, - // |prefs_images| is honored for now but |prefs_images_unsafe| will be - // migrated, overwriting the |prefs_images| entry, when the user logs in. const base::DictionaryValue* image_properties = NULL; - if (prefs_images_unsafe) { - needs_migration = prefs_images_unsafe->GetDictionaryWithoutPathExpansion( - user_id(), &image_properties); - if (needs_migration) - user_needs_migration_ = true; - } - if (prefs_images) { - prefs_images->GetDictionaryWithoutPathExpansion(user_id(), - &image_properties); - } + prefs_images->GetDictionaryWithoutPathExpansion(user_id(), &image_properties); // If the user image for |user_id| is managed by policy and the policy-set // image is being loaded and persisted right now, let that job continue. It @@ -561,11 +535,9 @@ void UserImageManagerImpl::LoadUserImage() { true); DCHECK(!image_path.empty() || image_index == user_manager::User::USER_IMAGE_PROFILE); - if (image_path.empty() || needs_migration) { - // Return if either of the following is true: - // * The profile image is to be used but has not been downloaded yet. The - // profile image will be downloaded after login. - // * The image needs migration. Migration will be performed after login. + if (image_path.empty()) { + // Return if the profile image is to be used but has not been downloaded + // yet. The profile image will be downloaded after login. return; } @@ -583,27 +555,6 @@ void UserImageManagerImpl::UserLoggedIn(bool user_is_new, UMA_HISTOGRAM_ENUMERATION("UserImage.LoggedIn", ImageIndexToHistogramIndex(user->image_index()), default_user_image::kHistogramImagesCount); - - if (!IsUserImageManaged() && user_needs_migration_) { - const base::DictionaryValue* prefs_images_unsafe = - g_browser_process->local_state()->GetDictionary(kUserImages); - const base::DictionaryValue* image_properties = NULL; - if (prefs_images_unsafe->GetDictionaryWithoutPathExpansion( - user_id(), &image_properties)) { - std::string image_path; - image_properties->GetString(kImagePathNodeName, &image_path); - job_.reset(new Job(this)); - if (!image_path.empty()) { - VLOG(0) << "Loading old user image, then migrating it."; - job_->SetToPath(base::FilePath(image_path), - user->image_index(), - user->image_url(), - false); - } else { - job_->SetToDefaultImage(user->image_index()); - } - } - } } // Reset the downloaded profile image as a new user logged in. @@ -683,7 +634,6 @@ void UserImageManagerImpl::SaveUserImageFromProfileImage() { void UserImageManagerImpl::DeleteUserImage() { job_.reset(); - DeleteUserImageAndLocalStateEntry(kUserImages); DeleteUserImageAndLocalStateEntry(kUserImageProperties); } @@ -957,52 +907,6 @@ void UserImageManagerImpl::OnJobDone() { base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, job_.release()); else NOTREACHED(); - - if (!user_needs_migration_) - return; - // Migration completed for |user_id|. - user_needs_migration_ = false; - - const base::DictionaryValue* prefs_images_unsafe = - g_browser_process->local_state()->GetDictionary(kUserImages); - const base::DictionaryValue* image_properties = NULL; - if (!prefs_images_unsafe->GetDictionaryWithoutPathExpansion( - user_id(), &image_properties)) { - NOTREACHED(); - return; - } - - int image_index = user_manager::User::USER_IMAGE_INVALID; - image_properties->GetInteger(kImageIndexNodeName, &image_index); - UMA_HISTOGRAM_ENUMERATION("UserImage.Migration", - ImageIndexToHistogramIndex(image_index), - default_user_image::kHistogramImagesCount); - - std::string image_path; - image_properties->GetString(kImagePathNodeName, &image_path); - if (!image_path.empty()) { - // If an old image exists, delete it and remove |user_id| from the old prefs - // dictionary only after the deletion has completed. This ensures that no - // orphaned image is left behind if the browser crashes before the deletion - // has been performed: In that case, local state will be unchanged and the - // migration will be run again on the user's next login. - background_task_runner_->PostTaskAndReply( - FROM_HERE, - base::Bind(base::IgnoreResult(&base::DeleteFile), - base::FilePath(image_path), - false), - base::Bind(&UserImageManagerImpl::UpdateLocalStateAfterMigration, - weak_factory_.GetWeakPtr())); - } else { - // If no old image exists, remove |user_id| from the old prefs dictionary. - UpdateLocalStateAfterMigration(); - } -} - -void UserImageManagerImpl::UpdateLocalStateAfterMigration() { - DictionaryPrefUpdate update(g_browser_process->local_state(), - kUserImages); - update->RemoveWithoutPathExpansion(user_id(), NULL); } void UserImageManagerImpl::TryToCreateImageSyncObserver() { diff --git a/chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.h b/chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.h index 7705109095e2df..38d912c127850c 100644 --- a/chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.h +++ b/chrome/browser/chromeos/login/users/avatar/user_image_manager_impl.h @@ -67,6 +67,14 @@ class UserImageManagerImpl static void IgnoreProfileDataDownloadDelayForTesting(); + // Key for a dictionary that maps user IDs to user image data with images + // stored in JPEG format. + static const char kUserImageProperties[]; + // Names of user image properties. + static const char kImagePathNodeName[]; + static const char kImageIndexNodeName[]; + static const char kImageURLNodeName[]; + private: friend class UserImageManagerTest; @@ -130,15 +138,9 @@ class UserImageManagerImpl // send a NOTIFICATION_LOGIN_USER_IMAGE_CHANGED notification. void OnJobChangedUserImage(); - // Called when a Job for the user finishes. If a migration was - // required for the user, the migration is now complete and the old - // image file for that user, if any, is deleted. + // Called when a Job for the user finishes. void OnJobDone(); - // Completes migration by removing the user from the old prefs - // dictionary. - void UpdateLocalStateAfterMigration(); - // Create a sync observer if a user is logged in, the user's user image is // allowed to be synced and no sync observer exists yet. void TryToCreateImageSyncObserver(); @@ -209,7 +211,6 @@ class UserImageManagerImpl scoped_ptr job_; bool has_managed_image_; - bool user_needs_migration_; base::WeakPtrFactory weak_factory_;