[Profile picker] Skip the picker when some switches are provided

This CL tweaks the logic on startup that decides whether the profile
picker should be displayed. After this CL, it is skipped in these cases:
  - Guest mode is requested,
  - a incognito window is requested,
  - a concrete profile is requested (such as by Windows shortcuts),
  - a notification for a concrete profile is handled (Win),
  - a concrete URL is requested.

Bug: 1122553
Change-Id: I1b3cbdf4ef74b84ffe687978b6b293e9e6ff4e54
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379913
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Auto-Submit: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802639}

chrome/browser/ui/startup/startup_browser_creator.cc

@@ -267,8 +267,58 @@ bool CanOpenProfileOnStartup(Profile* profile) {
 #endif
 }
 
+// Returns true if starting in guest mode is enforced. If |show_warning| is
+// true, send a warning if guest mode is requested but not allowed by policy.
+bool IsGuestModeEnforced(const base::CommandLine& command_line,
+                         bool show_warning) {
+#if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_WIN) || \
+    defined(OS_MAC)
+  PrefService* service = g_browser_process->local_state();
+  DCHECK(service);
+
+  // Check if guest mode enforcement commandline switch or policy are provided.
+  if (command_line.HasSwitch(switches::kGuest) ||
+      service->GetBoolean(prefs::kBrowserGuestModeEnforced)) {
+    // Check if guest mode is allowed by policy.
+    if (service->GetBoolean(prefs::kBrowserGuestModeEnabled))
+      return true;
+    if (show_warning) {
+      LOG(WARNING) << "Guest mode disabled by policy, launching a normal "
+                   << "browser session.";
+    }
+  }
+#endif
+  return false;
+}
+
 #if !defined(OS_CHROMEOS)
-bool ShouldShowProfilePicker() {
+bool ShouldShowProfilePicker(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 (IsGuestModeEnforced(command_line, /*show_warning=*/false) ||
+      command_line.HasSwitch(switches::kIncognito) ||
+      command_line.HasSwitch(switches::kProfileDirectory)) {
+    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;
+  }
+
   size_t number_of_profiles = g_browser_process->profile_manager()
                                   ->GetProfileAttributesStorage()
                                   .GetNumberOfProfiles();
@@ -304,30 +354,6 @@ bool IsSilentLaunchEnabled(const base::CommandLine& command_line,
   return false;
 }
 
-// Returns true if starting in guest mode is enforced. If |show_warning| is
-// true, send a warning if guest mode is requested but not allowed by policy.
-bool IsGuestModeEnforced(const base::CommandLine& command_line,
-                         bool show_warning) {
-#if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_WIN) || \
-    defined(OS_MAC)
-  PrefService* service = g_browser_process->local_state();
-  DCHECK(service);
-
-  // Check if guest mode enforcement commandline switch or policy are provided.
-  if (command_line.HasSwitch(switches::kGuest) ||
-      service->GetBoolean(prefs::kBrowserGuestModeEnforced)) {
-    // Check if guest mode is allowed by policy.
-    if (service->GetBoolean(prefs::kBrowserGuestModeEnabled))
-      return true;
-    if (show_warning) {
-      LOG(WARNING) << "Guest mode disabled by policy, launching a normal "
-                   << "browser session.";
-    }
-  }
-#endif
-  return false;
-}
-
 }  // namespace
 
 StartupBrowserCreator::StartupBrowserCreator() = default;
@@ -818,7 +844,10 @@ bool StartupBrowserCreator::LaunchBrowserForLastProfiles(
     Profile* last_used_profile,
     const Profiles& last_opened_profiles) {
 #if !defined(OS_CHROMEOS)
-  if (ShouldShowProfilePicker()) {
+  const std::vector<GURL> urls_to_launch =
+      StartupBrowserCreator::GetURLsFromCommandLine(command_line, cur_dir,
+                                                    last_used_profile);
+  if (ShouldShowProfilePicker(command_line, urls_to_launch)) {
     ProfilePicker::Show();
     return true;
   }

chrome/browser/ui/startup/startup_browser_creator_browsertest.cc

@@ -48,6 +48,7 @@
 #include "chrome/browser/ui/startup/startup_browser_creator_impl.h"
 #include "chrome/browser/ui/startup/startup_tab_provider.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/common/buildflags.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
@@ -1882,4 +1883,110 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorInfobarsKioskTest,
               infobar->delegate()->GetIdentifier());
   }
 }
+
+// Checks the correct behavior of the profile picker on startup. This feature is
+// not available on ChromeOS.
+class StartupBrowserCreatorPickerTest : public InProcessBrowserTest {
+ public:
+  StartupBrowserCreatorPickerTest() {
+    scoped_feature_list_.InitAndEnableFeature(features::kNewProfilePicker);
+  }
+  ~StartupBrowserCreatorPickerTest() override = default;
+
+ protected:
+  void StartWithTwoProfiles(const base::CommandLine& command_line) {
+    Profile* profile = browser()->profile();
+    ProfileManager* profile_manager = g_browser_process->profile_manager();
+    // Create another profile.
+    base::FilePath dest_path = profile_manager->user_data_dir();
+    dest_path = dest_path.Append(FILE_PATH_LITERAL("New Profile 1"));
+
+    Profile* other_profile = nullptr;
+    {
+      base::ScopedAllowBlockingForTesting allow_blocking;
+      other_profile = profile_manager->GetProfile(dest_path);
+    }
+    ASSERT_TRUE(other_profile);
+
+    StartupBrowserCreator launch;
+    ASSERT_TRUE(launch.Start(command_line, profile_manager->user_data_dir(),
+                             profile, {}));
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(StartupBrowserCreatorPickerTest);
+};
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorPickerTest, OpensPicker) {
+  ASSERT_EQ(1u, chrome::GetTotalBrowserCount());
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  StartWithTwoProfiles(command_line);
+
+  // Opens the picker and thus does not open any new browser window for the main
+  // profile.
+  EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
+}
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorPickerTest, SkipsPickerWithURL) {
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  GURL url("https://www.foo.com/");
+  command_line.AppendArg(url.spec());
+
+  StartWithTwoProfiles(command_line);
+
+  // Skips the picker and creates a new browser window.
+  Browser* new_browser = FindOneOtherBrowser(browser());
+  EXPECT_TRUE(new_browser);
+  TabStripModel* tab_strip = new_browser->tab_strip_model();
+  ASSERT_EQ(1, tab_strip->count());
+  EXPECT_EQ(url, tab_strip->GetWebContentsAt(0)->GetURL());
+}
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorPickerTest, SkipsPickerWithGuest) {
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  command_line.AppendSwitch(switches::kGuest);
+
+  // The guest profile needs to get created before it gets opened.
+  Profile* guest_profile = nullptr;
+  {
+    base::ScopedAllowBlockingForTesting allow_blocking;
+    ProfileManager* profile_manager = g_browser_process->profile_manager();
+    guest_profile =
+        profile_manager->GetProfile(profile_manager->GetGuestProfilePath());
+  }
+
+  StartWithTwoProfiles(command_line);
+  // Skips the picker and creates a new browser window for the guest profile (by
+  // definition, it opens the OTR profile).
+  Browser* new_browser =
+      chrome::FindBrowserWithProfile(guest_profile->GetOffTheRecordProfile());
+  EXPECT_TRUE(new_browser);
+}
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorPickerTest,
+                       SkipsPickerWithIncognito) {
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  command_line.AppendSwitch(switches::kIncognito);
+
+  StartWithTwoProfiles(command_line);
+
+  // Skips the picker and creates a new browser window.
+  Browser* new_browser = chrome::FindBrowserWithProfile(
+      browser()->profile()->GetOffTheRecordProfile());
+  EXPECT_TRUE(new_browser);
+}
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorPickerTest,
+                       SkipsPickerWithProfileDirectory) {
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  command_line.AppendSwitch(switches::kProfileDirectory);
+
+  StartWithTwoProfiles(command_line);
+
+  // Skips the picker and creates a new browser window.
+  Browser* new_browser = FindOneOtherBrowser(browser());
+  EXPECT_TRUE(new_browser);
+}
 #endif  // !defined(OS_CHROMEOS)