Skip to content

GitLab

T
tangled
Commit 51797648 authored by David Roger's avatar David Roger Committed by Commit Bot
Browse files
Options

Convert ProfileManager to OnceCallback/RepeatingCallback 

Bug: 714018
TBR: avi
Change-Id: I7bdf746f928f0feb6cf08d63a12b506063256430
Reviewed-on: https://chromium-review.googlesource.com/931129Reviewed-by: default avatarBernhard Bauer <bauerb@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544343}
parent 1ada8926
Hide whitespace changes
Inline Side-by-side
Showing with 87 additions and 79 deletions
chrome/browser/app_controller_mac_browsertest.mm
View file @ 51797648
......@@ -10,6 +10,7 @@
#import <objc/runtime.h>
#include <stddef.h>
#include "base/bind_helpers.h"
#include "base/command_line.h"
#include "base/mac/foundation_util.h"
#include "base/mac/scoped_nsobject.h"
......@@ -620,8 +621,8 @@ IN_PROC_BROWSER_TEST_F(AppControllerMainMenuBrowserTest,
ServiceAccessType::EXPLICIT_ACCESS));
// Delete profile2.
profile_manager->ScheduleProfileForDeletion(
profile2->GetPath(), ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(profile2->GetPath(),
base::DoNothing());
base::RunLoop().RunUntilIdle();
// Verify the controller's history is back to profile1.
......
chrome/browser/app_controller_mac_unittest.mm
View file @ 51797648
......@@ -4,6 +4,7 @@
#import <Cocoa/Cocoa.h>
#include "base/bind_helpers.h"
#include "base/files/file_path.h"
#include "base/mac/scoped_nsobject.h"
#include "base/run_loop.h"
......@@ -81,7 +82,7 @@ TEST_F(AppControllerTest, LastProfile) {
// Delete the active profile.
profile_manager_.profile_manager()->ScheduleProfileForDeletion(
dest_path1, ProfileManager::CreateCallback());
dest_path1, base::DoNothing());
base::RunLoop().RunUntilIdle();
......
chrome/browser/profiles/profile_manager.cc
View file @ 51797648
......@@ -11,6 +11,7 @@
#include <string>
#include "base/bind.h"
#include "base/callback_helpers.h"
#include "base/command_line.h"
#include "base/deferred_sequenced_task_runner.h"
#include "base/feature_list.h"
......@@ -309,11 +310,10 @@ size_t GetEnabledAppCount(Profile* profile) {
// It might get called more than once with different values of
// |status| but only once the profile is fully initialized will
// |client_callback| be run.
void OnProfileLoaded(
const ProfileManager::ProfileLoadedCallback& client_callback,
bool incognito,
Profile* profile,
Profile::CreateStatus status) {
void OnProfileLoaded(ProfileManager::ProfileLoadedCallback client_callback,
bool incognito,
Profile* profile,
Profile::CreateStatus status) {
if (status == Profile::CREATE_STATUS_CREATED) {
// This is an intermediate state where the profile has been created, but is
// not yet initialized. Ignore this and wait for the next state change.
......@@ -321,11 +321,12 @@ void OnProfileLoaded(
}
if (status != Profile::CREATE_STATUS_INITIALIZED) {
LOG(WARNING) << "Profile not loaded correctly";
client_callback.Run(nullptr);
std::move(client_callback).Run(nullptr);
return;
}
DCHECK(profile);
client_callback.Run(incognito ? profile->GetOffTheRecordProfile() : profile);
std::move(client_callback)
.Run(incognito ? profile->GetOffTheRecordProfile() : profile);
}
#if !defined(OS_ANDROID)
......@@ -517,25 +518,30 @@ size_t ProfileManager::GetNumberOfProfiles() {
bool ProfileManager::LoadProfile(const std::string& profile_name,
bool incognito,
const ProfileLoadedCallback& callback) {
ProfileLoadedCallback callback) {
const base::FilePath profile_path = user_data_dir().AppendASCII(profile_name);
return LoadProfileByPath(profile_path, incognito, callback);
return LoadProfileByPath(profile_path, incognito, std::move(callback));
}
bool ProfileManager::LoadProfileByPath(const base::FilePath& profile_path,
bool incognito,
const ProfileLoadedCallback& callback) {
ProfileLoadedCallback callback) {
ProfileAttributesEntry* entry = nullptr;
if (!GetProfileAttributesStorage().GetProfileAttributesWithPath(profile_path,
&entry)) {
callback.Run(nullptr);
std::move(callback).Run(nullptr);
LOG(ERROR) << "Loading a profile path that does not exist";
return false;
}
CreateProfileAsync(profile_path,
base::Bind(&OnProfileLoaded, callback, incognito),
base::string16() /* name */, std::string() /* icon_url */,
std::string() /* supervided_user_id */);
CreateProfileAsync(
profile_path,
base::BindRepeating(&OnProfileLoaded,
// OnProfileLoaded may be called multiple times, but
// |callback| will be called only once.
base::AdaptCallbackForRepeating(std::move(callback)),
incognito),
base::string16() /* name */, std::string() /* icon_url */,
std::string() /* supervided_user_id */);
return true;
}
......@@ -816,16 +822,17 @@ ProfileShortcutManager* ProfileManager::profile_shortcut_manager() {
#if !defined(OS_ANDROID)
void ProfileManager::MaybeScheduleProfileForDeletion(
const base::FilePath& profile_dir,
const CreateCallback& callback,
ProfileLoadedCallback callback,
ProfileMetrics::ProfileDelete deletion_source) {
if (!ScheduleProfileDirectoryForDeletion(profile_dir))
return;
ProfileMetrics::LogProfileDeleteUser(deletion_source);
ScheduleProfileForDeletion(profile_dir, callback);
ScheduleProfileForDeletion(profile_dir, std::move(callback));
}
void ProfileManager::ScheduleProfileForDeletion(
const base::FilePath& profile_dir, const CreateCallback& callback) {
const base::FilePath& profile_dir,
ProfileLoadedCallback callback) {
DCHECK(profiles::IsMultipleProfilesEnabled());
DCHECK(!IsProfileDirectoryMarkedForDeletion(profile_dir));
......@@ -845,10 +852,10 @@ void ProfileManager::ScheduleProfileForDeletion(
BrowserList::CloseAllBrowsersWithProfile(
profile,
base::Bind(&ProfileManager::EnsureActiveProfileExistsBeforeDeletion,
base::Unretained(this), callback),
base::Unretained(this), base::Passed(std::move(callback))),
base::Bind(&CancelProfileDeletion), false);
} else {
EnsureActiveProfileExistsBeforeDeletion(callback, profile_dir);
EnsureActiveProfileExistsBeforeDeletion(std::move(callback), profile_dir);
}
}
#endif // !defined(OS_ANDROID)
......@@ -1438,7 +1445,8 @@ Profile* ProfileManager::CreateAndInitializeProfile(
#if !defined(OS_ANDROID)
void ProfileManager::EnsureActiveProfileExistsBeforeDeletion(
const CreateCallback& callback, const base::FilePath& profile_dir) {
ProfileLoadedCallback callback,
const base::FilePath& profile_dir) {
// In case we delete non-active profile and current profile is valid, proceed.
const base::FilePath last_used_profile_path =
GetLastUsedProfileDir(user_data_dir_);
......@@ -1460,8 +1468,8 @@ void ProfileManager::EnsureActiveProfileExistsBeforeDeletion(
cur_path != guest_profile_path &&
!profile->IsLegacySupervised() &&
!IsProfileDirectoryMarkedForDeletion(cur_path)) {
OnNewActiveProfileLoaded(profile_dir, cur_path, callback, profile,
Profile::CREATE_STATUS_INITIALIZED);
OnNewActiveProfileLoaded(profile_dir, cur_path, std::move(callback),
profile, Profile::CREATE_STATUS_INITIALIZED);
return;
}
}
......@@ -1501,11 +1509,15 @@ void ProfileManager::EnsureActiveProfileExistsBeforeDeletion(
}
// Create and/or load fallback profile.
CreateProfileAsync(fallback_profile_path,
base::Bind(&ProfileManager::OnNewActiveProfileLoaded,
base::Unretained(this), profile_dir,
fallback_profile_path, callback),
new_profile_name, new_avatar_url, std::string());
CreateProfileAsync(
fallback_profile_path,
base::BindRepeating(
&ProfileManager::OnNewActiveProfileLoaded, base::Unretained(this),
profile_dir, fallback_profile_path,
// OnNewActiveProfileLoaded may be called several times, but
// only once with CREATE_STATUS_INITIALIZED.
base::AdaptCallbackForRepeating(std::move(callback))),
new_profile_name, new_avatar_url, std::string());
}
void ProfileManager::OnLoadProfileForProfileDeletion(
......@@ -1582,9 +1594,10 @@ void ProfileManager::FinishDeletingProfile(
// Attempt to load the profile before deleting it to properly clean up
// profile-specific data stored outside the profile directory.
LoadProfileByPath(profile_dir, false,
base::Bind(&ProfileManager::OnLoadProfileForProfileDeletion,
base::Unretained(this), profile_dir));
LoadProfileByPath(
profile_dir, false,
base::BindOnce(&ProfileManager::OnLoadProfileForProfileDeletion,
base::Unretained(this), profile_dir));
// Prevents CreateProfileAsync from re-creating the profile.
MarkProfileDirectoryForDeletion(profile_dir);
......@@ -1788,7 +1801,7 @@ void ProfileManager::BrowserListObserver::OnBrowserSetLastActive(
void ProfileManager::OnNewActiveProfileLoaded(
const base::FilePath& profile_to_delete_path,
const base::FilePath& new_active_profile_path,
const CreateCallback& original_callback,
ProfileLoadedCallback callback,
Profile* loaded_profile,
Profile::CreateStatus status) {
DCHECK(status != Profile::CREATE_STATUS_LOCAL_FAIL &&
......@@ -1802,14 +1815,13 @@ void ProfileManager::OnNewActiveProfileLoaded(
// If the profile we tried to load as the next active profile has been
// deleted, then retry deleting this profile to redo the logic to load
// the next available profile.
EnsureActiveProfileExistsBeforeDeletion(original_callback,
EnsureActiveProfileExistsBeforeDeletion(std::move(callback),
profile_to_delete_path);
return;
}
FinishDeletingProfile(profile_to_delete_path, new_active_profile_path);
if (!original_callback.is_null())
original_callback.Run(loaded_profile, status);
std::move(callback).Run(loaded_profile);
}
void ProfileManager::ScheduleForcedEphemeralProfileForDeletion(
......
chrome/browser/profiles/profile_manager.h
View file @ 51797648
......@@ -34,8 +34,9 @@ class ProfileInfoCache;
class ProfileManager : public content::NotificationObserver,
public Profile::Delegate {
public:
typedef base::Callback<void(Profile*, Profile::CreateStatus)> CreateCallback;
typedef base::Callback<void(Profile*)> ProfileLoadedCallback;
typedef base::RepeatingCallback<void(Profile*, Profile::CreateStatus)>
CreateCallback;
typedef base::OnceCallback<void(Profile*)> ProfileLoadedCallback;
explicit ProfileManager(const base::FilePath& user_data_dir);
~ProfileManager() override;
......@@ -108,10 +109,10 @@ class ProfileManager : public content::NotificationObserver,
// as part of the callback.
bool LoadProfile(const std::string& profile_name,
bool incognito,
const ProfileLoadedCallback& callback);
ProfileLoadedCallback callback);
bool LoadProfileByPath(const base::FilePath& profile_path,
bool incognito,
const ProfileLoadedCallback& callback);
ProfileLoadedCallback callback);
// Explicit asynchronous creation of a profile located at |profile_path|.
// If the profile has already been created then callback is called
......@@ -201,14 +202,14 @@ class ProfileManager : public content::NotificationObserver,
// profile is either scheduling or marked for deletion.
void MaybeScheduleProfileForDeletion(
const base::FilePath& profile_dir,
const CreateCallback& callback,
ProfileLoadedCallback callback,
ProfileMetrics::ProfileDelete deletion_source);
// Schedules the profile at the given path to be deleted on shutdown. If we're
// deleting the last profile, a new one will be created in its place, and in
// that case the callback will be called when profile creation is complete.
void ScheduleProfileForDeletion(const base::FilePath& profile_dir,
const CreateCallback& callback);
ProfileLoadedCallback callback);
#endif
// Autoloads profiles if they are running background apps.
......@@ -315,7 +316,7 @@ class ProfileManager : public content::NotificationObserver,
// last non-supervised profile. In the Mac, loads the next non-supervised
// profile if the profile to be deleted is the active profile.
void EnsureActiveProfileExistsBeforeDeletion(
const CreateCallback& callback,
ProfileLoadedCallback callback,
const base::FilePath& profile_dir);
// Schedules the profile at the given path to be deleted on shutdown,
......@@ -389,7 +390,7 @@ class ProfileManager : public content::NotificationObserver,
void OnNewActiveProfileLoaded(
const base::FilePath& profile_to_delete_path,
const base::FilePath& last_non_supervised_profile_path,
const CreateCallback& original_callback,
ProfileLoadedCallback callback,
Profile* loaded_profile,
Profile::CreateStatus status);
......
chrome/browser/profiles/profile_manager_browsertest.cc
View file @ 51797648
......@@ -5,6 +5,7 @@
#include <stddef.h>
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/callback.h"
#include "base/command_line.h"
#include "base/macros.h"
......@@ -13,6 +14,7 @@
#include "base/run_loop.h"
#include "base/stl_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/bind_test_util.h"
#include "build/build_config.h"
#include "chrome/browser/password_manager/password_store_factory.h"
#include "chrome/browser/profiles/profile_attributes_entry.h"
......@@ -257,8 +259,8 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteSingletonProfile) {
storage.GetAllProfilesAttributes().front()->GetPath();
EXPECT_FALSE(singleton_profile_path.empty());
MultipleProfileDeletionObserver profile_deletion_observer(1u);
profile_manager->ScheduleProfileForDeletion(
singleton_profile_path, ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(singleton_profile_path,
base::DoNothing());
// Run the message loop until the profile is actually deleted (as indicated
// by the callback above being called).
......@@ -301,8 +303,7 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteInactiveProfile) {
// Delete inactive profile.
MultipleProfileDeletionObserver profile_deletion_observer(1u);
profile_manager->ScheduleProfileForDeletion(
new_path, ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(new_path, base::DoNothing());
profile_deletion_observer.Wait();
// Make sure there only preexisted profile left.
......@@ -334,8 +335,8 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteCurrentProfile) {
// Delete current profile.
MultipleProfileDeletionObserver profile_deletion_observer(1u);
profile_manager->ScheduleProfileForDeletion(
browser()->profile()->GetPath(), ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(browser()->profile()->GetPath(),
base::DoNothing());
profile_deletion_observer.Wait();
// Make sure a profile created earlier become the only profile.
......@@ -375,8 +376,8 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, DeleteAllProfiles) {
for (ProfileAttributesEntry* entry : entries) {
base::FilePath profile_path = entry->GetPath();
EXPECT_FALSE(profile_path.empty());
profile_manager->ScheduleProfileForDeletion(
profile_path, ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(profile_path,
base::DoNothing());
old_profile_paths.push_back(profile_path);
}
profile_deletion_observer.Wait();
......@@ -605,7 +606,8 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, MAYBE_DeletePasswords) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
base::RunLoop run_loop;
profile_manager->ScheduleProfileForDeletion(
profile->GetPath(), base::Bind(&OnUnblockOnProfileCreation, &run_loop));
profile->GetPath(),
base::BindLambdaForTesting([&run_loop](Profile*) { run_loop.Quit(); }));
run_loop.Run();
PasswordStoreConsumerVerifier verify_delete;
......
chrome/browser/profiles/profile_manager_unittest.cc
View file @ 51797648
......@@ -6,6 +6,7 @@
#include <string>
#include "base/bind_helpers.h"
#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
......@@ -1155,8 +1156,7 @@ TEST_F(ProfileManagerTest, ActiveProfileDeleted) {
local_state->SetString(prefs::kProfileLastUsed, profile_name1);
// Delete the active profile.
profile_manager->ScheduleProfileForDeletion(dest_path1,
ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(dest_path1, base::DoNothing());
content::RunAllTasksUntilIdle();
EXPECT_EQ(dest_path2, profile_manager->GetLastUsedProfile()->GetPath());
......@@ -1188,8 +1188,7 @@ TEST_F(ProfileManagerTest, LastProfileDeleted) {
local_state->SetString(prefs::kProfileLastUsed, profile_name1);
// Delete the active profile.
profile_manager->ScheduleProfileForDeletion(dest_path1,
ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(dest_path1, base::DoNothing());
content::RunAllTasksUntilIdle();
// A new profile should have been created
......@@ -1242,8 +1241,7 @@ TEST_F(ProfileManagerTest, LastProfileDeletedWithGuestActiveProfile) {
local_state->SetString(prefs::kProfileLastUsed, guest_profile_name);
// Delete the other profile.
profile_manager->ScheduleProfileForDeletion(dest_path1,
ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(dest_path1, base::DoNothing());
content::RunAllTasksUntilIdle();
// A new profile should have been created.
......@@ -1284,7 +1282,7 @@ TEST_F(ProfileManagerTest, ProfileDisplayNameResetsDefaultName) {
// Deleting a profile means returning to the default name.
profile_manager->ScheduleProfileForDeletion(profile2->GetPath(),
ProfileManager::CreateCallback());
base::DoNothing());
content::RunAllTasksUntilIdle();
EXPECT_EQ(default_profile_name,
profiles::GetAvatarNameForProfile(profile1->GetPath()));
......@@ -1329,7 +1327,7 @@ TEST_F(ProfileManagerTest, ProfileDisplayNamePreservesCustomName) {
// Deleting a profile means returning to the original, custom name.
profile_manager->ScheduleProfileForDeletion(profile2->GetPath(),
ProfileManager::CreateCallback());
base::DoNothing());
content::RunAllTasksUntilIdle();
EXPECT_EQ(custom_profile_name,
profiles::GetAvatarNameForProfile(profile1->GetPath()));
......@@ -1381,7 +1379,7 @@ TEST_F(ProfileManagerTest, ProfileDisplayNamePreservesSignedInName) {
// Deleting a profile means returning to the original, actual profile name.
profile_manager->ScheduleProfileForDeletion(profile2->GetPath(),
ProfileManager::CreateCallback());
base::DoNothing());
content::RunAllTasksUntilIdle();
EXPECT_EQ(gaia_given_name,
profiles::GetAvatarNameForProfile(profile1->GetPath()));
......@@ -1488,8 +1486,7 @@ TEST_F(ProfileManagerTest, ActiveProfileDeletedNeedsToLoadNextProfile) {
// Delete the active profile. This should switch and load the unloaded
// profile.
profile_manager->ScheduleProfileForDeletion(dest_path1,
ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(dest_path1, base::DoNothing());
content::RunAllTasksUntilIdle();
......@@ -1546,12 +1543,10 @@ TEST_F(ProfileManagerTest, ActiveProfileDeletedNextProfileDeletedToo) {
// Try to break this flow by setting the active profile to Profile2 in the
// middle (so after the first posted message), and trying to delete Profile2,
// so that the ProfileManager has to look for a different profile to load.
profile_manager->ScheduleProfileForDeletion(dest_path1,
ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(dest_path1, base::DoNothing());
local_state->SetString(prefs::kProfileLastUsed,
dest_path2.BaseName().MaybeAsASCII());
profile_manager->ScheduleProfileForDeletion(dest_path2,
ProfileManager::CreateCallback());
profile_manager->ScheduleProfileForDeletion(dest_path2, base::DoNothing());
content::RunAllTasksUntilIdle();
EXPECT_EQ(dest_path3, profile_manager->GetLastUsedProfile()->GetPath());
......
chrome/browser/ui/webui/profile_helper.cc
View file @ 51797648
......@@ -55,11 +55,7 @@ void ShowReauthDialog(const std::string& user_name,
}
void DeleteProfileCallback(std::unique_ptr<ScopedKeepAlive> keep_alive,
Profile* profile,
Profile::CreateStatus status) {
if (status != Profile::CREATE_STATUS_INITIALIZED)
return;
Profile* profile) {
OpenNewWindowForProfile(profile);
}
......@@ -88,10 +84,10 @@ void DeleteProfileAtPath(base::FilePath file_path,
return;
g_browser_process->profile_manager()->MaybeScheduleProfileForDeletion(
file_path,
base::Bind(&DeleteProfileCallback,
base::Passed(std::make_unique<ScopedKeepAlive>(
KeepAliveOrigin::PROFILE_HELPER,
KeepAliveRestartOption::DISABLED))),
base::BindOnce(
&DeleteProfileCallback,
std::make_unique<ScopedKeepAlive>(KeepAliveOrigin::PROFILE_HELPER,
KeepAliveRestartOption::DISABLED)),
deletion_source);
}
......
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