Fix the first_run regression issue related to skip_first_run_ui setting processing.

chrome/browser/first_run/first_run_posix.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/first_run/first_run.h"
 
 #include "base/path_service.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/first_run/first_run_internal.h"
 #include "chrome/browser/importer/importer_host.h"
@@ skipping 129 matching lines @@
   out_prefs->new_tabs = install_prefs->GetFirstRunTabs();
 
   internal::SetRLZPref(out_prefs, install_prefs.get());
 
   if (!internal::CopyPrefFile(user_data_dir, master_prefs_path))
     return true;
 
   internal::SetupMasterPrefsFromInstallPrefs(out_prefs,
       install_prefs.get());
 
+  // TODO(mirandac): Refactor skip-first-run-ui process into regular first run
+  // import process.  http://crbug.com/49647
+  // Note we are skipping all other master preferences if skip-first-run-ui
+  // is *not* specified. (That is, we continue only if skipping first run ui.)
   if (!internal::SkipFirstRunUI(install_prefs.get()))

[msw, 2012/07/10 17:31:46]

We probably shouldn't ignore the rest of the master prefs if skip_first_run_ui
is actually specified as false. The code here (and in first_run_win.cc) should
be something like the following (feel free to remove |internal::SkipFirstRunUI|
or add a separate |internal::HasValidMasterPrefs| that returns true iff
skip_first_run_ui is specified):

bool skip_first_run_ui = false;
// Ignore the remaining master prefs if skip-first-run-ui is not specified.
if
(!install_prefs->GetBool(installer::master_preferences::kDistroSkipFirstRunPref,
&skip_first_run_ui))
  return true;

// From here...
// bubble anyway...
SetShowFirstRunBubblePref(skip_first_run_ui);

[jennyz, 2012/07/10 18:42:52]

For the above change you proposed, I am a little concerned
ProcessMasterPreferences will return false in the case skip_first_run_ui is set
to false, which should return true originally. This will affect the
fist_run_ui_bypass_ value in the calling code of chrome_browser_main.cc, and
have an impact the later code for whether to call first_run::AutoImport. Does
this matter for the first_run flow?

[msw, 2012/07/10 19:33:19]

Ah, I see what you mean. This code is horribly convoluted and not maintainable,
but this Patch Set does restore the original logic before the regression in
crrev.com/122023 (returning true here if skip_first_run_ui is not found or if
its value is false).

     return true;
 
   // From here on we won't show first run so we need to do the work to show the
   // bubble anyway, unless it's already been explicitly suppressed.
   SetShowFirstRunBubblePref(true);
 
   // We need to be able to create the first run sentinel or else we cannot
   // proceed because ImportSettings will launch the importer process which
   // would end up here if the sentinel is not present.
   if (!CreateSentinel())
     return false;
 
   internal::SetShowWelcomePagePrefIfNeeded(install_prefs.get());
   internal::SetImportPreferencesAndLaunchImport(out_prefs, install_prefs.get());
   internal::SetDefaultBrowser(install_prefs.get());
 
   return false;
 }
 
 
 }  // namespace first_run