Omnibox: Create Bundled Field Trial; Convert SearchHistory trial to it

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 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 418 matching lines @@
   // Gets the relevance score for the keyword verbatim result.
   // |relevance_from_server| is handled as in GetVerbatimRelevance().
   // TODO(mpearson): Refactor so this duplication isn't necesary or
   // restructure so one static function takes all the parameters it needs
   // (rather than looking at internal state).
   int GetKeywordVerbatimRelevance(bool* relevance_from_server) const;
 
   // |time| is the time at which this query was last seen.  |is_keyword|
   // indicates whether the results correspond to the keyword provider or default
   // provider. |prevent_inline_autocomplete| is true if we should not inline
-  // autocomplete this query.
+  // autocomplete this query.  |prevent_search_history_inlining| is true
+  // if we would normally inline autocomplete this query but want to shift
+  // the relevance curve slightly downward.

[Peter Kasting, 2013/08/01 22:44:48]

Nit: You might want a clearer explanation of these last two params (and maybe
even different names for them).  Basically, you'd want to explain that we have
two scoring curves, and why; then if |prevent_inline_autocomplete| is true, note
that we force all searches to use the second curve, and the last arg is ignored,
while if |prevent_inline_autocomplete| is false but
|prevent_search_history_inlining| is true, we allow two scoring curves to be
used, but lower the top value of the first curve.

Writing all that out makes me think that maybe this whole "2-curve, 2-param"
combination is more complex than it really should be.  After all, if it's hard
to explain...

[Mark P, 2013/08/02 00:44:36]

With judicious renaming, I think I managed to rewrite the comment in a much
clearer way.  I agree the old way with two "prevent" variables was ugly.

   int CalculateRelevanceForHistory(const base::Time& time,
                                    bool is_keyword,
-                                   bool prevent_inline_autocomplete) const;
+                                   bool prevent_inline_autocomplete,
+                                   bool prevent_search_history_inlining) const;
 
   // Creates an AutocompleteMatch for "Search <engine> for |query_string|" with
   // the supplied relevance.  Adds this match to |map|; if such a match already
   // exists, whichever one has lower relevance is eliminated.
   void AddMatchToMap(const string16& query_string,
                      const string16& input_text,
                      int relevance,
                      bool relevance_from_server,
                      AutocompleteMatch::Type type,
                      int accepted_suggestion,
@@ skipping 71 matching lines @@
   // session.
   bool field_trial_triggered_in_session_;
 
   // Start margin of the omnibox. Used to construct search URLs.
   int omnibox_start_margin_;
 
   // If true, search history query suggestions will score low enough that
   // they will not be inlined.
   bool prevent_search_history_inlining_;
 
-  // If true, no search history query suggestions will be offered.
-  bool disable_search_history_;
-
   DISALLOW_COPY_AND_ASSIGN(SearchProvider);
 };
 
 #endif  // CHROME_BROWSER_AUTOCOMPLETE_SEARCH_PROVIDER_H_