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..db8c6655c3c824f384fd5de4eeb2b9ca8e191a6a 100644
--- a/chrome/browser/ui/startup/startup_browser_creator.cc
+++ b/chrome/browser/ui/startup/startup_browser_creator.cc
@@ -262,34 +262,42 @@ 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) {
+// Returns true if we should show the user manager on startup.

[Peter Kasting, 2016/12/22 00:26:33]

This comment is inaccurate.  How about:

// Returns whether |profile| can be opened during Chrome startup without
// explicit user action.
bool ProfileCanBeAutoOpened(...) {

This also renames the function, since "available" feels like a bit of a misnomer
when certain profiles are available after you take an explicit action (e.g.
signing in).

[zmin, 2016/12/22 18:49:45]

Done.

+bool IsProfileAvailable(Profile* profile,
+                        bool allow_incognito,
+                        const base::CommandLine& command_line) {
+  // Profiles are not available if browser needs to be launched with incognito
+  // mode with command line options that do not compatible with incognito mode.

[Peter Kasting, 2016/12/22 00:26:33]

Nit: do -> are

[zmin, 2016/12/22 18:49:45]

Done.

+  if (!allow_incognito && IncognitoModePrefs::ShouldLaunchIncognito(
+                              command_line, profile->GetPrefs())) {
+    return false;
+  }

[Peter Kasting, 2016/12/22 00:26:33]

Nit: I don't think you should put this conditional in this function.  I think
you should do it in the caller at the site where you compute
|is_profile_available|.

Fundamentally, the incognito profile is "available", and can be auto-opened,
it's just that it's not suitable for certain command-line options.  So it seems
like a minor layering violation to put this here.

Hoisting it also allows the code in ShowUserManagerOnStartupIfNeeded() to not
worry about this.

[zmin, 2016/12/22 18:49:45]

Done.

 #if defined(OS_CHROMEOS)
-  // ChromeOS never shows the User Manager on startup.
-  return false;
+  // User will always choose and login profile before using Chrome on ChromeOS.

[Peter Kasting, 2016/12/22 00:26:33]

Nit: Grammar; how about: "On ChromeOS, the user has already chosen and logged
into the profile before Chrome starts up."

[zmin, 2016/12/22 18:49:45]

Done.

+  return true;
 #else
+  // Profiles that require signin are not available.
   ProfileAttributesEntry* entry = nullptr;
-  bool has_entry =
-      g_browser_process->profile_manager()
+  if (g_browser_process->profile_manager()
           ->GetProfileAttributesStorage()
-          .GetProfileAttributesWithPath(last_used_profile->GetPath(), &entry);
-
-  if (!has_entry || !entry->IsSigninRequired()) {
-    // Signin is not required. However, guest, system or locked profiles cannot
-    // be re-opened on startup. The only exception is if there's already a Guest
-    // window open in a separate process (for example, launching a new browser
-    // after clicking on a downloaded file in Guest mode).
-    if ((!last_used_profile->IsGuestSession() &&
-         !last_used_profile->IsSystemProfile()) ||
-        (chrome::GetBrowserCount(last_used_profile->GetOffTheRecordProfile()) >
-         0)) {
-      return false;
-    }
+          .GetProfileAttributesWithPath(profile->GetPath(), &entry) &&
+      entry->IsSigninRequired()) {
+    return false;
   }
 
+  // Guest or system profiles are not available unless a separate process
+  // already has a window open for the profile,

[Peter Kasting, 2016/12/22 00:26:33]

Nit: , -> .

[zmin, 2016/12/22 18:49:45]

Done.

+  return (!profile->IsGuestSession() && !profile->IsSystemProfile()) ||
+         (chrome::GetBrowserCount(profile->GetOffTheRecordProfile()) > 0);
+#endif
+}
+
+// Returns whether the User Manager was shown.
+bool ShowUserManagerOnStartupIfNeeded(Profile* last_used_profile,
+                                      const base::CommandLine& command_line) {
+  if (IsProfileAvailable(last_used_profile, false, command_line))
+    return false;
+
   // Show the User Manager.
   profiles::UserManagerAction action =
       command_line.HasSwitch(switches::kShowAppList) ?
@@ -298,7 +306,6 @@ bool ShowUserManagerOnStartupIfNeeded(
   UserManager::Show(
       base::FilePath(), profiles::USER_MANAGER_NO_TUTORIAL, action);
   return true;
-#endif
 }
 
 }  // namespace
@@ -587,11 +594,14 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
   }
 
   bool silent_launch = false;
+  bool is_profile_available =

[Peter Kasting, 2016/12/22 00:26:33]

Nit: Maybe call this |can_use_last_profile|, which better accommodates the "can
be auto opened + is not an incompatible profile" concepts from above.

[zmin, 2016/12/22 18:49:45]

Done.

+      IsProfileAvailable(last_used_profile, true, command_line);
 
 #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) &&
+      is_profile_available &&
       print_dialog_cloud::CreatePrintDialogFromCommandLine(last_used_profile,
                                                            command_line)) {
     silent_launch = true;
@@ -683,8 +693,7 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
     return true;
 
   if (command_line.HasSwitch(extensions::switches::kLoadApps) &&
-      !IncognitoModePrefs::ShouldLaunchIncognito(
-          command_line, last_used_profile->GetPrefs())) {
+      is_profile_available) {
     if (!ProcessLoadApps(command_line, cur_dir, last_used_profile))
       return false;
 
@@ -698,9 +707,7 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
   }
 
   // Check for --load-and-launch-app.
-  if (command_line.HasSwitch(apps::kLoadAndLaunchApp) &&
-      !IncognitoModePrefs::ShouldLaunchIncognito(
-          command_line, last_used_profile->GetPrefs())) {
+  if (command_line.HasSwitch(apps::kLoadAndLaunchApp) && is_profile_available) {
     base::CommandLine::StringType path =
         command_line.GetSwitchValueNative(apps::kLoadAndLaunchApp);