Omnibox: Better Enforce Suggest Relevance Constraints in Keyword Mode

chrome/browser/autocomplete/search_provider.h

 // Copyright (c) 2012 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.
 //
 // This file contains the Search autocomplete provider.  This provider is
 // responsible for all non-keyword autocomplete entries that start with
 // "Search <engine> for ...", including searching for the current input string,
 // search history, and search suggestions.  An instance of it gets created and
 // managed by the autocomplete controller.
 
@@ skipping 233 matching lines @@
   // Parses results from the suggest server and updates the appropriate suggest
   // and navigation result lists, depending on whether |is_keyword| is true.
   // Returns whether the appropriate result list members were updated.
   bool ParseSuggestResults(base::Value* root_val, bool is_keyword);
 
   // Converts the parsed results to a set of AutocompleteMatches, |matches_|.
   void ConvertResultsToAutocompleteMatches();
 
   // Checks if suggested relevances violate certain expected constraints.
   // See UpdateMatches() for the use and explanation of these constraints.
-  bool IsTopMatchScoreTooLow() const;
-  bool IsTopMatchHighRankSearchForURL() const;
-  bool IsTopMatchNotInlinable() const;
+  // We pass in keyword_search_what_you_typed_relevance because the
+  // keyword verbatim result is not in |matches_| but can be important in
+  // testing these constraints if it's high-ranked than the best result

[Peter Kasting, 2013/01/21 20:22:57]

Nit: high-ranked -> ranked higher?

[Mark P, 2013/01/21 20:57:45]

Rephrased for precision and readability.

+  // in |matches_|.
+  bool IsTopMatchScoreTooLow(

[Peter Kasting, 2013/01/21 20:22:57]

Nit: We may want to comment what some of these test in the context of the larger
set of constraints we're aiming to enforce.  For example, "high rank search" is
a little unclear.

[Mark P, 2013/01/21 20:57:45]

These are explained in UpdateMatch().  (Note the comment above "See
UpdateMatches() for the use and explanation of these constraints.")

If the explanations there are no enough, please tell me.

+      int keyword_search_what_you_typed_relevance) const;
+  bool IsTopMatchHighRankSearchForURL(
+      int keyword_search_what_you_typed_relevance) const;
+  bool IsTopMatchNotInlinable(
+      int keyword_search_what_you_typed_relevance) const;
 
   // Updates |matches_| from the latest results; applies calculated relevances
   // if suggested relevances cause undesriable behavior. Updates |done_|.
   void UpdateMatches();
 
   // Converts the top navigation result in |navigation_results| to an
   // AutocompleteMatch and adds it to |matches_|. |is_keyword| must be true if
   // the results come from the keyword provider.
   void AddNavigationResultsToMatches(
       const NavigationResults& navigation_results,
@@ skipping 117 matching lines @@
   bool field_trial_triggered_;
 
   // Same as above except that it is maintained across the current Omnibox
   // session.
   bool field_trial_triggered_in_session_;
 
   DISALLOW_COPY_AND_ASSIGN(SearchProvider);
 };
 
 #endif  // CHROME_BROWSER_AUTOCOMPLETE_SEARCH_PROVIDER_H_