Move some command line handling to ProcessCmdlineImpl.

Some command line handling only needs to be done once, instead of
once per profile. This CL moves handling of kNotificationLaunchId and
kGcpwSigninSwitch from StartupBrowserCreatorImpl::Launch to
StartupBrowserCreator::ProcessCmdLineImpl.

Bug: 1132578
Change-Id: Ia7d0dbbb7ca3fe8c9a1ce04720c0e2205aff7678
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2491983
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820646}

chrome/browser/ui/startup/startup_browser_creator.cc

@@ -99,7 +99,9 @@
 #if defined(OS_WIN)
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/metrics/jumplist_metrics_win.h"
+#include "chrome/browser/notifications/notification_platform_bridge_win.h"
 #include "chrome/browser/notifications/win/notification_launch_id.h"
+#include "chrome/browser/ui/startup/credential_provider_signin_dialog_win.h"
 #include "chrome/browser/ui/webui/settings/reset_settings_handler.h"
 #include "chrome/credential_provider/common/gcp_strings.h"
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
@@ -411,6 +413,10 @@ bool StartupBrowserCreator::LaunchBrowser(
     chrome::startup::IsFirstRun is_first_run,
     std::unique_ptr<LaunchModeRecorder> launch_mode_recorder) {
   DCHECK(profile);
+#if defined(OS_WIN)
+  DCHECK(!command_line.HasSwitch(credential_provider::kGcpwSigninSwitch));
+  DCHECK(!command_line.HasSwitch(switches::kNotificationLaunchId));
+#endif  // defined(OS_WIN)
   in_synchronous_profile_launch_ =
       process_startup == chrome::startup::IS_PROCESS_STARTUP;
 
@@ -431,12 +437,6 @@ bool StartupBrowserCreator::LaunchBrowser(
       profile = profile->GetPrimaryOTRProfile();
   }
 
-#if defined(OS_WIN)
-  // Continue with the incognito profile if this is a credential provider logon.
-  if (command_line.HasSwitch(credential_provider::kGcpwSigninSwitch))
-    profile = profile->GetPrimaryOTRProfile();
-#endif
-
   if (!IsSilentLaunchEnabled(command_line, profile)) {
     StartupBrowserCreatorImpl lwp(cur_dir, command_line, this, is_first_run);
     const std::vector<GURL> urls_to_launch =
@@ -852,6 +852,35 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
         chrome::kJumpListIconDirname,
         base::WrapUnique(new base::SupportsUserData::Data()));
   }
+
+  // If the command line has the kNotificationLaunchId switch, then this
+  // call is from notification_helper.exe to process toast activation.
+  // Delegate to the notification system; do not open a browser window here.
+  if (command_line.HasSwitch(switches::kNotificationLaunchId)) {
+    if (NotificationPlatformBridgeWin::HandleActivation(command_line)) {
+      LaunchModeRecorder().SetLaunchMode(LaunchMode::kWinPlatformNotification);
+      return true;
+    }
+    return false;
+  }
+
+  // If being started for credential provider logon purpose, only show the
+  // signin page.
+  if (command_line.HasSwitch(credential_provider::kGcpwSigninSwitch)) {
+    // Use incognito profile since this is a credential provider logon.
+    Profile* profile = last_used_profile->GetPrimaryOTRProfile();
+    DCHECK(profile->IsIncognitoProfile());
+    // NOTE: All launch urls are ignored when running with --gcpw-signin since
+    // this mode only loads Google's sign in page.
+
+    // If GCPW signin dialog fails, returning false here will allow Chrome to
+    // exit gracefully during the launch.
+    if (!StartGCPWSignin(command_line, profile))
+      return false;
+
+    LaunchModeRecorder().SetLaunchMode(LaunchMode::kCredentialProviderSignIn);
+    return true;
+  }
 #endif  // defined(OS_WIN)
 
   return LaunchBrowserForLastProfiles(command_line, cur_dir, process_startup,

chrome/browser/ui/startup/startup_browser_creator.h

@@ -114,6 +114,10 @@ class StartupBrowserCreator {
                            ReadingWasRestartedAfterRestart);
   FRIEND_TEST_ALL_PREFIXES(StartupBrowserCreatorTest, UpdateWithTwoProfiles);
   FRIEND_TEST_ALL_PREFIXES(StartupBrowserCreatorTest, LastUsedProfileActivated);
+  FRIEND_TEST_ALL_PREFIXES(StartupBrowserCreatorTest,
+                           ValidNotificationLaunchId);
+  FRIEND_TEST_ALL_PREFIXES(StartupBrowserCreatorTest,
+                           InvalidNotificationLaunchId);
 
   bool ProcessCmdLineImpl(const base::CommandLine& command_line,
                           const base::FilePath& cur_dir,

chrome/browser/ui/startup/startup_browser_creator_browsertest.cc

@@ -545,12 +545,9 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest, ValidNotificationLaunchId) {
   command_line.AppendSwitchNative(
       switches::kNotificationLaunchId,
       L"1|1|0|Default|0|https://example.com/|notification_id");
-  chrome::startup::IsFirstRun first_run =
-      first_run::IsChromeFirstRun() ? chrome::startup::IS_FIRST_RUN
-                                    : chrome::startup::IS_NOT_FIRST_RUN;
-  StartupBrowserCreatorImpl launch(base::FilePath(), command_line, first_run);
-  ASSERT_TRUE(
-      launch.Launch(browser()->profile(), std::vector<GURL>(), false, nullptr));
+
+  ASSERT_TRUE(StartupBrowserCreator().ProcessCmdLineImpl(
+      command_line, base::FilePath(), false, browser()->profile(), {}));
 
   // The launch delegates to the notification system and doesn't open any new
   // browser window.
@@ -561,12 +558,9 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest, InvalidNotificationLaunchId) {
   // Simulate a launch with invalid launch id, which will fail.
   base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
   command_line.AppendSwitchNative(switches::kNotificationLaunchId, L"");
-  chrome::startup::IsFirstRun first_run =
-      first_run::IsChromeFirstRun() ? chrome::startup::IS_FIRST_RUN
-                                    : chrome::startup::IS_NOT_FIRST_RUN;
-  StartupBrowserCreatorImpl launch(base::FilePath(), command_line, first_run);
-  ASSERT_FALSE(
-      launch.Launch(browser()->profile(), std::vector<GURL>(), false, nullptr));
+  StartupBrowserCreator browser_creator;
+  ASSERT_FALSE(StartupBrowserCreator().ProcessCmdLineImpl(
+      command_line, base::FilePath(), false, browser()->profile(), {}));
 
   // No new browser window is open.
   ASSERT_EQ(1u, chrome::GetBrowserCount(browser()->profile()));

chrome/browser/ui/startup/startup_browser_creator_impl.cc

@@ -72,14 +72,9 @@
 #include "chrome/browser/ui/startup/mac_system_infobar_delegate.h"
 #endif
 
-#if defined(OS_WIN)
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
+#if defined(OS_WIN) && BUILDFLAG(GOOGLE_CHROME_BRANDING)
 #include "chrome/browser/win/conflicts/incompatible_applications_updater.h"
-#endif  // BUILDFLAG(GOOGLE_CHROME_BRANDING)
-#include "chrome/browser/notifications/notification_platform_bridge_win.h"
-#include "chrome/browser/ui/startup/credential_provider_signin_dialog_win.h"
-#include "chrome/credential_provider/common/gcp_strings.h"
-#endif  // defined(OS_WIN)
+#endif
 
 #if BUILDFLAG(ENABLE_PLUGINS)
 #include "chrome/browser/plugins/flash_deprecation_infobar_delegate.h"
@@ -201,40 +196,6 @@ bool StartupBrowserCreatorImpl::Launch(
   DCHECK(profile);
   profile_ = profile;
 
-#if defined(OS_WIN)
-  // If the command line has the kNotificationLaunchId switch, then this
-  // Launch() call is from notification_helper.exe to process toast activation.
-  // Delegate to the notification system; do not open a browser window here.
-  if (command_line_.HasSwitch(switches::kNotificationLaunchId)) {
-    if (NotificationPlatformBridgeWin::HandleActivation(command_line_)) {
-      if (launch_mode_recorder) {
-        launch_mode_recorder->SetLaunchMode(
-            LaunchMode::kWinPlatformNotification);
-      }
-      return true;
-    }
-    return false;
-  }
-  // If being started for credential provider logon purpose, only show the
-  // signin page.
-  if (command_line_.HasSwitch(credential_provider::kGcpwSigninSwitch)) {
-    DCHECK(profile_->IsIncognitoProfile());
-    // NOTE: All launch urls are ignored when running with --gcpw-signin since
-    // this mode only loads Google's sign in page.
-
-    // If GCPW signin dialog fails, returning false here will allow Chrome to
-    // exit gracefully during the launch.
-    if (!StartGCPWSignin(command_line_, profile_))
-      return false;
-
-    if (launch_mode_recorder) {
-      launch_mode_recorder->SetLaunchMode(
-          LaunchMode::kCredentialProviderSignIn);
-    }
-    return true;
-  }
-#endif  // defined(OS_WIN)
-
   if (command_line_.HasSwitch(switches::kAppId)) {
     std::string app_id = command_line_.GetSwitchValueASCII(switches::kAppId);
     // If |app_id| is a disabled or terminated platform app we handle it