Omnibox: Make the Controller Reorder Matches for Inlining

chrome/browser/omnibox/omnibox_field_trial.h

Index: chrome/browser/omnibox/omnibox_field_trial.h
diff --git a/chrome/browser/omnibox/omnibox_field_trial.h b/chrome/browser/omnibox/omnibox_field_trial.h
index 1ad3b16f9dd7159d05a04318deb4ee2677f2975d..0522486ffe4c5c5acb6490bb2d8f3fe439979bb6 100644
--- a/chrome/browser/omnibox/omnibox_field_trial.h
+++ b/chrome/browser/omnibox/omnibox_field_trial.h
@@ -118,6 +118,19 @@ class OmniboxFieldTrial {
   static bool SearchHistoryDisable(
       AutocompleteInput::PageClassification current_page_classification);
 
+  // ---------------------------------------------------------
+  // For the ReorderForLegalDefaultMatch experiment that's part of the
+  // bundled omnibox field trial.
+
+  // Returns true if the omnibox will reorder matches, in the provided
+  // |current_page_classification| context so that a match that's allowed to
+  // be the default match will appear first.  This means AutocompleteProviders
+  // can score matches however they desire without regard to making sure the
+  // top match when all the matches from all providers are merged is a legal
+  // default match.
+  static bool ReorderForLegalDefaultMatch(
+      AutocompleteInput::PageClassification current_page_classification);

[msw, 2013/08/07 20:09:21]

Why does the page classification matter?

[Mark P, 2013/08/07 22:13:05]

We may only want to reorder in certain contexts.  I'm not initially imagining
using context, but I can imagine using it.  For instance, reordering has a
tendency to show URLs more aggressively, something that may not appropriate on a
search results page doing search term replacement (where users are more likely
to want to refine to another query).

[msw, 2013/08/10 17:32:18]

I'd remove the arg for simplicity if it doesn't have an imminent planned use.

[Mark P, 2013/08/11 03:22:48]

I'm afraid removing the arg, though it would make this API simpler, would either
require more code or a more complex configuration language.  Note below that
GetValueForRuleInContext(), which is the basic function to lookup config
information about what experiments are active, takes |page_classification|.  If
you look at the comment by its implementation, you'll note that that's an
expected part of the configuration language.  So, if I want to remove the
parameter from ReorderForLegalDefaultMatch(), I have two options:
* have the implementation of this function call GetValueForRuleInContext() with
a dummy page classification, and expect people who write configs to know what to
use as the dummy value for the reorder rule.
* have the implementation of this function do it own lookup of the rule (with no
page classification context) and therefore have people who write a config have
to remember two different formats for the config, one format for most rules and
one format for the reorder rule.

I don't like either of these options much.

+
  private:
   FRIEND_TEST_ALL_PREFIXES(OmniboxFieldTrialTest, GetValueForRuleInContext);