Skip to content

GitLab

T
tangled
Commit b62892aa authored by Xida Chen's avatar Xida Chen Committed by Chromium LUCI CQ
Browse files
Options

Revert "[profiles] Clean up guest ephemeral profile on startup" 

This reverts commit d660a480.

Reason for revert:
Test fails:
https://ci.chromium.org/ui/p/chromium/builders/ci/Win%207%20Tests%20x64%20(1)/76721/overview

Original change's description:
> [profiles] Clean up guest ephemeral profile on startup
>
> This CL fixes a bug which leads to the guest ephemeral profile not being
> cleaned up on startup if it has been left over in the previous session.
>
> ProfileManager::CleanUpEphemeralProfiles() needs to pass the `true`
> parameter in ProfileAttributesStorage::GetAllProfilesAttributes() to get
> the full list of registered profiles.
>
> Bug: 1167883
> Change-Id: I3051a9174e8472eb2af046126be88dbfb44bcb47
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2635158
> Commit-Queue: Alex Ilin <alexilin@chromium.org>
> Reviewed-by: David Roger <droger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#844749}

TBR=droger@chromium.org,alexilin@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: I6fb55866ddcda42c54d60aaa2e823da18b353348
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1167883
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2637395Reviewed-by: default avatarXida Chen <xidachen@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844779}
parent 97095dcd
Hide whitespace changes
Inline Side-by-side
Showing with 12 additions and 181 deletions
chrome/browser/profiles/profile_manager.cc
View file @ b62892aa
......@@ -996,7 +996,7 @@ void ProfileManager::CleanUpEphemeralProfiles() {
std::vector<base::FilePath> profiles_to_delete;
ProfileAttributesStorage& storage = GetProfileAttributesStorage();
std::vector<ProfileAttributesEntry*> entries =
storage.GetAllProfilesAttributes(/*include_guest_profile=*/true);
storage.GetAllProfilesAttributes();
for (ProfileAttributesEntry* entry : entries) {
base::FilePath profile_path = entry->GetPath();
if (entry->IsEphemeral()) {
......
chrome/browser/profiles/profile_manager_browsertest.cc
View file @ b62892aa
......@@ -8,7 +8,6 @@
#include "base/callback.h"
#include "base/callback_helpers.h"
#include "base/command_line.h"
#include "base/files/file_util.h"
#include "base/macros.h"
#include "base/run_loop.h"
#include "base/stl_util.h"
......@@ -54,10 +53,6 @@
namespace {
#if !defined(OS_ANDROID) && !BUILDFLAG(IS_CHROMEOS_ASH)
const char kEphemeralProfilePath[] = "Ephemeral profile";
#endif
const ProfileManager::CreateCallback kOnProfileSwitchDoNothing;
// An observer that returns back to test code after a new profile is
......@@ -69,16 +64,14 @@ void OnUnblockOnProfileCreation(base::RunLoop* run_loop,
run_loop->Quit();
}
void ProfileCreationComplete(base::OnceClosure completion_callback,
Profile* profile,
Profile::CreateStatus status) {
void ProfileCreationComplete(Profile* profile, Profile::CreateStatus status) {
ASSERT_NE(status, Profile::CREATE_STATUS_LOCAL_FAIL);
ASSERT_NE(status, Profile::CREATE_STATUS_REMOTE_FAIL);
// No browser should have been created for this profile yet.
EXPECT_EQ(chrome::GetBrowserCount(profile), 0U);
EXPECT_EQ(chrome::GetTotalBrowserCount(), 1U);
if (status == Profile::CREATE_STATUS_INITIALIZED)
std::move(completion_callback).Run();
base::RunLoop::QuitCurrentWhenIdleDeprecated();
}
// An observer that returns back to test code after one or more profiles was
......@@ -154,12 +147,11 @@ class MultipleProfileDeletionObserver
DISALLOW_COPY_AND_ASSIGN(MultipleProfileDeletionObserver);
};
void EphemeralProfileCreationComplete(base::OnceClosure completion_callback,
Profile* profile,
void EphemeralProfileCreationComplete(Profile* profile,
Profile::CreateStatus status) {
if (status == Profile::CREATE_STATUS_INITIALIZED)
profile->GetPrefs()->SetBoolean(prefs::kForceEphemeralProfiles, true);
ProfileCreationComplete(std::move(completion_callback), profile, status);
ProfileCreationComplete(profile, status);
}
class ProfileRemovalObserver : public ProfileAttributesStorage::Observer {
......@@ -491,13 +483,12 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest,
// Create a profile, make sure callback is invoked before any callbacks are
// invoked (so they can do things like sign in the profile, etc).
base::RunLoop run_loop;
ProfileManager::CreateMultiProfileAsync(
base::string16(), // name
std::string(), // icon url
base::BindRepeating(&ProfileCreationComplete,
run_loop.QuitWhenIdleClosure()));
run_loop.Run();
base::BindRepeating(ProfileCreationComplete));
// Wait for profile to finish loading.
content::RunMessageLoop();
EXPECT_EQ(profile_manager->GetNumberOfProfiles(), 2U);
EXPECT_EQ(chrome::GetTotalBrowserCount(), 2U);
......@@ -630,13 +621,12 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, MAYBE_EphemeralProfile) {
// Create an ephemeral profile.
base::FilePath path_profile2 =
profile_manager->GenerateNextProfileDirectoryPath();
base::RunLoop run_loop;
profile_manager->CreateProfileAsync(
path_profile2,
base::BindRepeating(&EphemeralProfileCreationComplete,
run_loop.QuitWhenIdleClosure()),
path_profile2, base::BindRepeating(&EphemeralProfileCreationComplete),
base::string16(), std::string());
run_loop.Run();
// Spin to allow profile creation to take place.
content::RunMessageLoop();
BrowserList* browser_list = BrowserList::GetInstance();
ASSERT_EQ(initial_profile_count + 1U, storage.GetNumberOfProfiles());
......@@ -764,94 +754,6 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, IncognitoProfile) {
}
#if !defined(OS_ANDROID) && !BUILDFLAG(IS_CHROMEOS_ASH)
class ProfileManagerEphemeralGuestProfileBrowserTest
: public ProfileManagerBrowserTest {
public:
ProfileManagerEphemeralGuestProfileBrowserTest() {
scoped_feature_list_.InitAndEnableFeature(
features::kEnableEphemeralGuestProfilesOnDesktop);
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
};
IN_PROC_BROWSER_TEST_F(ProfileManagerEphemeralGuestProfileBrowserTest,
PRE_CleanUpEphemeralProfilesOnStartup) {
// If multiprofile mode is not enabled, you can't switch between profiles.
if (!profiles::IsMultipleProfilesEnabled())
return;
// Force the number of guest profiles created so that we have the same guest
// profile name in the post test.
// It's important to call this early in the test, before the guest profile
// path is computed for the first time.
g_browser_process->local_state()->SetInteger(prefs::kGuestProfilesNumCreated,
1u);
ProfileManager* profile_manager = g_browser_process->profile_manager();
ProfileAttributesStorage& storage =
profile_manager->GetProfileAttributesStorage();
EXPECT_EQ(1u, storage.GetNumberOfProfiles(true));
// Create an ephemeral profile.
base::FilePath ephemeral_profile_path =
profile_manager->user_data_dir().AppendASCII(kEphemeralProfilePath);
base::RunLoop run_loop;
profile_manager->CreateProfileAsync(
ephemeral_profile_path,
base::BindRepeating(&EphemeralProfileCreationComplete,
run_loop.QuitWhenIdleClosure()),
base::string16(), std::string());
run_loop.Run();
// Create an ephemeral guest profile.
base::FilePath guest_path = profile_manager->GetGuestProfilePath();
base::RunLoop run_loop2;
profile_manager->CreateProfileAsync(
guest_path,
base::BindRepeating(&ProfileCreationComplete,
run_loop2.QuitWhenIdleClosure()),
base::string16(), std::string());
run_loop2.Run();
Profile* guest = profile_manager->GetProfileByPath(guest_path);
EXPECT_TRUE(guest->IsEphemeralGuestProfile());
BrowserList* browser_list = BrowserList::GetInstance();
ASSERT_EQ(3U, storage.GetNumberOfProfiles(true));
EXPECT_EQ(1U, browser_list->size());
// Do not open browser windows for ephemeral profiles so that they don't
// get deleted in this session.
}
IN_PROC_BROWSER_TEST_F(ProfileManagerEphemeralGuestProfileBrowserTest,
CleanUpEphemeralProfilesOnStartup) {
// If multiprofile mode is not enabled, you can't switch between profiles.
if (!profiles::IsMultipleProfilesEnabled())
return;
// It's important to call this early in the test, before the guest profile
// path is computed for the first time.
g_browser_process->local_state()->SetInteger(prefs::kGuestProfilesNumCreated,
1u);
ProfileManager* profile_manager = g_browser_process->profile_manager();
ProfileAttributesStorage& storage =
profile_manager->GetProfileAttributesStorage();
base::FilePath ephemeral_profile_path =
profile_manager->user_data_dir().AppendASCII(kEphemeralProfilePath);
g_browser_process->local_state()->SetInteger(prefs::kGuestProfilesNumCreated,
0u);
base::FilePath guest_profile_path = profile_manager->GetGuestProfilePath();
// Check that ephemeral profiles got deleted.
base::ScopedAllowBlockingForTesting allow_blocking;
EXPECT_FALSE(base::DirectoryExists(guest_profile_path));
EXPECT_FALSE(base::DirectoryExists(ephemeral_profile_path));
EXPECT_EQ(1u, storage.GetNumberOfProfiles(true));
}
class EphemeralGuestProfilePolicyTest
: public policy::PolicyTest,
public ::testing::WithParamInterface<bool> {
......
chrome/browser/profiles/profile_manager_unittest.cc
View file @ b62892aa
......@@ -1268,77 +1268,6 @@ TEST_F(ProfileManagerTest, CleanUpEphemeralProfiles) {
ASSERT_EQ(0u, final_last_active_profile_list->GetSize());
}
TEST_P(ProfileManagerGuestTest, CleanUpGuestEphemeralProfile) {
// Create two profiles, one of them is guest.
ProfileManager* profile_manager = g_browser_process->profile_manager();
ProfileAttributesStorage& storage =
profile_manager->GetProfileAttributesStorage();
ASSERT_EQ(0u, storage.GetNumberOfProfiles());
// Create the guest profile and register it.
base::FilePath guest_path = ProfileManager::GetGuestProfilePath();
const std::string guest_profile_name = guest_path.BaseName().MaybeAsASCII();
TestingProfile::Builder builder;
builder.SetGuestSession();
builder.SetPath(guest_path);
builder.SetProfileName(guest_profile_name);
std::unique_ptr<TestingProfile> guest_profile = builder.Build();
profile_manager->RegisterTestingProfile(std::move(guest_profile), true);
// Create a regular profile.
const std::string profile_name = "Homer";
base::FilePath path =
profile_manager->user_data_dir().AppendASCII(profile_name);
storage.AddProfile(path, base::UTF8ToUTF16(profile_name), std::string(),
base::UTF8ToUTF16(profile_name), true, 0, std::string(),
EmptyAccountId());
ASSERT_TRUE(base::CreateDirectory(path));
size_t profiles_count = IsEphemeral() ? 2u : 1u;
ASSERT_EQ(profiles_count,
storage.GetNumberOfProfiles(/*include_guest_profile=*/true));
// Set the active profile.
PrefService* local_state = g_browser_process->local_state();
local_state->SetString(prefs::kProfileLastUsed, guest_profile_name);
// 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>(guest_path.BaseName().MaybeAsASCII()));
initial_last_active_profile_list->Append(
std::make_unique<base::Value>(path.BaseName().MaybeAsASCII()));
profile_manager->CleanUpEphemeralProfiles();
content::RunAllTasksUntilIdle();
const base::ListValue* final_last_active_profile_list =
local_state->GetList(prefs::kProfilesLastActive);
if (IsEphemeral()) {
// The ephemeral guest profile should be deleted, and the last used profile
// set to the other one. Also, the guest ephemeral profile should be removed
// from the kProfilesLastActive list.
EXPECT_FALSE(base::DirectoryExists(guest_path));
EXPECT_TRUE(base::DirectoryExists(path));
EXPECT_EQ(profile_name, local_state->GetString(prefs::kProfileLastUsed));
ASSERT_EQ(1u, storage.GetNumberOfProfiles(/*include_guest_profile=*/true));
ASSERT_EQ(1u, final_last_active_profile_list->GetSize());
ASSERT_EQ(path.BaseName().MaybeAsASCII(),
(final_last_active_profile_list->GetList())[0].GetString());
} else {
// The regular guest profile isn't impacted.
EXPECT_TRUE(base::DirectoryExists(guest_path));
EXPECT_TRUE(base::DirectoryExists(path));
EXPECT_EQ(guest_profile_name,
local_state->GetString(prefs::kProfileLastUsed));
ASSERT_EQ(1u, storage.GetNumberOfProfiles(/*include_guest_profile=*/true));
ASSERT_EQ(2u, final_last_active_profile_list->GetSize());
ASSERT_EQ(guest_path.BaseName().MaybeAsASCII(),
(final_last_active_profile_list->GetList())[0].GetString());
}
}
TEST_F(ProfileManagerTest, CleanUpEphemeralProfilesWithGuestLastUsedProfile) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
ProfileAttributesStorage& storage =
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment