Refactoring startup logic for upcoming FRE changes (non-Win 10).

chrome/browser/ui/startup/startup_browser_creator_impl.cc

Index: chrome/browser/ui/startup/startup_browser_creator_impl.cc
diff --git a/chrome/browser/ui/startup/startup_browser_creator_impl.cc b/chrome/browser/ui/startup/startup_browser_creator_impl.cc
index 0e66bf556c084f51720c1550641addcb1cd8e5de..4ffa4c2c21b20187235b1d733dd38eb51033f126 100644
--- a/chrome/browser/ui/startup/startup_browser_creator_impl.cc
+++ b/chrome/browser/ui/startup/startup_browser_creator_impl.cc
@@ -18,6 +18,7 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/environment.h"
+#include "base/feature_list.h"
 #include "base/lazy_instance.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/statistics_recorder.h"
@@ -70,6 +71,7 @@
 #include "chrome/browser/ui/startup/obsolete_system_infobar_delegate.h"
 #include "chrome/browser/ui/startup/session_crashed_infobar_delegate.h"
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
+#include "chrome/browser/ui/startup/startup_features.h"
 #include "chrome/browser/ui/tabs/pinned_tab_codec.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_constants.h"
@@ -501,6 +503,13 @@ bool StartupBrowserCreatorImpl::OpenApplicationWindow(Profile* profile) {
 void StartupBrowserCreatorImpl::ProcessLaunchURLs(
     bool process_startup,
     const std::vector<GURL>& urls_to_open) {
+  if (base::FeatureList::IsEnabled(features::kUseConsolidatedStartupFlow)) {
+    ProcessLaunchUrlsUsingConsolidatedFlow(process_startup, urls_to_open);
+    return;
+  }
+  // TODO(tmartino): Remainder of this function is deprecated. Remove when
+  // kUseConsolidatedStartupFlow is on by default.
+
   // Don't open any browser windows if we're starting up in "background mode".
   if (process_startup && command_line_.HasSwitch(switches::kNoStartupWindow))
     return;
@@ -508,15 +517,6 @@ void StartupBrowserCreatorImpl::ProcessLaunchURLs(
   // Determine whether or not this launch must include the welcome page.
   InitializeWelcomeRunType(urls_to_open);
 
-// TODO(tapted): Move this to startup_browser_creator_win.cc after refactor.
-#if defined(OS_WIN)
-  if (base::win::GetVersion() >= base::win::VERSION_WIN8) {
-    // See if there are apps for this profile that should be launched on startup
-    // due to a switch from Metro mode.
-    app_metro_launch::HandleAppLaunchForMetroRestart(profile_);
-  }
-#endif
-
   if (process_startup && ProcessStartupURLs(urls_to_open)) {
     // ProcessStartupURLs processed the urls, nothing else to do.
     return;
@@ -568,6 +568,96 @@ void StartupBrowserCreatorImpl::ProcessLaunchURLs(
   AddInfoBarsIfNecessary(browser, is_process_startup);
 }
 
+void StartupBrowserCreatorImpl::ProcessLaunchUrlsUsingConsolidatedFlow(
+    bool process_startup,
+    const std::vector<GURL>& cmd_line_urls) {
+  // Don't open any browser windows if starting up in "background mode".
+  if (process_startup && command_line_.HasSwitch(switches::kNoStartupWindow))
+    return;
+
+  // If command-line URLs were passed, they supercede all other policies and

[grt (UTC plus 2), 2016/09/21 08:59:08]

at least some(!) of the old codepaths put the settings reset tab in front of
URLs passed on the command line. please check with robertshield to see if that
was intentional and if it should be carried forward. off the top of my head, i
think it should.

+  // logic.
+  StartupTabs tabs;
+  if (!cmd_line_urls.empty()) {
+    UrlsToTabs(cmd_line_urls, &tabs);
+  } else {
+    bool is_incognito = IncognitoModePrefs::ShouldLaunchIncognito(
+        command_line_, profile_->GetPrefs());
+    bool is_crash = HasPendingUncleanExit(profile_);
+    tabs =
+        DetermineStartupTabs(StartupTabProviderImpl(), is_incognito, is_crash);
+  }
+
+  // TODO(tmartino): If this is not process startup, attempt to restore
+  // asynchronously and return here. This logic is self-contained in
+  // SessionService and therefore can't be combined with the other Browser
+  // creation logic.
+
+  // TODO(tmartino): Function which determines what behavior of session
+  // restore, if any, is necessary. Incorporates code from ProcessStartupUrls.
+
+  Browser* browser =
+      RestoreOrCreateBrowser(process_startup, tabs
+                             /* TODO(tmartino): Also pass behavior here */);
+
+  // Finally, add info bars.
+  AddInfoBarsIfNecessary(
+      browser, process_startup ? chrome::startup::IS_PROCESS_STARTUP
+                               : chrome::startup::IS_NOT_PROCESS_STARTUP);
+}
+
+StartupTabs StartupBrowserCreatorImpl::DetermineStartupTabs(
+    const StartupTabProvider& provider,
+    bool is_incognito,
+    bool is_post_crash_launch) {
+  StartupTabs tabs;
+
+  // No policy- or profile-related content is shown in incognito mode.
+  // Similarly, when recovering from a crash, nothing should be opened
+  // automatically to prevent getting the user stuck in a crash loop.
+  if (!is_incognito && !is_post_crash_launch) {
+    // A Master Preferences file provided with this distribution may specify
+    // tabs to be displayed on first run, overriding any other tabs which would
+    // normally be shown.
+    if (provider.AddDistributionFirstRunTabs(browser_creator_, &tabs)) {

[grt (UTC plus 2), 2016/09/21 08:59:08]

nit: omit braces :-)

+      return tabs;
+    }
+
+    // Reads and adds any tabs which the user has previously pinned. Pinned tabs
+    // always appear as the leftmost tabs; insert at the beginning for
+    // consistency's sake.
+    provider.AddPinnedTabs(&tabs);
+
+    // Policies for onboarding (e.g., first run) may show promotional and
+    // introductory content depending on a number of system status factors,
+    // including OS and whether or not this is First Run.
+    provider.AddOnboardingTabs(&tabs);
+
+    // A trigger on a profile may indicate that we should show a tab which
+    // offers to reset the user's settings.
+    provider.AddResetTriggerTabs(profile_, &tabs);

[grt (UTC plus 2), 2016/09/21 08:59:08]

i think the settings reset page is supposed to be the first tab the user sees,
so it should be the first one added, no?

+
+    // If the user has set the preference indicating URLs to show on opening,
+    // read and add those.
+    provider.AddPreferencesTabs(&tabs);
+  }
+
+  // Default to showing New Tab Page when nothing else is cued up.
+  if (tabs.empty())
+    tabs.push_back(StartupTab(GURL(chrome::kChromeUINewTabURL), false));

[grt (UTC plus 2), 2016/09/21 08:59:08]

the old code would add the NTP if no URLs were specified on the command line or
in first_run_tabs. this also suppresses it if the settings reset page is shown
or if tabs are pinned. is this intentional?

+
+  return tabs;
+}
+
+Browser* StartupBrowserCreatorImpl::RestoreOrCreateBrowser(
+    bool process_startup,
+    const StartupTabs& tabs) {
+  // TODO(tmartino): Based on passed behavior flag, possibly restore session
+  // instead of creating a new Browser.
+
+  return OpenTabsInBrowser(nullptr, process_startup, tabs);
+}
+
 bool StartupBrowserCreatorImpl::ProcessStartupURLs(
     const std::vector<GURL>& urls_to_open) {
   VLOG(1) << "StartupBrowserCreatorImpl::ProcessStartupURLs";
@@ -650,6 +740,9 @@ bool StartupBrowserCreatorImpl::ProcessStartupURLs(
 
 Browser* StartupBrowserCreatorImpl::ProcessSpecifiedURLs(
     const std::vector<GURL>& urls_to_open) {
+  // TODO(tmartino): Deprecated, remove this once UseConsolidatedStartupFlow is
+  // enabled.
+
   SessionStartupPref pref =
       StartupBrowserCreator::GetSessionStartupPref(command_line_, profile_);
   StartupTabs tabs;
@@ -829,8 +922,8 @@ void StartupBrowserCreatorImpl::AddInfoBarsIfNecessary(
 
 void StartupBrowserCreatorImpl::AddStartupURLs(
     std::vector<GURL>* startup_urls) const {
-  // TODO(atwilson): Simplify the logic that decides which tabs to open on
-  // start-up and make it more consistent. http://crbug.com/248883
+  // TODO(tmartino): Deprecated, remove this once UseConsolidatedStartupFlow is
+  // enabled.
 
   // If we have urls specified by the first run master preferences use them
   // and nothing else.
@@ -896,6 +989,9 @@ void StartupBrowserCreatorImpl::AddStartupURLs(
 
 void StartupBrowserCreatorImpl::AddSpecialURLs(
     std::vector<GURL>* url_list) const {
+  // TODO(tmartino): Deprecated, remove this once UseConsolidatedStartupFlow is
+  // enabled.
+
   // Optionally include the welcome page.
   if (welcome_run_type_ == WelcomeRunType::FIRST_TAB)
     url_list->insert(url_list->begin(), internals::GetWelcomePageURL());
@@ -913,6 +1009,9 @@ void StartupBrowserCreatorImpl::AddSpecialURLs(
 // will be NONE for all systems except for Windows 10+, where it will be
 // ANY_RUN_FIRST if this is the first somewhat normal launch since an OS
 // upgrade.
+
+// TODO(tmartino): Deprecated, remove this once UseConsolidatedStartupFlow is
+// enabled.
 void StartupBrowserCreatorImpl::InitializeWelcomeRunType(
     const std::vector<GURL>& urls_to_open) {
   DCHECK_EQ(static_cast<int>(WelcomeRunType::NONE),
@@ -987,6 +1086,8 @@ void StartupBrowserCreatorImpl::RecordRapporOnStartupURLs(
   }
 }
 
+// TODO(tmartino): Deprecated, remove this once UseConsolidatedStartupFlow is
+// enabled.
 bool StartupBrowserCreatorImpl::ProfileHasResetTrigger() const {
   bool has_reset_trigger = false;
 #if defined(OS_WIN)