Some first_run code cleanups (extracted from https://codereview.chromium.org/12670013 to keep that one minimal and to the point).

A) CreateSentinel outside of AutoImport (it didn't belong in there). (this fixes issue 176354 as a side-effect).

B) Move first_run::MasterPrefsPath to first_run::internal:: (it wasn't used outside of first_run).

C) All decisions mapping MasterPreferences to MasterPrefs at first run should happen in SetupMasterPrefsFromInstallPrefs --> removed individual methods that did their own tweak.

D) MasterPrefs default values should be set even if MasterPreferences is NULL.

Precursor CL to https://codereview.chromium.org/12670013.

BUG=219419, 176354

Review URL: https://chromiumcodereview.appspot.com/14328019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@196317 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 61 insertions, 74 deletions

diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index d600b64..6f0d66f 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -903,9 +903,12 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() {
             prefs::kBrowserSuppressDefaultBrowserPrompt,
             master_prefs_->suppress_default_browser_prompt_for_version);
       }
-    } else if (parsed_command_line().HasSwitch(switches::kNoFirstRun)) {
-      // Create the First Run beacon anyways if --no-first-run was passed on the
-      // command line.
+    }
+
+    if (do_first_run_tasks_ ||
+        parsed_command_line().HasSwitch(switches::kNoFirstRun)) {
+      // Create the First Run sentinel whether first run tasks are executed
+      // or not.
       first_run::CreateSentinel();
     }
   }
diff --git a/chrome/browser/first_run/first_run.cc b/chrome/browser/first_run/first_run.cc
index 53a8db5..38ec392 100644
--- a/chrome/browser/first_run/first_run.cc
+++ b/chrome/browser/first_run/first_run.cc
@@ -9,6 +9,7 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
+#include "base/files/file_path.h"
 #include "base/lazy_instance.h"
 #include "base/metrics/histogram.h"
 #include "base/path_service.h"
@@ -63,8 +64,8 @@ using content::UserMetricsAction;
 namespace {
 
 // Flags for functions of similar name.
-bool should_show_welcome_page_ = false;
-bool should_do_autofill_personal_data_manager_first_run_ = false;
+bool g_should_show_welcome_page = false;
+bool g_should_do_autofill_personal_data_manager_first_run = false;
 
 // Flags indicating whether a first-run profile auto import was performed, and
 // whether the importer process exited successfully.
@@ -150,7 +151,7 @@ void SetImportItem(PrefService* user_prefs,
                    int import_items,
                    int dont_import_items,
                    importer::ImportItem import_type,
-                   int& items) {
+                   int* items) {
   // Work out whether an item is to be imported according to what is specified
   // in master preferences.
   bool should_import = false;
@@ -159,12 +160,11 @@ void SetImportItem(PrefService* user_prefs,
   bool master_pref = ((import_items & ~dont_import_items) & import_type) != 0;
 
   if (import_type == importer::HISTORY ||
-      ((import_type != importer::FAVORITES) &&
-      first_run::internal::IsOrganicFirstRun())) {
+      (import_type != importer::FAVORITES &&
+       first_run::internal::IsOrganicFirstRun())) {
     // History is always imported unless turned off in master_preferences.
-    // Search engines are only imported in certain builds unless overridden
-    // in master_preferences.Home page is imported in organic builds only unless
-    // turned off in master_preferences.
+    // Search engines and home page are imported in organic builds only
+    // unless turned off in master_preferences.
     should_import = !master_pref_set || master_pref;
   } else {
     // Bookmarks are never imported, unless turned on in master_preferences.
@@ -184,10 +184,10 @@ void SetImportItem(PrefService* user_prefs,
 
   if (!user_prefs->FindPreference(pref_path)->IsDefaultValue()) {
     if (user_prefs->GetBoolean(pref_path))
-      items |= import_type;
+      *items |= import_type;
   } else { // no policy (recommended or managed) is set
     if (should_import)
-      items |= import_type;
+      *items |= import_type;
   }
 
   user_prefs->ClearPref(pref_path);
@@ -283,6 +283,12 @@ bool CopyPrefFile(const base::FilePath& user_data_dir,
 void SetupMasterPrefsFromInstallPrefs(
     const installer::MasterPreferences& install_prefs,
     MasterPrefs* out_prefs) {
+  ConvertStringVectorToGURLVector(
+      install_prefs.GetFirstRunTabs(), &out_prefs->new_tabs);
+
+  install_prefs.GetInt(installer::master_preferences::kDistroPingDelay,
+                       &out_prefs->ping_delay);
+
   bool value = false;
   if (install_prefs.GetBool(
           installer::master_preferences::kDistroImportSearchPref, &value)) {
@@ -374,16 +380,6 @@ void SetDefaultBrowser(installer::MasterPreferences* install_prefs){
   }
 }
 
-void SetRLZPref(first_run::MasterPrefs* out_prefs,
-                installer::MasterPreferences* install_prefs) {
-  if (!install_prefs->GetInt(installer::master_preferences::kDistroPingDelay,
-                    &out_prefs->ping_delay)) {
-    // Default value in case master preferences is missing or corrupt,
-    // or ping_delay is missing.
-    out_prefs->ping_delay = 90;
-  }
-}
-
 // -- Platform-specific functions --
 
 #if !defined(OS_LINUX) && !defined(OS_BSD)
@@ -477,22 +473,22 @@ bool SetShowFirstRunBubblePref(FirstRunBubbleOptions show_bubble_option) {
 }
 
 void SetShouldShowWelcomePage() {
-  should_show_welcome_page_ = true;
+  g_should_show_welcome_page = true;
 }
 
 bool ShouldShowWelcomePage() {
-  bool retval = should_show_welcome_page_;
-  should_show_welcome_page_ = false;
+  bool retval = g_should_show_welcome_page;
+  g_should_show_welcome_page = false;
   return retval;
 }
 
 void SetShouldDoPersonalDataManagerFirstRun() {
-  should_do_autofill_personal_data_manager_first_run_ = true;
+  g_should_do_autofill_personal_data_manager_first_run = true;
 }
 
 bool ShouldDoPersonalDataManagerFirstRun() {
-  bool retval = should_do_autofill_personal_data_manager_first_run_;
-  should_do_autofill_personal_data_manager_first_run_ = false;
+  bool retval = g_should_do_autofill_personal_data_manager_first_run;
+  g_should_do_autofill_personal_data_manager_first_run = false;
   return retval;
 }
 
@@ -622,26 +618,26 @@ ProcessMasterPreferencesResult ProcessMasterPreferences(
   base::FilePath master_prefs_path;
   scoped_ptr<installer::MasterPreferences>
       install_prefs(internal::LoadMasterPrefs(&master_prefs_path));
-  if (!install_prefs.get())
-    return DO_FIRST_RUN_TASKS;
-
-  ConvertStringVectorToGURLVector(
-      install_prefs->GetFirstRunTabs(), &out_prefs->new_tabs);
 
-  internal::SetRLZPref(out_prefs, install_prefs.get());
+  // Default value in case master preferences is missing or corrupt, or
+  // ping_delay is missing.
+  out_prefs->ping_delay = 90;
+  if (install_prefs.get()) {
+    if (!internal::ShowPostInstallEULAIfNeeded(install_prefs.get()))
+      return EULA_EXIT_NOW;
 
-  if (!internal::ShowPostInstallEULAIfNeeded(install_prefs.get()))
-    return EULA_EXIT_NOW;
+    if (!internal::CopyPrefFile(user_data_dir, master_prefs_path))
+      DLOG(ERROR) << "Failed to copy master_preferences to user data dir.";
 
-  if (!internal::CopyPrefFile(user_data_dir, master_prefs_path))
-    DLOG(ERROR) << "Failed to copy master_preferences to user data dir.";
+    DoDelayedInstallExtensionsIfNeeded(install_prefs.get());
 
-  DoDelayedInstallExtensionsIfNeeded(install_prefs.get());
+    internal::SetupMasterPrefsFromInstallPrefs(*install_prefs, out_prefs);
 
-  internal::SetupMasterPrefsFromInstallPrefs(*install_prefs, out_prefs);
+    internal::SetImportPreferencesAndLaunchImport(out_prefs,
+                                                  install_prefs.get());
 
-  internal::SetImportPreferencesAndLaunchImport(out_prefs, install_prefs.get());
-  internal::SetDefaultBrowser(install_prefs.get());
+    internal::SetDefaultBrowser(install_prefs.get());
+  }
 
   return DO_FIRST_RUN_TASKS;
 }
@@ -700,25 +696,25 @@ void AutoImport(
                   import_items,
                   dont_import_items,
                   importer::HISTORY,
-                  items);
+                  &items);
     SetImportItem(user_prefs,
                   prefs::kImportHomepage,
                   import_items,
                   dont_import_items,
                   importer::HOME_PAGE,
-                  items);
+                  &items);
     SetImportItem(user_prefs,
                   prefs::kImportSearchEngine,
                   import_items,
                   dont_import_items,
                   importer::SEARCH_ENGINES,
-                  items);
+                  &items);
     SetImportItem(user_prefs,
                   prefs::kImportBookmarks,
                   import_items,
                   dont_import_items,
                   importer::FAVORITES,
-                  items);
+                  &items);
 
     importer::LogImporterUseToMetrics(
         "AutoImport", importer_list->GetSourceProfileAt(0).importer_type);
@@ -730,7 +726,6 @@ void AutoImport(
 
   content::RecordAction(UserMetricsAction("FirstRunDef_Accept"));
 
-  first_run::CreateSentinel();
 #endif  // !defined(USE_AURA)
   did_perform_profile_import = true;
 }
diff --git a/chrome/browser/first_run/first_run.h b/chrome/browser/first_run/first_run.h
index 7221d6c..0a03d7d 100644
--- a/chrome/browser/first_run/first_run.h
+++ b/chrome/browser/first_run/first_run.h
@@ -157,9 +157,6 @@ bool DidPerformProfileImport(bool* exited_successfully);
 // cmdline parameters.
 int ImportNow(Profile* profile, const CommandLine& cmdline);
 
-// Returns the path for the master preferences file.
-base::FilePath MasterPrefsPath();
-
 // Set a master preferences file path that overrides platform defaults.
 void SetMasterPrefsPathForTesting(const base::FilePath& master_prefs);
 
diff --git a/chrome/browser/first_run/first_run_internal.h b/chrome/browser/first_run/first_run_internal.h
index 4532709..d2ccebf 100644
--- a/chrome/browser/first_run/first_run_internal.h
+++ b/chrome/browser/first_run/first_run_internal.h
@@ -32,6 +32,7 @@ class MasterPreferences;
 }
 
 namespace first_run {
+
 namespace internal {
 
 enum FirstRunState {
@@ -96,10 +97,6 @@ void SetupMasterPrefsFromInstallPrefs(
 
 void SetDefaultBrowser(installer::MasterPreferences* install_prefs);
 
-// Sets ping_delay.
-void SetRLZPref(first_run::MasterPrefs* out_prefs,
-                installer::MasterPreferences* install_prefs);
-
 // -- Platform-specific functions --
 
 void DoPostImportPlatformSpecificTasks();
@@ -140,6 +137,9 @@ bool ImportBookmarks(const base::FilePath& import_bookmarks_path);
 // should exit.
 bool ShowPostInstallEULAIfNeeded(installer::MasterPreferences* install_prefs);
 
+// Returns the path for the master preferences file.
+base::FilePath MasterPrefsPath();
+
 }  // namespace internal
 }  // namespace first_run
 
diff --git a/chrome/browser/first_run/first_run_linux.cc b/chrome/browser/first_run/first_run_linux.cc
index 8dc1c57..01ce84d 100644
--- a/chrome/browser/first_run/first_run_linux.cc
+++ b/chrome/browser/first_run/first_run_linux.cc
@@ -63,11 +63,6 @@ bool ImportBookmarks(const base::FilePath& import_bookmarks_path) {
   return base::LaunchProcess(import_cmd, options, NULL);
 }
 
-}  // namespace internal
-}  // namespace first_run
-
-namespace first_run {
-
 base::FilePath MasterPrefsPath() {
   // The standard location of the master prefs is next to the chrome binary.
   base::FilePath master_prefs;
@@ -76,4 +71,5 @@ base::FilePath MasterPrefsPath() {
   return master_prefs.AppendASCII(installer::kDefaultMasterPrefs);
 }
 
+}  // namespace internal
 }  // namespace first_run
diff --git a/chrome/browser/first_run/first_run_mac.mm b/chrome/browser/first_run/first_run_mac.mm
index cd6ecc6..543d3d5 100644
--- a/chrome/browser/first_run/first_run_mac.mm
+++ b/chrome/browser/first_run/first_run_mac.mm
@@ -20,13 +20,9 @@ bool ImportBookmarks(const base::FilePath& import_bookmarks_path) {
   return false;
 }
 
-}  // namespace internal
-}  // namespace first_run
-
-namespace first_run {
-
 base::FilePath MasterPrefsPath() {
   return master_prefs::MasterPrefsPath();
 }
 
-}  //namespace first_run
+}  // namespace internal
+}  // namespace first_run
diff --git a/chrome/browser/first_run/first_run_win.cc b/chrome/browser/first_run/first_run_win.cc
index 8a5a9a5..aeaab33 100644
--- a/chrome/browser/first_run/first_run_win.cc
+++ b/chrome/browser/first_run/first_run_win.cc
@@ -497,6 +497,14 @@ bool ShowPostInstallEULAIfNeeded(installer::MasterPreferences* install_prefs) {
   return true;
 }
 
+base::FilePath MasterPrefsPath() {
+  // The standard location of the master prefs is next to the chrome binary.
+  base::FilePath master_prefs;
+  if (!PathService::Get(base::DIR_EXE, &master_prefs))
+    return base::FilePath();
+  return master_prefs.AppendASCII(installer::kDefaultMasterPrefs);
+}
+
 }  // namespace internal
 }  // namespace first_run
 
@@ -512,12 +520,4 @@ int ImportNow(Profile* profile, const CommandLine& cmdline) {
   return return_code;
 }
 
-base::FilePath MasterPrefsPath() {
-  // The standard location of the master prefs is next to the chrome binary.
-  base::FilePath master_prefs;
-  if (!PathService::Get(base::DIR_EXE, &master_prefs))
-    return base::FilePath();
-  return master_prefs.AppendASCII(installer::kDefaultMasterPrefs);
-}
-
 }  // namespace first_run