Fix TemplateUrl::MatchesData comparison of search_terms_replacement_key

components/search_engines/template_url.cc

Index: components/search_engines/template_url.cc
diff --git a/components/search_engines/template_url.cc b/components/search_engines/template_url.cc
index 1291fbd1ba88f59401269ad60be4f5d8a675ffd8..a43c660e4c5f006c18edd0a9a87091b2adf2d367 100644
--- a/components/search_engines/template_url.cc
+++ b/components/search_engines/template_url.cc
@@ -67,6 +67,9 @@ const char kDefaultCount[] = "10";
 // Used if the output encoding parameter is required.
 const char kOutputEncodingType[] = "UTF-8";
 
+const char kGoogleInstantExtendedEnabledKey[] =

[Peter Kasting, 2016/11/21 02:24:50]

Nit: constexpr (the rest of these here can be constexpr too, but not required to
fix those up)

+    "{google:instantExtendedEnabledKey}";
+
 // Attempts to encode |terms| and |original_query| in |encoding| and escape
 // them.  |terms| may be escaped as path or query depending on |is_in_query|;
 // |original_query| is always escaped as query.  Returns whether the encoding
@@ -629,7 +632,7 @@ bool TemplateURLRef::ParseParameter(size_t start,
   } else if (parameter == "google:instantExtendedEnabledParameter") {
     replacements->push_back(Replacement(GOOGLE_INSTANT_EXTENDED_ENABLED,
                                         start));
-  } else if (parameter == "google:instantExtendedEnabledKey") {
+  } else if (parameter == kGoogleInstantExtendedEnabledKey) {
     url->insert(start, google_util::kInstantExtendedAPIParam);
   } else if (parameter == "google:iOSSearchLanguage") {
     replacements->push_back(Replacement(GOOGLE_IOS_SEARCH_LANGUAGE, start));
@@ -1192,8 +1195,7 @@ TemplateURL::TemplateURL(const TemplateURLData& data, Type type)
   ResizeURLRefVector();
   SetPrepopulateId(data_.prepopulate_id);
 
-  if (data_.search_terms_replacement_key ==
-      "{google:instantExtendedEnabledKey}") {
+  if (data_.search_terms_replacement_key == kGoogleInstantExtendedEnabledKey) {

[Peter Kasting, 2016/11/21 02:24:50]

Nit: {} no longer needed

     data_.search_terms_replacement_key = google_util::kInstantExtendedAPIParam;
   }
 }
@@ -1239,7 +1241,7 @@ bool TemplateURL::MatchesData(const TemplateURL* t_url,
   if (!t_url || !data)
     return !t_url && !data;
 
-  return (t_url->short_name() == data->short_name()) &&
+  if ((t_url->short_name() == data->short_name()) &&
       t_url->HasSameKeywordAs(*data, search_terms_data) &&
       (t_url->url() == data->url()) &&
       (t_url->suggestions_url() == data->suggestions_url) &&
@@ -1254,9 +1256,18 @@ bool TemplateURL::MatchesData(const TemplateURL* t_url,
       (t_url->favicon_url() == data->favicon_url) &&
       (t_url->safe_for_autoreplace() == data->safe_for_autoreplace) &&
       (t_url->input_encodings() == data->input_encodings) &&
-      (t_url->alternate_urls() == data->alternate_urls) &&
-      (t_url->search_terms_replacement_key() ==
-          data->search_terms_replacement_key);
+      (t_url->alternate_urls() == data->alternate_urls)) {
+    // Special case for search_terms_replacement_key comparison, because of
+    // its special initialization in TemplateUrl constructor.

[Peter Kasting, 2016/11/21 02:24:50]

Nit: Might be more readable to add a local helper to this file called
SearchTermsReplacementKeysMatch() which takes
t_url->search_terms_replacement_key() and data->search_terms_replacement_key as
args.  Then the code here can largely stay as before.

+    if (t_url->search_terms_replacement_key() ==
+               data->search_terms_replacement_key)

[Peter Kasting, 2016/11/21 02:24:50]

Nit: I don't think this indentation is what git cl format would do

+      return true;
+    return (t_url->search_terms_replacement_key() ==
+                google_util::kInstantExtendedAPIParam) &&
+           (data->search_terms_replacement_key ==
+                kGoogleInstantExtendedEnabledKey);
+  }
+  return false;
 }
 
 base::string16 TemplateURL::AdjustedShortNameForLocaleDirection() const {