Parametrize Suggest polling strategy and delay. Also add a parameter

components/omnibox/omnibox_field_trial_unittest.cc

 // 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.
 
 #include "components/omnibox/omnibox_field_trial.h"
 
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/field_trial.h"
@@ skipping 47 matching lines @@
       AutocompleteMatchType::Type match_type,
       float expected_value);
 
   // EXPECT()s that OmniboxFieldTrial::GetValueForRuleInContext(|rule|,
   // |page_classification|) returns |rule_value|.
   static void ExpectRuleValue(
       const std::string& rule_value,
       const std::string& rule,
       OmniboxEventProto::PageClassification page_classification);
 
+  // EXPECTS that OmniboxFieldTrial::GetSuggestPollingStrategy returns
+  // |expected_from_last_keystroke| and |expected_delay_ms| for the given
+  // experiment params.
+  void VerifySuggestPollingStrategy(
+      const std::string& from_last_keystroke_rule_value,
+      const std::string& polling_delay_ms_rule_value,
+      bool expected_from_last_keystroke,
+      int expected_delay_ms);
+
  private:
   scoped_ptr<base::FieldTrialList> field_trial_list_;
 
   DISALLOW_COPY_AND_ASSIGN(OmniboxFieldTrialTest);
 };
 
 // static
 void OmniboxFieldTrialTest::VerifyDemotion(
     const OmniboxFieldTrial::DemotionMultipliers& demotions,
     AutocompleteMatchType::Type match_type,
     float expected_value) {
   OmniboxFieldTrial::DemotionMultipliers::const_iterator demotion_it =
       demotions.find(match_type);
   ASSERT_TRUE(demotion_it != demotions.end());
   EXPECT_FLOAT_EQ(expected_value, demotion_it->second);
 }
 
 // static
 void OmniboxFieldTrialTest::ExpectRuleValue(
     const std::string& rule_value,
     const std::string& rule,
     OmniboxEventProto::PageClassification page_classification) {
   EXPECT_EQ(rule_value,
             OmniboxFieldTrial::GetValueForRuleInContext(
                 rule, page_classification));
 }
 
+void OmniboxFieldTrialTest::VerifySuggestPollingStrategy(
+    const std::string& from_last_keystroke_rule_value,
+    const std::string& polling_delay_ms_rule_value,
+    bool expected_from_last_keystroke,

[Mark P, 2014/10/22 23:03:15]

this and below might as well be const

[Bart N., 2014/10/23 01:42:54]

We don't typically put const for params passed by value. See examples above and
elsewhere. If you really insist, I can do it, but please provide a good reason
of doing so.

+    int expected_delay_ms) {
+  ResetFieldTrialList();
+  std::map<std::string, std::string> params;
+  params[std::string(
+      OmniboxFieldTrial::kMeasureSuggestPollingDelayFromLastKeystrokeRule)] =
+      from_last_keystroke_rule_value;
+  params[std::string(
+      OmniboxFieldTrial::kSuggestPollingDelayMsRule)] =
+      polling_delay_ms_rule_value;
+  ASSERT_TRUE(variations::AssociateVariationParams(
+      OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A", params));
+  base::FieldTrialList::CreateFieldTrial(
+      OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A");
+
+  bool from_last_keystroke;
+  int delay_ms;
+  OmniboxFieldTrial::GetSuggestPollingStrategy(&from_last_keystroke, &delay_ms);
+  EXPECT_EQ(expected_from_last_keystroke, from_last_keystroke);
+  EXPECT_EQ(expected_delay_ms, delay_ms);
+}
+
 // Test if GetDisabledProviderTypes() properly parses various field trial
 // group names.
 TEST_F(OmniboxFieldTrialTest, GetDisabledProviderTypes) {
   EXPECT_EQ(0, OmniboxFieldTrial::GetDisabledProviderTypes());
 
   {
     SCOPED_TRACE("Invalid groups");
     CreateTestTrial("AutocompleteDynamicTrial_0", "DisabledProviders_");
     EXPECT_EQ(0, OmniboxFieldTrial::GetDisabledProviderTypes());
     ResetFieldTrialList();
@@ skipping 266 matching lines @@
 
   // No decay by default.
   EXPECT_EQ(1.0, buckets.HalfLifeTimeDecay(base::TimeDelta::FromDays(7)));
 
   buckets.set_half_life_days(7);
   EXPECT_EQ(0.5, buckets.HalfLifeTimeDecay(base::TimeDelta::FromDays(7)));
   EXPECT_EQ(0.25, buckets.HalfLifeTimeDecay(base::TimeDelta::FromDays(14)));
   EXPECT_EQ(1.0, buckets.HalfLifeTimeDecay(base::TimeDelta::FromDays(0)));
   EXPECT_EQ(1.0, buckets.HalfLifeTimeDecay(base::TimeDelta::FromDays(-1)));
 }
+
+TEST_F(OmniboxFieldTrialTest, DisableResultsCaching) {
+  EXPECT_FALSE(OmniboxFieldTrial::DisableResultsCaching());
+
+  {
+    std::map<std::string, std::string> params;
+    params[std::string(OmniboxFieldTrial::kDisableResultsCachingRule)] = "true";
+    ASSERT_TRUE(variations::AssociateVariationParams(
+        OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A", params));
+    base::FieldTrialList::CreateFieldTrial(
+        OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A");
+
+    EXPECT_TRUE(OmniboxFieldTrial::DisableResultsCaching());
+  }
+}
+
+TEST_F(OmniboxFieldTrialTest, GetSuggestPollingStrategy) {

[Mark P, 2014/10/22 23:03:15]

The function this test uses VerifySuggestPollingStrategy() always sets both
parameters.  Can you revise this framework to allow testing that things work
right if only one of the polling strategy parameter fields is set?

[Bart N., 2014/10/23 01:42:53]

Done.

+  // Invalid params.
+  VerifySuggestPollingStrategy(
+      "", "", false,
+      OmniboxFieldTrial::kDefaultMinimumTimeBetweenSuggestQueriesMs);
+  VerifySuggestPollingStrategy(
+      "foo", "-1", false,
+      OmniboxFieldTrial::kDefaultMinimumTimeBetweenSuggestQueriesMs);
+  VerifySuggestPollingStrategy(
+      "TRUE", "xyz", false,
+      OmniboxFieldTrial::kDefaultMinimumTimeBetweenSuggestQueriesMs);
+
+  // Valid params.
+  VerifySuggestPollingStrategy("true", "50", true, 50);
+}

[Mark P, 2014/10/22 23:03:15]

Please add a test for the default values of this polling strategy.

[Bart N., 2014/10/23 01:42:54]

Done.