Import keywords for search engines imported 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 a682dbdf70d1bd85a81ba90f79ac42de44725c6f..1ac2a7ece6fe0e1e3ebdc4ff396aec6a50200966 100644
--- a/chrome/utility/importer/firefox_importer.cc
+++ b/chrome/utility/importer/firefox_importer.cc
@@ -480,6 +480,18 @@ void FirefoxImporter::GetSearchEnginesXMLData(
 
 void FirefoxImporter::GetSearchEnginesXMLDataFromJSON(
     std::vector<std::string>* search_engine_data) {
+  // search-metadata.json contains keywords for search engines. This
+  // file exists only if the user has set keywords for search engines.
+  base::FilePath search_metadata_json_file =
+      source_path_.AppendASCII("search-metadata.json");
+  JSONFileValueSerializer metadata_serializer(search_metadata_json_file);
+  scoped_ptr<base::Value> metadata_root(
+      metadata_serializer.Deserialize(NULL, NULL));
+  const base::DictionaryValue* search_metadata_root = NULL;
+  if (metadata_root)
+    metadata_root->GetAsDictionary(&search_metadata_root);
+
+  // search.json contains information about search engines to import.
   base::FilePath search_json_file = source_path_.AppendASCII("search.json");
   if (!base::PathExists(search_json_file))
     return;
@@ -556,6 +568,26 @@ void FirefoxImporter::GetSearchEnginesXMLDataFromJSON(
 
         std::string file_data;
         base::ReadFileToString(xml_file, &file_data);
+
+        // If a keyword is mentioned for this search engine, then add
+        // it to the XML string as an <Alias> element and use this updated
+        // string.
+        if (search_metadata_root && search_metadata_root->HasKey(file_path)) {
+          const base::DictionaryValue* search_xml_path = NULL;

[Ilya Sherman, 2014/07/29 17:24:55]

Optional nit: It might be cleaner to move this variable up before line 575, and
then combine the two if-stmts into one.

[Nikhil, 2014/07/30 07:31:29]

Done.

+          if (search_metadata_root->GetDictionaryWithoutPathExpansion(
+              file_path, &search_xml_path)) {
+            std::string alias;
+            search_xml_path->GetString("alias", &alias);
+
+            // Add <Alias> element as the last child element.
+            size_t end_of_parent = file_data.find("</SearchPlugin>");
+            if (end_of_parent != std::string::npos)

[Ilya Sherman, 2014/07/29 17:24:56]

nit: You should still use curly braces, since the body of the if-stmt spans two
lines of text.  (That is, the constraint is visual, rather than having to do
with what the compiler sees.)

[Nikhil, 2014/07/30 07:31:29]

Done.

+              file_data.insert(end_of_parent,
+                               "<Alias>" + alias + "</Alias> \n");

[Peter Kasting, 2014/07/29 18:33:31]

Why did you pick "alias" anyway?  Wouldn't "keyword" make more sense?  Does some
browser already use "alias" to mean this?  Is "keyword" already taken?

[Nikhil, 2014/07/30 07:31:29]

search-metadata.json uses "alias" as key to store keyword. I thought it would be
consistent to use same as element in XML string.

If you think "keyword" makes more sense, then please let me know and I'll update
the patch. I've no preference, either one works for me.

+          }
+        }
+
+        // No keyword is mentioned for this search-engine.

[Ilya Sherman, 2014/07/29 17:24:56]

nit: This comment is no longer correct.

[Nikhil, 2014/07/30 07:31:29]

Done.

         search_engine_data->push_back(file_data);
       }
     }