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

Unified Diff: chrome/browser/autocomplete/search_provider.cc

Issue 471673002: Omnibox: Prevent Asynchronous Suggestions from Changing Default Match (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: add clarifying comment Created 6 years, 4 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 side-by-side diff with in-line comments
Download patch
Index: chrome/browser/autocomplete/search_provider.cc
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index bf49576093038dd465cd265dc23fb206c6212e35..30fb42ecb2475daa7e6efa07411113ee9be237a8 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -260,6 +260,16 @@ void SearchProvider::Start(const AutocompleteInput& input,
input_ = input;
+ // If Start() is called without minimal_changes, it likely means the user
+ // has pressed a key. This is a good opportunity to change the inline
+ // autocompletion (if any). Restore the results to the original reply
+ // received from the server, thus clobbering any modification (if any) made
+ // to the results to demote an brand new async inline autocompletion.
+ if (!minimal_changes) {
+ default_results_ = orig_default_results_;
+ keyword_results_ = orig_keyword_results_;
+ }
+
DoHistoryQuery(minimal_changes);
DoAnswersQuery(input);
StartOrStopSuggestQuery(minimal_changes);
@@ -304,7 +314,60 @@ const AutocompleteInput SearchProvider::GetInput(bool is_keyword) const {
SearchSuggestionParser::Results* SearchProvider::GetResultsToFill(
bool is_keyword) {
- return is_keyword ? &keyword_results_ : &default_results_;
+ return is_keyword ? &orig_keyword_results_ : &orig_default_results_;
+}
+
+void SearchProvider::HandleReceivedResults(bool is_keyword) {
+ // The general strategy of this function:
+ // 1. Get the needed information from the previously-displayed results.
+ // 2. Copy the newly-received results over the previously-displayed ones.
+ // 3. Modify these results as necessary.
+
+ // 1. Extract needed information.
+ // Determine the previous inline autocompletion, if any.
+ // We store the inlined suggestion (if any) in one of the two following
+ // variables, leaving them blank if there is no inline suggestion or the
+ // inlined suggestion is not of that type.
+ base::string16 inlined_query_suggestion_match_contents;
+ GURL inlined_navsuggestion;
+ ACMatches::const_iterator first_match = FindTopMatch();
+ if ((first_match != matches_.end()) &&
+ !first_match->inline_autocompletion.empty()) {
+ // Identify if this match came from a query suggestion or a navsuggestion.
+ // In either case, extracts the identifying feature of the suggestion
+ // (query string or navigation url).
+ if (AutocompleteMatch::IsSearchType(first_match->type)) {
+ inlined_query_suggestion_match_contents = first_match->contents;
+ } else {
+ inlined_navsuggestion = first_match->destination_url;
+ }
+ }
+
+ // 2. Clobber the previous results.
+ SearchSuggestionParser::Results* results_from_server =
+ is_keyword ? &orig_keyword_results_ : &orig_default_results_;
+ SearchSuggestionParser::Results* results =
+ is_keyword ? &keyword_results_ : &default_results_;
+ (*results) = (*results_from_server);
+
+ // 3. Modify the results.
+ // Mark all results aside from the previous inline autocompletion as not
+ // allowed to be the default match.
+ for (SearchSuggestionParser::SuggestResults::iterator sug_it =
+ results->suggest_results.begin();
+ sug_it != results->suggest_results.end(); ++sug_it) {
+ if (sug_it->match_contents() !=
+ inlined_query_suggestion_match_contents) {
+ sug_it->set_never_allowed_to_be_default_match(true);
+ }
+ }
+ for (SearchSuggestionParser::NavigationResults::iterator nav_it =
+ results->navigation_results.begin();
+ nav_it != results->navigation_results.end(); ++nav_it) {
+ if (nav_it->url() != inlined_navsuggestion) {
+ nav_it->set_never_allowed_to_be_default_match(true);
+ }
+ }
}
bool SearchProvider::ShouldAppendExtraParams(
@@ -328,6 +391,8 @@ void SearchProvider::StopSuggest() {
void SearchProvider::ClearAllResults() {
keyword_results_.Clear();
default_results_.Clear();
+ orig_keyword_results_.Clear();
+ orig_default_results_.Clear();
}
int SearchProvider::GetDefaultResultRelevance() const {
@@ -624,6 +689,7 @@ void SearchProvider::ApplyCalculatedSuggestRelevance(
result.CalculateRelevance(input_, providers_.has_keyword_provider()) +
(list->size() - i - 1));
result.set_relevance_from_server(false);
+ result.set_never_allowed_to_be_default_match(false);
}
}
@@ -635,6 +701,7 @@ void SearchProvider::ApplyCalculatedNavigationRelevance(
result.CalculateRelevance(input_, providers_.has_keyword_provider()) +
(list->size() - i - 1));
result.set_relevance_from_server(false);
+ result.set_never_allowed_to_be_default_match(false);
}
}
@@ -1174,7 +1241,9 @@ AutocompleteMatch SearchProvider::NavigationToMatch(
// that we're not preventing, make sure we didn't trim any whitespace.
// We don't want to claim http://foo.com/bar is inlineable against the
// input "foo.com/b ".
- match.allowed_to_be_default_match = (prefix != NULL) &&
+ match.allowed_to_be_default_match =
+ !navigation.never_allowed_to_be_default_match() &&
+ (prefix != NULL) &&
(providers_.GetKeywordProviderURL() == NULL) &&
(match.inline_autocompletion.empty() ||
(!input_.prevent_inline_autocomplete() && !trimmed_whitespace));

Powered by Google App Engine
This is Rietveld 408576698