Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(69)

Unified Diff: components/search_engines/template_url.cc

Issue 2516963002: Fix TemplateUrl::MatchesData comparison of search_terms_replacement_key (Closed)
Patch Set: More fixes after review Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | components/search_engines/template_url_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..eb1998a24b0cdf47f116e2c476a62fbf4ab35963 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";
+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
@@ -160,6 +163,22 @@ bool IsTemplateParameterString(const std::string& param) {
(*(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
@@ -629,7 +648,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) {
Marc Treib 2016/11/23 10:27:43 Post-commit drive-by: This used to check for "goog
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));
@@ -1192,10 +1211,8 @@ 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)
data_.search_terms_replacement_key = google_util::kInstantExtendedAPIParam;
- }
}
TemplateURL::~TemplateURL() {
@@ -1240,23 +1257,23 @@ bool TemplateURL::MatchesData(const TemplateURL* t_url,
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 {
« no previous file with comments | « no previous file | components/search_engines/template_url_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698