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

chrome/browser/profiles/profile_manager.cc

Index: chrome/browser/profiles/profile_manager.cc
diff --git a/chrome/browser/profiles/profile_manager.cc b/chrome/browser/profiles/profile_manager.cc
index 29fb9884d9eb0e8e8084e4f3d0aae9e0ad5acb66..8953536228d81d3eccb0c1d0b522e10d9281f1ae 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -166,12 +166,10 @@ void QueueProfileDirectoryForDeletion(const base::FilePath& path) {
 void CheckCryptohomeIsMounted(chromeos::DBusMethodCallStatus call_status,
                               bool is_mounted) {
   if (call_status != chromeos::DBUS_METHOD_CALL_SUCCESS) {
-    LOG(ERROR) << "IsMounted call failed.";
-    return;
+      return;

[Alexei Svitkine (slow), 2013/06/12 21:02:31]

Nit: Indention looks wrong.

   }
   if (!is_mounted)

[Alexei Svitkine (slow), 2013/06/12 21:02:31]

What's this dangling if?

-    LOG(ERROR) << "Cryptohome is not mounted.";
-}
+  }
 
 #endif
 
@@ -399,8 +397,7 @@ std::vector<Profile*> ProfileManager::GetLastOpenedProfiles(
       std::string profile;
       for (it = profile_list->begin(); it != profile_list->end(); ++it) {
         if (!(*it)->GetAsString(&profile) || profile.empty()) {
-          LOG(WARNING) << "Invalid entry in " << prefs::kProfilesLastActive;
-          continue;
+              continue;

[Alexei Svitkine (slow), 2013/06/12 21:02:31]

Nit: Indention looks wrong and inner loop shouldn't have {}'s if its a single
line.

By the way, did you actually mean to remove these LOG statements?

[noms (inactive), 2013/06/13 14:23:55]

This is a rebase gone wrong. Same for the two above. :)  
On 2013/06/12 21:02:31, Alexei Svitkine wrote:

         }
         to_return.push_back(GetProfile(user_data_dir.AppendASCII(profile)));
       }
@@ -1043,23 +1040,26 @@ void ProfileManager::ScheduleProfileForDeletion(
     const base::FilePath& profile_dir,
     const CreateCallback& callback) {
   DCHECK(IsMultipleProfilesEnabled());
-
   PrefService* local_state = g_browser_process->local_state();
   ProfileInfoCache& cache = GetProfileInfoCache();
+
   if (profile_dir.BaseName().MaybeAsASCII() ==
       local_state->GetString(prefs::kProfileLastUsed)) {
     // Update the last used profile pref before closing browser windows. This
     // way the correct last used profile is set for any notification observers.
-    std::string last_non_managed_profile;
+    base::FilePath last_non_managed_profile_path;
     for (size_t i = 0; i < cache.GetNumberOfProfiles(); ++i) {
       base::FilePath cur_path = cache.GetPathOfProfileAtIndex(i);
       if (cur_path != profile_dir && !cache.ProfileIsManagedAtIndex(i)) {
-        last_non_managed_profile = cur_path.BaseName().MaybeAsASCII();
+        last_non_managed_profile_path = cur_path;
         break;
       }
     }
+
     // If we're deleting the last (non-managed) profile, then create a new
     // profile in its place.
+    const std::string last_non_managed_profile =
+        last_non_managed_profile_path.BaseName().MaybeAsASCII();
     if (last_non_managed_profile.empty()) {
       base::FilePath new_path = GenerateNextProfileDirectoryPath();
       // Make sure the last used profile path is pointing at it. This way the
@@ -1072,15 +1072,58 @@ void ProfileManager::ScheduleProfileForDeletion(
                          string16(),
                          false);
     } else {
+      // On the Mac, the browser process is not killed when all browser windows

[sail, 2013/06/12 21:16:49]

Could we just implement this on all platforms?

[noms (inactive), 2013/06/13 14:23:55]

We could, but it would be a wasted operation -- on the other platforms, deleting
the active profile (with no other profiles opened) just kills the browser
process, so there's not much point waiting around to load a profile that won't
be used. Do you think we should do it anyway?

On 2013/06/12 21:16:49, sail wrote:

[sail, 2013/06/13 14:33:50]

Up to you.

There might be weird edge cases. For example, if you have a background extension
running and the browser process is not killed.

[Alexei Svitkine (slow), 2013/06/13 14:48:00]

Wouldn't any extension that's running still be running under a profile? So
presumably if its profile is deleted, it should also be stopped. (Or if it
isn't, that would be a separate bug.)

+      // are closed, so just in case we are deleting the active profile, and no
+      // other profile has been loaded, we must pre-load a next one.
+#if defined(OS_MACOSX)
+      CreateProfileAsync(last_non_managed_profile_path,
+                         base::Bind(&ProfileManager::OnNewActiveProfileLoaded,
+                                    base::Unretained(this),
+                                    profile_dir,
+                                    last_non_managed_profile,
+                                    callback),
+                         string16(),
+                         string16(),
+                         false);
+      return;
+#else
+      // For OS_MACOSX the pref is updated in the callback
+      // to make sure that it isn't used before the profile is actually loaded.
       local_state->SetString(prefs::kProfileLastUsed, last_non_managed_profile);
+#endif
     }
   }
+  FinishDeletingProfile(profile_dir);
+}
+
+void ProfileManager::OnNewActiveProfileLoaded(
+    const base::FilePath& profile_to_delete_dir,
+    const std::string& last_non_managed_profile,
+    const CreateCallback& original_callback,
+    Profile* loaded_profile,
+    Profile::CreateStatus status) {
+  if (status == Profile::CREATE_STATUS_INITIALIZED) {
+    // Update the local state as promised in the ScheduleProfileForDeletion.
+    PrefService* local_state = g_browser_process->local_state();
+    local_state->SetString(prefs::kProfileLastUsed, last_non_managed_profile);
+    FinishDeletingProfile(profile_to_delete_dir);
+  } else if (status == Profile::CREATE_STATUS_LOCAL_FAIL ||
+             status == Profile::CREATE_STATUS_REMOTE_FAIL) {
+    // This means that the profile we tried to load as the next active profile
+    // has been deleted. Retry deleting this profile to redo the logic
+    // and load the next available profile.
+    ScheduleProfileForDeletion(profile_to_delete_dir, original_callback);
+  }
+}
 
+void ProfileManager::FinishDeletingProfile(const base::FilePath& profile_dir) {
+  ProfileInfoCache& cache = GetProfileInfoCache();
   // TODO(sail): Due to bug 88586 we don't delete the profile instance. Once we
   // start deleting the profile instance we need to close background apps too.
   Profile* profile = GetProfileByPath(profile_dir);
+
   if (profile) {
-    BrowserList::CloseAllBrowsersWithProfile(profile);
+      BrowserList::CloseAllBrowsersWithProfile(profile);

[Alexei Svitkine (slow), 2013/06/12 21:02:31]

Nit:Indention is wrong.

[noms (inactive), 2013/06/13 20:56:25]

Done.

 
     // Disable sync for doomed profile.
     if (ProfileSyncServiceFactory::GetInstance()->HasProfileSyncService(