Show PhysicalWebProvider suggestions with omnibox input

components/omnibox/browser/omnibox_field_trial.h

Index: components/omnibox/browser/omnibox_field_trial.h
diff --git a/components/omnibox/browser/omnibox_field_trial.h b/components/omnibox/browser/omnibox_field_trial.h
index 46b8f490f14c55bffae193382be1f9add07869c4..0a7a0ab51deb2aa8db206cb7ec611debdc74aa94 100644
--- a/components/omnibox/browser/omnibox_field_trial.h
+++ b/components/omnibox/browser/omnibox_field_trial.h
@@ -95,6 +95,21 @@ struct HUPScoringParams {
   ScoreBuckets visited_count_buckets;
 };
 
+// The set of parameters customizing PhysicalWebProvider relevance scoring.

[Mark P, 2016/12/21 23:21:41]

This approach works but I feel it's a little heavy-handed.  (I see you copied it
from above.)  Plus, it's easy to make mistakes in configuration (the more
parameters, the more opportunities for mistakes.)  For example, your enabling
scoring code expects a "1" (or similar) to be enabled whereas your enabling
suggestions appearing code expected as "true".  I can easily imagine this biting
us at some point.  It's also the case that the current behavior is a problem if
we enable experimental scoring but don't, for example, specify one of your
current parameters (we'd try using the base relevance of 0).

Nowadays I prefer the approach where the scoring code isn't gated on a separate
flag (experimental_scoring_enabled).  Instead, simply have a function
GetPhysicalWebZeroSuggestBaseRelevance() that looks up the parameter directly
and returns it, returning a default value if the parameter doesn't exist.  This
will also let you get rid of the experiment gating test, making the
Get...Relevance function shorter (and more error-proof and maybe also easier to
read).  It'll also let you get rid of the default value in the physical web
code.  (Always get the value from the field trial code, regardless.  Make that
code have the default.)

This suggestion is optional.  I think it's cleaner.  Your approach is
nonetheless like others in this file so if you insist on this way, that's okay
too.

[mattreynolds, 2016/12/22 00:23:40]

Agreed, this is cleaner and less error-prone. Done.

+struct PhysicalWebScoringParams {
+  PhysicalWebScoringParams() : experimental_scoring_enabled(false) {}
+
+  bool experimental_scoring_enabled;
+
+  // The base relevance score for PhysicalWebProvider suggestions when the user
+  // has not entered any text in the omnibox.
+  int zero_suggest_base_relevance;
+
+  // The base relevance score for PhysicalWebprovider suggestions after the user
+  // has typed a query in the omnibox.
+  int after_typing_base_relevance;
+};
+
 // This class manages the Omnibox field trials.
 class OmniboxFieldTrial {
  public:
@@ -374,6 +389,25 @@ class OmniboxFieldTrial {
       metrics::OmniboxInputType::Type input_type);
 
   // ---------------------------------------------------------
+  // For PhysicalWebProvider related experiments.
+
+  // Returns whether the user is in a Physical Web field trial where the
+  // PhysicalWebProvider should be used to get suggestions when the user clicks
+  // on the omnibox but has not typed anything yet.
+  static bool InPhysicalWebZeroSuggestFieldTrial();
+
+  // Returns whether the user is in a Physical Web field trial and URL-based
+  // suggestions can continue to appear after the user has started typing.
+  static bool InPhysicalWebAfterTypingFieldTrial();
+
+  // Initializes the PhysicalWebProvider |scoring_params| based on the active
+  // Physical Web scoring experiment. If there is no such experiment, this
+  // function simply sets |scoring_params|->experimental_scoring_enabled to
+  // false.
+  static void GetExperimentalPhysicalWebScoringParams(
+      PhysicalWebScoringParams* scoring_params);
+
+  // ---------------------------------------------------------
   // Exposed publicly for the sake of unittests.
   static const char kBundledExperimentFieldTrialName[];
   // Rule names used by the bundled experiment.
@@ -405,6 +439,8 @@ class OmniboxFieldTrial {
   static const char kKeywordScoreForSufficientlyCompleteMatchRule[];
   static const char kHQPAllowDupMatchesForScoringRule[];
   static const char kEmphasizeTitlesRule[];
+  static const char kPhysicalWebZeroSuggestRule[];
+  static const char kPhysicalWebAfterTypingRule[];
 
   // Parameter names used by the HUP new scoring experiments.
   static const char kHUPNewScoringEnabledParam[];
@@ -421,6 +457,11 @@ class OmniboxFieldTrial {
   static const char kHQPExperimentalScoringBucketsParam[];
   static const char kHQPExperimentalScoringTopicalityThresholdParam[];
 
+  // Parameter names used by the Physical Web experimental scoring experiments.
+  static const char kPhysicalWebExperimentalScoringParam[];
+  static const char kPhysicalWebZeroSuggestBaseRelevanceParam[];
+  static const char kPhysicalWebAfterTypingBaseRelevanceParam[];
+
   // The amount of time to wait before sending a new suggest request after the
   // previous one unless overridden by a field trial parameter.
   // Non-const because some unittests modify this value.