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

chrome/browser/omnibox/omnibox_field_trial.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.
 
 #ifndef CHROME_BROWSER_OMNIBOX_OMNIBOX_FIELD_TRIAL_H_
 #define CHROME_BROWSER_OMNIBOX_OMNIBOX_FIELD_TRIAL_H_
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/gtest_prod_util.h"
+#include "chrome/browser/autocomplete/autocomplete_input.h"
 
 // This class manages the Omnibox field trials.
 class OmniboxFieldTrial {
  public:
   // Creates the static field trial groups.
   // *** MUST NOT BE CALLED MORE THAN ONCE. ***
   static void ActivateStaticTrials();
 
   // Activates all dynamic field trials.  The main difference between
   // the autocomplete dynamic and static field trials is that the former
@@ skipping 72 matching lines @@
   // if the field trial is not active, etc., returns false.
   // CalculateScore()'s return value is a product of this maximum
   // relevance score and some attenuating factors that are all between
   // 0 and 1.  (Note that Shortcuts results may have their scores
   // reduced later if the assigned score is higher than allowed for
   // non-inlineable results.  Shortcuts results are not allowed to be
   // inlined.)
   static bool ShortcutsScoringMaxRelevance(int* max_relevance);
 
   // ---------------------------------------------------------
-  // For the SearchHistory field trial.
+  // For the SearchHistory experiment that's part of the bundled omnibox
+  // field trial.
 
-  // Returns true if the user is in the experiment group that scores
-  // search history query suggestions less aggressively so that they don't
-  // inline.
-  static bool SearchHistoryPreventInlining();
+  // Returns true if the user is in the experiment group that, in the

[Peter Kasting, 2013/07/31 20:14:09]

Nit: last "in" -> "given"?  (2 places)

[Mark P, 2013/07/31 21:59:28]

Done.

+  // provided |current_page_classification| context, scores search history
+  // query suggestions less aggressively so that they don't inline.
+  static bool SearchHistoryPreventInlining(
+      AutocompleteInput::PageClassification current_page_classification);
 
-  // Returns true if the user is in the experiment group that disables
-  // all query suggestions from search history.
-  static bool SearchHistoryDisable();
+  // Returns true if the user is in the experiment group that, in the
+  // provided |current_page_classification| context, disables all query
+  // suggestions from search history.
+  static bool SearchHistoryDisable(
+      AutocompleteInput::PageClassification current_page_classification);
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(OmniboxFieldTrialTest,
+                           GetConsequencesOfRuleInPageClassificationContext);
+
+  // Returns the "consequences" for the |rule| that applies in the context

[Peter Kasting, 2013/07/31 20:14:09]

Nit: Putting "consequences" in quotes and never defining what it means leaves
the reader still unsure how this function is actually used.  I might hoist most
of the comments from the .cc file to here, since that background is all useful
in the .h file, rather than being low-level implementation detail that's only of
interest in deciphering the implementation.  I'd also probably use
"GetRuleValue" or "GetValueOfRule" or "GetValueForRule" instead of using the
word "consequences" anywhere.

[Mark P, 2013/07/31 21:59:28]

I think with the newly rewritten comment and removing the reference to
consequences things read pretty well.  I don't feel the need to drag the long
formatting of rules comment into the header.

+  // of |current_page_classification|.  If no such rule exists, looks for
+  // that rule in the global context and return it if found.  If the rule
+  // remains unfound in the global context, returns the empty string.  For
+  // more details, see the implementation.
+  static std::string GetConsequencesOfRuleInPageClassificationContext(
+      AutocompleteInput::PageClassification current_page_classification,
+      const std::string& rule);
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(OmniboxFieldTrial);
 };
 
 #endif  // CHROME_BROWSER_OMNIBOX_OMNIBOX_FIELD_TRIAL_H_