Show PhysicalWebProvider suggestions with omnibox input

components/omnibox/browser/omnibox_field_trial.h

 // Copyright 2014 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.
 
 #ifndef COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_FIELD_TRIAL_H_
 #define COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_FIELD_TRIAL_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 77 matching lines @@
   HUPScoringParams() : experimental_scoring_enabled(false) {}
 
   bool experimental_scoring_enabled;
 
   ScoreBuckets typed_count_buckets;
 
   // Used only when the typed count is 0.
   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:
   // A mapping that contains multipliers indicating that matches of the
   // specified type should have their relevance score multiplied by the
   // given number.  Omitted types are assumed to have multipliers of 1.0.
   typedef std::map<AutocompleteMatchType::Type, float> DemotionMultipliers;
 
   // A vector that maps from the number of matching pages to the document
   // specificity score used in HistoryQuick provider / ScoredHistoryMatch
@@ skipping 259 matching lines @@
   // field trial.
 
   // Returns the conditions under which the UI code should display the title
   // of a URL more prominently than the URL for an input of type |input_type|.
   // Normally the URL is displayed more prominently.  Returns NEVER_EMPHASIZE
   // if the experiment isn't active.
   static EmphasizeTitlesCondition GetEmphasizeTitlesConditionForInput(
       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.
   static const char kDisableProvidersRule[];
   static const char kShortcutsScoringMaxRelevanceRule[];
   static const char kSearchHistoryRule[];
   static const char kDemoteByTypeRule[];
   static const char kHQPBookmarkValueRule[];
   static const char kHQPTypedValueRule[];
   static const char kHQPDiscountFrecencyWhenFewVisitsRule[];
@@ skipping 11 matching lines @@
   static const char kHQPNumMatchesScoresRule[];
   static const char kHQPNumTitleWordsRule[];
   static const char kHQPAlsoDoHUPLikeScoringRule[];
   static const char kHUPSearchDatabaseRule[];
   static const char kPreventUWYTDefaultForNonURLInputsRule[];
   static const char kKeywordRequiresRegistryRule[];
   static const char kKeywordRequiresPrefixMatchRule[];
   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[];
   static const char kHUPNewScoringTypedCountRelevanceCapParam[];
   static const char kHUPNewScoringTypedCountHalfLifeTimeParam[];
   static const char kHUPNewScoringTypedCountScoreBucketsParam[];
   static const char kHUPNewScoringTypedCountUseDecayFactorParam[];
   static const char kHUPNewScoringVisitedCountRelevanceCapParam[];
   static const char kHUPNewScoringVisitedCountHalfLifeTimeParam[];
   static const char kHUPNewScoringVisitedCountScoreBucketsParam[];
   static const char kHUPNewScoringVisitedCountUseDecayFactorParam[];
 
   // Parameter names used by the HQP experimental scoring experiments.
   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.
   static int kDefaultMinimumTimeBetweenSuggestQueriesMs;
 
  private:
   friend class OmniboxFieldTrialTest;
 
   // The bundled omnibox experiment comes with a set of parameters
   // (key-value pairs).  Each key indicates a certain rule that applies in
@@ skipping 11 matching lines @@
   // prioritize different wildcard contexts, see the implementation.  How to
   // interpret the value is left to the caller; this is rule-dependent.
   static std::string GetValueForRuleInContext(
       const std::string& rule,
       metrics::OmniboxEventProto::PageClassification page_classification);
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(OmniboxFieldTrial);
 };
 
 #endif  // COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_FIELD_TRIAL_H_