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

chrome/browser/importer/firefox3_importer.cc

Index: chrome/browser/importer/firefox3_importer.cc
===================================================================
--- chrome/browser/importer/firefox3_importer.cc	(revision 208750)
+++ chrome/browser/importer/firefox3_importer.cc	(working copy)
@@ -21,7 +21,6 @@
 #include "chrome/browser/importer/firefox_importer_utils.h"
 #include "chrome/browser/importer/importer_bridge.h"
 #include "chrome/browser/importer/nss_decryptor.h"
-#include "chrome/browser/search_engines/template_url.h"
 #include "chrome/common/time_format.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/password_form.h"
@@ -43,24 +42,6 @@
   TYPE_DYNAMIC_CONTAINER = 4
 };
 
-// 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 keyword or url is invalid.
-  if (keyword.empty() || !url.is_valid())
-    return NULL;
-
-  TemplateURLData data;
-  // 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);
-}
-
 // Loads the default bookmarks in the Firefox installed at |app_path|,
 // and stores their locations in |urls|.
 void LoadDefaultBookmarks(const base::FilePath& app_path,
@@ -220,7 +201,7 @@
     GetWholeBookmarkFolder(&db, &list, i, NULL);
 
   std::vector<ImportedBookmarkEntry> bookmarks;
-  std::vector<TemplateURL*> template_urls;
+  std::vector<importer::URLKeywordInfo> url_keywords;
   FaviconMap favicon_map;
 
   // TODO(jcampan): http://b/issue?id=1196285 we do not support POST based
@@ -308,11 +289,14 @@
       if (item->favicon)
         favicon_map[item->favicon].insert(item->url);
 
-      // This bookmark has a keyword, we import it to our TemplateURL model.
-      TemplateURL* t_url = CreateTemplateURL(
-          item->title, UTF8ToUTF16(item->keyword), item->url);
-      if (t_url)
-        template_urls.push_back(t_url);
+      // This bookmark has a keyword, we should import it.
+      if (!item->keyword.empty() && item->url.is_valid()) {
+        importer::URLKeywordInfo url_keyword_info;
+        url_keyword_info.url = item->url;
+        url_keyword_info.keyword.assign(UTF8ToUTF16(item->keyword));
+        url_keyword_info.display_name = item->title;
+        url_keywords.push_back(url_keyword_info);
+      }
     }
   }
 
@@ -324,10 +308,9 @@
         bridge_->GetLocalizedString(IDS_BOOKMARK_GROUP_FROM_FIREFOX);
     bridge_->AddBookmarks(bookmarks, first_folder_name);
   }
-  if (!template_urls.empty() && !cancelled())
-    bridge_->SetKeywords(template_urls, false);
-  else
-    STLDeleteElements(&template_urls);
+  if (!url_keywords.empty() && !cancelled()) {
+    bridge_->SetKeywords(url_keywords, false);
+  }
   if (!favicon_map.empty() && !cancelled()) {
     std::vector<ImportedFaviconUsage> favicons;
     LoadFavicons(&db, favicon_map, &favicons);
@@ -368,13 +351,10 @@
 }
 
 void Firefox3Importer::ImportSearchEngines() {
-  std::vector<base::FilePath> files;
-  GetSearchEnginesXMLFiles(&files);
+  std::vector<std::string> search_engine_data;
+  GetSearchEnginesXMLData(&search_engine_data);
 
-  std::vector<TemplateURL*> search_engines;
-  ParseSearchEnginesFromXMLFiles(files, &search_engines);
-
-  bridge_->SetKeywords(search_engines, true);
+  bridge_->SetFirefoxSearchEnginesXMLData(search_engine_data);

[gab, 2013/06/27 22:07:29]

The reason to do the import out-of-process is mostly to avoid crashing the
browser process while reading arbitrary data.

It seems ParseSearchEnginesFromXMLFiles() was only used by this call, can't we
keep this call and move it in the utility side of the split along with the
importers (changing the call slightly to use URLKeywordInfo instead of
TemplateURL)?

[ananta, 2013/06/27 22:29:55]

The ParseSearchEnginesFromXMLFiles function uses the TemplateURLParser class to
parse the SAX XML data from Firefox. This class lives in
chrome\browser\search_engines and sadly depends on Profile* etc.
which is why I moved the whole function out

 }
 
 void Firefox3Importer::ImportHomepage() {
@@ -384,8 +364,8 @@
   }
 }
 
-void Firefox3Importer::GetSearchEnginesXMLFiles(
-    std::vector<base::FilePath>* files) {
+void Firefox3Importer::GetSearchEnginesXMLData(
+    std::vector<std::string>* search_engine_data) {
   base::FilePath file = source_path_.AppendASCII("search.sqlite");
   if (!file_util::PathExists(file))
     return;
@@ -432,7 +412,9 @@
         // Looks like absolute path to the file.
         file = base::FilePath::FromUTF8Unsafe(engine);
       }
-      files->push_back(file);
+      std::string file_data;
+      file_util::ReadFileToString(file, &file_data);
+      search_engine_data->push_back(file_data);
     } while (s.Step() && !cancelled());
   }
 
@@ -455,7 +437,9 @@
   base::FileEnumerator engines(app_path, false, base::FileEnumerator::FILES);
   for (base::FilePath engine_path = engines.Next();
        !engine_path.value().empty(); engine_path = engines.Next()) {
-    files->push_back(engine_path);
+    std::string file_data;
+    file_util::ReadFileToString(file, &file_data);
+    search_engine_data->push_back(file_data);
   }
 }