Importing certain bookmarks from firefox and HTML file as search engines.

chrome/browser/importer/in_process_importer_bridge.cc

Index: chrome/browser/importer/in_process_importer_bridge.cc
diff --git a/chrome/browser/importer/in_process_importer_bridge.cc b/chrome/browser/importer/in_process_importer_bridge.cc
index 853788ea8c1c3070adcd4aa19b91ba8f82917868..59640e26da653541decb86a2c8c5e3c48a844a57 100644
--- a/chrome/browser/importer/in_process_importer_bridge.cc
+++ b/chrome/browser/importer/in_process_importer_bridge.cc
@@ -101,13 +101,16 @@ class FirefoxURLParameterFilter : public TemplateURLParser::ParameterFilter {
   DISALLOW_COPY_AND_ASSIGN(FirefoxURLParameterFilter);
 };
 
-// Creates a TemplateURL with the |keyword| and |url|. |title| may be empty.
+// Creates a TemplateURL with the |keyword| and |url|. If the |url| is invalid
+// then |raw_url| is used in creating the TemplateURL so that it can be
+// imported as search engine. |title| and |raw_url| may be empty.

[Ilya Sherman, 2014/09/30 20:54:12]

nit: "as search engine" -> "as a search engine"

[Tapu Ghose, 2014/10/07 02:02:43]

Done.

 // This function transfers ownership of the created TemplateURL to the caller.
 TemplateURL* CreateTemplateURL(const base::string16& title,
                                const base::string16& keyword,
-                               const GURL& url) {
-  // Skip if the url is invalid.
-  if (!url.is_valid())
+                               const GURL& url,
+                               const base::string16& raw_url) {

[Ilya Sherman, 2014/09/30 20:54:12]

Peter understands the TemplateURL code better than I do; but my guess is that it
would be better to pass in a single parameter to this method, rather than
passing both a |url| and a |raw_url|.  That would, I think, make the code easier
to understand.

[Peter Kasting, 2014/10/01 01:02:54]

I agree.  The caller should simply not call this if we don't want to create a
TURL.

[Tapu Ghose, 2014/10/07 02:02:43]

Acknowledged.

[Tapu Ghose, 2014/10/07 02:02:43]

In that case I would use |raw_url| and get rid of |url|.
Reason: When the execution reaches this particular chunk of codes, both
url.spec() and url.possibly_invalid_spec() returns empty (if url is invalid).
Consequently, it becomes troublesome while passing parameter to SetURL function
at line 126. In addition, we can create a GURL object by using |raw_url|, if
needed.

+  // Skip if the url is invalid and raw_url is empty.
+  if (!url.is_valid() && raw_url.empty())
     return NULL;
 
   TemplateURLData data;
@@ -118,8 +121,10 @@ TemplateURL* CreateTemplateURL(const base::string16& title,
   // We set short name by using the title if it exists.
   // Otherwise, we use the shortcut.
   data.short_name = title.empty() ? keyword : title;
+  base::string16 str_url =
+      url.is_valid() ? base::UTF8ToUTF16(url.spec()) : raw_url;
   data.SetURL(
-      TemplateURLRef::DisplayURLToURLRef(base::UTF8ToUTF16(url.spec())));
+      TemplateURLRef::DisplayURLToURLRef(str_url));
   return new TemplateURL(data);
 }
 
@@ -238,7 +243,8 @@ void InProcessImporterBridge::SetKeywords(
     owned_template_urls.push_back(
         CreateTemplateURL(url_keywords[i].display_name,
                           url_keywords[i].keyword,
-                          url_keywords[i].url));
+                          url_keywords[i].url,
+                          url_keywords[i].raw_url));
   }
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       base::Bind(&ProfileWriter::AddKeywords, writer_,