Stop using the old-style profile names when using --new-profile-management

chrome/browser/profiles/profile_manager_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/scoped_testing_local_state.h"
 #include "chrome/test/base/test_browser_window.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "grit/generated_resources.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/login/mock_user_manager.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chromeos/chromeos_switches.h"
 #endif
 
 using base::ASCIIToUTF16;
@@ skipping 69 matching lines @@
                           MockObserver* mock_observer) {
     manager->CreateProfileAsync(
         temp_dir_.path().AppendASCII(name),
         base::Bind(&MockObserver::OnProfileCreated,
                    base::Unretained(mock_observer)),
         base::UTF8ToUTF16(name),
         base::UTF8ToUTF16(ProfileInfoCache::GetDefaultAvatarIconUrl(0)),
         is_managed ? "Dummy ID" : std::string());
   }
 
+  // Helper function to add a profile with |profile_name| to
+  // |profile_manager|'s ProfileInfoCache, and return the profile created.
+  Profile* AddProfileToCache(ProfileManager* profile_manager,
+                             const std::string& path_suffix,
+                             const base::string16& profile_name) {
+  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();

[groby-ooo-7-16, 2014/02/20 20:17:27]

Indent seems off

[noms (inactive), 2014/02/24 22:35:06]

Done.

+  size_t num_profiles = cache.GetNumberOfProfiles();
+  base::FilePath path = temp_dir_.path().AppendASCII(path_suffix);
+  cache.AddProfileToCache(path, profile_name,
+                          base::string16(), 0, std::string());
+  EXPECT_EQ(num_profiles + 1, cache.GetNumberOfProfiles());
+  return profile_manager->GetProfile(path);
+}
+
 #if defined(OS_CHROMEOS)
   chromeos::ScopedTestDeviceSettingsService test_device_settings_service_;
   chromeos::ScopedTestCrosSettings test_cros_settings_;
 #endif
 
   // The path to temporary directory used to contain the test operations.
   base::ScopedTempDir temp_dir_;
   ScopedTestingLocalState local_state_;
 
   content::TestBrowserThreadBundle thread_bundle_;
@@ skipping 680 matching lines @@
   EXPECT_EQ(normal_profile, last_opened_profiles[0]);
 }
 
 TEST_F(ProfileManagerTest, ActiveProfileDeleted) {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   ASSERT_TRUE(profile_manager);
 
   // Create and load two profiles.
   const std::string profile_name1 = "New Profile 1";
   const std::string profile_name2 = "New Profile 2";
-  base::FilePath dest_path1 =
-      temp_dir_.path().AppendASCII(profile_name1);
-  base::FilePath dest_path2 =
-      temp_dir_.path().AppendASCII(profile_name2);
+  base::FilePath dest_path1 = temp_dir_.path().AppendASCII(profile_name1);
+  base::FilePath dest_path2 = temp_dir_.path().AppendASCII(profile_name2);
 
   MockObserver mock_observer;
   EXPECT_CALL(mock_observer, OnProfileCreated(
       testing::NotNull(), NotFail())).Times(testing::AtLeast(3));
 
   CreateProfileAsync(profile_manager, profile_name1, false, &mock_observer);
   CreateProfileAsync(profile_manager, profile_name2, false, &mock_observer);
   base::RunLoop().RunUntilIdle();
 
   EXPECT_EQ(2u, profile_manager->GetLoadedProfiles().size());
@@ skipping 17 matching lines @@
 #if defined(OS_MACOSX)
 // These tests are for a Mac-only code path that assumes the browser
 // process isn't killed when all browser windows are closed.
 TEST_F(ProfileManagerTest, ActiveProfileDeletedNeedsToLoadNextProfile) {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   ASSERT_TRUE(profile_manager);
 
   // Create and load one profile, and just create a second profile.
   const std::string profile_name1 = "New Profile 1";
   const std::string profile_name2 = "New Profile 2";
-  base::FilePath dest_path1 =
-      temp_dir_.path().AppendASCII(profile_name1);
-  base::FilePath dest_path2 =
-      temp_dir_.path().AppendASCII(profile_name2);
+  base::FilePath dest_path1 = temp_dir_.path().AppendASCII(profile_name1);
+  base::FilePath dest_path2 = temp_dir_.path().AppendASCII(profile_name2);
 
   MockObserver mock_observer;
   EXPECT_CALL(mock_observer, OnProfileCreated(
       testing::NotNull(), NotFail())).Times(testing::AtLeast(2));
   CreateProfileAsync(profile_manager, profile_name1, false, &mock_observer);
   base::RunLoop().RunUntilIdle();
 
   // Track the profile, but don't load it.
   ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
   cache.AddProfileToCache(dest_path2, ASCIIToUTF16(profile_name2),
@@ skipping 25 matching lines @@
 // the next active profile has also been marked for deletion, so the
 // ProfileManager needs to recursively select a different next profile.
 TEST_F(ProfileManagerTest, ActiveProfileDeletedNextProfileDeletedToo) {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   ASSERT_TRUE(profile_manager);
 
   // Create and load one profile, and create two more profiles.
   const std::string profile_name1 = "New Profile 1";
   const std::string profile_name2 = "New Profile 2";
   const std::string profile_name3 = "New Profile 3";
-  base::FilePath dest_path1 =
-      temp_dir_.path().AppendASCII(profile_name1);
-  base::FilePath dest_path2 =
-      temp_dir_.path().AppendASCII(profile_name2);
-  base::FilePath dest_path3 =
-      temp_dir_.path().AppendASCII(profile_name3);
+  base::FilePath dest_path1 = temp_dir_.path().AppendASCII(profile_name1);
+  base::FilePath dest_path2 = temp_dir_.path().AppendASCII(profile_name2);
+  base::FilePath dest_path3 = temp_dir_.path().AppendASCII(profile_name3);
 
   MockObserver mock_observer;
   EXPECT_CALL(mock_observer, OnProfileCreated(
       testing::NotNull(), NotFail())).Times(testing::AtLeast(2));
   CreateProfileAsync(profile_manager, profile_name1, false, &mock_observer);
   base::RunLoop().RunUntilIdle();
 
   // Create the other profiles, but don't load them. Assign a fake avatar icon
   // to ensure that profiles in the info cache are sorted by the profile name,
   // and not randomly by the avatar name.
@@ skipping 26 matching lines @@
                          dest_path2.BaseName().MaybeAsASCII());
   profile_manager->ScheduleProfileForDeletion(dest_path2,
                                               ProfileManager::CreateCallback());
   // Spin the message loop so that all the callbacks can finish running.
   base::RunLoop().RunUntilIdle();
 
   EXPECT_EQ(dest_path3, profile_manager->GetLastUsedProfile()->GetPath());
   EXPECT_EQ(profile_name3, local_state->GetString(prefs::kProfileLastUsed));
 }
 #endif  // !defined(OS_MACOSX)
+
+TEST_F(ProfileManagerTest, ProfileDisplayNameResetsDefaultName) {
+  // The command line is reset at the end of every test by the test suite.
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kNewProfileManagement);
+
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
+  EXPECT_EQ(0u, cache.GetNumberOfProfiles());
+
+  // Only one local profile means we display IDS_SINGLE_PROFILE_DISPLAY_NAME.
+  const base::string16 default_profile_name =
+      l10n_util::GetStringUTF16(IDS_SINGLE_PROFILE_DISPLAY_NAME);
+  const base::string16 profile_name1 = cache.ChooseNameForNewProfile(0);
+  Profile* profile1 = AddProfileToCache(profile_manager,
+                                        "path_1", profile_name1);
+  EXPECT_EQ(default_profile_name,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+
+  // Multiple profiles means displaying the actual profile names.
+  const base::string16 profile_name2 = cache.ChooseNameForNewProfile(1);
+  Profile* profile2 = AddProfileToCache(profile_manager,
+                                        "path_2", profile_name2);
+  EXPECT_EQ(profile_name1,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+  EXPECT_EQ(profile_name2,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile2));
+
+  // Deleting a profile means returning to the default name.
+  profile_manager->ScheduleProfileForDeletion(profile2->GetPath(),
+                                              ProfileManager::CreateCallback());
+  // Spin the message loop so that all the callbacks can finish running.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(default_profile_name,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+}
+
+TEST_F(ProfileManagerTest, ProfileDisplayNamePreservesCustomName) {
+  // The command line is reset at the end of every test by the test suite.
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kNewProfileManagement);
+
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
+  EXPECT_EQ(0u, cache.GetNumberOfProfiles());
+
+  // Only one local profile means we display IDS_SINGLE_PROFILE_DISPLAY_NAME.
+  const base::string16 default_profile_name =
+      l10n_util::GetStringUTF16(IDS_SINGLE_PROFILE_DISPLAY_NAME);
+  const base::string16 profile_name1 = cache.ChooseNameForNewProfile(0);
+  Profile* profile1 = AddProfileToCache(profile_manager,
+                                        "path_1", profile_name1);
+  EXPECT_EQ(default_profile_name,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+
+  // We should display custom names for local profiles.

[rpetterson, 2014/02/20 21:41:32]

How does the profile info cache know that it was a custom name vs. a given
default name?

[noms (inactive), 2014/02/24 22:35:06]

The ProfileInfoCache doesn't. The profiles::GetAvatarButtonDisplayNameForProfile
does a check whether the name is of the form "Person %d", which is what default
names look like. That function is the only one that cares whether the name is
the default one the profile was created with. I thought about adding a
preference to the ProfileInfoCache -- the problem is that it wouldn't be set for
all existing profiles, so we might display "you" for a lot of single, local
profiles where the user has changed the name.
 
On 2014/02/20 21:41:32, rpetterson wrote:

+  const base::string16 custom_profile_name = ASCIIToUTF16("Batman");
+  cache.SetNameOfProfileAtIndex(0, custom_profile_name);
+  EXPECT_EQ(custom_profile_name, cache.GetNameOfProfileAtIndex(0));
+  EXPECT_EQ(custom_profile_name,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+
+  // Multiple profiles means displaying the actual profile names.
+  const base::string16 profile_name2 = cache.ChooseNameForNewProfile(1);
+  Profile* profile2 = AddProfileToCache(profile_manager,
+                                        "path_2", profile_name2);
+  EXPECT_EQ(custom_profile_name,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+  EXPECT_EQ(profile_name2,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile2));
+
+  // Deleting a profile means returning to the original, custom name.
+  profile_manager->ScheduleProfileForDeletion(profile2->GetPath(),
+                                              ProfileManager::CreateCallback());
+  // Spin the message loop so that all the callbacks can finish running.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(custom_profile_name,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+}
+
+TEST_F(ProfileManagerTest, ProfileDisplayNamePreservesSignedInName) {
+    // The command line is reset at the end of every test by the test suite.
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kNewProfileManagement);
+
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
+  EXPECT_EQ(0u, cache.GetNumberOfProfiles());
+
+  // Only one local profile means we display IDS_SINGLE_PROFILE_DISPLAY_NAME.
+  const base::string16 default_profile_name =
+      l10n_util::GetStringUTF16(IDS_SINGLE_PROFILE_DISPLAY_NAME);
+  const base::string16 profile_name1 = cache.ChooseNameForNewProfile(0);
+  Profile* profile1 = AddProfileToCache(profile_manager,
+                                        "path_1", profile_name1);
+  EXPECT_EQ(default_profile_name,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+
+  // We should display the actual profile name for signed in profiles.
+  cache.SetUserNameOfProfileAtIndex(0, ASCIIToUTF16("user@gmail.com"));

[rpetterson, 2014/02/20 21:41:32]

Does username take precedence over name?

[noms (inactive), 2014/02/24 22:35:06]

Username just means that the user is signed in, and therefore we are probably
using the user's gaia name, if it exists (in cache.GetNameOfProfile). It's
essentially the same as having a "custom" name, and they're both treated in the
same way (whether the name is custom, or whether the profile is signed in, we do
the same thing, which is display the actual profile name)

On 2014/02/20 21:41:32, rpetterson wrote:

+  EXPECT_EQ(profile_name1, cache.GetNameOfProfileAtIndex(0));
+  EXPECT_EQ(profile_name1,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+
+  // Multiple profiles means displaying the actual profile names.
+  const base::string16 profile_name2 = cache.ChooseNameForNewProfile(1);
+  Profile* profile2 = AddProfileToCache(profile_manager,
+                                        "path_2", profile_name2);
+  EXPECT_EQ(profile_name1,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+  EXPECT_EQ(profile_name2,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile2));
+
+  // Deleting a profile means returning to the original, actual profile name.
+  profile_manager->ScheduleProfileForDeletion(profile2->GetPath(),
+                                              ProfileManager::CreateCallback());
+  // Spin the message loop so that all the callbacks can finish running.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(profile_name1,
+            profiles::GetAvatarButtonDisplayNameForProfile(profile1));
+}