[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();
   }
 
+  virtual void CreateProfileAsync(ProfileManager* manager,

[Alexei Svitkine (slow), 2013/06/19 14:58:31]

Why is this virtual? Also, it needs a comment.

[noms (inactive), 2013/06/21 23:11:47]

Done.

+                                  const std::string& name,
+                                  MockObserver* mock_observer) {
+    manager->CreateProfileAsync(
+        temp_dir_.path().Append(FILE_PATH_LITERAL(name)),
+        base::Bind(&MockObserver::OnProfileCreated,
+                   base::Unretained(mock_observer)),
+        UTF8ToUTF16(name),
+        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().Append(FILE_PATH_LITERAL(profile_name1));
+  base::FilePath dest_path2 =
+      temp_dir_.path().Append(FILE_PATH_LITERAL(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));
+}
+
+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().Append(FILE_PATH_LITERAL(profile_name1));
+  base::FilePath dest_path2 =
+      temp_dir_.path().Append(FILE_PATH_LITERAL(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.

[Alexei Svitkine (slow), 2013/06/19 14:58:31]

How does this work on platforms other than Mac? The recursion doesn't happen
there, so is the test still valid in that case? Same question for the other
tests.

[noms (inactive), 2013/06/21 23:11:47]

Tests shouldn't work, so I've ifdef'ed them.
On 2013/06/19 14:58:31, Alexei Svitkine wrote:

+TEST_F(ProfileManagerTest, ActiveProfileDeletedNextProfileDeletedToo) {
+

[Alexei Svitkine (slow), 2013/06/19 14:58:31]

Nit: Remove empty line.

[noms (inactive), 2013/06/21 23:11:47]

Done.

+  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().Append(FILE_PATH_LITERAL(profile_name1));
+  base::FilePath dest_path2 =
+      temp_dir_.path().Append(FILE_PATH_LITERAL(profile_name2));
+  base::FilePath dest_path3 =
+      temp_dir_.path().Append(FILE_PATH_LITERAL(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));
+}
+
 #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)