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..70fab6695531a78406f891c1a0a31175562dbfb4 100644
--- a/chrome/browser/importer/in_process_importer_bridge.cc
+++ b/chrome/browser/importer/in_process_importer_bridge.cc
@@ -101,25 +101,24 @@ 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 |raw_url|. |title| may be empty.

[Peter Kasting, 2014/10/07 23:17:38]

This comment should probably also talk about what constraints, if any, apply to
the keyword and URL.

[Tapu Ghose, 2014/10/12 00:58:20]

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 base::string16& raw_url) {

[Ilya Sherman, 2014/10/07 22:08:22]

nit: You might as well drop the "raw" prefix at this point.  Or, if you want to
emphasize that the URL might not be valid, I'd name this something like
"possibly_invalid_url".

[Tapu Ghose, 2014/10/12 00:58:20]

Dropped "raw" prefix. Here, the url can be either valid or invalid. Hence, not
naming it as "possibly_invalid_url".

+  // Skip if the |raw_url| is empty.

[Peter Kasting, 2014/10/07 23:17:38]

This comment just restates the code.  Instead, if a comment is needed, say _why_
the URL might be empty.

[Tapu Ghose, 2014/10/12 00:58:20]

Agree with you.

+  if (raw_url.empty())
     return NULL;
 
   TemplateURLData data;
   if (keyword.empty())
-    data.SetKeyword(TemplateURL::GenerateKeyword(url));
+    data.SetKeyword(TemplateURL::GenerateKeyword(GURL(raw_url)));

[Peter Kasting, 2014/10/07 23:17:38]

This assumes that |raw_url| will construct a valid GURL.  Is that assumption
guaranteed?  See also comments in importer_data_types.h.

[Tapu Ghose, 2014/10/12 00:58:20]

The assumption is not guaranteed. Updated logic so address it.

   else
     data.SetKeyword(keyword);
   // We set short name by using the title if it exists.
   // Otherwise, we use the shortcut.
   data.short_name = title.empty() ? keyword : title;
-  data.SetURL(
-      TemplateURLRef::DisplayURLToURLRef(base::UTF8ToUTF16(url.spec())));
+  data.SetURL(TemplateURLRef::DisplayURLToURLRef(raw_url));
   return new TemplateURL(data);
 }
 
@@ -238,7 +237,7 @@ 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].raw_url));
   }
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       base::Bind(&ProfileWriter::AddKeywords, writer_,