[Mac] AppController needs to update its "last profile" pointer when the active profile is deleted

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/message_loop.h"
@@ skipping 102 matching lines @@
   CommandLine* cl = CommandLine::ForCurrentProcess();
   cl->AppendSwitch(switches::kTestType);
 #endif
   }
 
   virtual void TearDown() {
     TestingBrowserProcess::GetGlobal()->SetProfileManager(NULL);
     message_loop_.RunUntilIdle();
   }
 
+  // Helper function to create a profile with |name| for a profile |manager|.
+  void CreateProfileAsync(ProfileManager* manager,
+                          const std::string& name,
+                          MockObserver* mock_observer) {
+    manager->CreateProfileAsync(
+        temp_dir_.path().AppendASCII(name),
+        base::Bind(&MockObserver::OnProfileCreated,
+                   base::Unretained(mock_observer)),
+        UTF8ToUTF16(name),

[Alexei Svitkine (slow), 2013/06/22 14:14:09]

Nice, I see you're now always setting the profile avatar name!

Maybe this will fix the flakyness of the other tests. After this CL lands, it
may be worthwhile to try to re-enable the other tests (in a separate CL), to see
if they're no longer flaky.

[noms, 2013/06/26 15:24:45]

Yes and no. We would still have to set the avatar url, I think, as the username
is ignored if that isn't set, but I didn't want to experiment with that on this
CL. It's on my to-do list. :)

On 2013/06/22 14:14:09, Alexei Svitkine wrote:

+        string16(),
+        false);
+  }
+
 #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_;
   scoped_refptr<extensions::EventRouterForwarder>
       extension_event_router_forwarder_;
@@ skipping 100 matching lines @@
 }
 
 MATCHER(NotFail, "Profile creation failure status is not reported.") {
   return arg == Profile::CREATE_STATUS_CREATED ||
          arg == Profile::CREATE_STATUS_INITIALIZED;
 }
 
 // Tests asynchronous profile creation mechanism.
 // Crashes: http://crbug.com/89421
 TEST_F(ProfileManagerTest, DISABLED_CreateProfileAsync) {
-  base::FilePath dest_path =
-      temp_dir_.path().Append(FILE_PATH_LITERAL("New Profile"));
-
   MockObserver mock_observer;
   EXPECT_CALL(mock_observer, OnProfileCreated(
       testing::NotNull(), NotFail())).Times(testing::AtLeast(1));
 
-  g_browser_process->profile_manager()->CreateProfileAsync(dest_path,
-      base::Bind(&MockObserver::OnProfileCreated,
-                 base::Unretained(&mock_observer)),
-      string16(), string16(), false);
+  CreateProfileAsync(g_browser_process->profile_manager(),
+                     "New Profile", &mock_observer);
 
   message_loop_.RunUntilIdle();
 }
 
 MATCHER(SameNotNull, "The same non-NULL value for all calls.") {
   if (!g_created_profile)
     g_created_profile = arg;
   return arg != NULL && arg == g_created_profile;
 }
 
 TEST_F(ProfileManagerTest, CreateProfileAsyncMultipleRequests) {
-  base::FilePath dest_path =
-      temp_dir_.path().Append(FILE_PATH_LITERAL("New Profile"));
-
   g_created_profile = NULL;
 
   MockObserver mock_observer1;
   EXPECT_CALL(mock_observer1, OnProfileCreated(
       SameNotNull(), NotFail())).Times(testing::AtLeast(1));
   MockObserver mock_observer2;
   EXPECT_CALL(mock_observer2, OnProfileCreated(
       SameNotNull(), NotFail())).Times(testing::AtLeast(1));
   MockObserver mock_observer3;
   EXPECT_CALL(mock_observer3, OnProfileCreated(
       SameNotNull(), NotFail())).Times(testing::AtLeast(1));
 
   ProfileManager* profile_manager = g_browser_process->profile_manager();
-
-  profile_manager->CreateProfileAsync(dest_path,
-      base::Bind(&MockObserver::OnProfileCreated,
-                 base::Unretained(&mock_observer1)),
-                 string16(), string16(), false);
-  profile_manager->CreateProfileAsync(dest_path,
-      base::Bind(&MockObserver::OnProfileCreated,
-                 base::Unretained(&mock_observer2)),
-                 string16(), string16(), false);
-  profile_manager->CreateProfileAsync(dest_path,
-      base::Bind(&MockObserver::OnProfileCreated,
-                 base::Unretained(&mock_observer3)),
-                 string16(), string16(), false);
+  const std::string profile_name = "New Profile";
+  CreateProfileAsync(profile_manager, profile_name, &mock_observer1);
+  CreateProfileAsync(profile_manager, profile_name, &mock_observer2);
+  CreateProfileAsync(profile_manager, profile_name, &mock_observer3);
 
   message_loop_.RunUntilIdle();
 }
 
 TEST_F(ProfileManagerTest, CreateProfilesAsync) {
-  base::FilePath dest_path1 =
-      temp_dir_.path().Append(FILE_PATH_LITERAL("New Profile 1"));
-  base::FilePath dest_path2 =
-      temp_dir_.path().Append(FILE_PATH_LITERAL("New Profile 2"));
+  const std::string profile_name1 = "New Profile 1";
+  const std::string profile_name2 = "New Profile 2";
 
   MockObserver mock_observer;
   EXPECT_CALL(mock_observer, OnProfileCreated(
       testing::NotNull(), NotFail())).Times(testing::AtLeast(3));
 
   ProfileManager* profile_manager = g_browser_process->profile_manager();
 
-  profile_manager->CreateProfileAsync(dest_path1,
-      base::Bind(&MockObserver::OnProfileCreated,
-                 base::Unretained(&mock_observer)),
-                 string16(), string16(), false);
-  profile_manager->CreateProfileAsync(dest_path2,
-      base::Bind(&MockObserver::OnProfileCreated,
-                 base::Unretained(&mock_observer)),
-                 string16(), string16(), false);
+  CreateProfileAsync(profile_manager, profile_name1, &mock_observer);
+  CreateProfileAsync(profile_manager, profile_name2, &mock_observer);
 
   message_loop_.RunUntilIdle();
 }
 
 TEST_F(ProfileManagerTest, AutoloadProfilesWithBackgroundApps) {
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
   local_state_.Get()->SetUserPref(prefs::kBackgroundModeEnabled,
                                   Value::CreateBooleanValue(true));
 
@@ skipping 115 matching lines @@
   EXPECT_FALSE(
       profile_manager->GetLastUsedProfileAllowedByPolicy()->IsOffTheRecord());
 
   // GetLastUsedProfileAllowedByPolicy() returns the incognito Profile when
   // incognito mode is forced.
   IncognitoModePrefs::SetAvailability(prefs, IncognitoModePrefs::FORCED);
   EXPECT_TRUE(
       profile_manager->GetLastUsedProfileAllowedByPolicy()->IsOffTheRecord());
 }
 
+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);
+
+  MockObserver mock_observer;
+  EXPECT_CALL(mock_observer, OnProfileCreated(
+      testing::NotNull(), NotFail())).Times(testing::AtLeast(3));
+
+  CreateProfileAsync(profile_manager, profile_name1, &mock_observer);
+  CreateProfileAsync(profile_manager, profile_name2, &mock_observer);
+  message_loop_.RunUntilIdle();
+
+  EXPECT_EQ(2u, profile_manager->GetLoadedProfiles().size());
+  EXPECT_EQ(2u, profile_manager->GetProfileInfoCache().GetNumberOfProfiles());
+
+  // Set the active profile.
+  PrefService* local_state = g_browser_process->local_state();
+  local_state->SetString(prefs::kProfileLastUsed, profile_name1);
+
+  // Delete the active profile.
+  profile_manager->ScheduleProfileForDeletion(dest_path1,
+                                              ProfileManager::CreateCallback());
+  // Spin the message loop so that all the callbacks can finish running.
+  message_loop_.RunUntilIdle();
+
+  EXPECT_EQ(dest_path2, profile_manager->GetLastUsedProfile()->GetPath());
+  EXPECT_EQ(profile_name2, local_state->GetString(prefs::kProfileLastUsed));
+}
+
+#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);
+
+  MockObserver mock_observer;
+  EXPECT_CALL(mock_observer, OnProfileCreated(
+      testing::NotNull(), NotFail())).Times(testing::AtLeast(2));
+  CreateProfileAsync(profile_manager, profile_name1, &mock_observer);
+  message_loop_.RunUntilIdle();
+
+  // Track the profile, but don't load it.
+  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
+  cache.AddProfileToCache(dest_path2, ASCIIToUTF16(profile_name2),
+                          string16(), 0, false);
+  message_loop_.RunUntilIdle();
+
+  EXPECT_EQ(1u, profile_manager->GetLoadedProfiles().size());
+  EXPECT_EQ(2u, cache.GetNumberOfProfiles());
+
+  // Set the active profile.
+  PrefService* local_state = g_browser_process->local_state();
+  local_state->SetString(prefs::kProfileLastUsed,
+                         dest_path1.BaseName().MaybeAsASCII());
+
+  // Delete the active profile. This should switch and load the unloaded
+  // profile.
+  profile_manager->ScheduleProfileForDeletion(dest_path1,
+                                              ProfileManager::CreateCallback());
+
+  // Spin the message loop so that all the callbacks can finish running.
+  message_loop_.RunUntilIdle();
+
+  EXPECT_EQ(dest_path2, profile_manager->GetLastUsedProfile()->GetPath());
+  EXPECT_EQ(profile_name2, local_state->GetString(prefs::kProfileLastUsed));
+}
+
+// This tests the recursive call in ProfileManager::OnNewActiveProfileLoaded
+// by simulating a scenario in which the profile that is being loaded as
+// 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);
+
+  MockObserver mock_observer;
+  EXPECT_CALL(mock_observer, OnProfileCreated(
+      testing::NotNull(), NotFail())).Times(testing::AtLeast(2));
+  CreateProfileAsync(profile_manager, profile_name1, &mock_observer);
+  message_loop_.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.
+  ProfileInfoCache& cache = profile_manager->GetProfileInfoCache();
+  cache.AddProfileToCache(dest_path2, ASCIIToUTF16(profile_name2),
+                          ASCIIToUTF16(profile_name2), 1, false);
+  cache.AddProfileToCache(dest_path3, ASCIIToUTF16(profile_name3),
+                          ASCIIToUTF16(profile_name3), 2, false);
+
+  message_loop_.RunUntilIdle();
+
+  EXPECT_EQ(1u, profile_manager->GetLoadedProfiles().size());
+  EXPECT_EQ(3u, cache.GetNumberOfProfiles());
+
+  // Set the active profile.
+  PrefService* local_state = g_browser_process->local_state();
+  local_state->SetString(prefs::kProfileLastUsed,
+                         dest_path1.BaseName().MaybeAsASCII());
+
+  // Delete the active profile, Profile1.
+  // This will post a CreateProfileAsync message, that tries to load Profile2,
+  // which checks that the profile is not being deleted, and then calls back
+  // FinishDeletingProfile for Profile1.
+  // Try to break this flow by setting the active profile to Profile2 in the
+  // middle (so after the first posted message), and trying to delete Profile2,
+  // so that the ProfileManager has to look for a different profile to load.
+  profile_manager->ScheduleProfileForDeletion(dest_path1,
+                                              ProfileManager::CreateCallback());
+  local_state->SetString(prefs::kProfileLastUsed,
+                         dest_path2.BaseName().MaybeAsASCII());
+  profile_manager->ScheduleProfileForDeletion(dest_path2,
+                                              ProfileManager::CreateCallback());
+  // Spin the message loop so that all the callbacks can finish running.
+  message_loop_.RunUntilIdle();
+
+  EXPECT_EQ(dest_path3, profile_manager->GetLastUsedProfile()->GetPath());
+  EXPECT_EQ(profile_name3, local_state->GetString(prefs::kProfileLastUsed));
+}
+#endif  // !defined(OS_MACOSX)
+
 #if !defined(OS_ANDROID)
 // There's no Browser object on Android.
 TEST_F(ProfileManagerTest, LastOpenedProfiles) {
   base::FilePath dest_path1 = temp_dir_.path();
   dest_path1 = dest_path1.Append(FILE_PATH_LITERAL("New Profile 1"));
 
   base::FilePath dest_path2 = temp_dir_.path();
   dest_path2 = dest_path2.Append(FILE_PATH_LITERAL("New Profile 2"));
 
   ProfileManager* profile_manager = g_browser_process->profile_manager();
@@ skipping 170 matching lines @@
   browser2b.reset();
   last_opened_profiles = profile_manager->GetLastOpenedProfiles();
   ASSERT_EQ(1U, last_opened_profiles.size());
   EXPECT_EQ(profile1, last_opened_profiles[0]);
 
   browser1.reset();
   last_opened_profiles = profile_manager->GetLastOpenedProfiles();
   ASSERT_EQ(0U, last_opened_profiles.size());
 }
 #endif  // !defined(OS_ANDROID)