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

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 179 matching lines @@
 
 // Physically remove deleted profile directories from disk.
 void NukeProfileFromDisk(const base::FilePath& profile_path) {
   // Delete both the profile directory and its corresponding cache.
   base::FilePath cache_path;
   chrome::GetUserCacheDirectory(profile_path, &cache_path);
   base::DeleteFile(profile_path, true);
   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) {
   if (call_status != chromeos::DBUS_METHOD_CALL_SUCCESS) {
     LOG(ERROR) << "IsMounted call failed.";
     return;
   }
   if (!is_mounted)
     LOG(ERROR) << "Cryptohome is not mounted.";
 }
@@ skipping 286 matching lines @@
     base::FilePath profile_path(user_data_dir);
     Profile* profile = GetProfile(profile_path.Append(profile_dir));
     return profile->IsGuestSession() ? profile->GetOffTheRecordProfile() :
                                        profile;
   }
 #endif
 
   return GetProfile(GetLastUsedProfileDir(user_data_dir));
 }
 
 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(
     const base::FilePath& user_data_dir) {
   PrefService* local_state = g_browser_process->local_state();
   DCHECK(local_state);
 
   std::vector<Profile*> to_return;
   if (local_state->HasPrefPath(prefs::kProfilesLastActive) &&
       local_state->GetList(prefs::kProfilesLastActive)) {
@@ skipping 167 matching lines @@
 
     ProfileMetrics::LogProfileAddNewUser(
         ProfileMetrics::ADD_NEW_USER_LAST_DELETED);
     return;
   }
 
 #if defined(OS_MACOSX)
   // 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),
                                   profile_dir,
                                   last_non_supervised_profile_path,
                                   callback),
                        base::string16(),
                        base::string16(),
                        std::string());
     return;
   }
 #endif  // defined(OS_MACOSX)
 
   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.
   PrefService* local_state = g_browser_process->local_state();
   DCHECK(local_state);
   if (!local_state->HasPrefPath(prefs::kBackgroundModeEnabled) ||
       !local_state->GetBoolean(prefs::kBackgroundModeEnabled)) {
     return;
   }
 
   ProfileInfoCache& cache = GetProfileInfoCache();
   size_t number_of_profiles = cache.GetNumberOfProfiles();
   for (size_t p = 0; p < number_of_profiles; ++p) {
     if (cache.GetBackgroundStatusOfProfileAtIndex(p)) {
       // If status is true, that profile is running background apps. By calling
       // GetProfile, we automatically cause the profile to be loaded which will
       // register it with the BackgroundModeManager.
       GetProfile(cache.GetPathOfProfileAtIndex(p));
     }
   }
 }
 
+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();
 
   if (profile->GetPath().DirName() != cache.GetUserDataDir())
     return;
 
   size_t avatar_index;
   std::string profile_name;
   std::string supervised_user_id;
@@ skipping 560 matching lines @@
 }
 
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
 void ProfileManager::UpdateLastUser(Profile* last_active) {
   PrefService* local_state = g_browser_process->local_state();
   DCHECK(local_state);
   // Only keep track of profiles that we are managing; tests may create others.
   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();
     size_t profile_index =
         cache.GetIndexOfProfileWithPath(last_active->GetPath());
     if (profile_index != std::string::npos)
       cache.SetProfileActiveTimeAtIndex(profile_index);
   }
 }
 
@@ skipping 71 matching lines @@
   }
 
   FinishDeletingProfile(profile_to_delete_path, new_active_profile_path);
   if (!original_callback.is_null())
     original_callback.Run(loaded_profile, status);
 }
 
 ProfileManagerWithoutInit::ProfileManagerWithoutInit(
     const base::FilePath& user_data_dir) : ProfileManager(user_data_dir) {
 }