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..4b46ff6900ac212270d275c36d29978085335b80 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,84 @@ 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
+  // logic.
+  StartupTabs tabs;
+  if (!cmd_line_urls.empty()) {
+    UrlsToTabs(cmd_line_urls, &tabs);
+  } else {
+    std::unique_ptr<StartupTabProvider> provider(new StartupTabProviderImpl());

[grt (UTC plus 2), 2016/09/12 07:28:46]

regardless of whether you pass this instance to callees by pointer or by
constref, there should be no need to create it on the heap rather than on the
stack.

+    bool is_incognito = IncognitoModePrefs::ShouldLaunchIncognito(
+        command_line_, profile_->GetPrefs());
+    bool is_crash = HasPendingUncleanExit(profile_);
+    tabs = DetermineStartupTabs(provider.get(), 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(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) {

[grt (UTC plus 2), 2016/09/12 07:28:46]

the body of this is so much easier to read than the old code!

+    if (provider->AddDistributionFirstRunTabs(browser_creator_, &tabs)) {
+      // Master Preferences first run content overrides any other policy.
+      return tabs;
+    }
+
+    // Pinned tabs always appear as the leftmost tabs; insert at the beginning
+    // for consistency's sake.
+    provider->AddPinnedTabs(&tabs);

[grt (UTC plus 2), 2016/09/12 07:28:46]

how about adding a very brief comment for each of these calls to explain why
they do or don't add any tabs? it saves the reader having to bounce around to
the impls to understand what will happen.

+
+    provider->AddOnboardingTabs(&tabs);
+
+    provider->AddResetTriggerTabs(profile_, &tabs);
+
+    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), 0));

[grt (UTC plus 2), 2016/09/12 07:28:46]

0 -> false

+
+  return tabs;
+}
+
+Browser* StartupBrowserCreatorImpl::RestoreOrCreateBrowser(StartupTabs tabs) {

[grt (UTC plus 2), 2016/09/12 07:28:46]

pass by constref

+  // TODO(tmartino): Based on passed behavior flag, possibly restore session
+  // instead of creating a new Browser.
+
+  return OpenTabsInBrowser(nullptr, true, tabs);

[grt (UTC plus 2), 2016/09/12 07:28:46]

why is |true| correct here rather than passing process_startup on down?

+}
+
 bool StartupBrowserCreatorImpl::ProcessStartupURLs(
     const std::vector<GURL>& urls_to_open) {
   VLOG(1) << "StartupBrowserCreatorImpl::ProcessStartupURLs";
@@ -650,6 +728,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 +910,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 +977,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 +997,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 +1074,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)