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

chrome/browser/profiles/profile_manager.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 "chrome/browser/profiles/profile_manager.h"
 
 #include <set>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 1052 matching lines @@
 
 // TODO(robertshield): ProfileManager should not be opening windows and should
 // not have to care about HostDesktopType. See http://crbug.com/153864
 void ProfileManager::ScheduleProfileForDeletion(
     const base::FilePath& profile_dir,
     chrome::HostDesktopType desktop_type) {
   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.
+    std::string last_non_managed_profile =

[Alexei Svitkine (slow), 2013/06/05 20:00:52]

Nit: Make it const

[noms (inactive), 2013/06/11 19:32:17]

Done.

+        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
       // correct last used profile is set for any notification observers.
       local_state->SetString(prefs::kProfileLastUsed,
                              new_path.BaseName().MaybeAsASCII());
       // TODO(robertshield): This desktop type needs to come from the invoker,
       // currently that involves plumbing this through web UI.
       chrome::HostDesktopType desktop_type = chrome::HOST_DESKTOP_TYPE_NATIVE;
       CreateProfileAsync(new_path,
                          base::Bind(&OnOpenWindowForNewProfile,
                                     desktop_type,
                                     CreateCallback()),
                          string16(),
                          string16(),
                          false);
     } else {
+      // On the Mac, the browser process is not killed when all browser windows
+      // 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,
+                                    desktop_type),
+                         string16(),
+                         string16(),
+                         false);
+      return;
+#else
+      // For OS_MACOSX we will update the local_state in the callback

[Alexei Svitkine (slow), 2013/06/05 20:00:52]

Nit: "update the local_state" -> "update the pref"

Also, don't use "we" in the comment.

[noms (inactive), 2013/06/11 19:32:17]

Done.

+      // 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_dir,

[Alexei Svitkine (slow), 2013/06/05 20:00:52]

This param should be on the next line, aligned with the others. Also, rename it
to |profile_to_delete_dir|, for clarity.

[noms (inactive), 2013/06/11 19:32:17]

Done.

+    const std::string last_non_managed_profile,

[Alexei Svitkine (slow), 2013/06/05 20:00:52]

Should be a "const std::string&".

[noms (inactive), 2013/06/11 19:32:17]

Done.

+    chrome::HostDesktopType desktop_type,
+    Profile* profile,

[Alexei Svitkine (slow), 2013/06/05 20:00:52]

Rename to |loaded_profile|.

[noms (inactive), 2013/06/11 19:32:17]

Done.

+    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_dir);
+  } else if (status == Profile::CREATE_STATUS_FAIL) {

[Alexei Svitkine (slow), 2013/06/05 20:00:52]

Hmm, I see CREATE_STATUS_LOCAL_FAIL and CREATE_STATUS_REMOTE_FAIL. Has the code
changed under you?

[noms (inactive), 2013/06/11 19:32:17]

And how!

On 2013/06/05 20:00:52, Alexei Svitkine wrote:

+    // If we are here, it means that the profile we tried to load as the

[Alexei Svitkine (slow), 2013/06/05 20:00:52]

Nit: No "we"'s

[noms (inactive), 2013/06/11 19:32:17]

Done.

+    // next active profile has been deleted. We should retry to delete
+    // this profile to redo the logic and load the next available profile.
+    ScheduleProfileForDeletion(profile_dir, desktop_type);
+  }
+}
+
+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);
 
     // Disable sync for doomed profile.
     if (ProfileSyncServiceFactory::GetInstance()->HasProfileSyncService(
         profile)) {
       ProfileSyncServiceFactory::GetInstance()->GetForProfile(
@@ skipping 67 matching lines @@
 ProfileManager::ProfileInfo::ProfileInfo(
     Profile* profile,
     bool created)
     : profile(profile),
       created(created) {
 }
 
 ProfileManager::ProfileInfo::~ProfileInfo() {
   ProfileDestroyer::DestroyProfileWhenAppropriate(profile.release());
 }