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

chrome/browser/importer/firefox_importer_browsertest.cc

Index: chrome/browser/importer/firefox_importer_browsertest.cc
diff --git a/chrome/browser/importer/firefox_importer_browsertest.cc b/chrome/browser/importer/firefox_importer_browsertest.cc
index c56af96e163f76e0da386b18c2b9d88f409deae5..68a1597e921e80dfcab55713b3282701a9af6232 100644
--- a/chrome/browser/importer/firefox_importer_browsertest.cc
+++ b/chrome/browser/importer/firefox_importer_browsertest.cc
@@ -225,31 +225,44 @@ class FirefoxProfileImporterBrowserTest : public InProcessBrowserTest {
     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,
+                                  bool import_search_plugins) {

[Ilya Sherman, 2014/06/17 23:26:42]

Ditto for this variable.

     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);
+    // default search engines
+    base::FilePath default_search_engine_path = app_path_;
+    default_search_engine_path =
+        default_search_engine_path.AppendASCII("searchplugins");
+    base::CreateDirectory(default_search_engine_path);

[Ilya Sherman, 2014/06/17 23:26:42]

nit: I'd condense these five lines a bit, like so:

    // Create a directory to house default search engines.
    base::FilePath default_search_engines_path =
        app_path_.AppendASCII("searchplugins");
    base::CreateDirectory(default_search_engines_path);

+
+    // custom/installed search engines
+    base::FilePath custom_search_engine_path = profile_path_;
+    custom_search_engine_path =
+        custom_search_engine_path.AppendASCII("searchplugins");

[Ilya Sherman, 2014/06/17 23:26:42]

Are you intentionally not calling base::CreateDirectory() for this path?

[Tapu Ghose, 2014/06/18 12:16:42]

It was unintentional.

+
     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));
+      data_path = data_path.AppendASCII("firefox_searchplugins");
+      base::FilePath default_search_engine_source_path = data_path;
+      base::FilePath custom_search_engine_source_path = data_path;
+      default_search_engine_source_path =
+          default_search_engine_source_path.AppendASCII("default");
+      custom_search_engine_source_path =
+          custom_search_engine_source_path.AppendASCII("custom");

[Ilya Sherman, 2014/06/17 23:26:42]

I'd similarly condense these six lines down to four.

+      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;
@@ -280,8 +293,8 @@ class FirefoxProfileImporterBrowserTest : public InProcessBrowserTest {
 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(), true);
 }
 
 IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest,
@@ -289,6 +302,14 @@ IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest,
   bool import_search_engines = false;
   scoped_refptr<FirefoxObserver> observer(
       new FirefoxObserver(import_search_engines));
-  Firefox3xImporterBrowserTest("firefox35_profile", observer.get(),
-                               observer.get(), import_search_engines);
+  FirefoxImporterBrowserTest("firefox35_profile",
+                             observer.get(),
+                             observer.get(),
+                             import_search_engines);
+}
+
+IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest, FirefoxImporter) {
+  scoped_refptr<FirefoxObserver> observer(new FirefoxObserver());
+  FirefoxImporterBrowserTest(
+      "firefox_profile", observer.get(), observer.get(), true);
 }