Parsed search.json for importing search engine settings from FireFox.

chrome/browser/importer/firefox_importer_browsertest.cc

 // Copyright (c) 2013 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 "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
@@ skipping 81 matching lines @@
     "http://www.webster.com/cgi-bin/dictionary?va={searchTerms}" },
   // Search keywords.
   { L"\x4E2D\x6587", "http://www.google.com/" },
 };
 
 class FirefoxObserver : public ProfileWriter,
                         public importer::ImporterProgressObserver {
  public:
   FirefoxObserver()
       : ProfileWriter(NULL), bookmark_count_(0), history_count_(0),
-        password_count_(0), keyword_count_(0), import_search_engines_(true) {
+        password_count_(0), keyword_count_(0) {
   }
 
   explicit FirefoxObserver(bool import_search_engines)
       : ProfileWriter(NULL), bookmark_count_(0), history_count_(0),
-        password_count_(0), keyword_count_(0),
-        import_search_engines_(import_search_engines) {
+        password_count_(0), keyword_count_(0) {
   }

[Ilya Sherman, 2014/06/19 00:49:15]

Please remove this constructor entirely.

 
   // importer::ImporterProgressObserver:
   virtual void ImportStarted() OVERRIDE {}
   virtual void ImportItemStarted(importer::ImportItem item) OVERRIDE {}
   virtual void ImportItemEnded(importer::ImportItem item) OVERRIDE {}
   virtual void ImportEnded() OVERRIDE {
     base::MessageLoop::current()->Quit();
     EXPECT_EQ(arraysize(kFirefoxBookmarks), bookmark_count_);
     EXPECT_EQ(1U, history_count_);
     EXPECT_EQ(arraysize(kFirefoxPasswords), password_count_);
-    if (import_search_engines_)
-      EXPECT_EQ(arraysize(kFirefoxKeywords), keyword_count_);
+    EXPECT_EQ(arraysize(kFirefoxKeywords), keyword_count_);
   }
 
   virtual bool BookmarkModelIsLoaded() const OVERRIDE {
     // Profile is ready for writing.
     return true;
   }
 
   virtual bool TemplateURLServiceIsLoaded() const OVERRIDE {
     return true;
   }
@@ skipping 62 matching lines @@
       const std::vector<ImportedFaviconUsage>& favicons) OVERRIDE {
   }
 
  private:
   virtual ~FirefoxObserver() {}
 
   size_t bookmark_count_;
   size_t history_count_;
   size_t password_count_;
   size_t keyword_count_;
-  bool import_search_engines_;
 };
 
 }  // namespace
 
 // These tests need to be browser tests in order to be able to run the OOP
 // import (via ExternalProcessImporterHost) which launches a utility process on
 // supported platforms.
 class FirefoxProfileImporterBrowserTest : public InProcessBrowserTest {
  protected:
   virtual void SetUp() OVERRIDE {
     // Creates a new profile in a new subdirectory in the temp directory.
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     base::FilePath test_path = temp_dir_.path().AppendASCII("ImporterTest");
     base::DeleteFile(test_path, true);
     base::CreateDirectory(test_path);
     profile_path_ = test_path.AppendASCII("profile");
     app_path_ = test_path.AppendASCII("app");
     base::CreateDirectory(app_path_);
 
     // This will launch the browser test and thus needs to happen last.
     InProcessBrowserTest::SetUp();
   }
 
-  void Firefox3xImporterBrowserTest(
-      std::string profile_dir,
-      importer::ImporterProgressObserver* observer,
-      ProfileWriter* writer,
-      bool import_search_plugins) {
+  void FirefoxImporterBrowserTest(std::string profile_dir,
+                                  importer::ImporterProgressObserver* observer,
+                                  ProfileWriter* writer) {
     base::FilePath data_path;
     ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_path));
     data_path = data_path.AppendASCII(profile_dir);
     ASSERT_TRUE(base::CopyDirectory(data_path, profile_path_, true));
+
     ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_path));
     data_path = data_path.AppendASCII("firefox3_nss");
     ASSERT_TRUE(base::CopyDirectory(data_path, profile_path_, false));
 
-    base::FilePath search_engine_path = app_path_;
-    search_engine_path = search_engine_path.AppendASCII("searchplugins");
-    base::CreateDirectory(search_engine_path);
-    if (import_search_plugins) {
-      ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_path));
-      data_path = data_path.AppendASCII("firefox3_searchplugins");
-      if (!base::PathExists(data_path)) {
-        // TODO(maruel):  Create search test data that we can open source!
-        LOG(ERROR) << "Missing internal test data";
-        return;
-      }
-      ASSERT_TRUE(base::CopyDirectory(data_path, search_engine_path, false));
-    }
+    // Create a directory to house default search engines.
+    base::FilePath default_search_engine_path =
+        app_path_.AppendASCII("searchplugins");
+    base::CreateDirectory(default_search_engine_path);
+
+    // Create a directory to house custom/installed search engines.
+    base::FilePath custom_search_engine_path =
+        profile_path_.AppendASCII("searchplugins");
+    base::CreateDirectory(custom_search_engine_path);
+
+    // Import search engines

[Ilya Sherman, 2014/06/19 00:49:15]

nit: "Import" -> "Copy over"

[Ilya Sherman, 2014/06/19 00:49:16]

nit: Please end the sentence with a period.

+    ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_path));
+    data_path = data_path.AppendASCII("firefox_searchplugins");
+    base::FilePath default_search_engine_source_path =
+        data_path.AppendASCII("default");
+    base::FilePath custom_search_engine_source_path =
+        data_path.AppendASCII("custom");
+    ASSERT_TRUE(base::CopyDirectory(
+        default_search_engine_source_path, default_search_engine_path, false));
+    ASSERT_TRUE(base::CopyDirectory(
+        custom_search_engine_source_path, custom_search_engine_path, false));
 
     importer::SourceProfile source_profile;
     source_profile.importer_type = importer::TYPE_FIREFOX;
     source_profile.app_path = app_path_;
     source_profile.source_path = profile_path_;
     source_profile.locale = "en-US";
 
-    int items = importer::HISTORY | importer::PASSWORDS | importer::FAVORITES;
-    if (import_search_plugins)
-      items = items | importer::SEARCH_ENGINES;
+    int items = importer::HISTORY | importer::PASSWORDS |
+        importer::FAVORITES | importer::SEARCH_ENGINES;
 
     // Deletes itself.
     ExternalProcessImporterHost* host = new ExternalProcessImporterHost;
     host->set_observer(observer);
     host->StartImportSettings(source_profile,
                               browser()->profile(),
                               items,
-                              make_scoped_refptr(writer).get());
+                              writer);
     base::MessageLoop::current()->Run();
   }
 
   base::ScopedTempDir temp_dir_;
   base::FilePath profile_path_;
   base::FilePath app_path_;
 };
 
 IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest,
                        MAYBE_IMPORTER(Firefox30Importer)) {
   scoped_refptr<FirefoxObserver> observer(new FirefoxObserver());
-  Firefox3xImporterBrowserTest("firefox3_profile", observer.get(),
-                               observer.get(), true);
+  FirefoxImporterBrowserTest(
+      "firefox3_profile", observer.get(), observer.get());
 }
 
 IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest,
                        MAYBE_IMPORTER(Firefox35Importer)) {
-  bool import_search_engines = false;
-  scoped_refptr<FirefoxObserver> observer(
-      new FirefoxObserver(import_search_engines));
-  Firefox3xImporterBrowserTest("firefox35_profile", observer.get(),
-                               observer.get(), import_search_engines);
+  scoped_refptr<FirefoxObserver> observer(new FirefoxObserver());
+  FirefoxImporterBrowserTest(
+      "firefox35_profile", observer.get(), observer.get());
 }
+
+IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest, FirefoxImporter) {

[Ilya Sherman, 2014/06/19 00:49:15]

After looking at the tests more closely, I think you will need the
MAYBE_IMPORTER macro for this test after all.  The search engine importing logic
might work correctly across all platforms now, but other importing logic still
does not, which is likely to make this test fail where it shouldn't.

+  scoped_refptr<FirefoxObserver> observer(new FirefoxObserver());
+  FirefoxImporterBrowserTest("firefox_profile", observer.get(), observer.get());
+}