Fix TemplateUrl::MatchesData comparison of search_terms_replacement_key

components/search_engines/template_url.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/search_engines/template_url.h"
 
 #include <string>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 49 matching lines @@
 // Display value for kGoogleUnescapedSearchTermsParameter.
 const char kDisplayUnescapedSearchTerms[] = "%S";
 
 // Used if the count parameter is not optional. Indicates we want 10 search
 // results.
 const char kDefaultCount[] = "10";
 
 // Used if the output encoding parameter is required.
 const char kOutputEncodingType[] = "UTF-8";
 
+constexpr char kGoogleInstantExtendedEnabledKey[] =
+    "{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
 // process succeeded.
 bool TryEncoding(const base::string16& terms,
                  const base::string16& original_query,
                  const char* encoding,
                  bool is_in_query,
                  base::string16* escaped_terms,
                  base::string16* escaped_original_query) {
@@ skipping 73 matching lines @@
   parameter_position->begin = begin;
   parameter_position->len = arraysize(kSearchTermsParameterFullEscaped) - 1;
   return true;
 }
 
 bool IsTemplateParameterString(const std::string& param) {
   return (param.length() > 2) && (*(param.begin()) == kStartParameter) &&
       (*(param.rbegin()) == kEndParameter);
 }
 
+// Special case for search_terms_replacement_key comparison, because of
+// its special initialization in TemplateUrl constructor.
+bool SearchTermsReplacementKeysMatch(
+    const std::string& search_terms_replacement_key1,
+    const std::string& search_terms_replacement_key2) {
+  if (search_terms_replacement_key1 == search_terms_replacement_key2)
+    return true;
+  if (search_terms_replacement_key1 == google_util::kInstantExtendedAPIParam &&
+      search_terms_replacement_key2 == kGoogleInstantExtendedEnabledKey)
+    return true;
+  if (search_terms_replacement_key2 == google_util::kInstantExtendedAPIParam &&
+      search_terms_replacement_key1 == kGoogleInstantExtendedEnabledKey)
+    return true;
+  return false;
+}
+
 }  // namespace
 
 
 // TemplateURLRef::SearchTermsArgs --------------------------------------------
 
 TemplateURLRef::SearchTermsArgs::SearchTermsArgs(
     const base::string16& search_terms)
     : search_terms(search_terms),
       input_type(metrics::OmniboxInputType::INVALID),
       accepted_suggestion(NO_SUGGESTIONS_AVAILABLE),
@@ skipping 449 matching lines @@
         Replacement(TemplateURLRef::GOOGLE_IMAGE_THUMBNAIL, start));
   } else if (parameter == "google:imageURL") {
     replacements->push_back(Replacement(TemplateURLRef::GOOGLE_IMAGE_URL,
                                         start));
   } else if (parameter == "google:inputType") {
     replacements->push_back(Replacement(TemplateURLRef::GOOGLE_INPUT_TYPE,
                                         start));
   } else if (parameter == "google:instantExtendedEnabledParameter") {
     replacements->push_back(Replacement(GOOGLE_INSTANT_EXTENDED_ENABLED,
                                         start));
-  } else if (parameter == "google:instantExtendedEnabledKey") {
+  } else if (parameter == kGoogleInstantExtendedEnabledKey) {

[Marc Treib, 2016/11/23 10:27:43]

Post-commit drive-by: This used to check for "google:instantExtendedEnabledKey",
but now it checks for "{google:instantExtendedEnabledKey}" (with braces). Was
that intended?

[Alexander Yashkin, 2016/11/23 10:41:51]

Not intended, i'll fix it today.

     url->insert(start, google_util::kInstantExtendedAPIParam);
   } else if (parameter == "google:iOSSearchLanguage") {
     replacements->push_back(Replacement(GOOGLE_IOS_SEARCH_LANGUAGE, start));
   } else if (parameter == "google:contextualSearchVersion") {
     replacements->push_back(
         Replacement(GOOGLE_CONTEXTUAL_SEARCH_VERSION, start));
   } else if (parameter == "google:contextualSearchContextData") {
     replacements->push_back(
         Replacement(GOOGLE_CONTEXTUAL_SEARCH_CONTEXT_DATA, start));
   } else if (parameter == "google:originalQueryForSuggestion") {
@@ skipping 542 matching lines @@
       suggestions_url_ref_(this, TemplateURLRef::SUGGEST),
       instant_url_ref_(this, TemplateURLRef::INSTANT),
       image_url_ref_(this, TemplateURLRef::IMAGE),
       new_tab_url_ref_(this, TemplateURLRef::NEW_TAB),
       contextual_search_url_ref_(this, TemplateURLRef::CONTEXTUAL_SEARCH),
       type_(type),
       engine_type_(SEARCH_ENGINE_UNKNOWN) {
   ResizeURLRefVector();
   SetPrepopulateId(data_.prepopulate_id);
 
-  if (data_.search_terms_replacement_key ==
-      "{google:instantExtendedEnabledKey}") {
+  if (data_.search_terms_replacement_key == kGoogleInstantExtendedEnabledKey)
     data_.search_terms_replacement_key = google_util::kInstantExtendedAPIParam;
-  }
 }
 
 TemplateURL::~TemplateURL() {
 }
 
 // static
 base::string16 TemplateURL::GenerateKeyword(const GURL& url) {
   DCHECK(url.is_valid());
   // Strip "www." off the front of the keyword; otherwise the keyword won't work
   // properly.  See http://code.google.com/p/chromium/issues/detail?id=6984 .
@@ skipping 24 matching lines @@
 }
 
 // static
 bool TemplateURL::MatchesData(const TemplateURL* t_url,
                               const TemplateURLData* data,
                               const SearchTermsData& search_terms_data) {
   if (!t_url || !data)
     return !t_url && !data;
 
   return (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) &&
-      (t_url->instant_url() == data->instant_url) &&
-      (t_url->image_url() == data->image_url) &&
-      (t_url->new_tab_url() == data->new_tab_url) &&
-      (t_url->search_url_post_params() == data->search_url_post_params) &&
-      (t_url->suggestions_url_post_params() ==
+         t_url->HasSameKeywordAs(*data, search_terms_data) &&
+         (t_url->url() == data->url()) &&
+         (t_url->suggestions_url() == data->suggestions_url) &&
+         (t_url->instant_url() == data->instant_url) &&
+         (t_url->image_url() == data->image_url) &&
+         (t_url->new_tab_url() == data->new_tab_url) &&
+         (t_url->search_url_post_params() == data->search_url_post_params) &&
+         (t_url->suggestions_url_post_params() ==
           data->suggestions_url_post_params) &&
-      (t_url->instant_url_post_params() == data->instant_url_post_params) &&
-      (t_url->image_url_post_params() == data->image_url_post_params) &&
-      (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->instant_url_post_params() == data->instant_url_post_params) &&
+         (t_url->image_url_post_params() == data->image_url_post_params) &&
+         (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) &&
+         SearchTermsReplacementKeysMatch(t_url->search_terms_replacement_key(),
+                                         data->search_terms_replacement_key);
 }
 
 base::string16 TemplateURL::AdjustedShortNameForLocaleDirection() const {
   base::string16 bidi_safe_short_name = data_.short_name();
   base::i18n::AdjustStringForLocaleDirection(&bidi_safe_short_name);
   return bidi_safe_short_name;
 }
 
 bool TemplateURL::SupportsReplacement(
     const SearchTermsData& search_terms_data) const {
@@ skipping 254 matching lines @@
       // patterns. This means that given patterns
       //    [ "http://foo/#q={searchTerms}", "http://foo/?q={searchTerms}" ],
       // calling ExtractSearchTermsFromURL() on "http://foo/?q=bar#q=' would
       // return false. This is important for at least Google, where such URLs
       // are invalid.
       return !search_terms->empty();
     }
   }
   return false;
 }