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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
 
 #include <stddef.h>
 
 #include <algorithm>  // For max().
 #include <limits>
@@ skipping 244 matching lines @@
 // The file is overwritten if it exists. This function should only be called in
 // the blocking pool.
 void DumpBrowserHistograms(const base::FilePath& output_file) {
   base::ThreadRestrictions::AssertIOAllowed();
 
   std::string output_string(base::StatisticsRecorder::ToJSON(std::string()));
   base::WriteFile(output_file, output_string.data(),
                   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);
+          .GetProfileAttributesWithPath(profile->GetPath(), &entry) &&
+      entry->IsSigninRequired()) {
+    return false;
+  }
 
-  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;
-    }
-  }
+  // 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) ?
           profiles::USER_MANAGER_SELECT_PROFILE_APP_LAUNCHER :
           profiles::USER_MANAGER_SELECT_PROFILE_NO_ACTION;
   UserManager::Show(
       base::FilePath(), profiles::USER_MANAGER_NO_TUTORIAL, action);
   return true;
-#endif
 }
 
 }  // namespace
 
 StartupBrowserCreator::StartupBrowserCreator()
     : is_default_browser_dialog_suppressed_(false),
       show_main_browser_window_(true) {}
 
 StartupBrowserCreator::~StartupBrowserCreator() {}
 
@@ skipping 268 matching lines @@
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   TRACE_EVENT0("startup", "StartupBrowserCreator::ProcessCmdLineImpl");
 
   DCHECK(last_used_profile);
   if (process_startup) {
     if (command_line.HasSwitch(switches::kDisablePromptOnRepost))
       content::NavigationController::DisablePromptOnRepost();
   }
 
   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;
   }
 #endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW)
 
   if (command_line.HasSwitch(switches::kExplicitlyAllowedPorts)) {
     std::string allowed_ports =
         command_line.GetSwitchValueASCII(switches::kExplicitlyAllowedPorts);
     net::SetExplicitlyAllowedPorts(allowed_ports);
@@ skipping 71 matching lines @@
   // If --no-startup-window is specified and Chrome is already running then do
   // not open a new window.
   if (!process_startup && command_line.HasSwitch(switches::kNoStartupWindow))
     silent_launch = true;
 
   // If we don't want to launch a new browser window or tab we are done here.
   if (silent_launch)
     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;
 
     // Return early here to avoid opening a browser window.
     // The exception is when there are no browser windows, since we don't want
     // chrome to shut down.
     // TODO(jackhou): Do this properly once keep-alive is handled by the
     // background page of apps. Tracked at http://crbug.com/175381
     if (chrome::GetBrowserCount(last_used_profile) != 0)
       return true;
   }
 
   // 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);
 
     if (!apps::AppLoadService::Get(last_used_profile)->LoadAndLaunch(
             base::FilePath(path), command_line, cur_dir)) {
       return false;
     }
 
     // Return early here since we don't want to open a browser window.
     // The exception is when there are no browser windows, since we don't want
@@ skipping 274 matching lines @@
     if (!entry->IsSigninRequired()) {
       Profile* profile = profile_manager->GetProfile(entry->GetPath());
       if (profile)
         return profile;
     }
   }
 
   return nullptr;
 }
 #endif  // !defined(OS_CHROMEOS) && !defined(OS_ANDROID)