Disable --load-and-launch-app, --load-apps and --cloud-print-file if last_used_profile is not avail…

chrome/browser/ui/startup/startup_browser_creator.cc

Index: chrome/browser/ui/startup/startup_browser_creator.cc
diff --git a/chrome/browser/ui/startup/startup_browser_creator.cc b/chrome/browser/ui/startup/startup_browser_creator.cc
index 9caec3b3b51986a3d74f665f9aa47bbd0971989f..0301388c14a786c24c2d22685d1636bb319ed72f 100644
--- a/chrome/browser/ui/startup/startup_browser_creator.cc
+++ b/chrome/browser/ui/startup/startup_browser_creator.cc
@@ -262,15 +262,9 @@ void DumpBrowserHistograms(const base::FilePath& output_file) {
                   static_cast<int>(output_string.size()));
 }
 
-// Shows the User Manager on startup if the last used profile must sign in or
-// if the last used profile was the guest or system profile.
-// Returns true if the User Manager was shown, false otherwise.
-bool ShowUserManagerOnStartupIfNeeded(
-    Profile* last_used_profile, const base::CommandLine& command_line) {
-#if defined(OS_CHROMEOS)
-  // ChromeOS never shows the User Manager on startup.
-  return false;
-#else

[Peter Kasting, 2016/12/20 01:43:49]

It's not obvious to me from the new callers whether it was correct to move this
#ifdef out of this function.  Maybe this is really a question for each of those
new callsites: why is it important to depend on this function in each of those
spots?

[zmin, 2016/12/20 20:32:14]

I have moved the #ifdef to ShouldShowUserManagerOnStartup() from
ShowUserManagerOnStartupIfNeeded().

ShouldShowUserManagerOnStartup() will check whether the last_used_profile is
available. If not we will show UserManager(what we had already) and disable
--load-and-launch-app, --load-apps and --cloud-print-file(in this CL). However,
based on my understanding, user have to choose and login a profile before using
Chrome on ChromeOS. So there is no last_used_profile unavailable issue on
ChromeOS. So I think it makes sense to let ShouldShowUserManagerOnStartup()
always returns false on ChromeOS.

+// Return true if the last used profile must sign in or the last used profile

[Peter Kasting, 2016/12/20 01:43:49]

Nit: Return -> returns

[zmin, 2016/12/20 20:32:13]

Done.

+// was the guest or system profile. Otherwise, return false.

[Peter Kasting, 2016/12/20 01:43:49]

Nit: Remove last sentence, it's implied.

This comment is a bit more "what" than "why".  Maybe state the "why" a little
more instead, e.g.

Returns true if we should show the user manager on startup instead of the last
used profile.

(The "what" can be done as comments within the function.)

[zmin, 2016/12/20 20:32:13]

Done.

+bool ShouldShowUserManagerOnStartup(Profile* last_used_profile) {
   ProfileAttributesEntry* entry = nullptr;
   bool has_entry =
       g_browser_process->profile_manager()
@@ -289,6 +283,22 @@ bool ShowUserManagerOnStartupIfNeeded(
       return false;
     }
   }
+  return true;
+}
+
+// Shows the User Manager on startup if the last used profile must sign in or
+// if the last used profile was the guest or system profile.
+// Returns true if the User Manager was shown, false otherwise.

[Peter Kasting, 2016/12/20 01:43:49]

Nit: Or just "Returns whether the User Manager was shown."

[zmin, 2016/12/20 20:32:14]

Done.

+bool ShowUserManagerOnStartupIfNeeded(Profile* last_used_profile,
+                                      const base::CommandLine& command_line) {
+#if defined(OS_CHROMEOS)
+  // ChromeOS never shows the User Manager on startup.
+  return false;
+#else
+
+  if (!ShouldShowUserManagerOnStartup(last_used_profile)) {

[Peter Kasting, 2016/12/20 01:43:49]

Nit: {} unnecessary (and inconsistent with other places in this file, e.g. line
596)

[zmin, 2016/12/20 20:32:13]

Done.

+    return false;
+  }
 
   // Show the User Manager.
   profiles::UserManagerAction action =
@@ -587,11 +597,14 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
   }
 
   bool silent_launch = false;
+  bool should_show_user_manager =
+      ShouldShowUserManagerOnStartup(last_used_profile);
 
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
   // If we are just displaying a print dialog we shouldn't open browser
   // windows.
   if (command_line.HasSwitch(switches::kCloudPrintFile) &&
+      !should_show_user_manager &&
       print_dialog_cloud::CreatePrintDialogFromCommandLine(last_used_profile,
                                                            command_line)) {
     silent_launch = true;
@@ -683,6 +696,7 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
     return true;
 
   if (command_line.HasSwitch(extensions::switches::kLoadApps) &&
+      !should_show_user_manager &&
       !IncognitoModePrefs::ShouldLaunchIncognito(
           command_line, last_used_profile->GetPrefs())) {
     if (!ProcessLoadApps(command_line, cur_dir, last_used_profile))
@@ -699,6 +713,7 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
 
   // Check for --load-and-launch-app.
   if (command_line.HasSwitch(apps::kLoadAndLaunchApp) &&
+      !should_show_user_manager &&
       !IncognitoModePrefs::ShouldLaunchIncognito(
           command_line, last_used_profile->GetPrefs())) {
     base::CommandLine::StringType path =