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

This is a reland of fa8de986

Original change's description:
> [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: David Roger <droger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#828665}

Bug: 1150330
Change-Id: I457871e2041eb09eb90fb31f773d095b47aeb462
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2547040
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837191}

chrome/browser/app_controller_mac.mm

@@ -578,19 +578,23 @@ static base::mac::ScopedObjCClassSwizzler* g_swizzle_imk_input_session;
 - (void)didEndMainMessageLoop {
   if (base::FeatureList::IsEnabled(features::kDestroyProfileOnBrowserClose)) {
     // With DestroyProfileOnBrowserClose, Profiles get deleted earlier. So
-    // _lastProfile is already null, and [self lastProfile] below would load it
-    // from disk (which we can't do).
+    // _lastProfile is already null, and and we cannot load it from disk now.
     DCHECK_EQ(nullptr, _lastProfile);
     return;
   }
-  DCHECK_EQ(0u, chrome::GetBrowserCount([self lastProfile]));
-  if (!chrome::GetBrowserCount([self lastProfile])) {
+  if (!_lastProfile) {
+    // If only the profile picker is open and closed again, there is no profile
+    // loaded when main message loop ends and we cannot load it from disk now.
+    return;
+  }
+  DCHECK_EQ(0u, chrome::GetBrowserCount(_lastProfile));
+  if (!chrome::GetBrowserCount(_lastProfile)) {
     // As we're shutting down, we need to nuke the TabRestoreService, which
     // will start the shutdown of the NavigationControllers and allow for
     // proper shutdown. If we don't do this, Chrome won't shut down cleanly,
     // and may end up crashing when some thread tries to use the IO thread (or
     // another thread) that is no longer valid.
-    TabRestoreServiceFactory::ResetForProfile([self lastProfile]);
+    TabRestoreServiceFactory::ResetForProfile(_lastProfile);
   }
 }
 
@@ -1305,12 +1309,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 {

chrome/browser/chrome_browser_main.cc

@@ -346,6 +346,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")
   base::Time start = base::Time::Now();
@@ -399,7 +400,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 && !last_used_profile_set)
     profile = GetFallbackStartupProfile();
@@ -1378,9 +1379,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;
 

chrome/browser/ui/profile_picker.cc

@@ -6,24 +6,17 @@
 
 #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
-
 namespace {
 
 ProfilePicker::AvailabilityOnStartup GetAvailabilityOnStartup() {
@@ -45,63 +38,26 @@ ProfilePicker::AvailabilityOnStartup GetAvailabilityOnStartup() {
 }  // namespace
 
 // static
-bool ProfilePicker::ShouldShowAtLaunch(
-    const base::CommandLine& command_line,
-    const std::vector<GURL>& urls_to_launch) {
+bool ProfilePicker::ShouldShowAtLaunch() {
   AvailabilityOnStartup availability_on_startup = GetAvailabilityOnStartup();
 
-  // 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;
-  }
-
   if (signin_util::IsForceSigninEnabled())
     return false;
 
   if (!base::FeatureList::IsEnabled(features::kNewProfilePicker))
     return false;
 
-  // TODO (crbug/1155158): Move this over the urls check once the
-  // profile picker can forward urls specified in command line.
+  // TODO (crbug/1155158): Move this over the urls check (in
+  // startup_browser_creator.cc) once the profile picker can forward urls
+  // specified in command line.
   if (availability_on_startup == AvailabilityOnStartup::kForced)
     return true;
 
   size_t number_of_profiles = g_browser_process->profile_manager()
                                   ->GetProfileAttributesStorage()
                                   .GetNumberOfProfiles();
-
-  if (number_of_profiles == 1)
+  // Need to consider 0 profiles as this is what happens in some browser-tests.
+  if (number_of_profiles <= 1)
     return false;
 
   bool pref_enabled = g_browser_process->local_state()->GetBoolean(

chrome/browser/ui/profile_picker.h

@@ -15,7 +15,6 @@
 class GURL;
 
 namespace base {
-class CommandLine;
 class FilePath;
 }  // namespace base
 
@@ -108,14 +107,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);

chrome/browser/ui/startup/startup_browser_creator.h

@@ -164,12 +164,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.
@@ -205,6 +199,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);
@@ -222,6 +221,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

chrome/browser/ui/startup/startup_browser_creator_browsertest.cc

@@ -2179,8 +2179,8 @@ INSTANTIATE_TEST_SUITE_P(
                            /*switch_name=*/base::nullopt,
                            /*switch_value_ascii=*/base::nullopt,
                            /*url_arg=*/GURL("https://www.foo.com/")},
-        // Picker should be shown also in session restore.
-        ProfilePickerSetup{/*expected_to_show=*/true,
+        // Picker should also not be shown in session restore.
+        ProfilePickerSetup{/*expected_to_show=*/false,
                            /*switch_name=*/base::nullopt,
                            /*switch_value_ascii=*/base::nullopt,
                            /*url_arg=*/base::nullopt,