Added safeguards for the contents of the kProfilesLastActive list when

handling ephemeral profiles

As described in https://crbug.com/1064758 the kProfilesLastActive pref
list is not explicitly cleared when ephemeral profiles are handled.
In some cases this can cause profiles to be created at browser start
when they should not be. This change adds safeguards that mitigate
against this issue.

Bug: 1064758
Change-Id: Ib16f85b2c511a27a5fe618c10bfea1704b8e567f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2122917
Commit-Queue: Constantin Manea <comanea@microsoft.com>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758872}

chrome/browser/profiles/profile_manager.cc

@@ -350,6 +350,17 @@ bool IsProfileEphemeral(ProfileAttributesStorage* storage,
 }
 #endif
 
+// Helper function that deletes entries from the kProfilesLastActive pref list.
+// It is called when every ephemeral profile is handled.
+void RemoveFromLastActiveProfilesPrefList(base::FilePath path) {
+  PrefService* local_state = g_browser_process->local_state();
+  DCHECK(local_state);
+  ListPrefUpdate update(local_state, prefs::kProfilesLastActive);
+  base::ListValue* profile_list = update.Get();
+  base::Value entry_value = base::Value(path.BaseName().MaybeAsASCII());
+  profile_list->EraseListValue(entry_value);
+}
+
 #if defined(OS_CHROMEOS)
 bool IsLoggedIn() {
   return user_manager::UserManager::IsInitialized() &&
@@ -896,6 +907,7 @@ void ProfileManager::CleanUpEphemeralProfiles() {
     base::FilePath profile_path = entry->GetPath();
     if (entry->IsEphemeral()) {
       profiles_to_delete.push_back(profile_path);
+      RemoveFromLastActiveProfilesPrefList(profile_path);
       if (profile_path.BaseName().MaybeAsASCII() == last_used_profile)
         last_active_profile_deleted = true;
     } else if (new_profile_path.empty()) {
@@ -1923,6 +1935,8 @@ void ProfileManager::ScheduleForcedEphemeralProfileForDeletion(
     }
   }
 
+  RemoveFromLastActiveProfilesPrefList(profile_dir);
+
   const base::FilePath new_active_profile_dir =
       found_entry ? found_entry->GetPath() : GenerateNextProfileDirectoryPath();
   FinishDeletingProfile(profile_dir, new_active_profile_dir);

chrome/browser/profiles/profile_manager_unittest.cc

@@ -1214,15 +1214,29 @@ TEST_F(ProfileManagerTest, CleanUpEphemeralProfiles) {
   PrefService* local_state = g_browser_process->local_state();
   local_state->SetString(prefs::kProfileLastUsed, profile_name1);
 
+  // Set the last used profiles.
+  ListPrefUpdate update(local_state, prefs::kProfilesLastActive);
+  base::ListValue* initial_last_active_profile_list = update.Get();
+  initial_last_active_profile_list->Append(
+      std::make_unique<base::Value>(path1.BaseName().MaybeAsASCII()));
+  initial_last_active_profile_list->Append(
+      std::make_unique<base::Value>(path2.BaseName().MaybeAsASCII()));
+
   profile_manager->CleanUpEphemeralProfiles();
   content::RunAllTasksUntilIdle();
+  const base::ListValue* final_last_active_profile_list =
+      local_state->GetList(prefs::kProfilesLastActive);
 
   // The ephemeral profile should be deleted, and the last used profile set to
-  // the other one.
+  // the other one. Also, the ephemeral profile should be removed from the
+  // kProfilesLastActive list.
   EXPECT_FALSE(base::DirectoryExists(path1));
   EXPECT_TRUE(base::DirectoryExists(path2));
   EXPECT_EQ(profile_name2, local_state->GetString(prefs::kProfileLastUsed));
   ASSERT_EQ(1u, storage.GetNumberOfProfiles());
+  ASSERT_EQ(1u, final_last_active_profile_list->GetSize());
+  ASSERT_EQ(path2.BaseName().MaybeAsASCII(),
+            (final_last_active_profile_list->GetList())[0].GetString());
 
   // Mark the remaining profile ephemeral and clean up.
   storage.GetAllProfilesAttributes()[0]->SetIsEphemeral(true);
@@ -1233,6 +1247,7 @@ TEST_F(ProfileManagerTest, CleanUpEphemeralProfiles) {
   EXPECT_FALSE(base::DirectoryExists(path2));
   EXPECT_EQ(0u, storage.GetNumberOfProfiles());
   EXPECT_EQ("Profile 1", local_state->GetString(prefs::kProfileLastUsed));
+  ASSERT_EQ(0u, final_last_active_profile_list->GetSize());
 }
 
 TEST_F(ProfileManagerTest, CleanUpEphemeralProfilesWithGuestLastUsedProfile) {