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

Side by Side Diff: components/omnibox/search_provider.cc

Issue 603683004: [AiS] Remove answer contents from all matches other than the first. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Add unit test, respond to comments Created 6 years, 2 months 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 unified diff | Download patch
« no previous file with comments | « components/omnibox/search_provider.h ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2012 The Chromium Authors. All rights reserved. 1 // Copyright 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/omnibox/search_provider.h" 5 #include "components/omnibox/search_provider.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <cmath> 8 #include <cmath>
9 9
10 #include "base/base64.h" 10 #include "base/base64.h"
(...skipping 855 matching lines...) Expand 10 before | Expand all | Expand 10 after
866 // that set a legal default match if possible. If Instant Extended is enabled 866 // that set a legal default match if possible. If Instant Extended is enabled
867 // and we have server-provided (and thus hopefully more accurate) scores for 867 // and we have server-provided (and thus hopefully more accurate) scores for
868 // some suggestions, we allow more of those, until we reach 868 // some suggestions, we allow more of those, until we reach
869 // AutocompleteResult::kMaxMatches total matches (that is, enough to fill the 869 // AutocompleteResult::kMaxMatches total matches (that is, enough to fill the
870 // whole popup). 870 // whole popup).
871 // 871 //
872 // We will always return any verbatim matches, no matter how we obtained their 872 // We will always return any verbatim matches, no matter how we obtained their
873 // scores, unless we have already accepted AutocompleteResult::kMaxMatches 873 // scores, unless we have already accepted AutocompleteResult::kMaxMatches
874 // higher-scoring matches under the conditions above. 874 // higher-scoring matches under the conditions above.
875 std::sort(matches.begin(), matches.end(), &AutocompleteMatch::MoreRelevant); 875 std::sort(matches.begin(), matches.end(), &AutocompleteMatch::MoreRelevant);
876 matches_.clear(); 876
877 // Guarantee that if there's a legal default match anywhere in the result 877 // Guarantee that if there's a legal default match anywhere in the result
878 // set that it'll get returned. The rotate() call does this by moving the 878 // set that it'll get returned. The rotate() call does this by moving the
879 // default match to the front of the list. 879 // default match to the front of the list.
880 ACMatches::iterator default_match = FindTopMatch(&matches); 880 ACMatches::iterator default_match = FindTopMatch(&matches);
881 if (default_match != matches.end()) 881 if (default_match != matches.end())
882 std::rotate(matches.begin(), default_match, default_match + 1); 882 std::rotate(matches.begin(), default_match, default_match + 1);
883 883
884 // It's possible to get a copy of an answer from previous matches and get the
885 // same or a different answer to another server-provided suggestion. In the
886 // future we may decide that we want to have answers attached to multiple
887 // suggestions, but the current assumption is that there should only ever be
888 // one suggestion with an answer. To maintain this assumption, remove any
889 // answers after the first.
890 RemoveExtraAnswers(&matches);
891
892 matches_.clear();
884 size_t num_suggestions = 0; 893 size_t num_suggestions = 0;
885 for (ACMatches::const_iterator i(matches.begin()); 894 for (ACMatches::const_iterator i(matches.begin());
886 (i != matches.end()) && 895 (i != matches.end()) &&
887 (matches_.size() < AutocompleteResult::kMaxMatches); 896 (matches_.size() < AutocompleteResult::kMaxMatches);
888 ++i) { 897 ++i) {
889 // SEARCH_OTHER_ENGINE is only used in the SearchProvider for the keyword 898 // SEARCH_OTHER_ENGINE is only used in the SearchProvider for the keyword
890 // verbatim result, so this condition basically means "if this match is a 899 // verbatim result, so this condition basically means "if this match is a
891 // suggestion of some sort". 900 // suggestion of some sort".
892 if ((i->type != AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED) && 901 if ((i->type != AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED) &&
893 (i->type != AutocompleteMatchType::SEARCH_OTHER_ENGINE)) { 902 (i->type != AutocompleteMatchType::SEARCH_OTHER_ENGINE)) {
894 // If we've already hit the limit on non-server-scored suggestions, and 903 // If we've already hit the limit on non-server-scored suggestions, and
895 // this isn't a server-scored suggestion we can add, skip it. 904 // this isn't a server-scored suggestion we can add, skip it.
896 if ((num_suggestions >= kMaxMatches) && 905 if ((num_suggestions >= kMaxMatches) &&
897 (!chrome::IsInstantExtendedAPIEnabled() || 906 (!chrome::IsInstantExtendedAPIEnabled() ||
898 (i->GetAdditionalInfo(kRelevanceFromServerKey) != kTrue))) { 907 (i->GetAdditionalInfo(kRelevanceFromServerKey) != kTrue))) {
899 continue; 908 continue;
900 } 909 }
901 910
902 ++num_suggestions; 911 ++num_suggestions;
903 } 912 }
904 913
905 matches_.push_back(*i); 914 matches_.push_back(*i);
906 } 915 }
907 UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.ConvertResultsTime", 916 UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.ConvertResultsTime",
908 base::TimeTicks::Now() - start_time); 917 base::TimeTicks::Now() - start_time);
909 } 918 }
910 919
920 void SearchProvider::RemoveExtraAnswers(ACMatches* matches) {
921 bool answer_seen = false;
922 for (ACMatches::iterator it = matches->begin(); it != matches->end(); ++it) {
923 if (!it->answer_contents.empty()) {
924 if (!answer_seen) {
925 answer_seen = true;
926 } else {
927 it->answer_contents.clear();
928 it->answer_type.clear();
929 }
930 }
931 }
932 }
933
911 ACMatches::const_iterator SearchProvider::FindTopMatch() const { 934 ACMatches::const_iterator SearchProvider::FindTopMatch() const {
912 ACMatches::const_iterator it = matches_.begin(); 935 ACMatches::const_iterator it = matches_.begin();
913 while ((it != matches_.end()) && !it->allowed_to_be_default_match) 936 while ((it != matches_.end()) && !it->allowed_to_be_default_match)
914 ++it; 937 ++it;
915 return it; 938 return it;
916 } 939 }
917 940
918 bool SearchProvider::IsTopMatchSearchWithURLInput() const { 941 bool SearchProvider::IsTopMatchSearchWithURLInput() const {
919 ACMatches::const_iterator first_match = FindTopMatch(); 942 ACMatches::const_iterator first_match = FindTopMatch();
920 return (input_.type() == metrics::OmniboxInputType::URL) && 943 return (input_.type() == metrics::OmniboxInputType::URL) &&
(...skipping 408 matching lines...) Expand 10 before | Expand all | Expand 10 after
1329 match->fill_into_edit.empty()) 1352 match->fill_into_edit.empty())
1330 return; 1353 return;
1331 1354
1332 // Valid answer encountered, cache it for further queries. 1355 // Valid answer encountered, cache it for further queries.
1333 answers_cache_.UpdateRecentAnswers(match->fill_into_edit, match->answer_type); 1356 answers_cache_.UpdateRecentAnswers(match->fill_into_edit, match->answer_type);
1334 } 1357 }
1335 1358
1336 void SearchProvider::DoAnswersQuery(const AutocompleteInput& input) { 1359 void SearchProvider::DoAnswersQuery(const AutocompleteInput& input) {
1337 prefetch_data_ = answers_cache_.GetTopAnswerEntry(input.text()); 1360 prefetch_data_ = answers_cache_.GetTopAnswerEntry(input.text());
1338 } 1361 }
OLDNEW
« no previous file with comments | « components/omnibox/search_provider.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698