Do not do AutoImport on Windows since the import process is already ran earlier as part of ProcessM…

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 bada01d575b2a70c69128f78f4cf6021fad1f0dd..e380c62d67d122bf8ba97decda2c45a6491074c0 100644
--- a/chrome/browser/first_run/first_run.cc
+++ b/chrome/browser/first_run/first_run.cc
@@ -28,6 +28,7 @@
 #include "chrome/browser/importer/importer_list.h"
 #include "chrome/browser/importer/importer_progress_dialog.h"
 #include "chrome/browser/importer/importer_progress_observer.h"
+#include "chrome/browser/importer/profile_writer.h"
 #include "chrome/browser/process_singleton.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/search_engines/template_url_service.h"
@@ -62,13 +63,45 @@ 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;
+
+// This class acts as an observer for the ImporterProgressObserver::ImportEnded
+// callback. When the import process is started, certain errors may cause
+// ImportEnded() to be called synchronously, but the typical case is that
+// ImportEnded() is called asynchronously. Thus we have to handle both cases.
+class ImportEndedObserver : public importer::ImporterProgressObserver {
+ public:
+  ImportEndedObserver() : ended_(false),
+                          should_quit_message_loop_(false) {}
+  virtual ~ImportEndedObserver() {}
+
+  // importer::ImporterProgressObserver:
+  virtual void ImportStarted() OVERRIDE {}
+  virtual void ImportItemStarted(importer::ImportItem item) OVERRIDE {}
+  virtual void ImportItemEnded(importer::ImportItem item) OVERRIDE {}
+  virtual void ImportEnded() OVERRIDE {
+    ended_ = true;
+    if (should_quit_message_loop_)
+      MessageLoop::current()->Quit();

[Joao da Silva, 2013/03/12 13:32:05]

#include "base/message_loop.h"

[gab, 2013/03/25 16:00:43]

Done.

+  }
+
+  void set_should_quit_message_loop() {
+    should_quit_message_loop_ = true;
+  }
+
+  bool ended() {

[Joao da Silva, 2013/03/12 13:32:05]

nit: const method

[gab, 2013/03/25 16:00:43]

Note that this is a pure move from first_run_posix.cc, but sure.

+    return ended_;
+  }
 
-// Flags indicating whether a first-run profile auto import was performed, and
-// whether the importer process exited successfully.
-bool did_perform_profile_import = false;
-bool profile_import_exited_successfully = false;
+ private:
+  // Set if the import has ended.
+  bool ended_;
+
+  // Set by the client (via set_should_quit_message_loop) if, when the import
+  // ends, this class should quit the message loop.
+  bool should_quit_message_loop_;
+};
 
 // Helper class that performs delayed first-run tasks that need more of the
 // chrome infrastructure to be up and running before they can be attempted.
@@ -149,7 +182,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;
@@ -183,10 +216,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);
@@ -236,6 +269,9 @@ void ConvertStringVectorToGURLVector(
 namespace first_run {
 namespace internal {
 
+bool g_did_perform_profile_import = false;
+bool g_profile_import_exited_successfully = false;
+
 FirstRunState first_run_ = FIRST_RUN_UNKNOWN;
 
 static base::LazyInstance<base::FilePath> master_prefs_path_for_testing
@@ -273,6 +309,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)) {
@@ -338,6 +380,10 @@ void SetupMasterPrefsFromInstallPrefs(
     out_prefs->suppress_first_run_default_browser_prompt = true;
   }
 
+  install_prefs.GetString(
+          installer::master_preferences::kDistroImportBookmarksFromFilePref,
+          &out_prefs->import_bookmarks_path);
+
   out_prefs->variations_seed = install_prefs.GetVariationsSeed();
 }
 
@@ -360,14 +406,34 @@ 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;
+bool ImportSettings(Profile* profile,
+                    scoped_refptr<ImporterHost> importer_host,
+                    scoped_refptr<ImporterList> importer_list,
+                    int items_to_import) {
+  const importer::SourceProfile& source_profile =
+      importer_list->GetSourceProfileAt(0);
+
+  // Ensure that importers aren't requested to import items that they do not
+  // support. If there is no overlap, skip.
+  items_to_import &= source_profile.services_supported;
+  if (items_to_import == 0)
+    return true;
+
+  scoped_ptr<ImportEndedObserver> observer(new ImportEndedObserver);
+  importer_host->SetObserver(observer.get());
+  importer_host->StartImportSettings(source_profile,
+                                     profile,
+                                     items_to_import,
+                                     new ProfileWriter(profile),
+                                     true);
+  // If the import process has not errored out, block on it.
+  if (!observer->ended()) {
+    observer->set_should_quit_message_loop();
+    MessageLoop::current()->Run();
   }
+
+  // Unfortunately there's no success/fail signal in ImporterHost.

[cpu_(ooo_6.6-7.5), 2013/03/11 20:54:20]

don't we watch the process handle and get the exit code?

[gab, 2013/03/12 12:32:19]

That's a pure copy/paste from first_run_posix.cc; I figured since the Windows
one is no longer used I would remove it from the platform specific section (or I
could leave it there and make the Windows impl NOTREACHED()...

+  return true;
 }
 
 // -- Platform-specific functions --
@@ -462,22 +528,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;
 }
 
@@ -594,26 +660,25 @@ 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);
+  // 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;
 
-  internal::SetRLZPref(out_prefs, install_prefs.get());

[cpu_(ooo_6.6-7.5), 2013/03/11 20:56:05]

where did the rlz stuff go?

[gab, 2013/03/12 12:32:19]

Down in SetupMasterPrefsFromInstallPrefs() beside every other pref mapped from
|install_prefs| to |out_prefs|.

[Joao da Silva, 2013/03/12 13:32:05]

SetRLZPref() used to happen before potentially returning EULA_EXIT_NOW, but not
that exit path comes before. Is it OK not to set the RLZ prefs with returning
EULA_EXIT_NOW?

[gab, 2013/03/25 16:00:43]

Returning EULA_EXIT_NOW means the user refused the EULA and the browser will
immediately exit
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...).

SetRLZPref() is only setting some in-memory state which is irrelevant if exiting
the browser immediately.

Also note that the First Run sentinel hasn't been created at that point so if
Chrome is launched again and the EULA accepted it will still go through the full
First Run experience.

+    if (!internal::CopyPrefFile(user_data_dir, master_prefs_path))
+      DLOG(ERROR) << "Failed to copy master_preferences to user data dir.";
 
-  if (!internal::ShowPostInstallEULAIfNeeded(install_prefs.get()))
-    return EULA_EXIT_NOW;
+    DoDelayedInstallExtensionsIfNeeded(install_prefs.get());
 
-  if (!internal::CopyPrefFile(user_data_dir, master_prefs_path))
-    DLOG(ERROR) << "Failed to copy master_preferences to user data dir.";
+    internal::SetupMasterPrefsFromInstallPrefs(*install_prefs, out_prefs);
 
-  DoDelayedInstallExtensionsIfNeeded(install_prefs.get());
-
-  internal::SetupMasterPrefsFromInstallPrefs(*install_prefs, out_prefs);
+    internal::SetDefaultBrowser(install_prefs.get());
+  }
 
-  internal::SetImportPreferencesAndLaunchImport(out_prefs, install_prefs.get());
-  internal::SetDefaultBrowser(install_prefs.get());
+  internal::SetImportPreferencesAndLaunchImport(out_prefs);
 
   return DO_FIRST_RUN_TASKS;
 }
@@ -680,37 +745,36 @@ 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);
 
-    profile_import_exited_successfully =
+    internal::g_profile_import_exited_successfully =
         internal::ImportSettings(profile, importer_host, importer_list, items);
-    DCHECK(profile_import_exited_successfully);
+    DCHECK(internal::g_profile_import_exited_successfully);
   }
 
   content::RecordAction(UserMetricsAction("FirstRunDef_Accept"));
 
   process_singleton->Unlock();
-  first_run::CreateSentinel();
 #endif  // !defined(USE_AURA)
-  did_perform_profile_import = true;
+  internal::g_did_perform_profile_import = true;
 }
 
 void DoPostImportTasks(Profile* profile, bool make_chrome_default) {
@@ -745,8 +809,8 @@ void DoPostImportTasks(Profile* profile, bool make_chrome_default) {
 
 bool DidPerformProfileImport(bool* exited_successfully) {
   if (exited_successfully)
-    *exited_successfully = profile_import_exited_successfully;
-  return did_perform_profile_import;
+    *exited_successfully = internal::g_profile_import_exited_successfully;
+  return internal::g_did_perform_profile_import;
 }
 
 }  // namespace first_run