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 54d468b826f90f25d1ea29eb5efde8edd6f1cce8..5df946fc90a6a583fb602b659437d3843ad364c2 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"
@@ -39,6 +40,7 @@
 #include "chrome/browser/extensions/launch_util.h"
 #include "chrome/browser/extensions/pack_extension_job.h"
 #include "chrome/browser/first_run/first_run.h"
+#include "chrome/browser/first_run/first_run_features.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
@@ -499,6 +501,12 @@ bool StartupBrowserCreatorImpl::OpenApplicationWindow(Profile* profile) {
 void StartupBrowserCreatorImpl::ProcessLaunchURLs(
     bool process_startup,
     const std::vector<GURL>& urls_to_open) {
+  if (base::FeatureList::IsEnabled(features::kUseConsolidatedFirstRun) &&
+      !first_run::IsWin10()) {

[grt (UTC plus 2), 2016/07/29 06:56:15]

i don't think "IsWin10" should be a public method in first_run.

+    ProcessLaunchURLsUsingConsolidatedFlow(process_startup, urls_to_open);
+    return;
+  }
+
   // Don't open any browser windows if we're starting up in "background mode".
   if (process_startup && command_line_.HasSwitch(switches::kNoStartupWindow))
     return;
@@ -566,6 +574,77 @@ void StartupBrowserCreatorImpl::ProcessLaunchURLs(
   AddInfoBarsIfNecessary(browser, is_process_startup);
 }
 
+void StartupBrowserCreatorImpl::ProcessLaunchURLsUsingConsolidatedFlow(
+    bool process_startup,
+    const std::vector<GURL>& urls_to_open) {
+  std::vector<GURL> adjusted_urls(urls_to_open);
+
+  // Don't open any browser windows if we're starting up in "background mode".
+  if (process_startup && command_line_.HasSwitch(switches::kNoStartupWindow))
+    return;
+
+// 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_);

[grt (UTC plus 2), 2016/07/29 06:56:15]

metro mode is gone. i don't think this dead code needs to be here any longer if
it truly is only for metro->desktop transitions

+  }
+#endif
+
+  // If Master Prefs contains any first run tabs, use only those.
+  std::vector<GURL> tabs_to_insert;
+  if (browser_creator_) {
+    tabs_to_insert =
+        first_run::ProcessMasterPrefsTabs(browser_creator_->first_run_tabs_);
+    browser_creator_->first_run_tabs_.clear();
+  }
+  // Otherwise, use the standard Onboarding logic to determine which, if any,
+  // tabs need to be surfaced.
+  if (tabs_to_insert.empty()) {
+    tabs_to_insert = first_run::GetOnboardingTabs();
+  }
+
+  // If this profile is marked for a reset prompt, put this before all other
+  // tabs.
+  if (ProfileHasResetTrigger()) {
+    tabs_to_insert.insert(tabs_to_insert.begin(),
+                          internals::GetTriggeredResetSettingsURL());
+  }
+
+  // TODO(tmartino): Function for processing pinned tabs.
+
+  // Prepend the resulting list of tabs to those passed in.
+  if (!tabs_to_insert.empty()) {
+    adjusted_urls.insert(adjusted_urls.begin(), tabs_to_insert.begin(),
+                         tabs_to_insert.end());
+  }
+
+  // TODO(tmartino): If this is not process startup, we need to restore
+  // asynchronously and return here.
+
+  // See if we can or should restore an old session synchronously.
+  Browser* browser = MaybeRestoreSession(adjusted_urls, process_startup);

[grt (UTC plus 2), 2016/07/29 06:56:15]

this is mixing policy with functionality. can you tease these apart? is it
possible to have different phases (even classes, if that makes sense) such as:
- gather inputs
- make policy decisions that result in outputs such as:
  - whether or not any browser windows should be created
  - the ordered list of URLs to open
  - whether or not any infobars (and which ones) should be included
 - act on the policy decisions

unittesting the second step will then be a simple matter of craft inputs, run
the policy function, and verify the outputs.

+
+  // Otherwise, open.
+  if (!browser) {
+    browser = OpenURLsInBrowser(browser, process_startup, adjusted_urls);
+  }
+
+  // Finally, add info bars.
+  chrome::startup::IsProcessStartup is_process_startup =
+      process_startup ? chrome::startup::IS_PROCESS_STARTUP
+                      : chrome::startup::IS_NOT_PROCESS_STARTUP;
+  AddInfoBarsIfNecessary(browser, is_process_startup);
+}
+
+Browser* StartupBrowserCreatorImpl::MaybeRestoreSession(
+    const std::vector<GURL>& urls,
+    bool is_process_startup) {
+  // TODO(tmartino): Move session restore logic into this function.
+  return NULL;
+}
+
 bool StartupBrowserCreatorImpl::ProcessStartupURLs(
     const std::vector<GURL>& urls_to_open) {
   VLOG(1) << "StartupBrowserCreatorImpl::ProcessStartupURLs";
@@ -648,6 +727,9 @@ bool StartupBrowserCreatorImpl::ProcessStartupURLs(
 
 Browser* StartupBrowserCreatorImpl::ProcessSpecifiedURLs(
     const std::vector<GURL>& urls_to_open) {
+  // TODO(tmartino): Deprecated, remove this once UseConsolidatedFirstRun is
+  // enabled.
+
   SessionStartupPref pref =
       StartupBrowserCreator::GetSessionStartupPref(command_line_, profile_);
   StartupTabs tabs;
@@ -826,8 +908,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 UseConsolidatedFirstRun is
+  // enabled.
 
   // If we have urls specified by the first run master preferences use them
   // and nothing else.
@@ -893,6 +975,9 @@ void StartupBrowserCreatorImpl::AddStartupURLs(
 
 void StartupBrowserCreatorImpl::AddSpecialURLs(
     std::vector<GURL>* url_list) const {
+  // TODO(tmartino): Deprecated, remove this once UseConsolidatedFirstRun is
+  // enabled.
+
   // Optionally include the welcome page.
   if (welcome_run_type_ == WelcomeRunType::FIRST_TAB)
     url_list->insert(url_list->begin(), internals::GetWelcomePageURL());
@@ -910,6 +995,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 UseConsolidatedFirstRun is
+// enabled.
 void StartupBrowserCreatorImpl::InitializeWelcomeRunType(
     const std::vector<GURL>& urls_to_open) {
   DCHECK_EQ(static_cast<int>(WelcomeRunType::NONE),