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

chrome/utility/importer/firefox_importer.cc

Index: chrome/utility/importer/firefox_importer.cc
diff --git a/chrome/utility/importer/firefox_importer.cc b/chrome/utility/importer/firefox_importer.cc
index c74947b685e91f288f07952175711e58a8890a2b..54c93f161c40cad7064cff12a7711055e3d21519 100644
--- a/chrome/utility/importer/firefox_importer.cc
+++ b/chrome/utility/importer/firefox_importer.cc
@@ -8,6 +8,7 @@
 
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
+#include "base/json/json_file_value_serializer.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
@@ -383,12 +384,58 @@ void FirefoxImporter::ImportHomepage() {
 
 void FirefoxImporter::GetSearchEnginesXMLData(
     std::vector<std::string>* search_engine_data) {
-  // TODO(mpawlowski): This may no longer work, search engines are stored in
-  // search.json since Firefox 3.5, not in search.sqlite. XML definitions are
-  // still necessary. http://crbug.com/329175
   base::FilePath file = source_path_.AppendASCII("search.sqlite");
-  if (!base::PathExists(file))
+  if (!base::PathExists(file)) {
+    // since Firefox 3.5, search engines are no longer stored in search.sqlite.

[Ilya Sherman, 2014/04/23 04:56:01]

nit: Please start the sentence with a capital letter.

+    // Instead, search.json is used for storing search engines.
+
+    base::FilePath search_json_file = source_path_.AppendASCII("search.json");
+    if (!base::PathExists(search_json_file))
+      return;
+
+    JSONFileValueSerializer serializer(search_json_file);
+    scoped_ptr<base::Value> root(serializer.Deserialize(NULL, NULL));
+
+    if (!root.get())

[Ilya Sherman, 2014/04/23 04:56:01]

nit: No need for the ".get()".

+      return;
+
+    base::DictionaryValue* search_root =
+        static_cast<base::DictionaryValue*>(root.release());

[Ilya Sherman, 2014/04/23 04:56:01]

Please use GetAsDictionary() for a type-safe conversion.

[Ilya Sherman, 2014/04/23 04:56:01]

Releasing the scoped_ptr here leaks memory.  It's quite rare to need to call
release().  If you ever need to transfer ownership, you should instead call
Pass().

+    const std::string kDirectories("directories");
+    const base::DictionaryValue* search_directories = NULL;
+
+    if (!search_root->GetDictionary(kDirectories, &search_directories))
+      return;
+
+    // search engine list can be found from key <engines> of the dictionary.

[Ilya Sherman, 2014/04/23 04:56:01]

nit: Please write this as a complete sentence, including capitalizing the first
letter.

+    // key <engines> is a grandchild of key <directories>.
+    // However, key <engines> parent's key is dynamic which
+    // depends on operating systems. For example,
+    // Ubuntu: /usr/lib/firefox/distribution/searchplugins/locale/en-US
+    // Windows: C:\\Program Files (x86)\\Mozilla Firefox\\browser\\searchplugins
+    // Therefore, it needs to be retrieved by using iterator
+
+    base::DictionaryValue::Iterator it(*search_directories);
+    const base::ListValue* search_engines = NULL;

[Ilya Sherman, 2014/04/23 04:56:01]

nit: Please move this down even closer to where it's first used.

+    const std::string kEngines(it.key() + ".engines");
+
+    if (search_directories->GetList(kEngines, &search_engines)) {
+      const std::string kFilePath("filePath");
+      for (size_t i = 0; i < search_engines->GetSize(); ++i) {
+        const base::DictionaryValue* engine_info = NULL;
+        if (search_engines->GetDictionary(i, &engine_info)) {
+          std::string file_path;
+          if (engine_info->GetString(kFilePath, &file_path)) {
+            std::string file_data;
+            base::FilePath xml_file = base::FilePath::FromUTF8Unsafe(file_path);
+            base::ReadFileToString(xml_file,  &file_data);
+            search_engine_data->push_back(file_data);
+          }
+        }
+      }
+    }
     return;
+  }

[Ilya Sherman, 2014/04/23 04:56:01]

Please factor this code out into a separate method.

 
   sql::Connection db;
   if (!db.Open(file))