Commit fa8de986 authored by Jan Krcal's avatar Jan Krcal Committed by Commit Bot

[Profile picker] Use system profile on startup when showing the picker

This CL changes the logic to load the startup profile whenever the
profile picker should be loaded. It instead loads the guest profile
to indicate the picker should be open. This speeds up showing the picker
when the last profile used is heavy to load. The CL also unifies the
logic to decide whether to show the new picker (or the old user manager)
into one function, GetStartupProfile().

There is one behavioral difference: the new code directly opens browser
windows if the user had multiple profiles open last time. This is desired
because having multiple profiles open simultaneously the last time is a
strong signal the user wants to continue this way. Note that having
multiple profiles open the last time happens only in limited situations,
e.g. via chrome://restart or via session restore (when logging out /
switching off the machine with multiple profiles' windows open).

Bug: 1150330
Change-Id: Iae9744f44bc242e9aaaf8f287cc29e1089326786
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532460
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828665}
parent 2e87369b
......@@ -1265,12 +1265,7 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session;
// manager is still shown).
UserManager::Show(base::FilePath(),
profiles::USER_MANAGER_SELECT_PROFILE_NO_ACTION);
} else if (ProfilePicker::ShouldShowAtLaunch(
// Pass an empty command line, because this is not application
// startup. The original arguments (e.g. --incognito) are no
// longer relevant.
base::CommandLine(base::CommandLine::NO_PROGRAM),
/*urls_to_launch=*/std::vector<GURL>())) {
} else if (ProfilePicker::ShouldShowAtLaunch()) {
ProfilePicker::Show(
ProfilePicker::EntryPoint::kNewSessionOnExistingProcess);
} else {
......
......@@ -340,6 +340,7 @@ void AddFirstRunNewTabs(StartupBrowserCreator* browser_creator,
// should not continue.
Profile* CreatePrimaryProfile(const content::MainFunctionParams& parameters,
const base::FilePath& user_data_dir,
const base::FilePath& cur_dir,
const base::CommandLine& parsed_command_line) {
TRACE_EVENT0("startup", "ChromeBrowserMainParts::CreateProfile")
......@@ -392,7 +393,7 @@ Profile* CreatePrimaryProfile(const content::MainFunctionParams& parameters,
CHECK(profile) << "Cannot get default profile.";
#else
profile = GetStartupProfile(user_data_dir, parsed_command_line);
profile = GetStartupProfile(user_data_dir, cur_dir, parsed_command_line);
if (!profile && !set_last_used_profile)
profile = GetFallbackStartupProfile();
......@@ -1360,9 +1361,8 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
// This step is costly and is already measured in Startup.CreateFirstProfile
// and more directly Profile.CreateAndInitializeProfile.
profile_ = CreatePrimaryProfile(parameters(),
user_data_dir_,
parsed_command_line());
profile_ = CreatePrimaryProfile(parameters(), user_data_dir_,
base::FilePath(), parsed_command_line());
if (!profile_)
return content::RESULT_CODE_NORMAL_EXIT;
......
......@@ -6,68 +6,24 @@
#include <string>
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/metrics/histogram_functions.h"
#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile_attributes_storage.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/browser/signin/signin_util.h"
#include "chrome/browser/ui/ui_features.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "components/prefs/pref_service.h"
#if defined(OS_WIN)
#include "chrome/browser/notifications/win/notification_launch_id.h"
#endif
// static
bool ProfilePicker::ShouldShowAtLaunch(
const base::CommandLine& command_line,
const std::vector<GURL>& urls_to_launch) {
// Don't show the picker if a certain profile (or an incognito window in the
// default profile) is explicitly requested.
if (profiles::IsGuestModeRequested(command_line,
g_browser_process->local_state(),
/*show_warning=*/false) ||
command_line.HasSwitch(switches::kIncognito) ||
command_line.HasSwitch(switches::kProfileDirectory)) {
return false;
}
// Don't show the picker if an app is explicitly requested to open. This URL
// param should be ideally paired with switches::kProfileDirectory but it's
// better to err on the side of opening the last profile than to err on the
// side of not opening the app directly.
if (command_line.HasSwitch(switches::kApp) ||
command_line.HasSwitch(switches::kAppId)) {
return false;
}
// If the browser is launched due to activation on Windows native notification,
// the profile id encoded in the notification launch id should be chosen over
// the profile picker.
#if defined(OS_WIN)
std::string profile_id =
NotificationLaunchId::GetNotificationLaunchProfileId(command_line);
if (!profile_id.empty()) {
return false;
}
#endif // defined(OS_WIN)
// Don't show the picker if a any URL is requested to launch via the
// command-line.
if (!urls_to_launch.empty()) {
return false;
}
bool ProfilePicker::ShouldShowAtLaunch() {
size_t number_of_profiles = g_browser_process->profile_manager()
->GetProfileAttributesStorage()
.GetNumberOfProfiles();
if (signin_util::IsForceSigninEnabled() || number_of_profiles == 1)
// Need to consider 0 profiles as this is what happens in some browser-tests.
if (signin_util::IsForceSigninEnabled() || number_of_profiles <= 1)
return false;
bool pref_enabled = g_browser_process->local_state()->GetBoolean(
......
......@@ -68,14 +68,11 @@ class ProfilePicker {
// Overrides the timeout delay for waiting for extended account info.
static void SetExtendedAccountInfoTimeoutForTesting(base::TimeDelta timeout);
// Returns whether the profile picker at launch. This can be called on
// Returns whether to show profile picker at launch. This can be called on
// startup or when Chrome is re-opened, e.g. when clicking on the dock icon on
// MacOS when there are no windows, or from Windows tray icon.
// This returns true if this is a new session. Returns false if a specific
// profile is passed in the command line, or if some parameters (such as the
// URLs to launch) cannot be handled by the picker.
static bool ShouldShowAtLaunch(const base::CommandLine& command_line,
const std::vector<GURL>& urls_to_launch);
// This returns true if the user has multiple profiles and has not opted-out.
static bool ShouldShowAtLaunch();
private:
DISALLOW_COPY_AND_ASSIGN(ProfilePicker);
......
......@@ -50,6 +50,7 @@
#include "chrome/browser/profiles/profile_observer.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/signin/signin_util.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_list.h"
......@@ -277,8 +278,59 @@ bool CanOpenProfileOnStartup(Profile* profile) {
#endif
}
void ShowUserManagerOnStartup() {
bool ShouldShowProfilePickerAtProcessLaunch(
ProfileManager* profile_manager,
const base::CommandLine& command_line) {
#if defined(OS_CHROMEOS)
return false;
#else
// If multiple profiles get restored, do not show the picker.
if (!profile_manager->GetLastOpenedProfiles().empty())
return false;
// Don't show the picker if a certain profile (or an incognito window in the
// default profile) is explicitly requested.
if (profiles::IsGuestModeRequested(command_line,
g_browser_process->local_state(),
/*show_warning=*/false) ||
command_line.HasSwitch(switches::kIncognito) ||
command_line.HasSwitch(switches::kProfileDirectory)) {
return false;
}
// Don't show the picker if an app is explicitly requested to open. This URL
// param should be ideally paired with switches::kProfileDirectory but it's
// better to err on the side of opening the last profile than to err on the
// side of not opening the app directly.
if (command_line.HasSwitch(switches::kApp) ||
command_line.HasSwitch(switches::kAppId)) {
return false;
}
// If the browser is launched due to activation on Windows native notification,
// the profile id encoded in the notification launch id should be chosen over
// the profile picker.
#if defined(OS_WIN)
std::string profile_id =
NotificationLaunchId::GetNotificationLaunchProfileId(command_line);
if (!profile_id.empty()) {
return false;
}
#endif // defined(OS_WIN)
return ProfilePicker::ShouldShowAtLaunch();
#endif // !defined(OS_CHROMEOS)
}
void ShowUserManager(bool is_process_startup) {
#if !defined(OS_CHROMEOS)
if (!signin_util::IsForceSigninEnabled() &&
base::FeatureList::IsEnabled(features::kNewProfilePicker)) {
ProfilePicker::Show(
is_process_startup
? ProfilePicker::EntryPoint::kOnStartup
: ProfilePicker::EntryPoint::kNewSessionOnExistingProcess);
return;
}
UserManager::Show(base::FilePath(),
profiles::USER_MANAGER_SELECT_PROFILE_NO_ACTION);
#endif // !defined(OS_CHROMEOS)
......@@ -603,86 +655,6 @@ void StartupBrowserCreator::RegisterProfilePrefs(PrefRegistrySimple* registry) {
#endif // defined(OS_WIN) && BUILDFLAG(GOOGLE_CHROME_BRANDING)
}
// static
std::vector<GURL> StartupBrowserCreator::GetURLsFromCommandLine(
const base::CommandLine& command_line,
const base::FilePath& cur_dir,
Profile* profile) {
DCHECK(profile);
std::vector<GURL> urls;
const base::CommandLine::StringVector& params = command_line.GetArgs();
for (size_t i = 0; i < params.size(); ++i) {
base::FilePath param = base::FilePath(params[i]);
// Handle Vista way of searching - "? <search-term>"
if ((param.value().size() > 2) && (param.value()[0] == '?') &&
(param.value()[1] == ' ')) {
GURL url(GetDefaultSearchURLForSearchTerms(
TemplateURLServiceFactory::GetForProfile(profile),
param.LossyDisplayName().substr(2)));
if (url.is_valid()) {
urls.push_back(url);
continue;
}
}
// Otherwise, fall through to treating it as a URL.
// This will create a file URL or a regular URL.
// This call can (in rare circumstances) block the UI thread.
// Allow it until this bug is fixed.
// http://code.google.com/p/chromium/issues/detail?id=60641
GURL url = GURL(param.MaybeAsASCII());
// http://crbug.com/371030: Only use URLFixerUpper if we don't have a valid
// URL, otherwise we will look in the current directory for a file named
// 'about' if the browser was started with a about:foo argument.
// http://crbug.com/424991: Always use URLFixerUpper on file:// URLs,
// otherwise we wouldn't correctly handle '#' in a file name.
if (!url.is_valid() || url.SchemeIsFile()) {
base::ThreadRestrictions::ScopedAllowIO allow_io;
url = url_formatter::FixupRelativeFile(cur_dir, param);
}
// Exclude dangerous schemes.
if (!url.is_valid())
continue;
const GURL settings_url = GURL(chrome::kChromeUISettingsURL);
bool url_points_to_an_approved_settings_page = false;
#if defined(OS_CHROMEOS)
// In ChromeOS, allow any settings page to be specified on the command line.
url_points_to_an_approved_settings_page =
url.GetOrigin() == settings_url.GetOrigin();
#else
// Exposed for external cleaners to offer a settings reset to the
// user. The allowed URLs must match exactly.
const GURL reset_settings_url =
settings_url.Resolve(chrome::kResetProfileSettingsSubPage);
url_points_to_an_approved_settings_page = url == reset_settings_url;
#if defined(OS_WIN)
// On Windows, also allow a hash for the Chrome Cleanup Tool.
const GURL reset_settings_url_with_cct_hash = reset_settings_url.Resolve(
std::string("#") +
settings::ResetSettingsHandler::kCctResetSettingsHash);
url_points_to_an_approved_settings_page =
url_points_to_an_approved_settings_page ||
url == reset_settings_url_with_cct_hash;
#endif // defined(OS_WIN)
#endif // defined(OS_CHROMEOS)
ChildProcessSecurityPolicy* policy =
ChildProcessSecurityPolicy::GetInstance();
if (policy->IsWebSafeScheme(url.scheme()) ||
url.SchemeIs(url::kFileScheme) ||
url_points_to_an_approved_settings_page ||
(url.spec().compare(url::kAboutBlankURL) == 0)) {
urls.push_back(url);
}
}
return urls;
}
bool StartupBrowserCreator::ProcessCmdLineImpl(
const base::CommandLine& command_line,
const base::FilePath& cur_dir,
......@@ -961,19 +933,6 @@ bool StartupBrowserCreator::LaunchBrowserForLastProfiles(
bool process_startup,
Profile* last_used_profile,
const Profiles& last_opened_profiles) {
#if !defined(OS_CHROMEOS)
const std::vector<GURL> urls_to_launch =
StartupBrowserCreator::GetURLsFromCommandLine(command_line, cur_dir,
last_used_profile);
if (ProfilePicker::ShouldShowAtLaunch(command_line, urls_to_launch)) {
ProfilePicker::Show(
process_startup
? ProfilePicker::EntryPoint::kOnStartup
: ProfilePicker::EntryPoint::kNewSessionOnExistingProcess);
return true;
}
#endif // !defined(OS_CHROMEOS)
chrome::startup::IsProcessStartup is_process_startup =
process_startup ? chrome::startup::IS_PROCESS_STARTUP
: chrome::startup::IS_NOT_PROCESS_STARTUP;
......@@ -1009,7 +968,7 @@ bool StartupBrowserCreator::LaunchBrowserForLastProfiles(
}
// Show UserManager if |last_used_profile| can't be auto opened.
ShowUserManagerOnStartup();
ShowUserManager(/*is_process_startup=*/process_startup);
return true;
}
return ProcessLastOpenedProfiles(command_line, cur_dir, is_process_startup,
......@@ -1082,7 +1041,7 @@ bool StartupBrowserCreator::ProcessLastOpenedProfiles(
// activation this one.
#if !defined(OS_CHROMEOS)
if (is_process_startup == chrome::startup::IS_PROCESS_STARTUP)
ShowUserManagerOnStartup();
ShowUserManager(/*is_process_startup=*/true);
else
#endif
profile_launch_observer.Get().set_profile_to_activate(last_used_profile);
......@@ -1188,6 +1147,82 @@ bool StartupBrowserCreator::ActivatedProfile() {
return profile_launch_observer.Get().activated_profile();
}
std::vector<GURL> GetURLsFromCommandLine(const base::CommandLine& command_line,
const base::FilePath& cur_dir,
Profile* profile) {
std::vector<GURL> urls;
const base::CommandLine::StringVector& params = command_line.GetArgs();
for (size_t i = 0; i < params.size(); ++i) {
base::FilePath param = base::FilePath(params[i]);
// Handle Vista way of searching - "? <search-term>"
if ((param.value().size() > 2) && (param.value()[0] == '?') &&
(param.value()[1] == ' ')) {
GURL url(GetDefaultSearchURLForSearchTerms(
TemplateURLServiceFactory::GetForProfile(profile),
param.LossyDisplayName().substr(2)));
if (url.is_valid()) {
urls.push_back(url);
continue;
}
}
// Otherwise, fall through to treating it as a URL.
// This will create a file URL or a regular URL.
// This call can (in rare circumstances) block the UI thread.
// Allow it until this bug is fixed.
// http://code.google.com/p/chromium/issues/detail?id=60641
GURL url = GURL(param.MaybeAsASCII());
// http://crbug.com/371030: Only use URLFixerUpper if we don't have a valid
// URL, otherwise we will look in the current directory for a file named
// 'about' if the browser was started with a about:foo argument.
// http://crbug.com/424991: Always use URLFixerUpper on file:// URLs,
// otherwise we wouldn't correctly handle '#' in a file name.
if (!url.is_valid() || url.SchemeIsFile()) {
base::ThreadRestrictions::ScopedAllowIO allow_io;
url = url_formatter::FixupRelativeFile(cur_dir, param);
}
// Exclude dangerous schemes.
if (!url.is_valid())
continue;
const GURL settings_url = GURL(chrome::kChromeUISettingsURL);
bool url_points_to_an_approved_settings_page = false;
#if defined(OS_CHROMEOS)
// In ChromeOS, allow any settings page to be specified on the command line.
url_points_to_an_approved_settings_page =
url.GetOrigin() == settings_url.GetOrigin();
#else
// Exposed for external cleaners to offer a settings reset to the
// user. The allowed URLs must match exactly.
const GURL reset_settings_url =
settings_url.Resolve(chrome::kResetProfileSettingsSubPage);
url_points_to_an_approved_settings_page = url == reset_settings_url;
#if defined(OS_WIN)
// On Windows, also allow a hash for the Chrome Cleanup Tool.
const GURL reset_settings_url_with_cct_hash = reset_settings_url.Resolve(
std::string("#") +
settings::ResetSettingsHandler::kCctResetSettingsHash);
url_points_to_an_approved_settings_page =
url_points_to_an_approved_settings_page ||
url == reset_settings_url_with_cct_hash;
#endif // defined(OS_WIN)
#endif // defined(OS_CHROMEOS)
ChildProcessSecurityPolicy* policy =
ChildProcessSecurityPolicy::GetInstance();
if (policy->IsWebSafeScheme(url.scheme()) ||
url.SchemeIs(url::kFileScheme) ||
url_points_to_an_approved_settings_page ||
(url.spec().compare(url::kAboutBlankURL) == 0)) {
urls.push_back(url);
}
}
return urls;
}
bool HasPendingUncleanExit(Profile* profile) {
return profile->GetLastSessionExitType() == Profile::EXIT_CRASHED &&
!profile_launch_observer.Get().HasBeenLaunched(profile);
......@@ -1225,8 +1260,31 @@ base::FilePath GetStartupProfilePath(const base::FilePath& user_data_dir,
#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
Profile* GetStartupProfile(const base::FilePath& user_data_dir,
const base::FilePath& cur_dir,
const base::CommandLine& command_line) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
#if !defined(OS_CHROMEOS)
if (ShouldShowProfilePickerAtProcessLaunch(profile_manager, command_line)) {
// Open the picker only if no URLs have been provided to launch Chrome. If
// URLs are provided, open them in the last profile, instead.
Profile* guest_profile =
profile_manager->GetProfile(ProfileManager::GetGuestProfilePath());
// TODO(crbug.com/1150326): Consider resolving urls_to_launch without any
// profile so that the guest profile does not need to get created in case
// some URLs are passed and the picker is not shown in the end.
const std::vector<GURL> urls_to_launch =
GetURLsFromCommandLine(command_line, cur_dir, guest_profile);
if (urls_to_launch.empty() &&
profile_manager->GetProfile(ProfileManager::GetSystemProfilePath())) {
// To indicate that we want to show the profile picker, return the guest
// profile. However, we can only do this if the system profile (where the
// profile picker lives) also exists (or is creatable).
// TODO(crbug.com/1150326): Refactor this to indicate more directly that
// profile picker should be shown (returning an enum, or so).
return guest_profile;
}
}
#endif // !defined(OS_CHROMEOS)
base::FilePath profile_path =
GetStartupProfilePath(user_data_dir, command_line);
......@@ -1247,6 +1305,8 @@ Profile* GetStartupProfile(const base::FilePath& user_data_dir,
// We want to show the user manager. To indicate this, return the guest
// profile. However, we can only do this if the system profile (where the user
// manager lives) also exists (or is creatable).
// TODO(crbug.com/1150326): Refactor this to indicate more directly that
// profile picker should be shown (returning an enum, or so).
return profile_manager->GetProfile(ProfileManager::GetSystemProfilePath())
? profile_manager->GetProfile(
ProfileManager::GetGuestProfilePath())
......
......@@ -161,12 +161,6 @@ class StartupBrowserCreator {
Profile* last_used_profile,
const Profiles& last_opened_profiles);
// Returns the list of URLs to open from the command line.
static std::vector<GURL> GetURLsFromCommandLine(
const base::CommandLine& command_line,
const base::FilePath& cur_dir,
Profile* profile);
// This function performs command-line handling and is invoked only after
// start up (for example when we get a start request for another process).
// |command_line| holds the command line being processed.
......@@ -202,6 +196,11 @@ class StartupBrowserCreator {
static bool in_synchronous_profile_launch_;
};
// Returns the list of URLs to open from the command line.
std::vector<GURL> GetURLsFromCommandLine(const base::CommandLine& command_line,
const base::FilePath& cur_dir,
Profile* profile);
// Returns true if |profile| has exited uncleanly and has not been launched
// after the unclean exit.
bool HasPendingUncleanExit(Profile* profile);
......@@ -219,6 +218,7 @@ base::FilePath GetStartupProfilePath(const base::FilePath& user_data_dir,
// opening the user manager, returns null if either the guest profile or the
// system profile cannot be opened.
Profile* GetStartupProfile(const base::FilePath& user_data_dir,
const base::FilePath& cur_dir,
const base::CommandLine& command_line);
// Returns the profile that should be loaded on process startup when
......
......@@ -1871,7 +1871,8 @@ class StartupBrowserCreatorPickerTest : public InProcessBrowserTest {
~StartupBrowserCreatorPickerTest() override = default;
protected:
void StartWithTwoProfiles(const base::CommandLine& command_line) {
void StartWithTwoProfiles(const base::CommandLine& command_line,
bool set_last_opened_profiles = false) {
Profile* profile = browser()->profile();
ProfileManager* profile_manager = g_browser_process->profile_manager();
// Create another profile.
......@@ -1886,8 +1887,13 @@ class StartupBrowserCreatorPickerTest : public InProcessBrowserTest {
ASSERT_TRUE(other_profile);
StartupBrowserCreator launch;
std::vector<Profile*> last_opened_profiles;
if (set_last_opened_profiles) {
last_opened_profiles.push_back(profile);
last_opened_profiles.push_back(other_profile);
}
ASSERT_TRUE(launch.Start(command_line, profile_manager->user_data_dir(),
profile, {}));
profile, last_opened_profiles));
}
private:
......@@ -1972,6 +1978,16 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorPickerTest, SkipsPickerWithAppId) {
EXPECT_TRUE(new_browser);
}
IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorPickerTest,
SkipsPickerWithLastOpenedProfiles) {
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
StartWithTwoProfiles(command_line, /*set_last_opened_profiles=*/true);
// Skips the picker and creates a new browser window.
Browser* new_browser = FindOneOtherBrowser(browser());
EXPECT_TRUE(new_browser);
}
class GuestStartupBrowserCreatorPickerTest
: public StartupBrowserCreatorPickerTest,
public ::testing::WithParamInterface<bool> {
......
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