Don't create a new profile when cleaning up stale ephemeral profiles.

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 5e02bee7af2c79596de38be0bce4e63ec5fa946c..20c269af4c644e0e7c575b419f12f7e24b010f01 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -197,6 +197,16 @@ void NukeProfileFromDisk(const base::FilePath& profile_path) {
   base::DeleteFile(cache_path, true);
 }
 
+std::string GetLastUsedProfileName() {
+  PrefService* local_state = g_browser_process->local_state();

[Mike Lerman, 2015/05/11 15:06:22]

nit: can we DCHECK(local_state)?

[Bernhard Bauer, 2015/05/11 16:52:08]

Done.

+  std::string last_used_profile_name =

[Mike Lerman, 2015/05/11 15:06:22]

tiny: can this be a const std::string&?

[Bernhard Bauer, 2015/05/11 16:52:08]

Done.

+      local_state->GetString(prefs::kProfileLastUsed);
+  if (!last_used_profile_name.empty())
+    return last_used_profile_name;
+
+  return chrome::kInitialProfile;
+}
+
 #if defined(OS_CHROMEOS)
 void CheckCryptohomeIsMounted(chromeos::DBusMethodCallStatus call_status,
                               bool is_mounted) {
@@ -505,16 +515,7 @@ Profile* ProfileManager::GetLastUsedProfile(
 
 base::FilePath ProfileManager::GetLastUsedProfileDir(

[Mike Lerman, 2015/05/11 15:06:22]

AppListServiceImpl.cc::GetProfilePath(), especially around line 295, now does
very similar things to this method. Can we somehow rewrite that function to call
this?

Or, if you don't want to include that change in this CL, can you leave a TODO to
make that change?

[Bernhard Bauer, 2015/05/11 16:52:08]

Sure, done.

FYI, I also came up with some more cleanups in the interface of this class
(getting rid of the need to pass in the user data directory everywhere, and
removing static helper methods that just forward to the corresponding instance
method) at https://codereview.chromium.org/1128173005/, which I will send out
once this one lands.

     const base::FilePath& user_data_dir) {
-  base::FilePath last_used_profile_dir(user_data_dir);
-  PrefService* local_state = g_browser_process->local_state();
-  DCHECK(local_state);
-
-  if (local_state->HasPrefPath(prefs::kProfileLastUsed)) {
-    return last_used_profile_dir.AppendASCII(
-        local_state->GetString(prefs::kProfileLastUsed));
-  }
-
-  return last_used_profile_dir.AppendASCII(chrome::kInitialProfile);
+  return user_data_dir.AppendASCII(GetLastUsedProfileName());
 }
 
 std::vector<Profile*> ProfileManager::GetLastOpenedProfiles(
@@ -702,12 +703,9 @@ void ProfileManager::ScheduleProfileForDeletion(
   // 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.
-  PrefService* local_state = g_browser_process->local_state();
-  DCHECK(local_state);
-  const std::string last_used_profile =
-      local_state->GetString(prefs::kProfileLastUsed);
-  if (last_used_profile == profile_dir.BaseName().MaybeAsASCII() ||
-      last_used_profile == GetGuestProfilePath().BaseName().MaybeAsASCII()) {
+  base::FilePath last_used_profile = GetLastUsedProfileDir(user_data_dir_);

[Mike Lerman, 2015/05/11 15:06:22]

nit: const

[Bernhard Bauer, 2015/05/11 16:52:08]

Done.

+  if (last_used_profile == profile_dir ||
+      last_used_profile == GetGuestProfilePath()) {
     CreateProfileAsync(last_non_supervised_profile_path,
                        base::Bind(&ProfileManager::OnNewActiveProfileLoaded,
                                   base::Unretained(this),
@@ -724,17 +722,6 @@ void ProfileManager::ScheduleProfileForDeletion(
   FinishDeletingProfile(profile_dir, last_non_supervised_profile_path);
 }
 
-// static
-void ProfileManager::CleanUpStaleProfiles(
-    const std::vector<base::FilePath>& profile_paths) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-
-  for (std::vector<base::FilePath>::const_iterator it = profile_paths.begin();
-       it != profile_paths.end(); ++it) {
-    NukeProfileFromDisk(*it);
-  }
-}
-
 void ProfileManager::AutoloadProfiles() {
   // If running in the background is disabled for the browser, do not autoload
   // any profiles.
@@ -757,6 +744,48 @@ void ProfileManager::AutoloadProfiles() {
   }
 }
 
+void ProfileManager::CleanUpEphemeralProfiles() {
+  std::string last_used_profile = GetLastUsedProfileName();
+
+  bool last_active_profile_deleted = false;
+  base::FilePath new_profile_path;
+  std::vector<base::FilePath> profiles_to_delete;
+  ProfileInfoCache& profile_cache = GetProfileInfoCache();
+  size_t profiles_count = profile_cache.GetNumberOfProfiles();
+  for (size_t i = 0; i < profiles_count; ++i) {
+    base::FilePath profile_path = profile_cache.GetPathOfProfileAtIndex(i);
+    bool profile_is_active =
+        profile_path.BaseName().MaybeAsASCII() == last_used_profile;

[Mike Lerman, 2015/05/11 15:06:22]

can you just inline this equality check within the condition where
profile_is_active is referenced? (l761)?

[Bernhard Bauer, 2015/05/11 16:52:08]

Yes, I've been meaning to do that :)

+    if (profile_cache.ProfileIsEphemeralAtIndex(i)) {
+      profiles_to_delete.push_back(profile_path);
+      if (profile_is_active)
+        last_active_profile_deleted = true;
+    } else if (new_profile_path.empty()) {
+      new_profile_path = profile_path;
+    }
+  }
+
+  // If the last active profile was ephemeral, set a new one.
+  if (last_active_profile_deleted) {
+    if (new_profile_path.empty())
+      new_profile_path = GenerateNextProfileDirectoryPath();
+
+    PrefService* local_state = g_browser_process->local_state();
+    local_state->SetString(prefs::kProfileLastUsed,
+                           new_profile_path.BaseName().MaybeAsASCII());
+  }
+
+  // This uses a separate loop, because deleting the profile from the
+  // ProfileInfoCache will modify indices.
+  for (const base::FilePath& profile_path : profiles_to_delete) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE,
+        base::Bind(&NukeProfileFromDisk, profile_path));
+
+    profile_cache.DeleteProfileFromCache(profile_path);
+  }
+}
+
 void ProfileManager::InitProfileUserPrefs(Profile* profile) {
   TRACE_EVENT0("browser", "ProfileManager::InitProfileUserPrefs");
   ProfileInfoCache& cache = GetProfileInfoCache();
@@ -1337,7 +1366,7 @@ void ProfileManager::UpdateLastUser(Profile* last_active) {
   if (profiles_info_.find(last_active->GetPath()) != profiles_info_.end()) {
     std::string profile_path_base =
         last_active->GetPath().BaseName().MaybeAsASCII();
-    if (profile_path_base != local_state->GetString(prefs::kProfileLastUsed))
+    if (profile_path_base != GetLastUsedProfileName())
       local_state->SetString(prefs::kProfileLastUsed, profile_path_base);
 
     ProfileInfoCache& cache = GetProfileInfoCache();