Merge 3 different ways of obtaining first run state into a single one.

chrome/browser/chrome_browser_main.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chrome_browser_main.h"
 
 #if defined(TOOLKIT_GTK)
 #include <gtk/gtk.h>
 #endif
 
@@ skipping 535 matching lines @@
       result_code_(content::RESULT_CODE_NORMAL_EXIT),
       startup_watcher_(new StartupTimeBomb()),
       shutdown_watcher_(new ShutdownWatcherHelper()),
       startup_timer_(new performance_monitor::StartupTimer()),
       browser_field_trials_(parameters.command_line),
       rvh_callback_(base::Bind(&RenderViewHostCreated)),
       translate_manager_(NULL),
       profile_(NULL),
       run_message_loop_(true),
       notify_result_(ProcessSingleton::PROCESS_NONE),
-      do_first_run_tasks_(false),
+      is_first_run_(false),
       local_state_(NULL),
       restart_last_session_(false) {
   // If we're running tests (ui_task is non-null).
   if (parameters.ui_task)
     browser_defaults::enable_help_app = false;
 
   // Chrome disallows cookies by default. All code paths that want to use
   // cookies need to go through one of Chrome's URLRequestContexts which have
   // a ChromeNetworkDelegate attached that selectively allows cookies again.
   if (!disable_enforcing_cookie_policies_for_tests_)
@@ skipping 247 matching lines @@
 
 int ChromeBrowserMainParts::PreCreateThreadsImpl() {
   TRACE_EVENT0("startup", "ChromeBrowserMainParts::PreCreateThreadsImpl")
   run_message_loop_ = false;
   {
     TRACE_EVENT0("startup",
       "ChromeBrowserMainParts::PreCreateThreadsImpl:GetUserDataDir");
     user_data_dir_ = chrome::GetUserDataDir(parameters());
   }
 
-  // Whether this is First Run. |do_first_run_tasks_| should be prefered to this
-  // unless the desire is actually to know whether this is really First Run
-  // (i.e., even if --no-first-run is passed).
-  bool is_first_run = false;
   // Android's first run is done in Java instead of native.
 #if defined(OS_ANDROID)
   // Force MediaCaptureDevicesDispatcher created on UI thread.
   MediaCaptureDevicesDispatcher::GetInstance();
 #else
   process_singleton_.reset(new ChromeProcessSingleton(
       user_data_dir_, base::Bind(&ProcessSingletonNotificationCallback)));
 
-  bool force_first_run =
-      parsed_command_line().HasSwitch(switches::kForceFirstRun);
-  bool force_skip_first_run_tasks =
-      (!force_first_run &&
-       parsed_command_line().HasSwitch(switches::kNoFirstRun));
-
-  is_first_run = force_first_run || first_run::IsChromeFirstRun();
+  is_first_run_ = first_run::IsChromeFirstRun();

[gab, 2013/07/26 15:41:13]

We could in fact completely get rid of is_first_run_ state and use static getter
first_run::IsChromeFirstRun() everywhere if that's preferable..?

[cpu_(ooo_6.6-7.5), 2013/07/27 21:16:31]

Yes please. You can have a function local is_first_run in case
IsChromeFirstRun() is doing disk IO but my last recollection is that it caches
the value.

[gab, 2013/07/29 17:39:50]

Done, indeed IsChromeFirstRun() caches the value.

 #endif
 
   scoped_refptr<base::SequencedTaskRunner> local_state_task_runner =
       JsonPrefStore::GetTaskRunnerForFile(
           base::FilePath(chrome::kLocalStorePoolName),
           BrowserThread::GetBlockingPool());
 
   {
     TRACE_EVENT0("startup",
       "ChromeBrowserMainParts::PreCreateThreadsImpl:InitBrowswerProcessImpl");
@@ skipping 17 matching lines @@
     tracked_objects::ThreadData::InitializeAndSetTrackingStatus(status);
   }
 
   if (parsed_command_line().HasSwitch(switches::kProfilingOutputFile)) {
     tracking_objects_.set_output_file_path(
         parsed_command_line().GetSwitchValuePath(
             switches::kProfilingOutputFile));
   }
 
   local_state_ = InitializeLocalState(
-      local_state_task_runner.get(), parsed_command_line(), is_first_run);
+      local_state_task_runner.get(), parsed_command_line(), is_first_run_);
 
   // These members must be initialized before returning from this function.
   master_prefs_.reset(new first_run::MasterPrefs);
 
 #if !defined(OS_ANDROID)
   // Android doesn't use StartupBrowserCreator.
   browser_creator_.reset(new StartupBrowserCreator);
   // TODO(yfriedman): Refactor Android to re-use UMABrowsingActivityObserver
   chrome::UMABrowsingActivityObserver::Init();
 #endif
@@ skipping 68 matching lines @@
           resources_pack_path, ui::SCALE_FACTOR_NONE);
     }
 #endif  // defined(OS_MACOSX)
   }
 
 #if defined(TOOLKIT_GTK)
   g_set_application_name(l10n_util::GetStringUTF8(IDS_PRODUCT_NAME).c_str());
 #endif
 
   // Android does first run in Java instead of native.
-#if !defined(OS_ANDROID)
+  // Chrome OS has its own out-of-box-experience code.
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)

[gab, 2013/07/26 15:41:13]

This comes from the old (removed in this CL) hidden side-effect in
ProcessMasterPreferences which I moved up here.

I think it's better to explicitly ifdef out things that shouldn't happen on
Chrome OS than to have do_first_run_tasks_ be set to false through some hidden
ifdef.

[cpu_(ooo_6.6-7.5), 2013/07/27 21:16:31]

Sure, It more along the lines of other things in this horrible horrible file.

   // On first run, we need to process the predictor preferences before the
   // browser's profile_manager object is created, but after ResourceBundle
   // is initialized.
-  if (is_first_run) {
+  if (is_first_run_) {
     first_run::ProcessMasterPreferencesResult pmp_result =
         first_run::ProcessMasterPreferences(user_data_dir_,
                                             master_prefs_.get());
     if (pmp_result == first_run::EULA_EXIT_NOW)
       return chrome::RESULT_CODE_EULA_REFUSED;
 
-    // Do first run tasks unless:
-    //  - Explicitly forced not to by |force_skip_first_run_tasks| or
-    //    |pmp_result|.
-    //  - Running in App (or App Launcher) mode, where showing the importer
-    //    (first run) UI is undesired.
-    do_first_run_tasks_ = (
-        !force_skip_first_run_tasks &&
-        pmp_result != first_run::SKIP_FIRST_RUN_TASKS &&
-        !parsed_command_line().HasSwitch(switches::kApp) &&

[gab, 2013/07/26 15:41:13]

We would previously set do_first_run_tasks_ to false if first run was in App
mode of AppLauncher mode.

I think this was previously done to avoid the import UI (now silent in
background -- i.e., AutoImport()) and avoid adding first run tabs...

Now a bunch of things have come under this logic which I don't think belong
here.... besides adding first run tabs, every other first run initialization
should happen even if running App Launcher or App mode first.

[cpu_(ooo_6.6-7.5), 2013/07/27 21:16:31]

In general I agree, but you should fire an email to benwells, it might be
possible to have a machine that has only IE and install a google calculator that
installs chrome under the hood. I don't know if that is ready or if we don't
plan to do that anymore. I guess if it works for a chromeframe only install it
should work with apps. This is just a suggestion.

[gab, 2013/07/29 17:39:50]

[cc'ing benwells here]

Right, so AFAIK this has been put to sleep for now while AppsV2 is brought up,
it will be revisited later.

Ideally we would have --no-first-run and --cancel-first-run. In most cases users
of that flag simply don't want first run now, but don't want to cancel it the
first time Chrome actually runs (i.e. CF, apps, tests, etc.).

We still need --cancel-first-run in some scenarios (i.e., self-destruct where we
don't want the system-level chrome being launched after destroying the
user-level chrome to launch in first run).

For now we only have --no-first-run which actually cancels first run from ever
happening in future runs of chrome and is likely too strong in most scenarios.

It will be very easy after this CL and https://codereview.chromium.org/20743002/
to implement separate --no-first-run and --cancel-first-run if required.

[gab, 2013/07/29 22:22:29]

Here https://codereview.chromium.org/21150006/ :)!

+    if (!parsed_command_line().HasSwitch(switches::kApp) &&
         !parsed_command_line().HasSwitch(switches::kAppId) &&
-        !parsed_command_line().HasSwitch(switches::kShowAppList));
-
-    if (do_first_run_tasks_) {
+        !parsed_command_line().HasSwitch(switches::kShowAppList)) {
       AddFirstRunNewTabs(browser_creator_.get(), master_prefs_->new_tabs);
-
-      // TODO(macourteau): refactor preferences that are copied from
-      // master_preferences into local_state, as a "local_state" section in
-      // master preferences. If possible, a generic solution would be prefered
-      // over a copy one-by-one of specific preferences. Also see related TODO
-      // in first_run.h.
-
-      // Store the initial VariationsService seed in local state, if it exists
-      // in master prefs.
-      if (!master_prefs_->variations_seed.empty()) {
-        local_state_->SetString(prefs::kVariationsSeed,
-                                master_prefs_->variations_seed);
-        // Set the variation seed date to the current system time. If the user's
-        // clock is incorrect, this may cause some field trial expiry checks to
-        // not do the right thing until the next seed update from the server,
-        // when this value will be updated.
-        local_state_->SetInt64(prefs::kVariationsSeedDate,
-                               base::Time::Now().ToInternalValue());
-      }
-
-      if (!master_prefs_->suppress_default_browser_prompt_for_version.empty()) {
-        local_state_->SetString(
-            prefs::kBrowserSuppressDefaultBrowserPrompt,
-            master_prefs_->suppress_default_browser_prompt_for_version);
-      }
-
-      AppListService::Get()->HandleFirstRun();
     }
 
-    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();

[gab, 2013/07/26 15:41:13]

Now handled inside first_run::IsChromeFirstRun(); no point having browser main
care about setting this internal state of first_run.

Note: first_run::CreateSentinel() is further moved to first_run::internal:: in
https://codereview.chromium.org/20743002/

+    // TODO(macourteau): refactor preferences that are copied from
+    // master_preferences into local_state, as a "local_state" section in

[cpu_(ooo_6.6-7.5), 2013/07/27 21:16:31]

btw, I pinged him on his 3 bugs (130598, 180304, 230115)

+    // master preferences. If possible, a generic solution would be prefered
+    // over a copy one-by-one of specific preferences. Also see related TODO
+    // in first_run.h.
+
+    // Store the initial VariationsService seed in local state, if it exists
+    // in master prefs.
+    if (!master_prefs_->variations_seed.empty()) {
+      local_state_->SetString(prefs::kVariationsSeed,
+                              master_prefs_->variations_seed);
+      // Set the variation seed date to the current system time. If the user's
+      // clock is incorrect, this may cause some field trial expiry checks to
+      // not do the right thing until the next seed update from the server,
+      // when this value will be updated.
+      local_state_->SetInt64(prefs::kVariationsSeedDate,
+                             base::Time::Now().ToInternalValue());
     }
+
+    if (!master_prefs_->suppress_default_browser_prompt_for_version.empty()) {
+      local_state_->SetString(
+          prefs::kBrowserSuppressDefaultBrowserPrompt,
+          master_prefs_->suppress_default_browser_prompt_for_version);
+    }
+
+    AppListService::Get()->HandleFirstRun();
   }
 #endif
 
 #if defined(OS_LINUX) || defined(OS_OPENBSD) || defined(OS_MACOSX)
   // Set the product channel for crash reports.
   child_process_logging::SetChannel(
       chrome::VersionInfo::GetVersionStringModifier());
 #endif
 
   // Initialize tracking synchronizer system.
@@ skipping 311 matching lines @@
   PostProfileInit();
 
   // Retrieve cached GL strings from local state and use them for GPU
   // blacklist decisions.
   if (g_browser_process->gl_string_manager())
     g_browser_process->gl_string_manager()->Initialize();
 
   // Create an instance of GpuModeManager to watch gpu mode pref change.
   g_browser_process->gpu_mode_manager();
 
-#if !defined(OS_ANDROID)
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
   // Show the First Run UI if this is the first time Chrome has been run on
   // this computer, or we're being compelled to do so by a command line flag.
   // Note that this be done _after_ the PrefService is initialized and all
   // preferences are registered, since some of the code that the importer
   // touches reads preferences.
-  if (do_first_run_tasks_) {
+  if (is_first_run_) {
     first_run::AutoImport(profile_,
                           master_prefs_->homepage_defined,
                           master_prefs_->do_import_items,
                           master_prefs_->dont_import_items,
                           master_prefs_->import_bookmarks_path);
 
     // Note: this can pop the first run consent dialog on linux.
     first_run::DoPostImportTasks(profile_, master_prefs_->make_chrome_default);
 
     if (!master_prefs_->suppress_first_run_default_browser_prompt) {
       browser_creator_->set_show_main_browser_window(
           !chrome::ShowFirstRunDefaultBrowserPrompt(profile_));
     } else {
       browser_creator_->set_is_default_browser_dialog_suppressed(true);
     }
-  }  // if (do_first_run_tasks_)
-#endif  // !defined(OS_ANDROID)
+  }  // if (is_first_run_)
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 
 #if defined(OS_WIN)
   // Sets things up so that if we crash from this point on, a dialog will
   // popup asking the user to restart chrome. It is done this late to avoid
   // testing against a bunch of special cases that are taken care early on.
   ChromeBrowserMainPartsWin::PrepareRestartOnCrashEnviroment(
       parsed_command_line());
 
   // Registers Chrome with the Windows Restart Manager, which will restore the
   // Chrome session when the computer is restarted after a system update.
@@ skipping 12 matching lines @@
         base::Bind(&NetworkProfileBubble::CheckNetworkProfile,
                    profile_->GetPath()));
   }
 #endif  // OS_WIN
 
 #if defined(ENABLE_RLZ) && !defined(OS_CHROMEOS)
   // Init the RLZ library. This just binds the dll and schedules a task on the
   // file thread to be run sometime later. If this is the first run we record
   // the installation event.
   PrefService* pref_service = profile_->GetPrefs();
-  int ping_delay = do_first_run_tasks_ ? master_prefs_->ping_delay :
+  int ping_delay = is_first_run_ ? master_prefs_->ping_delay :
       pref_service->GetInteger(first_run::GetPingDelayPrefName().c_str());
   // Negative ping delay means to send ping immediately after a first search is
   // recorded.
   RLZTracker::InitRlzFromProfileDelayed(
-      profile_, do_first_run_tasks_, ping_delay < 0,
+      profile_, is_first_run_, ping_delay < 0,
       base::TimeDelta::FromMilliseconds(abs(ping_delay)));
 #endif  // defined(ENABLE_RLZ) && !defined(OS_CHROMEOS)
 
   // Configure modules that need access to resources.
   net::NetModule::SetResourceProvider(chrome_common_net::NetResourceProvider);
 
   // In unittest mode, this will do nothing.  In normal mode, this will create
   // the global IntranetRedirectDetector instance, which will promptly go to
   // sleep for seven seconds (to avoid slowing startup), and wake up afterwards
   // to see if it should do anything else.
@@ skipping 202 matching lines @@
   return true;
 #else
   // Set the result code set in PreMainMessageLoopRun or set above.
   *result_code = result_code_;
   if (!run_message_loop_)
     return true;  // Don't run the default message loop.
 
   // These should be invoked as close to the start of the browser's
   // UI thread message loop as possible to get a stable measurement
   // across versions.
-  RecordBrowserStartupTime(do_first_run_tasks_);
+  RecordBrowserStartupTime(is_first_run_);
   startup_timer_->SignalStartupComplete(
       performance_monitor::StartupTimer::STARTUP_NORMAL);
 
   DCHECK_EQ(base::MessageLoop::TYPE_UI, base::MessageLoop::current()->type());
 #if !defined(USE_AURA) && defined(TOOLKIT_VIEWS)
   views::AcceleratorHandler accelerator_handler;
   base::RunLoop run_loop(&accelerator_handler);
 #else
   base::RunLoop run_loop;
 #endif
@@ skipping 71 matching lines @@
   chromeos::CrosSettings::Shutdown();
 #endif
 #endif
 }
 
 // Public members:
 
 void ChromeBrowserMainParts::AddParts(ChromeBrowserMainExtraParts* parts) {
   chrome_extra_parts_.push_back(parts);
 }