The browser importer code which runs in the utility process should not depend on chrome\browser dat…

chrome/browser/importer/in_process_importer_bridge.cc

Index: chrome/browser/importer/in_process_importer_bridge.cc
===================================================================
--- chrome/browser/importer/in_process_importer_bridge.cc	(revision 208948)
+++ chrome/browser/importer/in_process_importer_bridge.cc	(working copy)
@@ -5,11 +5,15 @@
 #include "chrome/browser/importer/in_process_importer_bridge.h"
 
 #include "base/bind.h"
+#include "base/file_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/bookmarks/imported_bookmark_entry.h"
 #include "chrome/browser/favicon/imported_favicon_usage.h"
 #include "chrome/browser/importer/importer_host.h"
 #include "chrome/browser/search_engines/template_url.h"
+#include "chrome/browser/search_engines/template_url_parser.h"
+#include "chrome/browser/search_engines/template_url_prepopulate_data.h"
+#include "chrome/browser/search_engines/template_url_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/password_form.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -22,6 +26,104 @@
 
 using content::BrowserThread;
 
+namespace {
+
+// FirefoxURLParameterFilter is used to remove parameter mentioning Firefox from
+// the search URL when importing search engines.
+class FirefoxURLParameterFilter : public TemplateURLParser::ParameterFilter {
+ public:
+  FirefoxURLParameterFilter() {}
+  virtual ~FirefoxURLParameterFilter() {}
+
+  // TemplateURLParser::ParameterFilter method.
+  virtual bool KeepParameter(const std::string& key,
+                             const std::string& value) OVERRIDE {
+    std::string low_value = StringToLowerASCII(value);
+    if (low_value.find("mozilla") != std::string::npos ||
+        low_value.find("firefox") != std::string::npos ||
+        low_value.find("moz:") != std::string::npos )

[gab, 2013/06/27 23:12:44]

Remove extra space before ')'

[gab, 2013/06/27 23:12:44]

Wrap in {} since conditional is more than 1 line.

[ananta, 2013/06/27 23:26:09]

Done.

[ananta, 2013/06/27 23:26:09]

Done.

+      return false;
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FirefoxURLParameterFilter);
+};
+
+// Creates a TemplateURL with the |keyword| and |url|. |title| may be empty.
+// This function transfers ownership of the created TemplateURL to the caller.
+TemplateURL* CreateTemplateURL(const string16& title,
+                               const string16& keyword,
+                               const GURL& url) {
+  // Skip if the url is invalid.
+  if (!url.is_valid())
+    return NULL;
+
+  TemplateURLData data;
+  if (keyword.empty()) {

[gab, 2013/06/27 23:12:44]

I realize that you are copying the old behavior for both IE and FF in a single
function here, but doing either based on keyword.empty() seems very obscure;
it's not clear to me why they need to be different at all (other than generating
the keyword if it's empty).

[ananta, 2013/06/27 23:26:09]

Done.

+    data.short_name = title;
+    data.SetKeyword(TemplateURLService::GenerateKeyword(url));
+    data.SetURL(url.spec());
+    data.show_in_default_list = true;
+  } else {
+    // We set short name by using the title if it exists.
+    // Otherwise, we use the shortcut.
+    data.short_name = title.empty() ? keyword : title;
+    data.SetKeyword(keyword);
+    data.SetURL(TemplateURLRef::DisplayURLToURLRef(UTF8ToUTF16(url.spec())));
+  }
+  return new TemplateURL(NULL, data);
+}
+
+// Parses the OpenSearch XML files in |xml_files| and populates |search_engines|
+// with the resulting TemplateURLs.
+void ParseSearchEnginesFromXMLData(

[gab, 2013/06/27 23:12:44]

Add "Firefox" to the name of this method since comments in the method suggest
this is parsing data specific to the way Firefox writes it.

[ananta, 2013/06/27 23:26:09]

Done.

+    const std::vector<std::string>& xml_data,
+    std::vector<TemplateURL*>* search_engines) {
+  DCHECK(search_engines);
+
+  typedef std::map<std::string, TemplateURL*> SearchEnginesMap;
+  SearchEnginesMap search_engine_for_url;
+  std::string content;
+  // The first XML file represents the default search engine in Firefox 3, so we
+  // need to keep it on top of the list.
+  SearchEnginesMap::const_iterator default_turl = search_engine_for_url.end();
+  for (std::vector<std::string>::const_iterator xml_iter =
+           xml_data.begin(); xml_iter != xml_data.end(); ++xml_iter) {
+    FirefoxURLParameterFilter param_filter;

[gab, 2013/06/27 23:12:44]

Can |param_filter| live outside the loop instead?

[ananta, 2013/06/27 23:26:09]

Done.

+    TemplateURL* template_url = TemplateURLParser::Parse(NULL, true,
+        xml_iter->data(), xml_iter->length(), &param_filter);
+    if (template_url) {
+      SearchEnginesMap::iterator iter =
+          search_engine_for_url.find(template_url->url());
+      if (iter == search_engine_for_url.end()) {
+        iter = search_engine_for_url.insert(
+            std::make_pair(template_url->url(), template_url)).first;
+      } else {
+        // We have already found a search engine with the same URL.  We give
+        // priority to the latest one found, as GetSearchEnginesXMLFiles()
+        // returns a vector with first Firefox default search engines and then
+        // the user's ones.  We want to give priority to the user ones.
+        delete iter->second;
+        iter->second = template_url;
+      }
+      if (default_turl == search_engine_for_url.end())
+        default_turl = iter;
+    }
+  }
+
+  // Put the results in the |search_engines| vector.
+  for (SearchEnginesMap::iterator t_iter = search_engine_for_url.begin();
+       t_iter != search_engine_for_url.end(); ++t_iter) {
+    if (t_iter == default_turl)
+      search_engines->insert(search_engines->begin(), default_turl->second);
+    else
+      search_engines->push_back(t_iter->second);
+  }
+}
+
+}

[gab, 2013/06/27 23:12:44]

+  // namespace

[ananta, 2013/06/27 23:26:09]

Done.

+
 InProcessImporterBridge::InProcessImporterBridge(
     ProfileWriter* writer,
     base::WeakPtr<ImporterHost> host) : writer_(writer),
@@ -68,16 +170,34 @@
 }
 
 void InProcessImporterBridge::SetKeywords(
-    const std::vector<TemplateURL*>& template_urls,
+    const std::vector<importer::URLKeywordInfo>& url_keywords,
     bool unique_on_host_and_path) {
   ScopedVector<TemplateURL> owned_template_urls;
-  std::copy(template_urls.begin(), template_urls.end(),
-            std::back_inserter(owned_template_urls));
+  for (size_t i = 0; i < url_keywords.size(); ++i) {
+    owned_template_urls.push_back(
+        CreateTemplateURL(url_keywords[i].display_name,
+                          url_keywords[i].keyword,
+                          url_keywords[i].url));
+  }
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       base::Bind(&ProfileWriter::AddKeywords, writer_,
                  base::Passed(&owned_template_urls), unique_on_host_and_path));
 }
 
+void InProcessImporterBridge::SetFirefoxSearchEnginesXMLData(
+    const std::vector<std::string>& search_engine_data) {
+  std::vector<TemplateURL*> search_engines;
+  ParseSearchEnginesFromXMLData(search_engine_data, &search_engines);
+
+  ScopedVector<TemplateURL> owned_template_urls;
+  std::copy(search_engines.begin(), search_engines.end(),
+            std::back_inserter(owned_template_urls));
+
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+      base::Bind(&ProfileWriter::AddKeywords, writer_,
+                 base::Passed(&owned_template_urls), true));
+}
+
 void InProcessImporterBridge::SetPasswordForm(
     const content::PasswordForm& form) {
   BrowserThread::PostTask(