Import keywords for search engines imported from Firefox

components/search_engines/template_url_parser.cc

Index: components/search_engines/template_url_parser.cc
diff --git a/components/search_engines/template_url_parser.cc b/components/search_engines/template_url_parser.cc
index 5c6eae5ef587141ae01d1cddb2f5123054ec9177..fc2943f467b43878f6a96107ff300f566a8b0bef 100644
--- a/components/search_engines/template_url_parser.cc
+++ b/components/search_engines/template_url_parser.cc
@@ -34,6 +34,7 @@ const char kImageElement[] = "Image";
 const char kOpenSearchDescriptionElement[] = "OpenSearchDescription";
 const char kFirefoxSearchDescriptionElement[] = "SearchPlugin";
 const char kInputEncodingElement[] = "InputEncoding";
+const char kAliasElement[] = "Alias";
 
 // Various XML attributes used.
 const char kURLTypeAttribute[] = "type";
@@ -115,6 +116,7 @@ class TemplateURLParsingContext {
     SHORT_NAME,
     IMAGE,
     INPUT_ENCODING,
+    ALIAS

[Peter Kasting, 2014/07/28 22:48:31]

Nit: Add trailing comma (means future additions won't have to touch this line)

[Nikhil, 2014/07/29 08:29:21]

Done.

   };
 
   enum Method {
@@ -243,6 +245,9 @@ void TemplateURLParsingContext::EndElementImpl(void* ctx, const xmlChar* name) {
     case TemplateURLParsingContext::SHORT_NAME:

[Peter Kasting, 2014/07/28 22:48:31]

Nit: The order of cases in this switch seems random -- can you change it to
match the enum order?

[Nikhil, 2014/07/29 08:29:21]

Done.

       context->data_.short_name = context->string_;
       break;
+    case TemplateURLParsingContext::ALIAS:
+      context->data_.SetKeyword(context->string_);
+      break;
     case TemplateURLParsingContext::IMAGE: {
       GURL image_url(base::UTF16ToUTF8(context->string_));
       if (image_url.SchemeIs(url::kDataScheme)) {
@@ -298,7 +303,12 @@ TemplateURL* TemplateURLParsingContext::GetTemplateURL(
   if (derive_image_from_url_ && data_.favicon_url.is_empty())
     data_.favicon_url = TemplateURL::GenerateFaviconURL(search_url);
 
-  data_.SetKeyword(TemplateURL::GenerateKeyword(search_url));
+  // When |keyword| is equal to "dummy", it means that no alias was mentioned
+  // in XML file while importing search engines. We generate based on url in
+  // that case. Else, we use alias mentioned in XML file.

[Peter Kasting, 2014/07/28 22:48:31]

Nit: Clearer and shorter:

  // Generate a keyword for this search engine if a custom one was not present
  // in the imported data.

[Nikhil, 2014/07/29 08:29:21]

Done.

+  if (base::UTF16ToUTF8(data_.keyword()) == "dummy")

[Peter Kasting, 2014/07/28 22:48:31]

I'm not a big fan of checking against "dummy", as in theory a user could have
manually set a keyword to that.  Can we instead actually track directly whether
a keyword has been set in the TemplateURLParsingContext?

[Nikhil, 2014/07/29 08:29:21]

Noted, it could very well be possible. To track this, I've introduced a boolean
in TemplateURLData. I'm setting it while extracting keyword from <Alias>
element. PTAL.

+    data_.SetKeyword(TemplateURL::GenerateKeyword(search_url));
+
   data_.show_in_default_list = show_in_default_list;
 
   // Bail if the search URL is empty or if either TemplateURLRef is invalid.
@@ -325,6 +335,7 @@ void TemplateURLParsingContext::InitMapping() {
   (*kElementNameToElementTypeMap)[kFirefoxSearchDescriptionElement] =
       OPEN_SEARCH_DESCRIPTION;
   (*kElementNameToElementTypeMap)[kInputEncodingElement] = INPUT_ENCODING;
+  (*kElementNameToElementTypeMap)[kAliasElement] = ALIAS;
 }
 
 void TemplateURLParsingContext::ParseURL(const xmlChar** atts) {