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

chrome/browser/first_run/first_run.cc

Index: chrome/browser/first_run/first_run.cc
diff --git a/chrome/browser/first_run/first_run.cc b/chrome/browser/first_run/first_run.cc
index 72945a63b443c94302c9cb01c20f054ad132aca1..9d5426fb2fe828eb1c1d888fbbc478bf0a03f0f8 100644
--- a/chrome/browser/first_run/first_run.cc
+++ b/chrome/browser/first_run/first_run.cc
@@ -25,6 +25,7 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/updater/extension_updater.h"
+#include "chrome/browser/first_run/first_run_features.h"
 #include "chrome/browser/first_run/first_run_internal.h"
 #include "chrome/browser/google/google_brand.h"
 #include "chrome/browser/importer/external_process_importer_host.h"
@@ -82,6 +83,11 @@ using base::UserMetricsAction;
 
 namespace {
 
+// Constants: Magic words used by Master Prefs files to indicate that internal
+// pages should appear on first run.
+const char* kNewTabMagicWord = "new_tab_page";

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

use "UrlHost" or something rather than "MagicWord"

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

constexpr char kNewTabMagicWord[] = ...

[grt (UTC plus 2), 2016/08/01 06:23:14]

I find that "Keyword" is still vague. Neither the constant's name nor the doc
comment that accompanies it convey that the constant is the host part of an URL.
By replacing "Keyword" with "UrlHost" (and updating the comment), I think this
is fairly clear.

[tmartino, 2016/08/02 23:46:50]

Aha! In fact, I've been pointedly avoiding calling these UrlHost because they
explicitly are *not* hosts, or any other part of an actual URL. They're constant
strings, defined by convention, which partners include *in place* of a valid URL
in their Master Prefs file, and which we are replacing with something resolvable
in this function.

[grt (UTC plus 2), 2016/08/04 10:59:27]

I see it from the other side: they are magic values placed in the host portion
of a URL. The fact that they don't resolve is irrelevant. An URL is still an URL
if the host doesn't resolve, and the host portion of the URL is still the host
portion of the URL even if it's gibberish.

+const char* kWelcomePageMagicWord = "welcome_page";
+
 // A bitfield formed from values in AutoImportState to record the state of
 // AutoImport. This is used in testing to verify import startup actions that
 // occur before an observer can be registered in the test.
@@ -491,7 +497,6 @@ void ProcessDefaultBrowserPolicy(bool make_chrome_default_for_user) {
     }
   }
 }
-

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

nit: retain this blank line

 }  // namespace
 
 namespace first_run {
@@ -647,6 +652,11 @@ bool IsChromeFirstRun() {
   return internal::g_first_run == internal::FIRST_RUN_TRUE;
 }
 
+void SetFirstRunForTesting(bool is_first_run) {
+  internal::g_first_run =
+      is_first_run ? internal::FIRST_RUN_TRUE : internal::FIRST_RUN_FALSE;
+}
+
 #if defined(OS_MACOSX)
 bool IsFirstRunSuppressed(const base::CommandLine& command_line) {
   return command_line.HasSwitch(switches::kNoFirstRun);
@@ -714,6 +724,47 @@ bool ShouldShowWelcomePage() {
   return retval;
 }
 
+GURL GetWelcomePageURL() {
+  // TODO(tmartino): Once Consolidated FRE page is implemented, return that
+  // here when kUseConsolidatedFirstRun is true.
+  return GURL(l10n_util::GetStringUTF8(IDS_WELCOME_PAGE_URL));
+}
+
+bool IsWin10() {
+#if defined(OS_WIN)
+  return os_info->version() >= base::win::VERSION_WIN10;

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

base::win::GetVersion()

and #include "base/win/windows_version.h" wrapped in #if defined(OS_WIN) up
above

+#else
+  return false;
+#endif
+}
+
+std::vector<GURL> GetOnboardingTabs() {
+  std::vector<GURL> tabs;
+  if (IsWin10()) {
+    // TODO(tmartino): Win 10 logic is more complex and will be added in its
+    // own change.
+  } else if (IsChromeFirstRun()) {
+    tabs.push_back(GetWelcomePageURL());
+  }
+  return tabs;
+}
+
+std::vector<GURL> ProcessMasterPrefsTabs(const std::vector<GURL>& tabs) {
+  std::vector<GURL> processed_tabs;
+  if (IsChromeFirstRun()) {
+    for (GURL tab : tabs) {

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

const GURL& tab to avoid a copy here

+      if (tab.host() == kNewTabMagicWord) {
+        processed_tabs.push_back(GURL(chrome::kChromeUINewTabURL));
+      } else if (tab.host() == kWelcomePageMagicWord) {
+        processed_tabs.push_back(GetWelcomePageURL());
+      } else {
+        processed_tabs.push_back(tab);
+      }
+    }
+  }
+  return processed_tabs;
+}
+
 void SetShouldDoPersonalDataManagerFirstRun() {
   g_should_do_autofill_personal_data_manager_first_run = true;
 }