Revert 201968 "Revert 201837 "OOP import on Windows.""

Sorry about that, seems to be a different issue - http://crbug.com/177163

> Revert 201837 "OOP import on Windows."
> 
> This broke browser_tests on XP Tests (dbg)(4):
> http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%28dbg%29%284%29/builds/33240
> 
> [1148:3528:0523/123340:2643015:WARNING:extension_apitest.cc(169)] Workaround for 177163, prematurely stopping test
> 
> > OOP import on Windows.
> > 
> > Gets rid of the import process on all platforms -- replaced by a utility process which communicates with a ProfileWriter back in the browser process to write to the profile (this uses the ExternalProcessImporterHost machinery written 2+ years ago by mirandac@ for import on Mac and still the state of the art today).
> > 
> > Gets rid of all issues regarding profile contention and races to profile creation between the browser and import processes on first run (example of issues this has previously caused: http://crbug.com/171475, http://crbug.com/174591, http://crbug.com/180459).
> > 
> > Makes bookmarks file import use the same mechanism on all platforms (this means bookmarks file import is now in-process on Linux which still uses ImporterHost (instead of ExternalProcessImporterHost) -- Linux used to use the import process solely for bookmarks file import -- but the work to switch Linux to ExternalProcessImporterHost should be very minimal after this CL and I intend to do it in a follow-up CL to unify the import flow cross-platform once and for all!).
> > 
> > Do not use the out-of-process import for Google Toolbar (this was already the case prior to this CL).
> > To make the Google Toolbar importer work out-of-process, we would have to augment the import IPC drastically to support the web auth flow required by this importer (it requires to login to import the google.com/bookmarks favorites).
> > 
> > This, as a side-effect, brings silent bookmarks file import from master_preferences to Mac (long standing issue 48880).
> > 
> > Also fixes issue 231710 (or at least removes the condition causing the bug by making the ImportLockDialog go away on first run on Windows -- as should already have been the case).
> > 
> > Also addresses issue 178083 since the early message loop spinning was caused by ImportSettingsWin which was called too early on Windows (actually resulting in running the full import twice on Windows!) -- via PreCreateThreadsImpl()-->ProcessMasterPreferences()-->SetImportPreferencesAndLaunchImport()-->ImportSettingsWin()... This whole flow is removed in this CL :).
> > 
> > This improves first run speed in a debug build from 4901ms to 1477ms, a 332% improvement!!!! (tested by instrumenting a first run browser test, the delta is between the time the test is constructed and the time the test case is called (which happens after the browser has been initialized and import has occurred)).
> > 
> > This supersedes https://codereview.chromium.org/12463030/ (which won't be committed because this fix is so much better).
> > 
> > BUG=219419, 22142, 56816, 178083, 178051, 48880, 232241, 231710, 223462, 87657, 236225
> > 
> > Review URL: https://chromiumcodereview.appspot.com/12670013
> 
> TBR=gab@chromium.org
> 
> Review URL: https://codereview.chromium.org/15968002

TBR=dbeam@chromium.org

Review URL: https://codereview.chromium.org/15876003

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

1 files changed, 106 insertions, 43 deletions

diff --git a/chrome/browser/first_run/first_run_browsertest.cc b/chrome/browser/first_run/first_run_browsertest.cc
index 07015a1..d2847a6 100644
--- a/chrome/browser/first_run/first_run_browsertest.cc
+++ b/chrome/browser/first_run/first_run_browsertest.cc
@@ -3,7 +3,9 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/files/file_path.h"
 #include "base/prefs/pref_service.h"
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/component_loader.h"
@@ -64,60 +66,97 @@ IN_PROC_BROWSER_TEST_F(FirstRunBrowserTest, SetShouldShowWelcomePage) {
 
 #if !defined(OS_CHROMEOS)
 namespace {
-class FirstRunIntegrationBrowserTest : public InProcessBrowserTest {
+
+// A generic test class to be subclassed by test classes testing specific
+// master_preferences. All subclasses must call SetMasterPreferencesForTest()
+// from their SetUp() method before deferring the remainder of Setup() to this
+// class.
+class FirstRunMasterPrefsBrowserTestBase : public InProcessBrowserTest {
  public:
-  FirstRunIntegrationBrowserTest() {}
+  FirstRunMasterPrefsBrowserTestBase() {}
+
  protected:
+  virtual void SetUp() OVERRIDE {
+    // All users of this test class need to call SetMasterPreferencesForTest()
+    // before this class' SetUp() is invoked.
+    ASSERT_TRUE(text_.get());
+
+    ASSERT_TRUE(file_util::CreateTemporaryFile(&prefs_file_));
+    EXPECT_TRUE(file_util::WriteFile(prefs_file_, text_->c_str(),
+                                     text_->size()));
+    first_run::SetMasterPrefsPathForTesting(prefs_file_);
+
+    // This invokes BrowserMain, and does the import, so must be done last.
+    InProcessBrowserTest::SetUp();
+  }
+
+  virtual void TearDown() OVERRIDE {
+    EXPECT_TRUE(file_util::Delete(prefs_file_, false));
+    InProcessBrowserTest::TearDown();
+  }
+
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     InProcessBrowserTest::SetUpCommandLine(command_line);
     command_line->AppendSwitch(switches::kForceFirstRun);
-    EXPECT_FALSE(first_run::DidPerformProfileImport(NULL));
+    EXPECT_EQ(first_run::AUTO_IMPORT_NONE, first_run::auto_import_state());
 
     extensions::ComponentLoader::EnableBackgroundExtensionsForTesting();
+  }
 
-    // The forked import process should run BrowserMain.
-    CommandLine import_arguments((CommandLine::NoProgram()));
-    import_arguments.AppendSwitch(content::kLaunchAsBrowser);
-    first_run::SetExtraArgumentsForImportProcess(import_arguments);
+  void SetMasterPreferencesForTest(const char text[]) {
+    text_.reset(new std::string(text));
   }
+
  private:
-  DISALLOW_COPY_AND_ASSIGN(FirstRunIntegrationBrowserTest);
+  base::FilePath prefs_file_;
+  scoped_ptr<std::string> text_;
+
+  DISALLOW_COPY_AND_ASSIGN(FirstRunMasterPrefsBrowserTestBase);
 };
 
-class FirstRunMasterPrefsBrowserTest : public FirstRunIntegrationBrowserTest {
+template<const char Text[]>
+class FirstRunMasterPrefsBrowserTestT
+    : public FirstRunMasterPrefsBrowserTestBase {
  public:
-  FirstRunMasterPrefsBrowserTest() {}
+  FirstRunMasterPrefsBrowserTestT() {}
 
  protected:
   virtual void SetUp() OVERRIDE {
-    ASSERT_TRUE(file_util::CreateTemporaryFile(&prefs_file_));
-    // TODO(tapted): Make this reusable.
-    const char text[] =
-        "{\n"
-        "  \"distribution\": {\n"
-        "    \"import_bookmarks\": false,\n"
-        "    \"import_history\": false,\n"
-        "    \"import_home_page\": false,\n"
-        "    \"import_search_engine\": false\n"
-        "  }\n"
-        "}\n";
-    EXPECT_TRUE(file_util::WriteFile(prefs_file_, text, strlen(text)));
-    first_run::SetMasterPrefsPathForTesting(prefs_file_);
-
-    // This invokes BrowserMain, and does the import, so must be done last.
-    FirstRunIntegrationBrowserTest::SetUp();
-  }
-
-  virtual void TearDown() OVERRIDE {
-    EXPECT_TRUE(file_util::Delete(prefs_file_, false));
-    FirstRunIntegrationBrowserTest::TearDown();
+    SetMasterPreferencesForTest(Text);
+    FirstRunMasterPrefsBrowserTestBase::SetUp();
   }
 
  private:
-  base::FilePath prefs_file_;
-
-  DISALLOW_COPY_AND_ASSIGN(FirstRunMasterPrefsBrowserTest);
+  DISALLOW_COPY_AND_ASSIGN(FirstRunMasterPrefsBrowserTestT);
 };
+
+}  // namespace
+
+// TODO(tapted): Investigate why this fails on Linux bots but does not
+// reproduce locally. See http://crbug.com/178062 .
+// TODO(tapted): Investigate why this fails on mac_asan flakily
+// http://crbug.com/181499 .
+#if defined(OS_LINUX) || (defined(OS_MACOSX) && defined(ADDRESS_SANITIZER))
+#define MAYBE_ImportDefault DISABLED_ImportDefault
+#else
+#define MAYBE_ImportDefault ImportDefault
+#endif
+
+extern const char kImportDefault[] =
+    "{\n"
+    "}\n";
+typedef FirstRunMasterPrefsBrowserTestT<kImportDefault>
+    FirstRunMasterPrefsImportDefault;
+IN_PROC_BROWSER_TEST_F(FirstRunMasterPrefsImportDefault, MAYBE_ImportDefault) {
+  int auto_import_state = first_run::auto_import_state();
+  // Aura builds skip over the import process.
+#if defined(USE_AURA)
+  EXPECT_EQ(first_run::AUTO_IMPORT_CALLED, auto_import_state);
+#else
+  EXPECT_EQ(first_run::AUTO_IMPORT_CALLED |
+                first_run::AUTO_IMPORT_PROFILE_IMPORTED,
+            auto_import_state);
+#endif
 }
 
 // TODO(tapted): Investigate why this fails on Linux bots but does not
@@ -125,28 +164,52 @@ class FirstRunMasterPrefsBrowserTest : public FirstRunIntegrationBrowserTest {
 // TODO(tapted): Investigate why this fails on mac_asan flakily
 // http://crbug.com/181499 .
 #if defined(OS_LINUX) || (defined(OS_MACOSX) && defined(ADDRESS_SANITIZER))
-#define MAYBE_WaitForImport DISABLED_WaitForImport
+#define MAYBE_ImportBookmarksFile DISABLED_ImportBookmarksFile
 #else
-#define MAYBE_WaitForImport WaitForImport
+#define MAYBE_ImportBookmarksFile ImportBookmarksFile
 #endif
 
-IN_PROC_BROWSER_TEST_F(FirstRunIntegrationBrowserTest, MAYBE_WaitForImport) {
-  bool success = false;
-  EXPECT_TRUE(first_run::DidPerformProfileImport(&success));
+// The bookmarks file doesn't actually need to exist for this integration test
+// to trigger the interaction being tested.
+extern const char kImportBookmarksFile[] =
+    "{\n"
+    "  \"distribution\": {\n"
+    "     \"import_bookmarks_from_file\": \"/foo/doesntexists.wtv\"\n"
+    "  }\n"
+    "}\n";
+typedef FirstRunMasterPrefsBrowserTestT<kImportBookmarksFile>
+    FirstRunMasterPrefsImportBookmarksFile;
+IN_PROC_BROWSER_TEST_F(FirstRunMasterPrefsImportBookmarksFile,
+                       MAYBE_ImportBookmarksFile) {
+  int auto_import_state = first_run::auto_import_state();
   // Aura builds skip over the import process.
 #if defined(USE_AURA)
-  EXPECT_FALSE(success);
+  EXPECT_EQ(first_run::AUTO_IMPORT_CALLED, auto_import_state);
 #else
-  EXPECT_TRUE(success);
+  EXPECT_EQ(first_run::AUTO_IMPORT_CALLED |
+                first_run::AUTO_IMPORT_PROFILE_IMPORTED |
+                first_run::AUTO_IMPORT_BOOKMARKS_FILE_IMPORTED,
+            auto_import_state);
 #endif
 }
 
 // Test an import with all import options disabled. This is a regression test
 // for http://crbug.com/169984 where this would cause the import process to
 // stay running, and the NTP to be loaded with no apps.
-IN_PROC_BROWSER_TEST_F(FirstRunMasterPrefsBrowserTest,
+extern const char kImportNothing[] =
+    "{\n"
+    "  \"distribution\": {\n"
+    "    \"import_bookmarks\": false,\n"
+    "    \"import_history\": false,\n"
+    "    \"import_home_page\": false,\n"
+    "    \"import_search_engine\": false\n"
+    "  }\n"
+    "}\n";
+typedef FirstRunMasterPrefsBrowserTestT<kImportNothing>
+    FirstRunMasterPrefsImportNothing;
+IN_PROC_BROWSER_TEST_F(FirstRunMasterPrefsImportNothing,
                        ImportNothingAndShowNewTabPage) {
-  EXPECT_TRUE(first_run::DidPerformProfileImport(NULL));
+  EXPECT_EQ(first_run::AUTO_IMPORT_CALLED, first_run::auto_import_state());
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), GURL(chrome::kChromeUINewTabURL), CURRENT_TAB,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);